Skip to content

fix: azure keep using upn if exists#2163

Merged
dpgaspar merged 4 commits intomasterfrom
fix/azure-auth
Nov 20, 2023
Merged

fix: azure keep using upn if exists#2163
dpgaspar merged 4 commits intomasterfrom
fix/azure-auth

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

@codecov
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.

@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