feat: use issuer url instead of well known url#1371
Conversation
ZohebShaikh
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1371 +/- ##
==========================================
+ Coverage 95.91% 95.93% +0.01%
==========================================
Files 44 44
Lines 3255 3269 +14
==========================================
+ Hits 3122 3136 +14
Misses 133 133 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ZohebShaikh
changed the title
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
I like this change - it makes the auth config much clearer, thanks.
That said, this is a breaking change that will need every beamline to update its config and for GDA to support the new API. We have a list of breaking issues that are waiting for a 2.0 release so it might be worth adding an issue there for this change and keeping this PR around until we decide to break everything at once.
| def _config_from_oidc_url(self) -> dict[str, Any]: | ||
| response: requests.Response = requests.get(self.well_known_url) | ||
| response: requests.Response = requests.get( | ||
| self.issuer + "/.well-known/openid-configuration" |
There was a problem hiding this comment.
May be worth calling out that this address must exist, that may determine what the "issuer" is.
There was a problem hiding this comment.
Spec for this is here as a MUST so we can probably rely on it.
OpenID Providers supporting Discovery MUST make a JSON document available at the path formed by concatenating the string /.well-known/openid-configuration to the Issuer
|
@tpoliaw I have made it backwards compatible so in v2 we can remove the backwards compatibility. |
|
@tpoliaw Can you have a look at this ? |
3 participants
No description provided.