microsoft / azurechat

🤖 💼 Azure Chat Solution Accelerator powered by Azure Open AI Service
MIT License
1.25k stars 1.22k forks source link

Fix sign-in loop when email is not set in Azure AD account #430

Open heguro opened 2 months ago

heguro commented 2 months ago

Fix #273, May related: #408, #418

The profile object returned when signing in from Azure AD may be missing the email property. In this case, the following will occur.

  1. The following code will fail on profile.email.toLowerCase(), so AzureADProvider doesn't return profile and cause an sign-in loop.
  2. The email property is used in the userHashedId function to identify users in the database, but if the email property is undefined, the hashing fails.

In this PR, if the email property is missing, the preferred_username property, which contains the user principal name, is used as the email instead.

(Note: Personally, I believe using an immutable identifier like sub for userHashedId would be more robust than using email or preferred_username (which Microsoft's documentation states are mutable). However, implementing this idea is not included in this PR to maintain compatibility with previously stored data.)

yasir-fayrooz commented 2 months ago

+1 on this. Was about to make the same PR

bwitzig-zen commented 2 months ago

+1 for this request.

Some orgs don't have the email field setup in O365 and this will allow them to not require an additional step for onboarding the app

oliverlabs commented 1 month ago

@thivy - this needs to be merged to main.