The necessity of line 51-56 in `lib/Auth/Source/MicrosoftHybridAuth.php`

[iamCristYe]

Recently I noticed that microsoft.name is a duplicated value of microsoft.displayName. Thus I doubt the necessity of line 51-56 in lib/Auth/Source/MicrosoftHybridAuth.php.

After some research, I found out the email and name value in token seems to be duplicate of microsoft.mail and microsoft.displayName. Plus, the original microsoft.mail field got overwritten in this way, so I think it's better to simply remove these lines.

[pradtke]

Hi @iamCristYe , Were you testing with Microsoft Live accounts or Azure AD accounts, and did the Azure AD accounts have office 365 subscriptions? When we wrote the original code we found each of those types of accounts returned the attributes in different places.

[iamCristYe]

@pradtke Hi! Thanks for your quick response. I modified the lines to

        if (array_key_exists('email', $idTokenData)) {
            $state['Attributes'][$prefix . 'email_from_profile'] = [$idTokenData['email']];
        }
        if (array_key_exists('name', $idTokenData)) {
            $state['Attributes'][$prefix . 'name_from_profile'] = [$idTokenData['name']];
        }

Then I tested with several accounts:
(I omitted real names, emails, and ID)

With my personal MS account: (Note that microsoft.mail is not returned.)

microsoft.@odata.context	https://graph.microsoft.com/v1.0/$metadata#users/$entity
microsoft.displayName	(My name)
microsoft.userPrincipalName	(my email)
microsoft.email_from_profile	(same as my userPrincipalName)
microsoft.name_from_profile	(same as microsoft.displayName)

With another testing personal MS account: (microsoft.mail is not returned and this account's primary alias has letters in upper case)

microsoft.@odata.context	https://graph.microsoft.com/v1.0/$metadata#users/$entity
microsoft.displayName	fn ln
microsoft.userPrincipalName	test3eipef@outlook.com (all in lower case)
microsoft.email_from_profile	test3eIPef@outlook.com (note some letters in upper case)
microsoft.name_from_profile	fn ln

(For Personal MS account, testing with https://docs.microsoft.com/en-us/graph/traverse-the-graph#code-try-1 also returned null for mail, but email is in userPrincipalName)

With my school account (This one with Microsoft 365 subscription):

microsoft.@odata.context	https://graph.microsoft.com/v1.0/$metadata#users/$entity
microsoft.displayName	(my name)
microsoft.mail	(same as my userPrincipalName)
microsoft.userPrincipalName	(my email)
microsoft.email_from_profile	(same as my userPrincipalName)
microsoft.name_from_profile	(same as microsoft.displayName)

With another school account (This one without Microsoft 365 subscription):

microsoft.@odata.context	https://graph.microsoft.com/v1.0/$metadata#users/$entity
microsoft.displayName	(my name)
microsoft.mail	(my email, in self-defined format)
microsoft.userPrincipalName	(my email in id format, generated by system based on school id)
microsoft.email_from_profile	(same as microsoft.mail)
microsoft.name_from_profile	(same as microsoft.displayName)

based on the testing result, you can see that name_from_profile always has the same value as microsoft.displayName returned from Microsoft Graph. The email_from_profile would either be the lower case of microsoft.userPrincipalName for personal MS account or microsoft.mail for organization Microsoft accounts.

Thus, I believe the lines are not necessary and the data from Microsoft Graph is enough already.

[pradtke]

Thanks for the details explanation and examples. I'll take a look at removing those as we create the next release for SSP version 2.

[iamCristYe]

It seems that Microsoft is now returning "microsoft.email" for personal Microsoft Accounts:
{"microsoft.@odata.context":["https:\/\/graph.microsoft.com\/v1.0\/$metadata#users\/$entity"],"microsoft.displayName":["fn ln"],"microsoft.surname":["ln"],"microsoft.givenName":["fn"],"microsoft.id":["baa3d4f918811ea8"],"microsoft.userPrincipalName":["test3eipef@outlook.com"],"microsoft.businessPhones":[],"microsoft.mail":["test3eIPef@outlook.com"],"microsoft.name":["fn ln"]}

Edit: I just realized it's there just because of line 53...