Online Accounts: Account plugins

Merge lp:~gary-wzl77/account-plugins/onedrive_provider into lp:account-plugins

  1. onedrive_provider
  2. Merge into trunk
Proposed by Gary.Wang
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 179
Merged at revision: 179
Proposed branch: lp:~gary-wzl77/account-plugins/onedrive_provider
Merge into: lp:account-plugins
Diff against target: 255 lines (+79/-41)
10 files modified
Makefile.am (+3/-2)
configure.ac (+9/-9)
data/providers/microsoft.provider.in.in (+26/-0)
data/providers/windows-live.provider.in.in (+0/-26)
debian/account-plugin-microsoft.install (+3/-0)
debian/control (+8/-1)
debian/rules (+2/-2)
qml/Makefile.am (+1/-0)
qml/microsoft/Main.qml (+27/-0)
src/config.vapi (+0/-1)
To merge this branch: bzr merge lp:~gary-wzl77/account-plugins/onedrive_provider
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Needs Fixing
Review via email: mp+308236@code.launchpad.net

Commit message

Add onedrive account plugin.

Description of the change

Add onedrive account plugin.

Please find the onedrive scope at the following link for a quick review.
https://drive.google.com/open?id=0B2H9ECPSSfqIeVhUTHo5dXZRcHM

To post a comment you must log in.
Revision history for this message
Alberto Mardegan (mardy) wrote :

Hi Gary! I think that the plugin should be called Windows Live, because OneDrive it's only one of its services.

We already had a Windows Live plugin, it got removed here:
http://bazaar.launchpad.net/~online-accounts/account-plugins/trunk/revision/150

but the file in data/providers/facebook.provider.in.in is still there, so you can update it instead.
You should also update the debian/control file to remove the line "Conflicts: account-plugin-windows-live," which is no longer relevant now.

review: Needs Fixing
Revision history for this message
Gary.Wang (gary-wzl77) wrote :

Hi mandy
    Please check the following link
    https://msdn.microsoft.com/en-us/library/office/dn631819.aspx

    "Instead of using the Live SDK, you might want to try out the new OneDrive API. The OneDrive API is a RESTful API, that works directly with OneDrive, on many platforms, and *completely independent* of the Live SDK. This API provides a faster way to authenticate users, allows large file uploads, and more."

    Also you can find the onedrive oauth dev page here.
    https://dev.onedrive.com/auth/msa_oauth.htm

    On the other hand, in order to easily to find "correct" account plugin from scopes/apps, I tend to use 'onedrive' as account plugin name, instead of "window live".
    As we have onedrive scope and storage provider here already.
    https://code.launchpad.net/storage-provider-onedrive
    https://code.launchpad.net/~hanloon-team/hanloon/onedrive_scope

    I'd like to hear you options on this.
    Thanks.

Revision history for this message
Alberto Mardegan (mardy) wrote :

> Hi mandy
> Please check the following link
> https://msdn.microsoft.com/en-us/library/office/dn631819.aspx
>
> "Instead of using the Live SDK, you might want to try out the new OneDrive
> API. The OneDrive API is a RESTful API, that works directly with OneDrive, on
> many platforms, and *completely independent* of the Live SDK. This API
> provides a faster way to authenticate users, allows large file uploads, and
> more."

Yes, that's about the recommended API to use, but:

> Also you can find the onedrive oauth dev page here.
> https://dev.onedrive.com/auth/msa_oauth.htm

OneDrive is still using the Live account for authentication. Apart from a change in the scopes list, all other authentication parameters are the same. Given that in the future we might support other services provided by the Live account, it's not a good idea to refer to this provider as "onedrive".

> On the other hand, in order to easily to find "correct" account plugin
> from scopes/apps, I tend to use 'onedrive' as account plugin name, instead of
> "window live".
> As we have onedrive scope and storage provider here already.
> https://code.launchpad.net/storage-provider-onedrive
> https://code.launchpad.net/~hanloon-team/hanloon/onedrive_scope

Given that at the moment OneDrive is the only service we support for the Live account, I don't have any strong objections if you want to set the display name of the Live account to "OneDrive", so that users will find it more easily. However, the provider id should be not specific to OneDrive, but instead refer to Microsoft in general.

I now see that the Windows Live brand is actually discontinued [1], so even though the website is called live.com, it seems that Microsoft is trying to forget that name and instead call it just "Microsoft account", as in [2] and [3].
So, my suggestion is to use "microsoft" as provider ID.

[1] https://en.wikipedia.org/wiki/Windows_Live
[2] https://www.microsoft.com/en-us/account/
[3] https://login.live.com/login.srf

lp:~gary-wzl77/account-plugins/onedrive_provider updated
177. By Gary.Wang

1.use microsoft as added account plugin name.
2.remove the deprecated account(windows-live).

Revision history for this message
Gary.Wang (gary-wzl77) wrote :

As provider id is changed to "microsoft", the click package needs some adjustment accordingly.
Please find the latest onedrive scope here.
https://drive.google.com/open?id=0B2H9ECPSSfqIOU5iUHYxaWVPM2M

Thanks.

Revision history for this message
Alberto Mardegan (mardy) wrote :

Thanks Gary! Please have a look at the comments inline, there are a few changes I'd recommend doing, but otherwise it's good!

review: Needs Fixing
Revision history for this message
Gary.Wang (gary-wzl77) wrote :

Sorry for the late reply. Please see the inline comment.
Thanks.

lp:~gary-wzl77/account-plugins/onedrive_provider updated
178. By Gary.Wang

1.fixed translations field("Microsoft" -> "account-plugins")
2.fixed redirect URL with recommended one.

179. By Gary.Wang

query user information so that we can display it on account page.

Revision history for this message
Alberto Mardegan (mardy) wrote :

Looks good, thanks!

review: Approve
lp:~gary-wzl77/account-plugins/onedrive_provider updated
180. By Gary.Wang

minor code change as windows-live.provider is deprecated.

Revision history for this message
Alberto Mardegan (mardy) wrote :

Hi Gary, after trying out this branch I would like to request some changes. You can see that I've proposed a branch, please merge it into this one.

The most serious problem I've found is that I had a live.com account I had been using for testing, but I never used onedrive before with that account. When I tried to create the account in my phone the authentication went fine, I could get the access code, but then getting the username was failing:

=================
ui-proxy.cpp 184 onDataReady QMap(("code", QVariant(QString, "finished") ) ( "data" , QVariant(QVariantMap, QMap(("UrlResponse", QVariant(QString, "https://login.live.com/oauth20_desktop.srf?lc=1040#access_token=XXX&token_type=bearer&expires_in=3600&scope=onedrive.readwrite onedrive.appfolder onedrive.readonly&user_id=AAAAAAAAAAAAAAAAAAAAAPywB7MPl1COV-NhKCYqWDw") ) ) ) ) ( "delay" , QVariant(int, 0) ) ( "id" , QVariant(int, 1) ) ( "interface" , QVariant(QString, "com.nokia.singlesignonui") ) )
browser-request.cpp 125 ~BrowserRequestPrivate
browser-request.cpp 325 closeView
qml: error: 403
qml: response text: {"error":{"code":"accessDenied","message":"Access Denied"}}
qml: Removing credentials...
qml: ====== PLUGIN FINISHED ======
provider-request.cpp 175 deny
=================

I tried many times, and it always failed. Then I tried to go to the onedrive site from the browser, and after logging in (with the same account) I was asked to accept the terms & conditions, and then I got to the main web page of onedrive where I can see my folders (all empty, since I never used it).
After doing this step, I tried creating the account on the phone again, and this time it worked.

To me, it sounds like a bug in the onedrive server, but I wonder if we should handle it better; maybe you can change the code in the onedrive QML plugin, and if the username cannot be obtained you can just do 'callback("")' (that is, you return an empty username) instead of calling cancel(). What do you think?

Also, I don't think we should include the user ID in the display name, it makes the display name too long (in my phone, it occupies 2 rows!). Maybe we should use the user ID only if the user name is empty. Can you please change this?

review: Needs Fixing
Revision history for this message
Gary.Wang (gary-wzl77) wrote :

Yes, I hit the same problem if user's not accepted the terms & conditions yet.

1...Return an empty username instead of calling cancel
Basically, I think empty username is sth we need to avoid since we have been discussing this issue with michi and james that users can't distinguish two Microsoft account when they have two microsoft account logged in if they hit the above problem. Yes, I know it's a rare case.
2. Maybe we should use the user ID only if the user name is empty. Can you please change this?
The reason why I append user id here is that Microsoft didn't provider user email address in response body so that there is probably a rare case that two same accounts (e.g. two "Gary" in display name field) are shown in account page, which makes users confused.

However, I saw your merge proposal, You call cancel() if user name is not be obtained and still append user id into display name, which look good.

lp:~gary-wzl77/account-plugins/onedrive_provider updated
181. By Gary.Wang

Merge mardy's branch to use microsoft graph API.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile.am'
2--- Makefile.am 2016-08-31 06:50:13 +0000
3+++ Makefile.am 2016-11-08 08:02:34 +0000
4@@ -90,12 +90,12 @@
5 data/providers/linkedin.provider.in.in \
6 data/providers/instagram.provider.in.in \
7 data/providers/mcloud.provider.in.in \
8+ data/providers/microsoft.provider.in.in \
9 data/providers/owncloud.provider.in.in \
10 data/providers/sina.provider.in.in \
11 data/providers/sohu.provider.in.in \
12 data/providers/twitter.provider.in.in \
13- data/providers/vk.provider.in.in \
14- data/providers/windows-live.provider.in.in
15+ data/providers/vk.provider.in.in
16
17 providers_DATA = \
18 $(providers_in_in_files:.provider.in.in=.provider)
19@@ -141,6 +141,7 @@
20 iconsdir = $(datadir)/icons/hicolor/32x32/apps
21 dist_icons_DATA = \
22 data/icons/mcloud.png \
23+ data/icons/microsoft.png \
24 data/icons/owncloud.png \
25 data/icons/vk.png
26
27
28=== modified file 'configure.ac'
29--- configure.ac 2016-06-16 11:48:50 +0000
30+++ configure.ac 2016-11-08 08:02:34 +0000
31@@ -242,6 +242,14 @@
32 AC_SUBST(MCLOUD_CLIENT_ID, ["$mcloud_client_id"])
33 AC_SUBST(MCLOUD_CLIENT_SECRET, ["$mcloud_client_secret"])
34
35+# Set microsoft client id
36+AC_ARG_WITH(microsoft-client-id,
37+ [AS_HELP_STRING([--with-microsoft-client-id],
38+ [Microsoft client id])],
39+ [microsoft_client_id=$withval],
40+ [microsoft_client_id="478797ed-02c1-442b-bebd-709401131e7d"])
41+AC_SUBST(MICROSOFT_CLIENT_ID, ["$microsoft_client_id"])
42+
43 # Set VK client id
44 AC_ARG_WITH(vk-client-id,
45 [AS_HELP_STRING([--with-vk-client-id],
46@@ -250,14 +258,6 @@
47 [vk_client_id="5404010"])
48 AC_SUBST(VK_CLIENT_ID, ["$vk_client_id"])
49
50-# Set Windows Live client id
51-AC_ARG_WITH(windows-live-client-id,
52- [AS_HELP_STRING([--with-windows-live-client-id],
53- [Windows Live client ID])],
54- [windows_live_client_id=$withval],
55- [windows_live_client_id="00000000480CBF28"])
56-AC_SUBST(WINDOWS_LIVE_CLIENT_ID, ["$windows_live_client_id"])
57-
58 AC_CONFIG_FILES([
59 data/providers/facebook.provider.in
60 data/providers/flickr.provider.in
61@@ -267,12 +267,12 @@
62 data/providers/linkedin.provider.in
63 data/providers/instagram.provider.in
64 data/providers/mcloud.provider.in
65+ data/providers/microsoft.provider.in
66 data/providers/owncloud.provider.in
67 data/providers/sina.provider.in
68 data/providers/sohu.provider.in
69 data/providers/twitter.provider.in
70 data/providers/vk.provider.in
71- data/providers/windows-live.provider.in
72 Makefile
73 po/Makefile.in
74 qml/Makefile
75
76=== added file 'data/icons/microsoft.png'
77Binary files data/icons/microsoft.png 1970-01-01 00:00:00 +0000 and data/icons/microsoft.png 2016-11-08 08:02:34 +0000 differ
78=== added file 'data/providers/microsoft.provider.in.in'
79--- data/providers/microsoft.provider.in.in 1970-01-01 00:00:00 +0000
80+++ data/providers/microsoft.provider.in.in 2016-11-08 08:02:34 +0000
81@@ -0,0 +1,26 @@
82+<?xml version="1.0" encoding="UTF-8"?>
83+<provider id="microsoft">
84+ <name>Microsoft</name>
85+ <icon>microsoft</icon>
86+ <translations>account-plugins</translations>
87+ <domains>.*live\.com</domains>
88+ <plugin>generic-oauth</plugin>
89+ <single-account>true</single-account>
90+ <template>
91+ <group name="auth">
92+ <setting name="method">oauth2</setting>
93+ <setting name="mechanism">user_agent</setting>
94+ <group name="oauth2">
95+ <group name="user_agent">
96+ <setting name="Host">login.live.com</setting>
97+ <setting name="AuthPath">oauth20_authorize.srf</setting>
98+ <setting name="RedirectUri">https://login.live.com/oauth20_desktop.srf</setting>
99+ <setting name="ResponseType">token</setting>
100+ <setting name="ClientId">@MICROSOFT_CLIENT_ID@</setting>
101+ <setting type="as" name="Scope">['User.Read', 'Files.ReadWrite']</setting>
102+ <setting type="b" name="DisableStateParameter">true</setting>
103+ </group>
104+ </group>
105+ </group>
106+ </template>
107+</provider>
108
109=== removed file 'data/providers/windows-live.provider.in.in'
110--- data/providers/windows-live.provider.in.in 2013-11-14 11:42:15 +0000
111+++ data/providers/windows-live.provider.in.in 1970-01-01 00:00:00 +0000
112@@ -1,26 +0,0 @@
113-<?xml version="1.0" encoding="UTF-8" ?>
114-<provider id="windows-live">
115- <name>Windows Live</name>
116- <icon>live</icon>
117- <translations>account-plugins</translations>
118- <domains>.*live\.com</domains>
119- <plugin>generic-oauth</plugin>
120-
121- <template>
122- <group name="auth">
123- <setting name="method">oauth2</setting>
124- <setting name="mechanism">web_server</setting>
125- <group name="oauth2">
126- <group name="web_server">
127- <setting name="Host">login.live.com</setting>
128- <setting name="AuthPath">/oauth20_authorize.srf</setting>
129- <setting name="TokenPath">/oauth20_token.srf</setting>
130- <setting name="RedirectUri">https://login.live.com/oauth20_desktop.srf</setting>
131- <setting name="ResponseType">code</setting>
132- <setting name="Scope" type="as">['wl.messenger','wl.offline_access','wl.emails', 'wl.imap']</setting>
133- <setting name="ClientId">@WINDOWS_LIVE_CLIENT_ID@</setting>
134- </group>
135- </group>
136- </group>
137- </template>
138-</provider>
139
140=== added file 'debian/account-plugin-microsoft.install'
141--- debian/account-plugin-microsoft.install 1970-01-01 00:00:00 +0000
142+++ debian/account-plugin-microsoft.install 2016-11-08 08:02:34 +0000
143@@ -0,0 +1,3 @@
144+usr/share/accounts/providers/microsoft.provider
145+usr/share/accounts/qml-plugins/microsoft/*.qml
146+usr/share/icons/hicolor/32x32/apps/microsoft.png
147
148=== modified file 'debian/control'
149--- debian/control 2016-08-31 06:50:13 +0000
150+++ debian/control 2016-11-08 08:02:34 +0000
151@@ -26,7 +26,6 @@
152 Architecture: any
153 Depends: ${shlibs:Depends}, ${misc:Depends},
154 signon-plugin-oauth2,
155-Conflicts: account-plugin-windows-live,
156 Description: Online account plugin support lib for Unity 7 - generic OAuth
157 Support library for the Unity 7 OAuth-based online account plugins
158
159@@ -156,6 +155,14 @@
160 This plugin adds support for creating mCloud accounts in the Unity Control
161 Center
162
163+Package: account-plugin-microsoft
164+Architecture: all
165+Depends: ${misc:Depends},
166+ ubuntu-system-settings-online-accounts,
167+Description: Online account plugin for Unity - Microsoft
168+ This plugin adds support for creating microsoft accounts in the Unity Control
169+ Center
170+
171 Package: account-plugin-owncloud
172 Architecture: all
173 Depends: ${misc:Depends},
174
175=== modified file 'debian/rules'
176--- debian/rules 2016-05-31 08:53:33 +0000
177+++ debian/rules 2016-11-08 08:02:34 +0000
178@@ -11,7 +11,6 @@
179 dh_auto_configure -- --enable-qml-plugins \
180 --with-twitter-consumer-key="NGOB5S7sICsj6epjh0PhAw" \
181 --with-twitter-consumer-secret="rbUEJCBEokMnGZd8bubd0QL2cSmoCjJeyiSJpnx3OM0" \
182- --with-windows-live-client-id="00000000400D5635" \
183 --with-facebook-client-id="271112146382618" \
184 --with-foursquare-client-id="1I2UNJXPHNDZT3OPZOOA5LCPIUEUJFMKRXSF42UFCN1KXKTK" \
185 --with-google-client-id="759250720802-4sii0me9963n9fdqdmi7cepn6ub8luoh.apps.googleusercontent.com" \
186@@ -24,12 +23,13 @@
187 --with-instagram-client-secret="4751ccdc39c648719ea83cfb1c866c26" \
188 --with-mcloud-client-id="APP1ZtqoN3R0002" \
189 --with-mcloud-client-secret="A70EFCDC91456349E7FDECF0A33574AC" \
190+ --with-microsoft-client-id="478797ed-02c1-442b-bebd-709401131e7d" \
191 --with-vk-client-id="5402699"
192
193 override_dh_install:
194 rm -f debian/*/usr/lib/*/*/*.la
195 # the service is not valid anymore so we don't ship those
196- rm debian/tmp/etc/signon-ui/webkit-options.d/login.live.com.conf debian/tmp/usr/share/accounts/providers/windows-live.provider
197+ rm debian/tmp/etc/signon-ui/webkit-options.d/login.live.com.conf
198 mkdir -p debian/tmp/usr/share/account-plugins/tools
199 dh_install --fail-missing
200
201
202=== modified file 'qml/Makefile.am'
203--- qml/Makefile.am 2016-06-01 13:01:07 +0000
204+++ qml/Makefile.am 2016-11-08 08:02:34 +0000
205@@ -3,6 +3,7 @@
206 flickr/Main.qml \
207 google/Main.qml \
208 mcloud/Main.qml \
209+ microsoft/Main.qml \
210 owncloud/Main.qml \
211 owncloud/NewAccount.qml \
212 twitter/Main.qml \
213
214=== added directory 'qml/microsoft'
215=== added file 'qml/microsoft/Main.qml'
216--- qml/microsoft/Main.qml 1970-01-01 00:00:00 +0000
217+++ qml/microsoft/Main.qml 2016-11-08 08:02:34 +0000
218@@ -0,0 +1,27 @@
219+import Ubuntu.OnlineAccounts.Plugin 1.0
220+
221+OAuthMain {
222+ creationComponent: OAuth {
223+ function getUserName(reply, callback) {
224+ var http = new XMLHttpRequest();
225+ var url = "https://graph.microsoft.com/v1.0/me";
226+ http.open("GET", url, true);
227+ http.setRequestHeader("Authorization", "bearer " + reply.AccessToken);
228+ http.onreadystatechange = function() {
229+ if (http.readyState === 4) {
230+ if (http.status === 200) {
231+ console.log("response text: " + http.responseText);
232+ var response = JSON.parse(http.responseText);
233+ callback(response.userPrincipalName ? response.userPrincipalName : response.displayName);
234+ } else {
235+ console.log("error: " + http.status);
236+ console.log("response text: " + http.responseText);
237+ cancel();
238+ }
239+ }
240+ };
241+ http.send(null);
242+ return true;
243+ }
244+ }
245+}
246
247=== modified file 'src/config.vapi'
248--- src/config.vapi 2013-04-09 08:45:38 +0000
249+++ src/config.vapi 2016-11-08 08:02:34 +0000
250@@ -36,5 +36,4 @@
251 public const string SINA_CONSUMER_SECRET;
252 public const string SOHU_CLIENT_ID;
253 public const string SOHU_CLIENT_SECRET;
254- public const string WINDOWS_LIVE_CLIENT_ID;
255 }

Subscribers

People subscribed via source and target branches