iainmcgin / AppAuth-Demo

A demonstration of using the AppAuth library with Google, Facebook, Microsoft and Github
Apache License 2.0
21 stars 6 forks source link

Microsoft sign in broken #1

Open gabrielittner opened 6 years ago

gabrielittner commented 6 years ago

When using the Microsoft sign in I'm getting a "Token refresh failed" snackbar. The AuthorizationException looks like this

W: AuthorizationException: {"type":0,"code":5,"errorDescription":"JSON deserialization error"}
W:     at net.openid.appauth.AuthorizationService$TokenRequestTask.onPostExecute(AuthorizationService.java:478)
W:     at net.openid.appauth.AuthorizationService$TokenRequestTask.onPostExecute(AuthorizationService.java:375)
W:     at android.os.AsyncTask.finish(AsyncTask.java:695)
W:     at android.os.AsyncTask.-wrap1(Unknown Source:0)
W:     at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:712)
W:     at android.os.Handler.dispatchMessage(Handler.java:105)
W:     at android.os.Looper.loop(Looper.java:164)
W:     at android.app.ActivityThread.main(ActivityThread.java:6541)
W:     at java.lang.reflect.Method.invoke(Native Method)
W:     at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
W:     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)
W: Caused by: org.json.JSONException: field "token_type" not found in json object
W:     at net.openid.appauth.JsonUtil.getString(JsonUtil.java:171)
W:     at net.openid.appauth.TokenResponse$Builder.fromResponseJson(TokenResponse.java:216)
W:     at net.openid.appauth.AuthorizationService$TokenRequestTask.onPostExecute(AuthorizationService.java:475)
W:  ... 10 more

Does this also happen for you?

iainmcgin commented 6 years ago

It's been a while since I've tested the Microsoft integration, let me try it out and I'll get back to you.

iainmcgin commented 6 years ago

Microsoft seem to be returning an invalid response for the access code exchange request, which is missing the token_type field (which is mandatory, according to the OAuth2 spec). It's possible that the endpoints I'm using in this demo have changed; I'll compare it to the Microsoft docs.

gabrielittner commented 6 years ago

When I tried to integrate Microsoft sign in to my app I was following this documentation. The documented response seems to be spec compliant. The two differences to your implementation are that the client secret isn't required anymore and that they support redirects with custom schemes (the default when adding a native app in the console is msal[clientid]://auth). Both won't fix the incomplete response though.

iainmcgin commented 6 years ago

Yeah, I'm also looking at this documentation and the documented response there is spec compliant, but doesn't match what we're actually seeing - we only get a refresh_token and and id_token in the response, with no access_token. The absence of the access_token would explain why no token_type is described in the response, so perhaps I should be a little more tolerant of that, though the response is definitely not compliant with the OAuth2 spec.

I'll see if any of my contacts at Microsoft can help explain the change in behavior.

selfissued commented 6 years ago

This is a known issue in which Microsoft currently has an incomplete OpenID Connect implementation. I'll plan to let you know once the issue has been addressed. Thanks for writing about this.

selfissued commented 6 years ago

BTW, @gabrielittner - what was the path to the .well-known/openid-configuration document you were using when you experienced the problem?

gabrielittner commented 6 years ago

https://login.microsoftonline.com/common/v2.0/.well-known/openid-configuration I've also tried using https://login.microsoftonline.com/common/oauth2/v2.0/authorize and /token directly without the discovery.

gabrielittner commented 6 years ago

@selfissued When requesting Microsoft Graph scopes it seems to work now, but when using scopes for the Outlook API like https://outlook.office.com/calendars.readwrite.shared instead of Calendars.ReadWrite.Shared I'm still seeing the issue.