search

MicrosoftDocs / azure-docs

Open source documentation of Microsoft Azure
https://docs.microsoft.com/azure
Creative Commons Attribution 4.0 International
10.28k stars 21.45k forks source link

"upn" is not always included in v1.0 claims #41385

Closed jaffadog closed 3 years ago

jaffadog commented 5 years ago

If the user is a guest rather than a member, then the v1.0 claim will not include "upn".

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

frankhu-2021 commented 5 years ago

@jaffadog Thanks for your feedback! We will investigate and update as appropriate.

jaffadog commented 5 years ago

On further investigation, I found that i was able to make the "upn" claim be emitted by using the "include_externally_authenticated_upn" and "include_externally_authenticated_upn_without_hash" options. But this statement is not accurate:

"Although this claim is automatically included, you can specify it as an optional claim to attach additional properties to modify its behavior in the guest user case."

It may be a bug that "upn" is not emitted at all for guests... I'd actually love for the unmangled UPN to be emitted for guests by default if neither of the above additionalProperties options are invoked. i.e. foo@hometenant.com. And the switches permit picking alternate forms of the UPN.

SaurabhSharma-MSFT commented 5 years ago

@jaffadog Thanks for the feedback ! I have assigned this issue to content author to investigate and update the document as appropriate.

rwike77 commented 4 years ago

@paulgarn @hpsin can you comment on this? Thanks.

jdelforno commented 4 years ago

AAD Guest, UPN is emitted without #EXT#.onmicrosoft.com External Guest, UPN is emitted with #EXT#.onmicrosoft.com

It'd be great if the UPN was as is from Get-MsolUser and uniform. You get what you see kind of thinking.

Please also note that during User Provisioning from Enterprise Applications, the UPN is set without #EXT# / EXT regardless of what you've updated in the manifest file. This means, the UPN you register a user with will be different than the UPN you use with the Optional Claim - assuming you make use of UPN for user registration, which a lot of the default applications do.

This is a serious issue where Salesforce SSO is concerned as SalesForce Cloud has a single user database, no duplicates, all unique. If you register with your.name@company.com, you can't re-register under a different instance with Salesforce using that email address and because the UPN omits the #EXT#.onmicrosoft.com (As you see form Get-MsolUser), you effectively need to apply a transformation for both the Attribute Mapping and the SAML Claim (via manifest file).

Rather ugly but it's workable.

mmacy commented 3 years ago

@paulgarn @hpsin can you comment on this? Thanks.

Reassigning to @hpsin for visibility: #reassign:hpsin

Hirsch, let me know if there are any doc updates required here, happy to help.

hpsin commented 3 years ago

Thanks @mmacy

UPN is not issued for guests (external identities, those from another tenant) by default. You must use the optional claim in order to have it emitted, and it will be emitted with the mangled name. Certainly we cannot change the default behavior, as this will cause issues for code like "if (token.upn==null)". Additionally, we're wary of issuing it without mangling for a couple reasons:

  1. Apps that aren't aware of B2B will accept tokens for User A@Bar in Tenant Foo as if they're users in Tenant Bar. This is a security issue that we want to prevent, as it exposes Tenant Bar data to Tenant Foo - this is a data exfiltration concern amongst others. While making it a configurable option makes it more likely that apps will consider B2B while configuring it, what we've seen with similar features is that developers say "the claim looks wrong, and my API stops returning errors when I set this option" -> and now you have a security issue.

  2. We'd like to not make further investments in the fragile UPN claim. UPNs are mutable, and apps that use the UPN for ACLs are going to break when you change a user's UPN. Doubling down on the UPN claim (to make it usable in scenarios as if it wasn't for a guest user) runs counter to our goals here.

I'll update the upn claim in the access/ID/optional claims docs to make this more clear.

mmacy commented 3 years ago

@jaffadog @hpsin Moving another stale issue (☢️ 490 days) to Azure DevOps work item backlog for review in recurring sprint triage. We'll move on it as appropriate and resolve when possible.

ID Title Iteration Path Assigned To
1308773 [ISSUE][41385] "upn" is not always included in v1.0 claims Technical Content\Backlog Hirsch Singhal

please-close