Skip to content

OIDC discovery#11

Open
ski7777 wants to merge 11 commits intothegcat:mainfrom
ski7777:oidc-discovery
Open

OIDC discovery#11
ski7777 wants to merge 11 commits intothegcat:mainfrom
ski7777:oidc-discovery

Conversation

@ski7777
Copy link
Contributor

@ski7777 ski7777 commented Aug 21, 2025

add OIDC discovery functionality. This reduces configuration because all endpoints are discovered automatically based on the issuer

Some testing is still needed. I will convert draft to final later :)

@ski7777 ski7777 marked this pull request as ready for review August 23, 2025 13:37
@ski7777
Copy link
Contributor Author

ski7777 commented Aug 23, 2025

Seems to work. Just one problem:
As far as I understand, the OIDCAuthBackend class is initialized for every request thus we already fetched the OIDC Keys for every request. After my changes and OIDC discovery enabled, we would always run a discovery request. Maybe we should move the initialization, key request and a periodic key refresh to a mechanism outside the class.

Where can we do the initialization of the oic.oic.Client?
If the response with JWTs with a unknows key, the oic KeyJar will automatically fetch new keys. This refetch will not remove revoked keys. So we should register a timer for a periodic overriding refetch:

self.client.keyjar.load_keys(self.client.provider_info, self.client.issuer, replace=True)

How to register a timer for this purpuse? Do you think pretix.base.signals.periodic_task is the right signal for this?

@ski7777
Copy link
Contributor Author

ski7777 commented Sep 9, 2025

I just fixed the merge conflicts :)

@ski7777
Copy link
Contributor Author

ski7777 commented Nov 18, 2025

@thegcat I just updated this branch. There were some bugs but now it should work just filling out the issuer field.
We should still investigate on the other topic. Maybe KIF will be the right time :)

@thegcat
Copy link
Owner

thegcat commented Nov 28, 2025

This looks mostly fine to me, see the comments above.

@ski7777 ski7777 mentioned this pull request Dec 25, 2025
@ski7777
Copy link
Contributor Author

ski7777 commented Feb 18, 2026

Ping

logger.error(
"Please specify jwks_uri in [oidc] section in pretix.cfg or ensure that the issuer supports jwks_uri discovery."
)
self.client.handle_provider_config(op_info, op_info["issuer"])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed this before sorry, but we do handle_provider_config twice now?

@thegcat
Copy link
Owner

thegcat commented Feb 25, 2026

I have a few formatting gripes but I can take care of those myself.

I had a closer look and I'm a bit unsure about the flow now though. Starting on new line 44:

  • if skip_provider_discovery is not True/True-ish we call provider_config to use provider discovery
  • then we unconditionally run handle_provider_config with the provided _endpoint values which may override what was autodiscovered
  • then we handle missing values
  • then we re-run handle_provider_config?

Point 4 is probably a but.

I would make Points 1 and 2 mutually exclusive though? Either autodiscover or use the static configuration? Going one step further I'd also build op_info only in the case we use the static configuration, but I can also change this myself.

provider_config also does not seem to check if the passed URL looks like an URL and might throw a "http_request failed" if the URL is not valid. We might want to check what error the pyoic throws if the issuer URL is malformed/not reachable and if it is "http_request failed" maybe check the URL ourselves first at least syntactically to give a better error.

What do you think? Do you want to tackle those points?

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