chore: change User.authProvider to User.jwtIssuer #1749
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1749 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 165 165
Lines 5137 5136 -1
=========================================
- Hits 5137 5136 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| const user = User( | ||
| id: 42, | ||
| email: email, | ||
| jwtIssuer: googleJwtIssuer, |
There was a problem hiding this comment.
We're probably going to end up with these being enums and just getting the .toJson value out of them as a url for the db, rather than using strings inside our models. 🤷
There was a problem hiding this comment.
I don't think it's required that the iss value needs to be a well-formed URL (see https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.1).
| if (payload.iss.startsWith('https://login.microsoftonline.com')) { | ||
| return MicrosoftAuthProvider(); | ||
| } else if (payload.iss == 'https://accounts.google.com') { | ||
| if (googleJwtIssuerRegexp.hasMatch(payload.iss)) { |
There was a problem hiding this comment.
Instead of having this thing do the iss -> auth provider, you might consider having a middle step which is iss string -> enum -> auth-provider. Then the string -> enum/object could hide all the regexp goop. 🤷
There was a problem hiding this comment.
Regex goop removed, although I'm not sure that removing it addresses this comment.
I think our VC convo got cut short as you were explaining this, but I'm not sure I understand how adding an intermediate step would make this cleaner.
Description
Because JWT issuers can vary between Azure tenants, we're recording the full issuer instead of simply that it was Microsoft.
Type of Change