BingAds / BingAds-Python-SDK

Other
118 stars 164 forks source link

KeyError: 'refresh_token' #87

Closed jozsi closed 6 years ago

jozsi commented 6 years ago

Hello,

request_oauth_tokens_by_refresh_token fails with

    return OAuthTokens(r_json['access_token'], int(r_json['expires_in']), r_json['refresh_token'])
KeyError: 'refresh_token'

because the response from https://login.live.com/oauth20_token.srf is missing the refresh_token (but includes all the other fields correctly: token_type, expires_in, scope, access_token, user_id)

It started happening recently.

Could be related: https://stackoverflow.com/questions/49016329/microsoft-oauth-2-0-missing-refresh-token-field

qitia commented 6 years ago

@jozsi Do you have Request ID or Tracking ID to investigate with?

jozsi commented 6 years ago

@qitia - this is happening at https://login.live.com/oauth20_token.srf which does not seem to have a Request ID nor Tracking ID in the header (unlike reporting calls), or I'm looking at the wrong place?

Meanwhile I've been experimenting and it only happens to some users. They only seem to get wl.signin wl.basic scopes granted (even though only bingads.manage has been requested), while others get wl.signin wl.basic wl.offline_access from the same requested scope.

eric-urban commented 6 years ago

@jozsi Thanks for raising this issue!

Authorization service implementation details

Putting aside SDK implementation for now, the authorization code provisioned via https://login.live.com/oauth20_authorize.srf enables access to specific resources or scopes e.g., bingads.manage or wl.signin. You can (again, without the SDK) request an authorization code for multiple resources e.g., scope=bingads.manage%20wl.signin; however, when you request access tokens only one scope is allowed e.g., scope=bingads.manage and the returned access token is only valid for that scope. When you request access tokens via https://login.live.com/oauth20_token.srf the scope query parameter is optional, since the authorization service remembers the scope or scopes that you used to get the authorization code. It is important to note that if multiple scopes were used initially to acquire the authorization code, and then later you do not specify the scope parameter via the access token request, the authorization service default to the first scope originally requested by your app. For example if you requested the authorization code with scope=wl.signin%20bingads.manage, and then did not set the scope for the token request, the returned access token will not be valid for Bing Ads API access (and no refresh token would be returned by default for the wl.signin scope).

How might this apply to the SDK issue you observed?

Its important to mention that the Bing Ads SDKs do not currently support multiple scopes i.e., internally the SDK sets scope=bingads.manage for authorization code requests, and we do not allow any scope to be set when requesting access tokens. I am making an educated guess in your case that some users granted your app access for another scope first e.g. wl.basic or wl.signin. Then when you attempt to request access tokens by refresh token the SDK a) does not set the scope to bingads.manage, and therefore lets the authorization service default to the original scope e.g., wl.basic or wl.signin and b) does not gracefully handle an empty refresh_token in the response. I can repro the same error that you observed under these conditions.

Instead of depending on a variable default scope, probably the SDK should internally set scope=bingads.manage when requesting access tokens e.g., here. You might prefer this as a workaround in the meantime. For now as a best practice I suggest that you use the SDK end-to-end and only request access tokens via the authorization code that was provisioned via the SDK in the first place i.e., where scope=bingads.manage (only).

jozsi commented 6 years ago

Dear @eric-urban,

Thank you, this helped working around the issue. For some reason, there's an inconsistency on the frontend library that unwillingly appends wl.signin and wl.basic to the requested bingads.manage in some scenarios (when consent dialog appears vs. when it's automatically granted; didn't find a useful force consent parameter) during the authorize phase.

Explicitly providing the bingads.manage scope to the token call resolved the issue!

Thanks again, Józsi