From 6d712765139eb2f39c8041b39a5310b58cba7c00 Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Mon, 27 Jul 2026 07:27:54 -0700 Subject: [PATCH] Auth: pass the OIDC issuer through untouched, trailing slash and all Deploying against the real Authentik turned this up immediately: its issuer ends in a slash, OIDC requires the discovered issuer to match the configured one byte-for-byte, and trimming it made discovery fail every time. The stub in the tests happened to advertise a slashless issuer, so the whole suite passed while the only provider Petal actually talks to could not be reached. The stub now takes its issuer as a knob, and a regression test runs the flow against one that ends in a slash. Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua --- internal/auth/oidc.go | 6 +++++- internal/auth/oidc_test.go | 43 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/internal/auth/oidc.go b/internal/auth/oidc.go index 0c9d95b..aa07ead 100644 --- a/internal/auth/oidc.go +++ b/internal/auth/oidc.go @@ -91,7 +91,11 @@ func (o *OIDC) discover(ctx context.Context) error { if o.provider != nil { return nil } - provider, err := oidc.NewProvider(ctx, strings.TrimSuffix(o.opts.IssuerURL, "/")) + // The issuer is passed through exactly as configured, trailing slash and + // all: OIDC requires the discovered issuer to match the requested one + // byte-for-byte, and Authentik's ends in a slash. (go-oidc trims it itself + // when building the .well-known URL, so a slash here costs nothing.) + provider, err := oidc.NewProvider(ctx, o.opts.IssuerURL) if err != nil { return err } diff --git a/internal/auth/oidc_test.go b/internal/auth/oidc_test.go index 87d914f..5965964 100644 --- a/internal/auth/oidc_test.go +++ b/internal/auth/oidc_test.go @@ -29,6 +29,10 @@ type stubIdP struct { *httptest.Server key *rsa.PrivateKey clientID string + // issuer as advertised by discovery and asserted in tokens. Defaults to the + // server's URL; a test can give it a trailing slash, which is what Authentik + // does and which OIDC requires to match byte-for-byte. + issuer string // Claims the next token exchange will assert. sub, email, name string @@ -48,11 +52,12 @@ func newStubIdP(t *testing.T, clientID string) *stubIdP { mux := http.NewServeMux() idp.Server = httptest.NewServer(mux) + idp.issuer = idp.URL t.Cleanup(idp.Close) mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { _ = json.NewEncoder(w).Encode(map[string]any{ - "issuer": idp.URL, + "issuer": idp.issuer, "authorization_endpoint": idp.URL + "/authorize", "token_endpoint": idp.URL + "/token", "jwks_uri": idp.URL + "/jwks", @@ -91,7 +96,7 @@ func (idp *stubIdP) idToken(t *testing.T) string { t.Fatal(err) } payload, _ := json.Marshal(map[string]any{ - "iss": idp.URL, + "iss": idp.issuer, "aud": idp.clientID, "sub": idp.sub, "email": idp.email, @@ -329,6 +334,40 @@ func TestCallbackHandlesProviderError(t *testing.T) { assertNoSession(t, sessions, rec) } +// Authentik's issuer ends in a slash, and OIDC requires the discovered issuer to +// match the configured one byte-for-byte. Normalising it away made discovery +// fail against the real provider while every stub test still passed. +func TestDiscoveryKeepsTrailingSlashIssuer(t *testing.T) { + sessions, users, _ := newStores(t) + idp := newStubIdP(t, "petal") + idp.issuer = idp.URL + "/" + idp.sub, idp.email = "sub-her", "her@example.com" + + o := NewOIDC(context.Background(), Options{ + IssuerURL: idp.issuer, + ClientID: "petal", + ClientSecret: "shh", + BaseURL: "http://petal.test", + }, sessions, users) + flow := o.Routes() + + // A failed discovery renders the 503 "sign-in is unavailable" page instead + // of redirecting, so reaching the provider at all is the assertion. + target, jar := start(t, flow) + if !strings.HasPrefix(target.String(), idp.URL+"/authorize") { + t.Fatalf("login went to %q, want the provider's authorize endpoint", target) + } + + // And the ID token it issues, whose `iss` carries the same slash, verifies. + idp.nonce = jar[nonceCookie] + rec := httptest.NewRecorder() + flow.ServeHTTP(rec, jar.attach( + httptest.NewRequest(http.MethodGet, "/callback?code=abc&state="+url.QueryEscape(jar[stateCookie]), nil))) + if rec.Code != http.StatusFound { + t.Fatalf("callback status=%d body=%s", rec.Code, rec.Body) + } +} + // Signing in when already signed in shouldn't bounce a good session through the // identity provider. func TestLoginSkipsWhenAlreadySignedIn(t *testing.T) {