diff --git a/admin/server/auth/auth.go b/admin/server/auth/auth.go index 73a4c156cb1e..2977f8ac6af2 100644 --- a/admin/server/auth/auth.go +++ b/admin/server/auth/auth.go @@ -2,6 +2,8 @@ package auth import ( "context" + "fmt" + "strings" "github.com/coreos/go-oidc/v3/oidc" "github.com/rilldata/rill/admin" @@ -28,21 +30,33 @@ type AuthenticatorOptions struct { // It provides endpoints for login/logout, creates users, issues cookie-based auth tokens, and provides middleware for authenticating requests. // The implementation was derived from: https://auth0.com/docs/quickstart/webapp/golang/01-login. type Authenticator struct { - logger *zap.Logger - admin *admin.Service - cookies *cookies.Store - opts *AuthenticatorOptions - oidc *oidc.Provider - oauth2 oauth2.Config + logger *zap.Logger + admin *admin.Service + cookies *cookies.Store + opts *AuthenticatorOptions + oidc *oidc.Provider + oauth2 oauth2.Config + endSessionEndpoint string } // NewAuthenticator creates an Authenticator. func NewAuthenticator(logger *zap.Logger, adm *admin.Service, cookieStore *cookies.Store, opts *AuthenticatorOptions) (*Authenticator, error) { - oidcProvider, err := oidc.NewProvider(context.Background(), "https://"+opts.AuthDomain+"/") + issuer := issuerURL(opts.AuthDomain) + oidcProvider, err := oidc.NewProvider(context.Background(), issuer) if err != nil { return nil, err } + var claims struct { + EndSessionEndpoint string `json:"end_session_endpoint"` + } + if err := oidcProvider.Claims(&claims); err != nil { + return nil, fmt.Errorf("failed to parse the auth provider's discovery document: %w", err) + } + if claims.EndSessionEndpoint == "" && !isBareDomain(opts.AuthDomain) { + logger.Warn("auth provider does not publish an end_session_endpoint, so logging out will only end the Rill session", zap.String("issuer", issuer)) + } + oauth2Config := oauth2.Config{ ClientID: opts.AuthClientID, ClientSecret: opts.AuthClientSecret, @@ -52,13 +66,29 @@ func NewAuthenticator(logger *zap.Logger, adm *admin.Service, cookieStore *cooki } a := &Authenticator{ - logger: logger, - admin: adm, - cookies: cookieStore, - opts: opts, - oidc: oidcProvider, - oauth2: oauth2Config, + logger: logger, + admin: adm, + cookies: cookieStore, + opts: opts, + oidc: oidcProvider, + oauth2: oauth2Config, + endSessionEndpoint: claims.EndSessionEndpoint, } return a, nil } + +// issuerURL returns the OIDC issuer for authDomain. +// AuthDomain with "://" is a full issuer URL (Keycloak, Dex, etc.) used verbatim; +// without it, assume Auth0-style domain and append trailing slash. +func issuerURL(authDomain string) string { + if isBareDomain(authDomain) { + return "https://" + authDomain + "/" + } + return authDomain +} + +// isBareDomain reports whether authDomain is an Auth0-style domain (e.g. "rill.auth0.com") rather than a full issuer URL. +func isBareDomain(authDomain string) bool { + return !strings.Contains(authDomain, "://") +} diff --git a/admin/server/auth/auth_test.go b/admin/server/auth/auth_test.go new file mode 100644 index 000000000000..c0b16e01d2b0 --- /dev/null +++ b/admin/server/auth/auth_test.go @@ -0,0 +1,111 @@ +package auth + +import ( + "crypto/rand" + "crypto/rsa" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-jose/go-jose/v3" + "github.com/go-jose/go-jose/v3/jwt" + "github.com/rilldata/rill/admin" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +// testProvider is a minimal OIDC provider serving a discovery document and a JWKS, and signing ID tokens with its key. +type testProvider struct { + *httptest.Server + key *rsa.PrivateKey +} + +// newTestProvider starts a testProvider. The discovery document can be extended (or fields overridden) with extra. +func newTestProvider(t *testing.T, extra map[string]any) *testProvider { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + + p := &testProvider{key: key} + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) { + doc := map[string]any{ + "issuer": p.URL, + "authorization_endpoint": p.URL + "/authorize", + "token_endpoint": p.URL + "/token", + "jwks_uri": p.URL + "/jwks", + "id_token_signing_alg_values_supported": []string{"RS256"}, + } + for k, v := range extra { + doc[k] = v + } + _ = json.NewEncoder(w).Encode(doc) + }) + mux.HandleFunc("/jwks", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{Key: &key.PublicKey, KeyID: "test", Algorithm: "RS256", Use: "sig"}}}) + }) + p.Server = httptest.NewServer(mux) + t.Cleanup(p.Close) + return p +} + +// signIDToken returns an ID token for the given claims, signed with key (the provider's own key if nil). +func (p *testProvider) signIDToken(t *testing.T, key *rsa.PrivateKey, claims map[string]any) string { + if key == nil { + key = p.key + } + signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.RS256, Key: key}, (&jose.SignerOptions{}).WithHeader("kid", "test")) + require.NoError(t, err) + raw, err := jwt.Signed(signer).Claims(claims).CompactSerialize() + require.NoError(t, err) + return raw +} + +func TestIssuerURL(t *testing.T) { + tests := []struct { + authDomain string + want string + }{ + {"rill.auth0.com", "https://rill.auth0.com/"}, + {"https://idp.example.com/realms/rill", "https://idp.example.com/realms/rill"}, + {"https://idp.example.com/realms/rill/", "https://idp.example.com/realms/rill/"}, + {"http://localhost:5556/dex", "http://localhost:5556/dex"}, + } + for _, tt := range tests { + require.Equal(t, tt.want, issuerURL(tt.authDomain), tt.authDomain) + } +} + +func TestNewAuthenticator(t *testing.T) { + urls, err := admin.NewURLs("http://localhost:8080", "http://localhost:3000") + require.NoError(t, err) + adm := &admin.Service{URLs: urls} + + tests := []struct { + name string + endSession any // value of end_session_endpoint in the discovery document; nil leaves it out + wantEndSession string + wantErrorSubstring string + }{ + {name: "with end_session_endpoint", endSession: "https://idp.example.com/logout", wantEndSession: "https://idp.example.com/logout"}, + {name: "without end_session_endpoint"}, + {name: "malformed end_session_endpoint", endSession: 42, wantErrorSubstring: "discovery document"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var discovery map[string]any + if tt.endSession != nil { + discovery = map[string]any{"end_session_endpoint": tt.endSession} + } + p := newTestProvider(t, discovery) + + a, err := NewAuthenticator(zap.NewNop(), adm, nil, &AuthenticatorOptions{AuthDomain: p.URL, AuthClientID: "rill-client"}) + if tt.wantErrorSubstring != "" { + require.ErrorContains(t, err, tt.wantErrorSubstring) + return + } + require.NoError(t, err) + require.Equal(t, tt.wantEndSession, a.endSessionEndpoint) + }) + } +} diff --git a/admin/server/auth/handlers.go b/admin/server/auth/handlers.go index ba8c736f34e1..e4912ac21568 100644 --- a/admin/server/auth/handlers.go +++ b/admin/server/auth/handlers.go @@ -13,6 +13,7 @@ import ( "time" "github.com/coreos/go-oidc/v3/oidc" + "github.com/gorilla/sessions" "github.com/rilldata/rill/admin/database" "github.com/rilldata/rill/admin/pkg/urlutil" "github.com/rilldata/rill/runtime/pkg/httputil" @@ -30,6 +31,8 @@ const ( cookieFieldRedirect = "redirect" cookieFieldCustomDomainFlow = "custom_domain_flow" cookieFieldAccessToken = "access_token" + idTokenCookieName = "auth_id_token" // nolint:gosec // cookie name, not a credential + cookieFieldIDToken = "id_token" ) var ( @@ -207,9 +210,8 @@ func (a *Authenticator) authStart(w http.ResponseWriter, r *http.Request, signup // Redirect to auth provider (canonical domain flow) redirectURL := a.oauth2.AuthCodeURL(state) if signup { - // Set custom parameters for signup using AuthCodeOption - customOption := oauth2.SetAuthURLParam("screen_hint", "signup") - redirectURL = a.oauth2.AuthCodeURL(state, customOption) + // Send both signup hints: Auth0 only honors screen_hint, standard OIDC providers only honor prompt=create. + redirectURL = a.oauth2.AuthCodeURL(state, oauth2.SetAuthURLParam("screen_hint", "signup"), oauth2.SetAuthURLParam("prompt", "create")) } http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) @@ -279,33 +281,9 @@ func (a *Authenticator) authLoginCallback(w http.ResponseWriter, r *http.Request http.Error(w, err.Error(), http.StatusInternalServerError) return } - email, ok := profile["email"].(string) - if !ok || email == "" { - http.Error(w, "claim 'email' not found", http.StatusInternalServerError) - return - } - emailVerified, ok := profile["email_verified"].(bool) - if !ok { - // For SAML flows, it is passed as a string - emailVerifiedStr, ok := profile["email_verified"].(string) - if !ok { - http.Error(w, "claim 'email_verified' not found", http.StatusInternalServerError) - return - } - emailVerified, err = strconv.ParseBool(emailVerifiedStr) - if err != nil { - http.Error(w, fmt.Sprintf("claim 'email_verified' could not be parsed as a boolean (got %q)", emailVerifiedStr), http.StatusInternalServerError) - return - } - } - name, ok := profile["name"].(string) - if !ok { - http.Error(w, "claim 'name' not found", http.StatusInternalServerError) - return - } - photoURL, ok := profile["picture"].(string) - if !ok { - http.Error(w, "claim 'picture' not found", http.StatusInternalServerError) + info, err := parseUserProfile(profile) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -317,19 +295,22 @@ func (a *Authenticator) authLoginCallback(w http.ResponseWriter, r *http.Request delete(sess.Values, cookieFieldRedirect) // Check that the user's email is verified - if !emailVerified { + if !info.emailVerified { redirectURL := a.admin.URLs.WithCustomDomainFromRedirectURL(redirect).AuthVerifyEmailUI() http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) return } // Create (or update) user in our DB - user, err := a.admin.CreateOrUpdateUser(r.Context(), email, name, photoURL) + user, err := a.admin.CreateOrUpdateUser(r.Context(), info.email, info.name, info.photoURL) if err != nil { http.Error(w, fmt.Sprintf("failed to update user: %s", err), http.StatusInternalServerError) return } + // Keep the ID token for logout. This callback and authLogoutProvider both run on the canonical domain, so it works for both flows below. + a.saveIDTokenHint(w, r, rawIDToken) + // If it's part of a custom domain login flow, redirect back to the custom domain with a short-lived access token for the user. customDomainFlow, ok := sess.Values[cookieFieldCustomDomainFlow].(bool) delete(sess.Values, cookieFieldCustomDomainFlow) @@ -383,6 +364,47 @@ func (a *Authenticator) authLoginCallback(w http.ResponseWriter, r *http.Request http.Redirect(w, r, redirect, http.StatusTemporaryRedirect) } +// userProfile is the user information authLoginCallback reads from the ID token claims. +type userProfile struct { + email string + emailVerified bool + name string + photoURL string +} + +// parseUserProfile reads the user's profile from the ID token claims. +func parseUserProfile(claims map[string]any) (*userProfile, error) { + email, ok := claims["email"].(string) + if !ok || email == "" { + return nil, errors.New("claim 'email' not found") + } + emailVerified, ok := claims["email_verified"].(bool) + if !ok { + // For SAML flows, it is passed as a string + emailVerifiedStr, ok := claims["email_verified"].(string) + if !ok { + return nil, errors.New("claim 'email_verified' not found") + } + var err error + emailVerified, err = strconv.ParseBool(emailVerifiedStr) + if err != nil { + return nil, fmt.Errorf("claim 'email_verified' could not be parsed as a boolean (got %q)", emailVerifiedStr) + } + } + name, ok := claims["name"].(string) + if !ok { + return nil, errors.New("claim 'name' not found") + } + // The picture claim is optional: some providers never emit it (e.g. Dex), or only for users who have one (e.g. Keycloak) + photoURL, _ := claims["picture"].(string) + return &userProfile{ + email: email, + emailVerified: emailVerified, + name: name, + photoURL: photoURL, + }, nil +} + // authLoginCustomDomainCallback first verifies the state for CSRF protection, then extracts // a nonce token from the query parameters and validates it. If valid, it issues a new // long-lived token, stores it in a cookie, and redirects the user to the frontend. @@ -611,19 +633,106 @@ func (a *Authenticator) authLogoutProvider(w http.ResponseWriter, r *http.Reques } } - // Build and redirect to the auth provider logout URL. - logoutURL, err := url.Parse("https://" + a.opts.AuthDomain + "/v2/logout") + // Take the ID token saved at login (if any) to send as id_token_hint + idTokenHint := a.takeIDTokenHint(w, r) + + // Build the provider logout URL. + // Standard OIDC providers expose end_session_endpoint; Auth0 uses /v2/logout with "returnTo". + logoutEndpoint := a.endSessionEndpoint + redirectParam := "post_logout_redirect_uri" + if logoutEndpoint == "" { + if !isBareDomain(a.opts.AuthDomain) { + // The provider has no logout endpoint we can call (e.g. Dex), so only the Rill session is ended. + http.Redirect(w, r, a.admin.URLs.AuthLogoutCallback(), http.StatusTemporaryRedirect) + return + } + logoutEndpoint = "https://" + a.opts.AuthDomain + "/v2/logout" + redirectParam = "returnTo" + } + + logoutURL, err := url.Parse(logoutEndpoint) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - parameters := url.Values{} - parameters.Add("returnTo", a.admin.URLs.AuthLogoutCallback()) - parameters.Add("client_id", a.opts.AuthClientID) - logoutURL.RawQuery = parameters.Encode() + params := url.Values{} + params.Set("client_id", a.opts.AuthClientID) + params.Set(redirectParam, a.admin.URLs.AuthLogoutCallback()) + if a.endSessionEndpoint != "" && idTokenHint != "" { + params.Set("id_token_hint", idTokenHint) + } + logoutURL.RawQuery = params.Encode() http.Redirect(w, r, logoutURL.String(), http.StatusTemporaryRedirect) } +// saveIDTokenHint keeps the raw ID token from login so that authLogoutProvider can send it as id_token_hint. +// Without the hint, standard OIDC providers (e.g. Keycloak) show a logout confirmation page and keep the user's +// session alive until it is confirmed. +// +// It is only kept when the provider publishes an end_session_endpoint: Auth0's /v2/logout does not use it, so +// Auth0 deployments are unaffected. +// +// It goes in its own cookie rather than in the auth cookie: ID tokens can be large enough to push the auth cookie +// past the 4096-byte cookie limit, which would fail the login. If the ID token does not fit on its own either, it +// is skipped, and logout falls back to the provider's confirmation page. +// +// The cookie is scoped to the path of authLogoutProvider, so browsers only send it on logout rather than adding +// the ID token to every request to the admin service. +func (a *Authenticator) saveIDTokenHint(w http.ResponseWriter, r *http.Request, rawIDToken string) { + if a.endSessionEndpoint == "" { + return + } + + sess := a.cookies.Get(r, idTokenCookieName) + sess.Options = a.idTokenCookieOptions(sess.Options, false) + sess.Values[cookieFieldIDToken] = rawIDToken + if err := sess.Save(r, w); err != nil { + a.logger.Info("not keeping ID token for logout", zap.Error(err), observability.ZapCtx(r.Context())) + } +} + +// takeIDTokenHint returns the ID token kept by saveIDTokenHint and clears its cookie. +// It returns an empty string if there is none, or if its signature no longer verifies (e.g. after the provider +// rotated its keys): providers such as Keycloak fail the whole logout on an invalid hint, whereas without one +// they only ask for confirmation. Expiry is not checked: the Rill session outlives the ID token by weeks, and the +// OIDC RP-Initiated Logout spec asks providers to accept hints whose exp has passed. +func (a *Authenticator) takeIDTokenHint(w http.ResponseWriter, r *http.Request) string { + if _, err := r.Cookie(idTokenCookieName); err != nil { + return "" + } + + sess := a.cookies.Get(r, idTokenCookieName) + rawIDToken, _ := sess.Values[cookieFieldIDToken].(string) + sess.Options = a.idTokenCookieOptions(sess.Options, true) + if err := sess.Save(r, w); err != nil { + a.logger.Info("failed to clear ID token cookie", zap.Error(err), observability.ZapCtx(r.Context())) + } + if rawIDToken == "" { + return "" + } + + verifier := a.oidc.Verifier(&oidc.Config{ClientID: a.oauth2.ClientID, SkipExpiryCheck: true}) + if _, err := verifier.Verify(r.Context(), rawIDToken); err != nil { + a.logger.Info("not sending ID token as logout hint", zap.Error(err), observability.ZapCtx(r.Context())) + return "" + } + return rawIDToken +} + +// idTokenCookieOptions returns a copy of opts for the ID token cookie, scoped to the path of authLogoutProvider. +// It copies because cookies.Store.Get may return the store's shared options. +func (a *Authenticator) idTokenCookieOptions(opts *sessions.Options, expire bool) *sessions.Options { + res := *opts + res.Path = "/" + if u, err := url.Parse(a.admin.URLs.AuthLogoutProvider("")); err == nil { + res.Path = u.Path + } + if expire { + res.MaxAge = -1 + } + return &res +} + // authLogoutCallback is called by the auth provider when a logout flow iniated by authLogout has completed. // // For orgs with a custom domain configured, the auth provider will still redirect back to the canonical domain's authLogoutCallback. diff --git a/admin/server/auth/handlers_test.go b/admin/server/auth/handlers_test.go new file mode 100644 index 000000000000..1a56c601729b --- /dev/null +++ b/admin/server/auth/handlers_test.go @@ -0,0 +1,265 @@ +package auth + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/rilldata/rill/admin" + "github.com/rilldata/rill/admin/server/cookies" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + "golang.org/x/oauth2" +) + +// newTestAuthenticator returns an Authenticator wired just enough to exercise the login and logout +// redirect handlers without a database or a live auth provider. +func newTestAuthenticator(t *testing.T, authDomain string) *Authenticator { + urls, err := admin.NewURLs("http://localhost:8080", "http://localhost:3000") + require.NoError(t, err) + + // Cookie options as set in server.New + cookieStore := cookies.New(zap.NewNop(), []byte("0123456789abcdef0123456789abcdef"), []byte("0123456789abcdef")) + cookieStore.Options.HttpOnly = true + cookieStore.Options.SameSite = http.SameSiteLaxMode + + return &Authenticator{ + logger: zap.NewNop(), + admin: &admin.Service{URLs: urls}, + cookies: cookieStore, + opts: &AuthenticatorOptions{ + AuthDomain: authDomain, + AuthClientID: "rill-client", + }, + oauth2: oauth2.Config{ + ClientID: "rill-client", + RedirectURL: urls.AuthLoginCallback(), + Endpoint: oauth2.Endpoint{AuthURL: "https://idp.example.com/authorize"}, + }, + } +} + +func TestAuthStartSignup(t *testing.T) { + a := newTestAuthenticator(t, "idp.example.com") + + for _, signup := range []bool{false, true} { + req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/auth/login", nil) + w := httptest.NewRecorder() + a.authStart(w, req, signup) + require.Equal(t, http.StatusTemporaryRedirect, w.Code) + + loc, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + q := loc.Query() + if !signup { + require.Empty(t, q.Get("prompt")) + require.Empty(t, q.Get("screen_hint")) + continue + } + // Auth0 only honors screen_hint, standard OIDC providers only honor prompt=create. + require.Equal(t, "create", q.Get("prompt")) + require.Equal(t, "signup", q.Get("screen_hint")) + } +} + +func TestAuthLogoutProvider(t *testing.T) { + tests := []struct { + name string + authDomain string + endSessionEndpoint string + want string + }{ + { + // Must stay identical to what Auth0 deployments get today. + name: "auth0 domain without end_session_endpoint", + authDomain: "rill.auth0.com", + want: "https://rill.auth0.com/v2/logout?client_id=rill-client&returnTo=http%3A%2F%2Flocalhost%3A8080%2Fauth%2Flogout%2Fcallback", + }, + { + name: "issuer URL without end_session_endpoint", + authDomain: "https://idp.example.com/realms/rill", + want: "http://localhost:8080/auth/logout/callback", + }, + { + name: "issuer URL with end_session_endpoint", + authDomain: "https://idp.example.com/realms/rill", + endSessionEndpoint: "https://idp.example.com/realms/rill/protocol/openid-connect/logout", + want: "https://idp.example.com/realms/rill/protocol/openid-connect/logout?client_id=rill-client&post_logout_redirect_uri=http%3A%2F%2Flocalhost%3A8080%2Fauth%2Flogout%2Fcallback", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := newTestAuthenticator(t, tt.authDomain) + a.endSessionEndpoint = tt.endSessionEndpoint + + req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/auth/logout/provider", nil) + w := httptest.NewRecorder() + a.authLogoutProvider(w, req) + require.Equal(t, http.StatusTemporaryRedirect, w.Code) + require.Equal(t, tt.want, w.Header().Get("Location")) + }) + } +} + +func TestIDTokenHint(t *testing.T) { + p := newTestProvider(t, nil) + provider, err := oidc.NewProvider(context.Background(), p.URL) + require.NoError(t, err) + + newAuthenticator := func(endSessionEndpoint string) *Authenticator { + a := newTestAuthenticator(t, p.URL) + a.oidc = provider + a.endSessionEndpoint = endSessionEndpoint + return a + } + idToken := func(key *rsa.PrivateKey, exp time.Time) string { + return p.signIDToken(t, key, map[string]any{"iss": p.URL, "aud": "rill-client", "sub": "user", "iat": exp.Add(-5 * time.Minute).Unix(), "exp": exp.Unix()}) + } + // save runs saveIDTokenHint as the login callback would and returns the cookie it set, if any. + save := func(a *Authenticator, rawIDToken string) *http.Cookie { + req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/auth/callback", nil) + w := httptest.NewRecorder() + a.saveIDTokenHint(w, req, rawIDToken) + require.Equal(t, http.StatusOK, w.Code) // It never fails the login + for _, c := range w.Result().Cookies() { + if c.Name == idTokenCookieName { + return c + } + } + return nil + } + // logout runs authLogoutProvider with the given cookie and returns the id_token_hint it sent and whether it cleared the cookie. + logout := func(a *Authenticator, c *http.Cookie) (string, bool) { + req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/auth/logout/provider", nil) + req.AddCookie(c) + w := httptest.NewRecorder() + a.authLogoutProvider(w, req) + require.Equal(t, http.StatusTemporaryRedirect, w.Code) + loc, err := url.Parse(w.Header().Get("Location")) + require.NoError(t, err) + cleared := false + for _, rc := range w.Result().Cookies() { + if rc.Name == idTokenCookieName && rc.MaxAge < 0 && rc.Path == "/auth/logout/provider" { + cleared = true + } + } + return loc.Query().Get("id_token_hint"), cleared + } + + t.Run("saved only for providers with an end_session_endpoint", func(t *testing.T) { + require.Nil(t, save(newAuthenticator(""), idToken(nil, time.Now().Add(5*time.Minute)))) + }) + + t.Run("scoped to the logout path", func(t *testing.T) { + c := save(newAuthenticator(p.URL+"/logout"), idToken(nil, time.Now().Add(5*time.Minute))) + require.NotNil(t, c) + require.Equal(t, "/auth/logout/provider", c.Path) + require.True(t, c.HttpOnly) + }) + + t.Run("skipped when too large for a cookie", func(t *testing.T) { + require.Nil(t, save(newAuthenticator(p.URL+"/logout"), strings.Repeat("x", 5000))) + }) + + tests := []struct { + name string + idToken string + wantHint bool + }{ + {"valid", idToken(nil, time.Now().Add(5*time.Minute)), true}, + // The ID token expires within minutes but the Rill session lasts for weeks, and providers accept expired hints. + {"expired", idToken(nil, time.Now().Add(-24*time.Hour)), true}, + // An invalid hint makes Keycloak fail the logout with an error page, so it is dropped. + {"signed by another key", idToken(mustRSAKey(t), time.Now().Add(5*time.Minute)), false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + a := newAuthenticator(p.URL + "/logout") + c := save(a, tt.idToken) + require.NotNil(t, c) + + hint, cleared := logout(a, c) + require.True(t, cleared) + if tt.wantHint { + require.Equal(t, tt.idToken, hint) + } else { + require.Empty(t, hint) + } + }) + } + + t.Run("undecodable cookie leaves the store options alone", func(t *testing.T) { + a := newAuthenticator(p.URL + "/logout") + before := *a.cookies.Options + + hint, cleared := logout(a, &http.Cookie{Name: idTokenCookieName, Value: "garbage"}) + require.Empty(t, hint) + require.True(t, cleared) + require.Equal(t, before, *a.cookies.Options) + }) +} + +func mustRSAKey(t *testing.T) *rsa.PrivateKey { + key, err := rsa.GenerateKey(rand.Reader, 2048) + require.NoError(t, err) + return key +} + +func TestParseUserProfile(t *testing.T) { + tests := []struct { + name string + claims map[string]any + want *userProfile + wantErr string + }{ + { + name: "all claims", + claims: map[string]any{"email": "a@example.com", "email_verified": true, "name": "A", "picture": "https://example.com/a.png"}, + want: &userProfile{email: "a@example.com", emailVerified: true, name: "A", photoURL: "https://example.com/a.png"}, + }, + { + // Dex never emits picture, and Keycloak omits it for users without a picture attribute. + name: "without picture", + claims: map[string]any{"email": "a@example.com", "email_verified": true, "name": "A"}, + want: &userProfile{email: "a@example.com", emailVerified: true, name: "A"}, + }, + { + name: "email_verified as a string", + claims: map[string]any{"email": "a@example.com", "email_verified": "false", "name": "A", "picture": ""}, + want: &userProfile{email: "a@example.com", emailVerified: false, name: "A"}, + }, + { + name: "without email", + claims: map[string]any{"email_verified": true, "name": "A"}, + wantErr: "claim 'email' not found", + }, + { + name: "without email_verified", + claims: map[string]any{"email": "a@example.com", "name": "A"}, + wantErr: "claim 'email_verified' not found", + }, + { + name: "without name", + claims: map[string]any{"email": "a@example.com", "email_verified": true}, + wantErr: "claim 'name' not found", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseUserProfile(tt.claims) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tt.want, got) + }) + } +}