search

Azure-Samples / ms-identity-javascript-angular-spa-aspnet-webapi-multitenant

Multi-tenancy tutorial demonstrating how to expose your app to users from other tenants, provide consent as admin and deploy it on Azure App Services using MSAL Angular
17 stars 7 forks source link

Chapter 2: Consent for fetching tasks does not work #10

Closed mrtes closed 3 years ago

mrtes commented 3 years ago

Please follow the issue template below. Failure to do so will result in a delay in answering your question.

Library

Description

Hello I'm currently trying to do the sample you provided and I have an issue in Chapter 2. I have setup the app registrations exactly according to the guide in the Readme.

It works fine if I try it with a user from the same tenant where I have registered the apps (Tenant A). However if I use a admin-user from another tenant (Tenant B) I have a few issues.

With this admin user I perform the following steps:

I would expect to get another consent screen here to give consent to both the "User.Read.All" scope as well as the API "access_as_user" scope (since I am an admin, I should be able to consent to those). However, i don't see this screen and am redirected to the start page again. I see the following error in the console: image Here the first blacked out segment corresponds to the Application Id URI of my API. The second segment is the Tenant Id of Tenant B.

I have googled for this error and the commonly suggested solution is to add the TodoListSPA ClientId to the "KnownClientApplications" array in the manifest of TodoListAPI. I have entered this value already during the setup of the sample (I double checked if its there).

Afterwards I have tried to do the admin-consent step with the provided button. Here I get a consent screen and after consent everything seems to work. However, the consent screen doesn't show the permission scope of the TodoListApi. The TodoListApi is only mentioned in the text below the permissions list. image

Does this have a relation with the .default scope which is provided for the adminconsent endpoint but not provided when the consent is requested from the MsalGuard?

derisen commented 3 years ago

@mrtes thank you for the detailed explanation.

Does this have a relation with the .default scope which is provided for the adminconsent endpoint but not provided when the consent is requested from the MsalGuard?

This is actually possible, I'll check to make sure. I'm wondering what are the versions of the packages you are using? did you update msal and msal-angular? (newer versions might cause the issue here).

Regardless, I'm investigating the issue, but in the meantime, would it be possible to send me a network trace, using something like Fiddler? You can send it to 'dogan.erisen at gmail.com'. I'll get back as soon as I can.

mrtes commented 3 years ago

Thanks for the quick response. I have not changed the package.json of the example. My installed versions are: @azure/msal-angular: 1.1.2 msal: 1.4.4

I will try to send you a trace later.

derisen commented 3 years ago

@mrtes indeed you're right

Our intention was (apparently) for the admin-user to sign-in first and provide admin consent via the Consent as Admin button (sorry this wasn't clear in README), which as you recognized, uses .default scope to combine SPAs and APIs permissions and triggers admin consent prompt. Going directly to Get my tasks won't do the trick since MsalGuard does not use .default but what is defined in app.module.ts (in protectedResourceMap).

So two alternatives to resolve this:

a) admin-user should use the Consent as Admin button after signing-in b) In app-config.json file (SPA), todoListApi resource scope should be written as api://{client-id}/.default instead of api://{client-id}/access_as_user

Let me know if that actually solves the issue (please delete your service principals first for both tenant A and B before trying). Also, I would appreciate if you tell me which alternative seems more natural here and I will update the sample accordingly (or go ahead and make a PR!)

mrtes commented 3 years ago

So you are saying that for any initial provisioning of the application to TenantB the scopes need to use the .default scope? That is good to know. I thought this was only required for the /adminconsent endpoint.

But this leaves me with a few other questions:

  1. What about provisioning an app as a user account?? According to the docs about converting an app to multi-tenancy this is possible if only delegated permissions are used and if those permissions do not require admin consent. In this case we don't have an admin-consent step. I have tried a setup like this (basically this sample, without the "User.Read.All" scope). When I try to access it as a normal user (or as an admin, doesn't matter) I get the same error as above (AADST650052) as long as I try to use a specific API scope api://{clientId}/{myScope}. However, with the .default scope it works again. Does this mean .default is always required for provisioning?

2) After deleting the service principals in Tenant B I have followed the explore the sample section of Chapter 2 again. This time I have logged in as a normal user. In step 1 you write that the user should press Get my tasks after logging in, and that he will see the "Admin approval required screen". However, in my tests I will see the Error "AADST650052" again, same as with the admin. Has something changed with the way AzureAd handles these things?

3) After I have given admin consent for the sample and try to use the sample as a normal user again, I have the issue that now I get the "Need admin approval" screen. image

Shouldn't I be able to use the sample (or give consent to the remaining scopes) now?

4) You haven't yet commented on the issue I mentioned regarding the admin consent screen. Is it expected that the API Permissions are not shown there? (this only seems to happen during provisioning with the .default scope). In my understanding the consent screen should show all permissions that are required, in this case User.Read, User.Read.All and api://{client-id}/access_as_user

Regarding your question, I would find it more natural if the admin consent step would be carried out before/during the login step. In this case, the user cannot run into the issue that some part of the application is forbidden to him due to lacking admin consent, but he could still decide if he wants to give permissions himself.

Sorry for the barrage of questions. I hope you can help me understand this better :)

derisen commented 3 years ago

no worries : )

So you are saying that for any initial provisioning of the application to TenantB the scopes need to use the .default scope? That is good to know. I thought this was only required for the /adminconsent endpoint.

Not quite. I mean the point is providing admin consent. The .default scope just has the behaviour to trigger admin consent. So indirectly yes.

But this leaves me with a few other questions:

  1. What about provisioning an app as a user account?? According to the docs about converting an app to multi-tenancy this is possible if only delegated permissions are used and if those permissions do not require admin consent. In this case we don't have an admin-consent step. I have tried a setup like this (basically this sample, without the "User.Read.All" scope). When I try to access it as a normal user (or as an admin, doesn't matter) I get the same error as above (AADST650052) as long as I try to use a specific API scope api://{clientId}/{myScope}. However, with the .default scope it works again. Does this mean .default is always required for provisioning?

  2. After deleting the service principals in Tenant B I have followed the explore the sample section of Chapter 2 again. This time I have logged in as a normal user. In step 1 you write that the user should press Get my tasks after logging in, and that he will see the "Admin approval required screen". However, in my tests I will see the Error "AADST650052" again, same as with the admin. Has something changed with the way AzureAd handles these things?

  3. After I have given admin consent for the sample and try to use the sample as a normal user again, I have the issue that now I get the "Need admin approval" screen. image

Shouldn't I be able to use the sample (or give consent to the remaining scopes) now?

It's a bit of a chicken-egg problem. The web api needs to know the client app calling it, so that if the user consents to client, it means consenting to web api as well. That is defined in the service principal (knownClientApplications). But before consenting to web api, there is no service principal provisioned (in tenant B). So one way to provision in such cases (i.e. customer tenants) is using admin consent, which allows admin to 'sign-up' for that service. Another way is presenting a sign-in/sign-up screen for the web api mentioned in the docs here

  1. You haven't yet commented on the issue I mentioned regarding the admin consent screen. Is it expected that the API Permissions are not shown there? (this only seems to happen during provisioning with the .default scope). In my understanding the consent screen should show all permissions that are required, in this case User.Read, User.Read.All and api://{client-id}/access_as_user

I've seen that behavior before with the /.default scope. Let me ask around and confirm this. Please also send me a network trace if you can.

Regarding your question, I would find it more natural if the admin consent step would be carried out before/during the login step. In this case, the user cannot run into the issue that some part of the application is forbidden to him due to lacking admin consent, but he could still decide if he wants to give permissions himself.

Thanks a lot! I'll make an update to the code.

Sorry for the barrage of questions. I hope you can help me understand this better :)

mrtes commented 3 years ago

I have shared you a fiddler session. I hope that helps with investigating my issue. Please contact me if you need more information.

It's a bit of a chicken-egg problem. The web api needs to know the client app calling it, so that if the user consents to client, it means consenting to web api as well. That is defined in the service principal (knownClientApplications). But before consenting to web api, there is no service principal provisioned (in tenant B). So one way to provision in such cases (i.e. customer tenants) is using admin consent, which allows admin to 'sign-up' for that service. Another way is presenting a sign-in/sign-up screen for the web api mentioned in the docs here

I think what confuses me is also why this only seems to work when I'm using the .default. According to the docs I should be able to provide dynamic scopes. But this doesn't work in neither this sample nor in other similar setups I have tried. Does this have something to do with the v1/v2 Endpoint behavior that is mentioned there?

derisen commented 3 years ago

@mrtes got the session, thanks. Just letting you know that I'm running the issue around and it might take some time. I'll get back as soon as I can.

derisen commented 3 years ago

@mrtes sorry this took a bit long, but I got the confirmation. Getting back to your 4th question

  1. You haven't yet commented on the issue I mentioned regarding the admin consent screen. Is it expected that the API Permissions are not shown there? (this only seems to happen during provisioning with the .default scope). In my understanding the consent screen should show all permissions that are required, in this case User.Read, User.Read.All and api://{client-id}/access_as_user

In this case, the specific permissions for the web API (access_as_user) not showing up on the consent screen is expected. The rationale is that, this is considered as an implementation detail between the client app and the service app, and so the users are not presented with the consent text for this particular scope. There will be, however, a line of text below that says "if you accept, {YOUR_WEB_API'S_NAME_HERE} will also have access to your user profile information"

Also, we have a pending PR that addresses the consent issue you mentioned and it will also replace the samples to use auth code flow with PKCE instead of implicit flow using the newer msal-angular (v2).

Let me know if there's anything else I can help with.

mrtes commented 3 years ago

Sorry, I kinda missed the notifications in my Mail.

In this case, the specific permissions for the web API (access_as_user) not showing up on the consent screen is expected. The rationale is that, this is considered as an implementation detail between the client app and the service app, and so the users are not presented with the consent text for this particular scope. There will be, however, a line of text below that says "if you accept, {YOUR_WEB_API'S_NAME_HERE} will also have access to your user profile information"

Ok, I was just confused there, because if you are in a single tenant scenario (user is from the same tenant as the application) you seem to see the API permission in the consent screen. I just expected it would be the same for a multitenant scenario.

What about additional permissions that are requested only by the backend? Would those be shown in the consent screen (e.g. offline_access to your Graph, or something similar).

I will check out the new version. According to the msal github, the implementation of msal-angular (v2) is still in alpha. Do you think it is already useable in a production scenario?

Thank you for the answers and your work regarding this issue, as well as the whole repository. It has been a huge help for me!

derisen commented 3 years ago

@mrtes no worries : )

What about additional permissions that are requested only by the backend? Would those be shown in the consent screen (e.g. offline_access to your Graph, or something similar).

I'm looking at my test here and yes, they should show up. Let me know if it doesn't.

I will check out the new version. According to the msal github, the implementation of msal-angular (v2) is still in alpha. Do you think it is already useable in a production scenario?

It's coming out for "public preview" in a few weeks, but that's still not recommended for production yet. Watch the roadmap here.