feat: use issuer url instead of well known url#1371
Conversation
ZohebShaikh
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1371 +/- ##
==========================================
- Coverage 95.86% 95.85% -0.02%
==========================================
Files 44 44
Lines 3292 3306 +14
==========================================
+ Hits 3156 3169 +13
- Misses 136 137 +1 ☔ 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 ? |
| ) | ||
|
|
||
| @model_validator(mode="after") | ||
| def check_well_know_urls(self) -> Self: |
There was a problem hiding this comment.
| def check_well_know_urls(self) -> Self: | |
| def check_urls(self) -> Self: |
| if self.well_known_url: | ||
| LOGGER.warning( | ||
| DeprecationWarning( | ||
| "well_known_url and issuer both are set. " |
There was a problem hiding this comment.
| "well_known_url and issuer both are set. " | |
| "well_known_url and issuer are both set. " |
| ) | ||
|
|
||
|
|
||
| def test_oidc_config_urls(): |
3 participants
No description provided.