Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADAL creates OAuth token for unauthorized user #1362

Open
fowlerp-qlik opened this issue Nov 7, 2018 · 10 comments
Open

ADAL creates OAuth token for unauthorized user #1362

fowlerp-qlik opened this issue Nov 7, 2018 · 10 comments
Assignees
Labels
Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue More Info Additional information is required to diagnose, troubleshoot, or confirm

Comments

@fowlerp-qlik
Copy link

fowlerp-qlik commented Nov 7, 2018

A tester performed a (negative) test where he attempted to login via ADAL on Android with an account that is not authorized to use the webapi. More specifically here is the flow:

Our code calls mAuthenticationContext.acquireToken(mActivity, url, clientId, mAuthenticationContext.getRedirectUriForBroker(), loginHint ,PromptBehavior.Auto, null, mCallback); where the url is equal to the Azure webapi we are trying to use
Tester is asked to pick an account via MS Authenticator or Company Portal. He purposely picks an account that is not authorized within Azure to access the webapi
Even so we receive an OAuth token and use that token to make HTTP GET requests but these fail with HTTP 403:

2018-10-31 10:01:30,647-DEBUG response to request https://mytenant.msappproxy.net/win received HTTP status code 403 =>static void ApiHandler::finishedSlot(QNetworkReply *)(503) [0x103b9eb80]<=

The thing is, even if the Tester kills our App and retries he isn’t given a chance to pick another account since a valid OAuth access token is found in the cache. As a result the Tester is completely stuck.

Since the acquireToken call includes the webapi URL shouldn’t ADAL or MS Authenticator block or deal with this scenario? Is there a way for the Tester to tell Azure/ADAL to “forget” the user?

The above testing was performed with ADAL version 1.14.1

@shoatman
Copy link
Contributor

shoatman commented Nov 7, 2018

@fowlerp-qlik - Was the web api in question the Azure Resource Manager API or a web api published by you?

@fowlerp-qlik
Copy link
Author

It was a webapi published by me.

@shoatman
Copy link
Contributor

shoatman commented Nov 7, 2018

K. So the libraries do not make authorization decisions. AAD, acting as the authorization server for your Web API decides whether or not the user in question should be issued a token.

Anyone can potentially request an access token to any resource protected by AAD within a tenant. With the possible exception being when you tell AAD to only allow assigned users access to your app. See the following doc: https://docs.microsoft.com/en-us/azure/active-directory/manage-apps/methods-for-assigning-users-and-groups

In your case if you want to turn on user assignment being required you'll need to find the enterprise application (service principal) associated with your web api app registration in the azure portal. Select properties and turn on "user assignment required".

@danieldobalian
Copy link
Contributor

danieldobalian commented Nov 7, 2018

This is a challenging case as the web API can deny the request for a variety of reasons. The two common cases are generally:

  1. The user did not have the necessary claims to continue based on some security posture, like MFA. In this case, the API should convey what's necessary through some form. If it's happening as a result of Conditional Access, this is the claims parameter which can be provided to ADAL to get the required claims.
  2. The API has a custom authorization based access control and has blocked the request. If it's the user trying to sign in, then another account with access may be necessary (as you mention in the issue). There's some reliance on the API signaling what went wrong so the client can remedy the situation. Note, in some cases it may choose to not convey this as to not help attackers narrow down what's needed to access the information.

Either way, to answer the specific question of signing in as a new user.

  1. To clear the tokens in the cache.

You can clear the cache of users using the following code:

mAuthContext.getCache().removeAll();

Alternatively, you can maintain the cache and perform a new acquireTokenAsync(...) request passing [PromptBehavior.Always](https://github.com/AzureAD/azure-activedirectory-library-for-android/blob/dev/adal/src/main/java/com/microsoft/aad/adal/PromptBehavior.java#L41). This will bypass any cache entries and force a user to enter their credentials, giving them an opportunity to do a fresh sign in.

Edit: Looks like there was a delay in this post making it's way to Github. Sounds like you're in the 2nd bucket. Definitely follow Shane's guidance and if you still need to clear cache / force sign in again you can take advantage of the code above.

@fowlerp-qlik
Copy link
Author

ok. I turned on "user assignment required" and picked an account that is not authorized. Sure enough I did not get an OAuth token. Instead I get an AuthenticationCallBack.onError callback where the
AuthenticationException.getCode() returns ADALError.AUTH_FAILED_CANCELLED.

Thing is I get the same callback and same error code if a press the back button on the MS Authenticator app to actually cancel the login flow.

So is there a way to distinguish these scenarios since in the latter case I don't want to present an error dialog to the user.

@iambmelt
Copy link
Member

iambmelt commented Nov 8, 2018

Unfortunately, I suspect the answer to this question presently is 'no' based on #726

/cc @danieldobalian @shoatman

Related: #1018 (very stale, was not merged)

@shoatman
Copy link
Contributor

shoatman commented Nov 8, 2018

@fowlerp-qlik - I wanted to confirm what the user experience was in this case. Did you make a silent request or an interactive request? (Did you see UI?) If you saw UI, what was the user presented with when access was denied?

I think there are multiple possible scenarios here....

New request (UI)
Request using existing refresh token (NO UI - Does this return interaction required)
Request using broker SSO (special refresh token) (NO UI - does this return interaction required).

Anyway... i'll add this to my scenario backlog to add tests for.

Thanks

shane

@fowlerp-qlik
Copy link
Author

I made an interactive request. As a result the MS Authenticator App came to the foreground at which point I clicked "ADD ACCOUNT" at which point I added an account that was not authorized to use the webapi in question. MS Authenticator did not present any error dialogs and my App was placed in the foreground at which point I got an OnError() callback but the error was ADALError.AUTH_FAILED_CANCELLED.

But this is the same error code I get if the user presses the back button when in the MS Authenticator App.

I want to display an error dialog in the first scenario but not the second (the user already is well aware that he/she cancelled the login flow explicitly).

Is there any trick that I could use to distinguish between these two scenarios?

@fowlerp-qlik
Copy link
Author

Hi .. Is there any trick that I could use to distinguish between these two scenarios?

@shoatman
Copy link
Contributor

@danieldobalian - I think we need to put this on the backlog for both ADAL & MSAL. @fowlerp-qlik - I'm not aware of a work around myself. I'd confirm that when using acquireTokenSilent that the error messages is likewise as unhelpful. I'd also consider whether it's possible to read from Graph whether the user is assigned to the app... https://developer.microsoft.com/en-us/graph/docs/api-reference/beta/resources/approleassignment

@iambmelt iambmelt added Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue More Info Additional information is required to diagnose, troubleshoot, or confirm and removed Questions labels Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue Triage The engineering team has looked into the issue, understood the issue, labelled/classified the issue More Info Additional information is required to diagnose, troubleshoot, or confirm
Projects
None yet
Development

No branches or pull requests

4 participants