Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: azure keep using upn if exists #2163

Merged
merged 4 commits into from
Nov 20, 2023
Merged

fix: azure keep using upn if exists #2163

merged 4 commits into from
Nov 20, 2023

Conversation

dpgaspar
Copy link
Owner

@dpgaspar dpgaspar commented Nov 20, 2023

Description

Keep backward compatibility with Azure, caused by this breaking change: #2121

although id_token does not include an upn field, it can optionally be included.

https://learn.microsoft.com/en-us/entra/identity-platform/id-token-claims-reference

On this change we use upn if it exists else use email

ADDITIONAL INFORMATION

  • Has associated issue:
  • Is CRUD MVC related.
  • Is Auth, RBAC security related.
  • Changes the security db schema.
  • Introduces new feature
  • Removes existing feature

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f591ee5) 74.50% compared to head (ee14539) 73.84%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2163      +/-   ##
==========================================
- Coverage   74.50%   73.84%   -0.66%     
==========================================
  Files          72       72              
  Lines        8970     8695     -275     
==========================================
- Hits         6683     6421     -262     
+ Misses       2287     2274      -13     
Flag Coverage Δ
python 73.84% <ø> (-0.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dpgaspar dpgaspar merged commit 05e518e into master Nov 20, 2023
13 of 14 checks passed
@dpgaspar dpgaspar deleted the fix/azure-auth branch November 20, 2023 12:38
@hterik
Copy link

hterik commented Dec 1, 2023

Thanks for the fix. Do you happen to know under which conditions Azure choose to include email vs upn fields? It seems odd that it is some times used and other times not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants