feat(authentik-gate-sync): OIDC clients from OIDCClient resources #1535

Merged
k.renner merged 3 commits from feat/gate-sync-oidc into main 2026-11-01 09:59:01 +00:00
Member

authentik-gate-sync learns OIDC clients, declared as an OIDCClient resource next to the app, so apps like YouTrack need no blueprint entries and nothing on their Ingress.

What

  • CRD oidcclients.auth.renner.uno (v1alpha1, namespaced, status subresource) in infrastructure/authentik-gate-sync-crds, like kuma-sync-crds.
    Spec: launchURL, redirectURIs[] {url, matchingMode strict|regex}, optional slug (default metadata.name), group, authorizationFlow.
  • Controller, per OIDCClient:
    • confidential OAuth2/OpenID provider oidc-<slug>: client ID = slug, secret generated by Authentik, grants authorization_code + refresh_token, scopes openid/profile/email, signed with authentik Self-signed Certificate;
    • application <slug> → issuer https://auth.renner.uno/application/o/<slug>/, meta_publisher: authentik-gate-sync;
    • with group: the same group expression policy + binding as gates (the group itself stays managed in Authentik);
    • status: Ready condition with the reason, and issuer.
  • Fail closed and ownership as for gates: a missing group, a foreign application/provider with the name, or two OIDCClients with one slug withhold/remove the client; owned clients whose OIDCClient is gone are swept. Missing CRD = no clients, gates unaffected.
  • RBAC: ClusterRole gets get/list/watch on oidcclients and patch on oidcclients/status; the Authentik role (blueprint gate-sync.yaml) gets authentik_providers_oauth2.{view,add,change,delete}_oauth2provider, view_scopemapping, authentik_crypto.view_certificatekeypair.

Verified

  • python3 -m unittest: 49 tests green (spec parsing/validation, status, creation, idempotent second pass, missing group, foreign app, redirect drift, ownership scan + removal next to a foreign OAuth2 app).
  • API fields/endpoints checked against Authentik 2026.8.4 source (providers/oauth2/api/providers.py, urls.py; client_secret defaults to generate_client_secret, readable with change permission).
  • kubectl kustomize for infrastructure, apps/authentik, the CRD dir; yaml-citations.sh green. Server-side CRD validation not possible with my read-only access.

Not verified until merge: the image against the live API. #1534 (YouTrack) uses this and should merge after it is live.

authentik-gate-sync learns OIDC clients, declared as an `OIDCClient` resource next to the app, so apps like YouTrack need no blueprint entries and nothing on their Ingress. ## What - **CRD** `oidcclients.auth.renner.uno` (v1alpha1, namespaced, status subresource) in `infrastructure/authentik-gate-sync-crds`, like `kuma-sync-crds`. Spec: `launchURL`, `redirectURIs[] {url, matchingMode strict|regex}`, optional `slug` (default `metadata.name`), `group`, `authorizationFlow`. - **Controller**, per OIDCClient: - confidential OAuth2/OpenID provider `oidc-<slug>`: client ID = slug, secret generated by Authentik, grants `authorization_code` + `refresh_token`, scopes openid/profile/email, signed with `authentik Self-signed Certificate`; - application `<slug>` → issuer `https://auth.renner.uno/application/o/<slug>/`, `meta_publisher: authentik-gate-sync`; - with `group`: the same group expression policy + binding as gates (the group itself stays managed in Authentik); - status: `Ready` condition with the reason, and `issuer`. - Fail closed and ownership as for gates: a missing group, a foreign application/provider with the name, or two OIDCClients with one slug withhold/remove the client; owned clients whose OIDCClient is gone are swept. Missing CRD = no clients, gates unaffected. - RBAC: ClusterRole gets get/list/watch on `oidcclients` and patch on `oidcclients/status`; the Authentik role (blueprint `gate-sync.yaml`) gets `authentik_providers_oauth2.{view,add,change,delete}_oauth2provider`, `view_scopemapping`, `authentik_crypto.view_certificatekeypair`. ## Verified - `python3 -m unittest`: 49 tests green (spec parsing/validation, status, creation, idempotent second pass, missing group, foreign app, redirect drift, ownership scan + removal next to a foreign OAuth2 app). - API fields/endpoints checked against Authentik 2026.8.4 source (`providers/oauth2/api/providers.py`, `urls.py`; `client_secret` defaults to `generate_client_secret`, readable with change permission). - `kubectl kustomize` for infrastructure, apps/authentik, the CRD dir; `yaml-citations.sh` green. Server-side CRD validation not possible with my read-only access. Not verified until merge: the image against the live API. #1534 (YouTrack) uses this and should merge after it is live.
hermes-bot added 1 commit 2026-11-01 09:22:27 +00:00
feat(authentik-gate-sync): OIDC clients from auth.renner.uno/oidc-client
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
8739efe026
An Ingress annotated auth.renner.uno/oidc-client: <slug> gets a
confidential OAuth2/OpenID provider (client ID = slug, secret generated by
Authentik), an application with that slug and, with auth.renner.uno/group,
the same group expression policy as gates. auth.renner.uno/oidc-redirect-uri
pins strict redirect URIs on the Ingress host; default is any path on it.

Same fail-closed and ownership rules as gates: owned clients are found via
the OAuth2 provider list and meta_publisher and swept when their Ingress
goes. The service-account role gains the OAuth2 provider permissions plus
view on scope mappings and certificate key pairs.

So apps that log in through Authentik via OIDC (first: YouTrack, #1534)
need no blueprint entries; the groups stay managed in Authentik.
hermes-bot changed title from feat(authentik-gate-sync): OIDC clients from auth.renner.uno/oidc-client to feat(authentik-gate-sync): OIDC clients from OIDCClient resources 2026-11-01 09:31:44 +00:00
hermes-bot added 1 commit 2026-11-01 09:31:45 +00:00
feat(authentik-gate-sync): OIDC clients as OIDCClient resources, not Ingress annotations
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
3e0557cfe8
OIDC is not part of the Ingress. The client is now declared by an
OIDCClient (auth.renner.uno/v1alpha1, CRD in
infrastructure/authentik-gate-sync-crds, like kuma-sync-crds) next to the
app: launchURL, redirectURIs, optional slug, group, authorizationFlow.
The controller writes a Ready condition and the issuer to its status.

The auth.renner.uno/oidc-* Ingress annotations are gone; RBAC gains
get/list/watch on oidcclients and patch on their status.
k.renner requested review from review-bot 2026-11-01 09:38:51 +00:00
review-bot approved these changes 2026-11-01 09:42:47 +00:00
Dismissed
review-bot left a comment
Member

🤖 Automated review — model claude-opus-5-5, commit 3e0557cfe812d377c831ad1e1a61d4485014afa5.

Automated review by the review-bot: it approves only when it found nothing red or yellow, and a human still merges. Request a review from review-bot again for a fresh one.


Summary

This PR teaches authentik-gate-sync to reconcile a new namespaced OIDCClient CRD into a confidential Authentik OAuth2 provider, an application and an optional group policy. It also extends the ownership sweep to OAuth2 providers and adds the RBAC and Authentik permissions the controller needs. The change is well built: it fails closed, refuses foreign applications and providers, detects duplicate slugs and comes with unit tests. I found no verified defect. The main gap is that OIDC slugs and gate names share one namespace and nothing separates them. mergeable is true, and no other open or closed PR covers this work (#1534 depends on it; #1222 was an earlier per-app OIDC change and is closed).

⚪ Unverified

  • OAuth2 field names and the live API. I did not check the grant_types field, the propertymappings/provider/scope/ endpoint, or whether the redirect_uris returned by the API compare equal to the desired body (controller.py:303-313, 547). If any of these differ from what the controller expects, every pass would re-PATCH the provider. The PR says it checked them against the Authentik source, but not against a running instance. After the deploy, check the logs for repeated "updated OAuth2 provider" lines.
  • Signing-key name. The certificate is matched only by its exact name, authentik Self-signed Certificate (controller.py:57, 544). If the instance names it differently, every client stays in "transient" state. That is safe, but worth confirming before #1534 merges.

🟡 Suggestions

  • Gate names and OIDC slugs can collide. Both are application slugs and both go into the same gates set, but ensure_oidc only checks meta_publisher == MANAGER (controller.py:560-562). If an OIDCClient's slug equals an existing gate name (<ns>-<ingress>-<hash6>, controller.py:113-118), two things go wrong:

    • On success, ensure_oidc creates oidc-<slug> and then PATCHes the gate's application so its provider points to the OAuth2 provider (controller.py:585-586). This takes over the gate.
    • On a GateError, gates.discard(slug) (controller.py:703) drops the gate's name, so the sweep deletes the gate (controller.py:725-727).

    The hash suffix makes an accidental collision unlikely, but anyone who can create an OIDCClient in any namespace can cause it on purpose. A cheap guard is to refuse an existing application whose meta_description does not start with OIDCClient (or whose provider is not in providers/oauth2/). Another option is to keep OIDC slugs in their own set and reject a slug that is already in the gates set.

  • Unassigned providers are adopted. If a provider named oidc-<slug> already exists without an application, it is taken over and PATCHed (controller.py:565, 570-577). For gates this is intended, after an interrupted teardown. For OIDC it could also take over a hand-made provider of the same name. Consider requiring that the existing provider was created by this controller, or document this behaviour in the module docstring.

  • Status is patched on every pass. Each OIDCClient's status is written every pass (every 60 s), even when nothing changed (controller.py:710-712). Comparing oidc_status(...) with cr.get("status") first would avoid constant writes to the API server and watch events for anyone watching the resource.

🟢 Nits

  • The removal log line still says "removed gate from Authentik" for OIDC clients too (controller.py:615).
  • The CRD's slug pattern ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$ plus maxLength: 50 matches SLUG_RE, but a default slug taken from metadata.name is only checked by the controller. That is fine, but the CRD description could say so.

Tests

The new tests cover spec parsing and validation, status and transition-time stability, creation, a second idempotent pass, a missing group, a foreign application, redirect drift, and the ownership scan plus removal next to a foreign OAuth2 app. No test drives reconcile() with OIDC clients, so several paths are untested:

  • a duplicate slug;
  • the transient path keeping the client;
  • the sweep of an OIDC client whose CR is gone;
  • an OIDCClient slug that collides with a gate name (see the first suggestion).

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `claude-opus-5-5`, commit `3e0557cfe812d377c831ad1e1a61d4485014afa5`. > Automated review by the `review-bot`: it approves only when it found nothing red or yellow, and a human still merges. Request a review from `review-bot` again for a fresh one. --- ## Summary This PR teaches authentik-gate-sync to reconcile a new namespaced `OIDCClient` CRD into a confidential Authentik OAuth2 provider, an application and an optional group policy. It also extends the ownership sweep to OAuth2 providers and adds the RBAC and Authentik permissions the controller needs. The change is well built: it fails closed, refuses foreign applications and providers, detects duplicate slugs and comes with unit tests. I found no verified defect. The main gap is that OIDC slugs and gate names share one namespace and nothing separates them. `mergeable` is true, and no other open or closed PR covers this work (#1534 depends on it; #1222 was an earlier per-app OIDC change and is closed). ## ⚪ Unverified - **OAuth2 field names and the live API.** I did not check the `grant_types` field, the `propertymappings/provider/scope/` endpoint, or whether the `redirect_uris` returned by the API compare equal to the desired body (`controller.py:303-313`, `547`). If any of these differ from what the controller expects, every pass would re-PATCH the provider. The PR says it checked them against the Authentik source, but not against a running instance. After the deploy, check the logs for repeated "updated OAuth2 provider" lines. - **Signing-key name.** The certificate is matched only by its exact name, `authentik Self-signed Certificate` (`controller.py:57`, `544`). If the instance names it differently, every client stays in "transient" state. That is safe, but worth confirming before #1534 merges. ## 🟡 Suggestions - **Gate names and OIDC slugs can collide.** Both are application slugs and both go into the same `gates` set, but `ensure_oidc` only checks `meta_publisher == MANAGER` (`controller.py:560-562`). If an OIDCClient's `slug` equals an existing gate name (`<ns>-<ingress>-<hash6>`, `controller.py:113-118`), two things go wrong: - On success, `ensure_oidc` creates `oidc-<slug>` and then PATCHes the gate's application so its `provider` points to the OAuth2 provider (`controller.py:585-586`). This takes over the gate. - On a `GateError`, `gates.discard(slug)` (`controller.py:703`) drops the gate's name, so the sweep deletes the gate (`controller.py:725-727`). The hash suffix makes an accidental collision unlikely, but anyone who can create an OIDCClient in any namespace can cause it on purpose. A cheap guard is to refuse an existing application whose `meta_description` does not start with `OIDCClient ` (or whose provider is not in `providers/oauth2/`). Another option is to keep OIDC slugs in their own set and reject a slug that is already in the gates set. - **Unassigned providers are adopted.** If a provider named `oidc-<slug>` already exists without an application, it is taken over and PATCHed (`controller.py:565`, `570-577`). For gates this is intended, after an interrupted teardown. For OIDC it could also take over a hand-made provider of the same name. Consider requiring that the existing provider was created by this controller, or document this behaviour in the module docstring. - **Status is patched on every pass.** Each OIDCClient's status is written every pass (every 60 s), even when nothing changed (`controller.py:710-712`). Comparing `oidc_status(...)` with `cr.get("status")` first would avoid constant writes to the API server and watch events for anyone watching the resource. ## 🟢 Nits - The removal log line still says "removed gate from Authentik" for OIDC clients too (`controller.py:615`). - The CRD's slug pattern `^[a-z0-9]([a-z0-9-]*[a-z0-9])?$` plus `maxLength: 50` matches `SLUG_RE`, but a default slug taken from `metadata.name` is only checked by the controller. That is fine, but the CRD description could say so. ## Tests The new tests cover spec parsing and validation, status and transition-time stability, creation, a second idempotent pass, a missing group, a foreign application, redirect drift, and the ownership scan plus removal next to a foreign OAuth2 app. No test drives `reconcile()` with OIDC clients, so several paths are untested: - a duplicate slug; - the transient path keeping the client; - the sweep of an OIDC client whose CR is gone; - an OIDCClient slug that collides with a gate name (see the first suggestion). ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=3e0557cfe812d377c831ad1e1a61d4485014afa5 -->
Author
Member

Addressed in the latest commit:

  • Live API comparison: checked. RedirectURISerializer (2026.8.4) also returns redirect_uri_type (default authorization), so the provider would have been re-PATCHed on every pass. The desired body now includes it. grant_types is a serializer field, and propertymappings/provider/scope is registered in providers/oauth2/urls.py.
  • Signing key: authentik Self-signed Certificate is the key the four existing OAuth2 providers in the blueprint use, so it exists live.
  • Gate/OIDC collision: fixed in three places. A slug equal to a gate name is refused. An existing application without this controller's oidc-<slug> provider is refused, so no re-pointing. The sweep keeps proxy and OAuth2 applications in separate sets.
  • Unassigned provider: adoption stays as it is, because teardown deletes the application first. It is now documented in the module docstring.
  • Status: it is written only when it changed.
  • Nits: the removal log now says "OIDC client", and the CRD slug description mentions the controller-only check.
  • Tests: reconcile() takes injectable clients. There are now loop tests for create-then-quiet (no API calls or status writes on pass 2), duplicate slug, the transient path keeping the client, the sweep after the CR is gone, and a slug equal to a gate name leaving the gate untouched. 55 tests pass.
Addressed in the latest commit: - **Live API comparison:** checked. `RedirectURISerializer` (2026.8.4) also returns `redirect_uri_type` (default `authorization`), so the provider would have been re-PATCHed on every pass. The desired body now includes it. `grant_types` is a serializer field, and `propertymappings/provider/scope` is registered in `providers/oauth2/urls.py`. - **Signing key:** `authentik Self-signed Certificate` is the key the four existing OAuth2 providers in the blueprint use, so it exists live. - **Gate/OIDC collision:** fixed in three places. A slug equal to a gate name is refused. An existing application without this controller's `oidc-<slug>` provider is refused, so no re-pointing. The sweep keeps proxy and OAuth2 applications in separate sets. - **Unassigned provider:** adoption stays as it is, because teardown deletes the application first. It is now documented in the module docstring. - **Status:** it is written only when it changed. - **Nits:** the removal log now says "OIDC client", and the CRD slug description mentions the controller-only check. - **Tests:** `reconcile()` takes injectable clients. There are now loop tests for create-then-quiet (no API calls or status writes on pass 2), duplicate slug, the transient path keeping the client, the sweep after the CR is gone, and a slug equal to a gate name leaving the gate untouched. 55 tests pass.
hermes-bot requested review from review-bot 2026-11-01 09:49:34 +00:00
hermes-bot added 1 commit 2026-11-01 09:49:35 +00:00
fix(authentik-gate-sync): review of the OIDC clients
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
f76defb1e1
- Desired redirect URIs carry redirect_uri_type: authorization, which the
  API returns on every entry (RedirectURISerializer default in 2026.8.4);
  without it every pass would re-PATCH the provider.
- OIDC slugs and gate names are kept apart: a slug that is a gate name is
  refused, an existing application without this controller's OAuth2
  provider is refused, and the sweep keeps proxy and OAuth2 applications
  by their own sets, so an OIDCClient can neither take over nor delete a
  gate.
- Status is written only when it changed.
- Adoption of an unassigned oidc-<slug> provider is documented; removal
  logs say OIDC client.
- reconcile() takes k8s/ak for tests; new loop tests: create then quiet,
  duplicate slug, transient error keeps the client, sweep after the
  resource is gone, slug equal to a gate name.
hermes-bot dismissed review-bot's review 2026-11-01 09:49:35 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

review-bot approved these changes 2026-11-01 09:52:15 +00:00
review-bot left a comment
Member

🤖 Automated review — model claude-opus-5-5, commit f76defb1e1688a0c757cad51ab7a4374aa1dc7fe.

Automated review by the review-bot: it approves only when it found nothing red or yellow, and a human still merges. Request a review from review-bot again for a fresh one.


Summary

Since my review at 3e0557cf, one commit (f76defb1) has been pushed. It answers every earlier point:

  • Gate/OIDC slug collision (🟡): fixed. reconcile() now refuses a slug that equals a gate name (controller.py:704-705). ensure_oidc refuses an existing application unless it already points at this controller's oidc-<slug> provider (controller.py:570-572). The sweep checks OAuth2 applications against keep_clients and proxy applications against gates, so a failing OIDCClient can no longer remove a gate's name (controller.py:741-742).
  • Adoption of an unassigned oidc-<slug> provider (🟡): the author kept this on purpose, because teardown deletes the application first, and documented it in the module docstring (lines 27-29). I consider it closed.
  • Status written on every pass (🟡): fixed. Status is now patched only when oidc_status(...) differs from the stored status (controller.py:726-727). Unchanged conditions keep their lastTransitionTime, so the comparison holds.
  • Nits: both fixed. The removal log line now says "OIDC client", and the CRD slug description says the default slug is checked only by the controller.
  • Re-PATCH on every pass (earlier ⚪): handled by adding redirect_uri_type: "authorization" to the desired redirect entries (controller.py:268-269).

The new code has no blocking problems. mergeable is true.

⚪ Unverified

  • redirect_uri_type in API responses. The fix relies on the Authentik serializer returning redirect_uri_type on every redirect entry. That is the author's reading of the 2026.8.4 source, and I did not check it against the source or a running instance. If the field were missing from responses, the only effect would be a PATCH on every pass. After the deploy, check that "updated OAuth2 provider" is not logged on every pass.

🟢 Nits

  • The second half of the elif in ensure_oidc can no longer be reached. The new check just above it already raises whenever the application exists and does not point at the provider:
    (app is not None and app.get("provider") != provider["pk"])
    
    The clause could be dropped.
  • An empty line was doubled before test_refuses_application_without_its_provider in test_controller.py.

Tests

The new ReconcileLoop tests drive reconcile() end to end through the injected FakeK8s and FakeAuthentik. They cover:

  • create, then a quiet second pass with no API calls and no status writes;
  • a duplicate slug withholding both clients;
  • a transient error keeping the client;
  • the sweep after the OIDCClient is gone;
  • a slug equal to a gate name leaving the gate untouched.

test_refuses_application_without_its_provider covers the no-re-pointing guard. Together these cover every test gap I listed in the earlier review.

Recommendation

Looks reasonable — no blocking issues; comments above are optional. The earlier review had no 🔴 points, every 🟡 point has been fixed or answered with a reason, and the new commit adds no 🔴.

🤖 **Automated review** — model `claude-opus-5-5`, commit `f76defb1e1688a0c757cad51ab7a4374aa1dc7fe`. > Automated review by the `review-bot`: it approves only when it found nothing red or yellow, and a human still merges. Request a review from `review-bot` again for a fresh one. --- ## Summary Since my review at `3e0557cf`, one commit (`f76defb1`) has been pushed. It answers every earlier point: - **Gate/OIDC slug collision (🟡):** fixed. `reconcile()` now refuses a slug that equals a gate name (`controller.py:704-705`). `ensure_oidc` refuses an existing application unless it already points at this controller's `oidc-<slug>` provider (`controller.py:570-572`). The sweep checks OAuth2 applications against `keep_clients` and proxy applications against `gates`, so a failing OIDCClient can no longer remove a gate's name (`controller.py:741-742`). - **Adoption of an unassigned `oidc-<slug>` provider (🟡):** the author kept this on purpose, because teardown deletes the application first, and documented it in the module docstring (lines 27-29). I consider it closed. - **Status written on every pass (🟡):** fixed. Status is now patched only when `oidc_status(...)` differs from the stored status (`controller.py:726-727`). Unchanged conditions keep their `lastTransitionTime`, so the comparison holds. - **Nits:** both fixed. The removal log line now says "OIDC client", and the CRD slug description says the default slug is checked only by the controller. - **Re-PATCH on every pass (earlier ⚪):** handled by adding `redirect_uri_type: "authorization"` to the desired redirect entries (`controller.py:268-269`). The new code has no blocking problems. `mergeable` is true. ## ⚪ Unverified - **`redirect_uri_type` in API responses.** The fix relies on the Authentik serializer returning `redirect_uri_type` on every redirect entry. That is the author's reading of the 2026.8.4 source, and I did not check it against the source or a running instance. If the field were missing from responses, the only effect would be a PATCH on every pass. After the deploy, check that "updated OAuth2 provider" is not logged on every pass. ## 🟢 Nits - The second half of the `elif` in `ensure_oidc` can no longer be reached. The new check just above it already raises whenever the application exists and does not point at the provider: ``` (app is not None and app.get("provider") != provider["pk"]) ``` The clause could be dropped. - An empty line was doubled before `test_refuses_application_without_its_provider` in `test_controller.py`. ## Tests The new `ReconcileLoop` tests drive `reconcile()` end to end through the injected `FakeK8s` and `FakeAuthentik`. They cover: - create, then a quiet second pass with no API calls and no status writes; - a duplicate slug withholding both clients; - a transient error keeping the client; - the sweep after the OIDCClient is gone; - a slug equal to a gate name leaving the gate untouched. `test_refuses_application_without_its_provider` covers the no-re-pointing guard. Together these cover every test gap I listed in the earlier review. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. The earlier review had no 🔴 points, every 🟡 point has been fixed or answered with a reason, and the new commit adds no 🔴. <!-- pr-review:sha=f76defb1e1688a0c757cad51ab7a4374aa1dc7fe -->
k.renner merged commit 09a03dfd76 into main 2026-11-01 09:59:01 +00:00
Sign in to join this conversation.