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
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user