AzureAD / azure-activedirectory-library-for-android

The ADAL SDK for Android gives you the ability to add support for Work Accounts to your application with just a few lines of additional code. This SDK gives your application the full functionality of Microsoft Azure AD, including industry standard protocol support for OAuth2, Web API integration with user level consent, and two factor authentication support.
http://www.windowsazure.com/en-us/services/active-directory/
MIT License
177 stars 108 forks source link

ADAL creates OAuth token for unauthorized user #1362

Open fowlerp-qlik opened 6 years ago

fowlerp-qlik commented 6 years ago

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 commented 6 years ago

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

fowlerp-qlik commented 6 years ago

It was a webapi published by me.

shoatman commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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

shoatman commented 6 years ago

@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