diff --git a/AUTHENTICATION.md b/AUTHENTICATION.md index ea56c0f3ff..c331dfbd43 100644 --- a/AUTHENTICATION.md +++ b/AUTHENTICATION.md @@ -35,27 +35,49 @@ auth: #### Auth Settings -| Field | Type | Description | -| -------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `enabled` | boolean | Enable or disable authentication | -| `redirectToProvider` | boolean | Skip the Temporal UI login page and redirect unauthenticated users directly to the configured OIDC provider | -| `maxSessionDuration` | duration | Maximum session duration before forced re-login (e.g., `8h`, `24h`, `168h`). Set to `0` or omit for unlimited session duration | -| `providers` | array | List of auth providers (currently only the first is used) | +| Field | Type | Description | +| -------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------- | +| `enabled` | boolean | Enable or disable authentication | +| `redirectToProvider` | boolean | Skip the Temporal UI login page and redirect unauthenticated users directly to the configured OIDC provider | +| `maxSessionDuration` | duration | Maximum session duration before forced re-login (e.g., `8h`, `24h`, `168h`). Omit, or set to `0s`, for unlimited session duration | +| `providers` | array | List of auth providers (currently only the first is used) | #### Provider Settings -| Field | Type | Description | -| -------------------- | ------- | --------------------------------------------------------------- | -| `label` | string | Display name for the provider | -| `type` | string | Provider type. Only `oidc` is supported | -| `providerUrl` | string | OIDC discovery URL (e.g., `https://accounts.google.com/`) | -| `issuerUrl` | string | Optional. Set only if issuer differs from provider URL | -| `clientId` | string | OAuth2 client ID | -| `clientSecret` | string | OAuth2 client secret | -| `scopes` | array | OAuth2 scopes. Include `offline_access` to enable token refresh | -| `callbackUrl` | string | OAuth2 callback URL for your deployment | -| `options` | object | Additional URL parameters for the auth redirect | -| `useIdTokenAsBearer` | boolean | Use ID token instead of access token in Authorization header | +| Field | Type | Description | +| ---------------------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `label` | string | Display name for the provider | +| `type` | string | Provider type. Only `oidc` is supported | +| `providerUrl` | string | OIDC discovery URL (e.g., `https://accounts.google.com/`) | +| `issuerUrl` | string | Optional. Set only if issuer differs from provider URL | +| `clientId` | string | OAuth2 client ID | +| `clientSecret` | string | OAuth2 client secret | +| `scopes` | array | OAuth2 scopes. Include `offline_access` to enable token refresh | +| `callbackUrl` | string | OAuth2 callback URL for your deployment | +| `options` | object | Additional URL parameters for the auth redirect | +| `useIdTokenAsBearer` | boolean | Use ID token instead of access token in Authorization header | +| `refreshTokenDuration` | duration | Lifetime of the refresh token this provider issues. Only needed for providers that issue opaque refresh tokens. See [Refresh token lifetime](#refresh-token-lifetime) | + +#### Docker Environment Variables + +The bundled `docker.yaml` maps each auth setting to an environment variable, so a +Docker deployment can be configured without supplying a custom config file. + +| Environment Variable | Config Field | Default | +| -------------------------------------- | -------------------------------------------- | ----------------- | +| `TEMPORAL_AUTH_ENABLED` | `auth.enabled` | `false` | +| `TEMPORAL_AUTH_REDIRECT_TO_PROVIDER` | `auth.redirectToProvider` | `false` | +| `TEMPORAL_MAX_SESSION_DURATION` | `auth.maxSessionDuration` | unset (unlimited) | +| `TEMPORAL_AUTH_LABEL` | `auth.providers[0].label` | `sso` | +| `TEMPORAL_AUTH_TYPE` | `auth.providers[0].type` | `oidc` | +| `TEMPORAL_AUTH_PROVIDER_URL` | `auth.providers[0].providerUrl` | unset | +| `TEMPORAL_AUTH_ISSUER_URL` | `auth.providers[0].issuerUrl` | unset | +| `TEMPORAL_AUTH_CLIENT_ID` | `auth.providers[0].clientId` | unset | +| `TEMPORAL_AUTH_CLIENT_SECRET` | `auth.providers[0].clientSecret` | unset | +| `TEMPORAL_AUTH_CALLBACK_URL` | `auth.providers[0].callbackUrl` | unset | +| `TEMPORAL_AUTH_SCOPES` | `auth.providers[0].scopes` (comma separated) | unset | +| `TEMPORAL_AUTH_USE_ID_TOKEN_AS_BEARER` | `auth.providers[0].useIdTokenAsBearer` | `false` | +| `TEMPORAL_AUTH_REFRESH_TOKEN_DURATION` | `auth.providers[0].refreshTokenDuration` | unset | ## Session Duration Management @@ -83,7 +105,10 @@ auth: - `8h` - 8 hours (typical workday) - `24h` - 24 hours - `168h` - 1 week -- `0` or omitted - No maximum (session lasts until refresh token expires) +- `0s` or omitted - No maximum (session lasts until refresh token expires) + +> Write an explicit zero as `0s`, not `0`. A bare `0` is parsed as a number rather +> than a duration and the server rejects the config file. ### How It Works @@ -93,6 +118,41 @@ auth: This is useful for compliance requirements where users must periodically re-verify their identity, independent of token validity. +The `user*` cookies that carry the access token to the browser are also held to the +session boundary: they are issued for one minute, or for whatever is left of the +session if that is shorter. Without this, a refresh performed shortly before the +boundary would hand the browser a full minute of cookie, leaving the UI looking +signed in while every API call behind it returned 401. + +## Refresh Token Lifetime + +The `refresh` cookie must outlive the access token, since its whole purpose is to +obtain the next one. Its lifetime is taken from the first of these that is available: + +1. The `exp` claim of the refresh token itself, when the provider issues a JWT + refresh token. Keycloak and similar providers are handled automatically, with no + configuration. +2. The `refreshTokenDuration` configured for the provider. This is the setting for + providers that issue **opaque** refresh tokens, whose lifetime the server has no + way to read. +3. A default of 7 days. + +Whichever applies, the cookie is capped at 30 days. + +```yaml +auth: + providers: + - label: My IdP + # ... + refreshTokenDuration: 24h # match your IdP's refresh token lifetime +``` + +Note that this is not derived from the token response's `expires_in`. That field +describes the **access** token, per [RFC 6749 section 5.1](https://datatracker.ietf.org/doc/html/rfc6749#section-5.1) +and [OIDC Core section 3.2.2.5](https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.3.2.2.5), +and using it would expire the refresh cookie at the same moment as the token it +exists to replace. + ## Provider-Specific Configuration ### Azure AD / Entra ID @@ -220,6 +280,13 @@ Ensure: - Refresh tokens are enabled in your IdP configuration - The refresh token hasn't expired (check IdP settings) +If refresh starts failing with 401 at the moment the access token expires, check +the `refresh` cookie in browser devtools. A Max-Age matching the access token +lifetime means the cookie is being dropped before it can be used. Providers that +issue JWT refresh tokens are handled automatically; for one that issues opaque +refresh tokens, set `refreshTokenDuration` on the provider to its refresh token +lifetime. See [Refresh token lifetime](#refresh-token-lifetime). + ### Redirect loop after login Verify: diff --git a/server/config/docker.yaml b/server/config/docker.yaml index fbe2577581..15ef7d21b9 100644 --- a/server/config/docker.yaml +++ b/server/config/docker.yaml @@ -52,6 +52,7 @@ uiServerTLS: auth: enabled: {{ env "TEMPORAL_AUTH_ENABLED" | default "false" }} redirectToProvider: {{ env "TEMPORAL_AUTH_REDIRECT_TO_PROVIDER" | default "false" }} + maxSessionDuration: {{ env "TEMPORAL_MAX_SESSION_DURATION" | default "" }} providers: - label: {{ env "TEMPORAL_AUTH_LABEL" | default "sso" }} type: {{ env "TEMPORAL_AUTH_TYPE" | default "oidc" }} @@ -61,6 +62,7 @@ auth: clientSecret: {{ env "TEMPORAL_AUTH_CLIENT_SECRET" }} callbackUrl: {{ env "TEMPORAL_AUTH_CALLBACK_URL" }} useIdTokenAsBearer: {{ env "TEMPORAL_AUTH_USE_ID_TOKEN_AS_BEARER" | default "false" }} + refreshTokenDuration: {{ env "TEMPORAL_AUTH_REFRESH_TOKEN_DURATION" | default "" }} scopes: {{- if env "TEMPORAL_AUTH_SCOPES" }} {{- range env "TEMPORAL_AUTH_SCOPES" | split "," }} diff --git a/server/config/e2e-auth.yaml b/server/config/e2e-auth.yaml new file mode 100644 index 0000000000..4a4d994900 --- /dev/null +++ b/server/config/e2e-auth.yaml @@ -0,0 +1,49 @@ +# ============================================================================= +# Auth configuration for the Playwright E2E suite. +# +# This is the counterpart to with-auth.yaml, which is tuned for `pnpm +# dev:with-auth` and a human clicking through the UI. The durations here are +# deliberately short so that tests/e2e/auth-cookie-lifetimes.spec.ts can wait +# out an access token in a few seconds, and so that maxSessionDuration is below +# the 60s user cookie lifetime and the clamp on those cookies is observable. +# +# The matching identity provider TTLs are set in tests/global-setup.ts via the +# OIDC_*_TTL environment variables. +# ============================================================================= +publicPath: +port: 8081 +enableUi: true +cors: + cookieInsecure: true + allowOrigins: + - http://localhost:8081 +refreshInterval: 1m +defaultNamespace: default +showTemporalSystemNamespace: false +disableWriteActions: false +auth: + enabled: true + redirectToProvider: false + # Below the 60s user cookie lifetime, so that the clamp is visible at login. + maxSessionDuration: 45s + providers: + - label: E2E OIDC + type: oidc + providerUrl: http://localhost:8889 + issuerUrl: '' + clientId: temporal-ui + clientSecret: temporal-secret + scopes: + - openid + - profile + - email + - offline_access + callbackUrl: http://localhost:8081/auth/sso/callback + # The mock provider issues opaque refresh tokens, so this is the value the + # refresh cookie should fall back to. A provider issuing JWT refresh + # tokens would have its own exp claim take precedence over this. + refreshTokenDuration: 24h +codec: + endpoint: + passAccessToken: false + includeCredentials: false diff --git a/server/config/with-auth.yaml b/server/config/with-auth.yaml index 2c0dc59e3f..93d344f89e 100644 --- a/server/config/with-auth.yaml +++ b/server/config/with-auth.yaml @@ -33,13 +33,13 @@ refreshWorkflowCountsDisabled: false # This forces refresh before session expiry. Current: 2m session, 1m tokens. # 2. Session Expiry: Wait for maxSessionDuration to elapse. User will be # redirected to login and must re-authenticate at the OIDC provider. -# 3. No Session Limit: Set maxSessionDuration to 0 or remove it entirely. +# 3. No Session Limit: Set maxSessionDuration to 0s or remove it entirely. # Tokens will refresh indefinitely until the OIDC refresh token expires. # ============================================================================= auth: enabled: true redirectToProvider: false - maxSessionDuration: 2m # Max time before forced re-login (0 = unlimited) + maxSessionDuration: 2m # Max time before forced re-login (0s = unlimited) providers: - label: Dummy OIDC type: oidc # for futureproofing; only oidc is supported today diff --git a/server/plugins/fs_config_provider/loader_test.go b/server/plugins/fs_config_provider/loader_test.go index b6eb7afbb3..41a7256266 100644 --- a/server/plugins/fs_config_provider/loader_test.go +++ b/server/plugins/fs_config_provider/loader_test.go @@ -28,6 +28,7 @@ import ( "io/ioutil" "os" "testing" + "time" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -165,3 +166,34 @@ func buildConfig(env string) string { item1: ` + item1 + ` item2: ` + item2 } + +// TestDockerConfigSessionDefaultsAreUnset pins the defaults for the auth duration +// settings in docker.yaml. Both must render to an unset duration, so that adding +// them does not silently start expiring sessions, or shorten refresh cookies, for +// Docker deployments that set neither environment variable. +func TestDockerConfigSessionDefaultsAreUnset(t *testing.T) { + t.Setenv("TEMPORAL_AUTH_ENABLED", "true") + + cfg, err := LoadConfig("../../config", "docker") + require.NoError(t, err) + + require.Zero(t, cfg.Auth.MaxSessionDuration, "TEMPORAL_MAX_SESSION_DURATION must default to unset") + require.Len(t, cfg.Auth.Providers, 1) + require.Zero(t, cfg.Auth.Providers[0].RefreshTokenDuration, "TEMPORAL_AUTH_REFRESH_TOKEN_DURATION must default to unset") +} + +// TestDockerConfigSessionDurationsFromEnv covers the reason these fields were added +// to docker.yaml: without them a Docker operator has no way to set either value +// short of supplying a wholly custom config file. +func TestDockerConfigSessionDurationsFromEnv(t *testing.T) { + t.Setenv("TEMPORAL_AUTH_ENABLED", "true") + t.Setenv("TEMPORAL_MAX_SESSION_DURATION", "8h") + t.Setenv("TEMPORAL_AUTH_REFRESH_TOKEN_DURATION", "24h") + + cfg, err := LoadConfig("../../config", "docker") + require.NoError(t, err) + + require.Equal(t, 8*time.Hour, cfg.Auth.MaxSessionDuration) + require.Len(t, cfg.Auth.Providers, 1) + require.Equal(t, 24*time.Hour, cfg.Auth.Providers[0].RefreshTokenDuration) +} diff --git a/server/server/auth/auth.go b/server/server/auth/auth.go index 1fbc776fec..338e66d032 100644 --- a/server/server/auth/auth.go +++ b/server/server/auth/auth.go @@ -46,8 +46,31 @@ const ( AuthorizationExtrasHeader = "authorization-extras" cookieLen = 4000 sessionStartCookie = "session_start" + + // defaultUserCookieDuration is how long the user* cookies live when no max + // session duration is configured. + defaultUserCookieDuration = time.Minute + // defaultRefreshCookieDuration applies when the refresh token's lifetime can + // neither be read from the token nor found in the provider config. + defaultRefreshCookieDuration = 7 * 24 * time.Hour + // maxRefreshCookieDuration bounds the refresh cookie, so a provider that + // issues a very long lived token cannot pin one in the browser for years. + maxRefreshCookieDuration = 30 * 24 * time.Hour ) +// CookieOptions carries the deployment settings that decide how the auth cookies +// are written. +type CookieOptions struct { + // Secure marks the cookies Secure. + Secure bool + // SessionExpiresAt is when the current session reaches the configured + // maxSessionDuration. The zero value means no max duration is configured. + SessionExpiresAt time.Time + // RefreshTokenDuration is the provider's configured refresh token lifetime. + // Zero means unset. + RefreshTokenDuration time.Duration +} + var tokenVerifier *oidc.IDTokenVerifier func SetVerifier(v *oidc.IDTokenVerifier) { @@ -58,7 +81,7 @@ func stripBearerPrefix(token string) string { return strings.TrimPrefix(token, "Bearer ") } -func SetUser(c echo.Context, user *User, secure bool) error { +func SetUser(c echo.Context, user *User, opts CookieOptions) error { if user.OAuth2Token == nil { return errors.New("no OAuth2Token") } @@ -84,12 +107,14 @@ func SetUser(c echo.Context, user *User, secure bool) error { s := base64.StdEncoding.EncodeToString(b) parts := splitCookie(s) + userMaxAge := userCookieMaxAge(opts.SessionExpiresAt, time.Now()) + for i, p := range parts { cookie := &http.Cookie{ Name: "user" + strconv.Itoa(i), Value: p, - MaxAge: int(time.Minute.Seconds()), - Secure: secure, + MaxAge: userMaxAge, + Secure: opts.Secure, HttpOnly: false, Path: "/", SameSite: http.SameSiteStrictMode, @@ -100,33 +125,13 @@ func SetUser(c echo.Context, user *User, secure bool) error { if rt := user.OAuth2Token.RefreshToken; rt != "" { log.Println("[Auth] Setting refresh token cookie") - // Calculate MaxAge from OAuth2 token expiry. - // IMPORTANT: When a refresh token is issued, oauth2.Token.Expiry typically - // reflects the refresh token's lifetime, not the access token's lifetime. - // This is standard behavior in OIDC flows with offline_access scope. - // See: https://pkg.go.dev/golang.org/x/oauth2#Token - var refreshMaxAge int - if user.OAuth2Token.Expiry.IsZero() { - // Fallback: if IdP doesn't provide expiry, use 7 days - refreshMaxAge = int((7 * 24 * time.Hour).Seconds()) - log.Printf("[Auth] Warning: No refresh token expiry from IdP, using 7-day default") - } else { - // Use IdP's expiry, capped at 30 days for safety - maxAge := time.Until(user.OAuth2Token.Expiry) - if maxAge > 30*24*time.Hour { - maxAge = 30 * 24 * time.Hour - log.Printf("[Auth] Warning: IdP refresh token expiry > 30 days, capping at 30 days") - } - refreshMaxAge = int(maxAge.Seconds()) - log.Printf("[Auth] Setting refresh cookie MaxAge to %d seconds (%.1f days) from IdP", - refreshMaxAge, maxAge.Hours()/24) - } + refreshMaxAge := refreshCookieMaxAge(rt, opts.RefreshTokenDuration, time.Now()) refreshCookie := &http.Cookie{ Name: "refresh", Value: rt, MaxAge: refreshMaxAge, - Secure: secure, + Secure: opts.Secure, HttpOnly: true, Path: "/", SameSite: http.SameSiteStrictMode, @@ -139,6 +144,122 @@ func SetUser(c echo.Context, user *User, secure bool) error { return nil } +// userCookieMaxAge returns the MaxAge for the user* cookies. +// +// Those cookies carry the access token, so they must not outlive the session +// boundary. When maxSessionDuration is shorter than the cookie lifetime, or when +// the last refresh before the boundary issues a fresh full-length cookie, the +// browser keeps presenting a signed-in UI whose every API call returns 401. +// Clamping to the time left in the session closes that window. +func userCookieMaxAge(sessionExpiresAt time.Time, now time.Time) int { + maxAge := defaultUserCookieDuration + if !sessionExpiresAt.IsZero() { + if remaining := sessionExpiresAt.Sub(now); remaining < maxAge { + maxAge = remaining + } + } + + // A MaxAge of 0 means "session cookie" rather than "expire now", so floor at + // one second for a session that is already over. + if maxAge < time.Second { + return 1 + } + return int(maxAge.Seconds()) +} + +// refreshCookieMaxAge returns the MaxAge for the refresh cookie. +// +// It deliberately ignores oauth2.Token.Expiry. That field is populated from the +// token response's expires_in, which per RFC 6749 section 5.1 and OIDC Core +// section 3.2.2.5 describes the access token, not the refresh token. Deriving the +// refresh cookie from it expires the cookie at the same moment as the access +// token, so the refresh the cookie exists to perform fails with a 401. +// +// The lifetime is taken from, in order: the refresh token's own exp claim, the +// lifetime configured for the provider, then a 7 day default. +func refreshCookieMaxAge(refreshToken string, configured time.Duration, now time.Time) int { + if exp, ok := jwtExp(refreshToken); ok { + if remaining := exp.Sub(now); remaining > 0 { + log.Printf("[Auth] Refresh cookie MaxAge from refresh token exp claim: %s", remaining.Round(time.Second)) + return cappedMaxAge(remaining) + } + log.Printf("[Auth] Refresh token exp claim is already past, falling back to configured lifetime") + } + + if configured > 0 { + log.Printf("[Auth] Refresh cookie MaxAge from configured refreshTokenDuration: %s", configured) + return cappedMaxAge(configured) + } + + log.Printf("[Auth] Refresh token is opaque and refreshTokenDuration is unset, using %s default", defaultRefreshCookieDuration) + return cappedMaxAge(defaultRefreshCookieDuration) +} + +// cappedMaxAge converts d to whole seconds, bounded by maxRefreshCookieDuration. +func cappedMaxAge(d time.Duration) int { + if d > maxRefreshCookieDuration { + log.Printf("[Auth] Refresh token lifetime %s exceeds the %s cap, capping", d.Round(time.Second), maxRefreshCookieDuration) + d = maxRefreshCookieDuration + } + return int(d.Seconds()) +} + +// jwtExp reads the exp claim from a JWT without verifying its signature. It +// reports false for opaque tokens, malformed JWTs, and tokens carrying no exp. +// +// The signature is not checked because the claim is only used to choose a cookie +// lifetime. Nothing is trusted on the strength of it: the token itself is still +// validated by the identity provider on every refresh, and a forged exp can only +// make the browser drop a cookie sooner or later than it needed to. +func jwtExp(token string) (time.Time, bool) { + parts := strings.Split(token, ".") + if len(parts) != 3 { + return time.Time{}, false + } + + // JWTs use unpadded base64url, but tolerate padding rather than give up on it. + payload, err := base64.RawURLEncoding.DecodeString(strings.TrimRight(parts[1], "=")) + if err != nil { + return time.Time{}, false + } + + var claims struct { + Exp json.Number `json:"exp"` + } + if err := json.Unmarshal(payload, &claims); err != nil || claims.Exp == "" { + return time.Time{}, false + } + + // exp is a NumericDate, which permits a fractional part. + seconds, err := claims.Exp.Float64() + if err != nil || seconds <= 0 { + return time.Time{}, false + } + + return time.Unix(int64(seconds), 0), true +} + +// SessionExpiresAt reports when the session recorded by the session_start cookie +// reaches maxSessionDuration. It returns the zero time when no max duration is +// configured, or when the cookie is missing or unreadable. +func SessionExpiresAt(c echo.Context, maxSessionDuration time.Duration) time.Time { + if maxSessionDuration <= 0 { + return time.Time{} + } + + cookie, err := c.Request().Cookie(sessionStartCookie) + if err != nil { + return time.Time{} + } + + startTime, err := strconv.ParseInt(cookie.Value, 10, 64) + if err != nil { + return time.Time{} + } + + return time.Unix(startTime, 0).Add(maxSessionDuration) +} + // SetSessionStart sets a cookie with the current timestamp to track when the session began. // This should only be called on initial login, NOT on token refresh. func SetSessionStart(c echo.Context, maxSessionDuration time.Duration, secure bool) { diff --git a/server/server/auth/auth_test.go b/server/server/auth/auth_test.go index e243e9a716..c96d43d827 100644 --- a/server/server/auth/auth_test.go +++ b/server/server/auth/auth_test.go @@ -96,7 +96,7 @@ func TestSetUser(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - err := auth.SetUser(tt.ctx, &tt.user, true) + err := auth.SetUser(tt.ctx, &tt.user, auth.CookieOptions{Secure: true}) cookies := tt.ctx.Cookies() if tt.wantErr { @@ -123,7 +123,7 @@ func TestSetUserSecureFlag(t *testing.T) { user := auth.User{OAuth2Token: &oauth2.Token{AccessToken: "AAA", RefreshToken: "RRR"}} a := assert.New(t) - a.NoError(auth.SetUser(c, &user, secure)) + a.NoError(auth.SetUser(c, &user, auth.CookieOptions{Secure: secure})) for _, sc := range c.Response().Header()[echo.HeaderSetCookie] { cookieName := strings.SplitN(sc, "=", 2)[0] diff --git a/server/server/auth/cookie_test.go b/server/server/auth/cookie_test.go new file mode 100644 index 0000000000..dffc8ed4df --- /dev/null +++ b/server/server/auth/cookie_test.go @@ -0,0 +1,198 @@ +// The MIT License +// +// Copyright (c) 2020 Temporal Technologies Inc. All rights reserved. +// +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package auth + +import ( + "encoding/base64" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testJWT builds an unsigned token with the shape of a JWT. Only the payload is +// read, so the header and signature segments just need to be present. +func testJWT(t *testing.T, payload string) string { + t.Helper() + enc := base64.RawURLEncoding.EncodeToString + return fmt.Sprintf("%s.%s.%s", + enc([]byte(`{"alg":"RS256","typ":"JWT"}`)), + enc([]byte(payload)), + "c2lnbmF0dXJl", + ) +} + +func TestJwtExp(t *testing.T) { + exp := time.Date(2030, time.January, 2, 3, 4, 5, 0, time.UTC) + + t.Run("reads the exp claim", func(t *testing.T) { + token := testJWT(t, fmt.Sprintf(`{"sub":"user","exp":%d}`, exp.Unix())) + + got, ok := jwtExp(token) + + require.True(t, ok) + assert.Equal(t, exp.Unix(), got.Unix()) + }) + + t.Run("accepts a fractional exp", func(t *testing.T) { + token := testJWT(t, fmt.Sprintf(`{"exp":%d.75}`, exp.Unix())) + + got, ok := jwtExp(token) + + require.True(t, ok) + assert.Equal(t, exp.Unix(), got.Unix()) + }) + + t.Run("tolerates base64 padding", func(t *testing.T) { + payload := base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf(`{"sub":"u","exp":%d}`, exp.Unix()))) + require.Contains(t, payload, "=", "this case is only meaningful with padding present") + + got, ok := jwtExp("aGVhZGVy." + payload + ".c2ln") + + require.True(t, ok) + assert.Equal(t, exp.Unix(), got.Unix()) + }) + + rejected := map[string]string{ + "opaque token": "0dGhpcy1pcy1ub3QtYS1qd3Q", + "empty": "", + "two segments": "aGVhZGVy.cGF5bG9hZA", + "four segments": "a.b.c.d", + "payload not b64": "aGVhZGVy.!!!not-base64!!!.c2ln", + "payload not json": testJWT(t, `not json at all`), + "no exp claim": testJWT(t, `{"sub":"user"}`), + "exp is a string": testJWT(t, `{"exp":"tomorrow"}`), + "exp is zero": testJWT(t, `{"exp":0}`), + "exp is negative": testJWT(t, `{"exp":-1}`), + } + + for name, token := range rejected { + t.Run("rejects "+name, func(t *testing.T) { + _, ok := jwtExp(token) + assert.False(t, ok) + }) + } +} + +func TestRefreshCookieMaxAge(t *testing.T) { + now := time.Date(2030, time.June, 1, 12, 0, 0, 0, time.UTC) + jwtExpiringIn := func(d time.Duration) string { + return testJWT(t, fmt.Sprintf(`{"exp":%d}`, now.Add(d).Unix())) + } + + tests := map[string]struct { + refreshToken string + configured time.Duration + want time.Duration + }{ + "exp claim wins over configured value": { + refreshToken: jwtExpiringIn(30 * time.Minute), + configured: 24 * time.Hour, + want: 30 * time.Minute, + }, + "exp claim is used when nothing is configured": { + refreshToken: jwtExpiringIn(12 * time.Hour), + want: 12 * time.Hour, + }, + "opaque token falls back to the configured value": { + refreshToken: "opaque-refresh-token", + configured: 24 * time.Hour, + want: 24 * time.Hour, + }, + "opaque token with no configured value falls back to the default": { + refreshToken: "opaque-refresh-token", + want: defaultRefreshCookieDuration, + }, + "already expired exp claim falls back to the configured value": { + refreshToken: jwtExpiringIn(-time.Hour), + configured: 24 * time.Hour, + want: 24 * time.Hour, + }, + "already expired exp claim with no configured value falls back to the default": { + refreshToken: jwtExpiringIn(-time.Hour), + want: defaultRefreshCookieDuration, + }, + "a very long lived token is capped": { + refreshToken: jwtExpiringIn(365 * 24 * time.Hour), + want: maxRefreshCookieDuration, + }, + "a very long configured value is capped": { + refreshToken: "opaque-refresh-token", + configured: 365 * 24 * time.Hour, + want: maxRefreshCookieDuration, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got := refreshCookieMaxAge(tt.refreshToken, tt.configured, now) + assert.Equal(t, int(tt.want.Seconds()), got) + }) + } +} + +func TestUserCookieMaxAge(t *testing.T) { + now := time.Date(2030, time.June, 1, 12, 0, 0, 0, time.UTC) + + tests := map[string]struct { + sessionExpiresAt time.Time + want int + }{ + "no max session duration configured": { + sessionExpiresAt: time.Time{}, + want: 60, + }, + "session outlasts the cookie": { + sessionExpiresAt: now.Add(8 * time.Hour), + want: 60, + }, + "session ends exactly when the cookie would": { + sessionExpiresAt: now.Add(time.Minute), + want: 60, + }, + "session ends before the cookie would": { + sessionExpiresAt: now.Add(10 * time.Second), + want: 10, + }, + "session has under a second left": { + sessionExpiresAt: now.Add(500 * time.Millisecond), + want: 1, + }, + "session is already over": { + sessionExpiresAt: now.Add(-time.Hour), + want: 1, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + got := userCookieMaxAge(tt.sessionExpiresAt, now) + assert.Equal(t, tt.want, got) + assert.Positive(t, got, "MaxAge of 0 would make this a session cookie") + }) + } +} diff --git a/server/server/config/config.go b/server/server/config/config.go index 1086707f2a..1d668b6326 100644 --- a/server/server/config/config.go +++ b/server/server/config/config.go @@ -136,6 +136,12 @@ type ( Options map[string]interface{} `yaml:"options"` // UseIDTokenAsBearer - Use ID token instead of access token as Bearer in Authorization header UseIDTokenAsBearer bool `yaml:"useIdTokenAsBearer"` + // RefreshTokenDuration - optional lifetime of the refresh token this provider issues. + // It is only needed for providers that issue opaque (non-JWT) refresh tokens, whose + // lifetime the server cannot read. For JWT refresh tokens the exp claim is used and + // this value is ignored. If neither is available, a 7 day default applies. + // Example values: "8h", "24h", "168h" (1 week). + RefreshTokenDuration time.Duration `yaml:"refreshTokenDuration"` } Codec struct { diff --git a/server/server/route/auth.go b/server/server/route/auth.go index 3eb5802ae8..cdbc133077 100644 --- a/server/server/route/auth.go +++ b/server/server/route/auth.go @@ -113,11 +113,14 @@ func SetAuthRoutes(e *echo.Echo, cfgProvider *config.ConfigProviderWithRefresh) log.Fatal(err) } + maxSessionDuration := serverCfg.Auth.MaxSessionDuration + refreshTokenDuration := providerCfg.RefreshTokenDuration + api := e.Group("/auth") api.GET("/sso", authenticate(&oauthCfg, providerCfg.Options, serverCfg.CORS.AllowOrigins, secure)) - api.GET("/sso/callback", authenticateCb(ctx, &oauthCfg, provider, serverCfg.Auth.MaxSessionDuration, secure)) - api.GET("/sso_callback", authenticateCb(ctx, &oauthCfg, provider, serverCfg.Auth.MaxSessionDuration, secure)) // compatibility with UI v1 - api.GET("/refresh", refreshTokens(ctx, &oauthCfg, provider, serverCfg.Auth.MaxSessionDuration, secure)) + api.GET("/sso/callback", authenticateCb(ctx, &oauthCfg, provider, maxSessionDuration, refreshTokenDuration, secure)) + api.GET("/sso_callback", authenticateCb(ctx, &oauthCfg, provider, maxSessionDuration, refreshTokenDuration, secure)) // compatibility with UI v1 + api.GET("/refresh", refreshTokens(ctx, &oauthCfg, provider, maxSessionDuration, refreshTokenDuration, secure)) api.GET("/logout", logout(secure)) } @@ -182,14 +185,24 @@ func authenticate(config *oauth2.Config, options map[string]interface{}, allowed } } -func authenticateCb(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc.Provider, maxSessionDuration time.Duration, secure bool) func(echo.Context) error { +func authenticateCb(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc.Provider, maxSessionDuration time.Duration, refreshTokenDuration time.Duration, secure bool) func(echo.Context) error { return func(c echo.Context) error { user, err := auth.ExchangeCode(ctx, c.Request(), oauthCfg, provider) if err != nil { return err } - err = auth.SetUser(c, user, secure) + // The session starts now, so SetSessionStart below records this same instant. + var sessionExpiresAt time.Time + if maxSessionDuration > 0 { + sessionExpiresAt = time.Now().Add(maxSessionDuration) + } + + err = auth.SetUser(c, user, auth.CookieOptions{ + Secure: secure, + SessionExpiresAt: sessionExpiresAt, + RefreshTokenDuration: refreshTokenDuration, + }) if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, "unable to set user: "+err.Error()) } @@ -217,7 +230,7 @@ func authenticateCb(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc // refreshTokens exchanges a refresh token (stored in an HttpOnly cookie) for a new access token // and optionally a new ID token. It resets the cookies using auth.SetUser and returns 200. -func refreshTokens(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc.Provider, maxSessionDuration time.Duration, secure bool) func(echo.Context) error { +func refreshTokens(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc.Provider, maxSessionDuration time.Duration, refreshTokenDuration time.Duration, secure bool) func(echo.Context) error { return func(c echo.Context) error { startTime := time.Now() clientIP := c.RealIP() @@ -267,7 +280,15 @@ func refreshTokens(ctx context.Context, oauthCfg *oauth2.Config, provider *oidc. } } - if err := auth.SetUser(c, &user, secure); err != nil { + // The session began at the session_start cookie, not now: a refresh extends + // the tokens but never the session. + opts := auth.CookieOptions{ + Secure: secure, + SessionExpiresAt: auth.SessionExpiresAt(c, maxSessionDuration), + RefreshTokenDuration: refreshTokenDuration, + } + + if err := auth.SetUser(c, &user, opts); err != nil { duration := time.Since(startTime).Milliseconds() log.Printf("token_refresh_failed reason=set_user_failed ip=%s error=%q duration_ms=%d", clientIP, err.Error(), duration) return echo.NewHTTPError(http.StatusInternalServerError, "unable to set refreshed user: "+err.Error()) diff --git a/server/server/route/auth_cookies_test.go b/server/server/route/auth_cookies_test.go new file mode 100644 index 0000000000..f8ab3242b3 --- /dev/null +++ b/server/server/route/auth_cookies_test.go @@ -0,0 +1,243 @@ +// The MIT License +// +// Copyright (c) 2020 Temporal Technologies Inc. All rights reserved. +// +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package route + +import ( + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strconv" + "testing" + "time" + + "github.com/labstack/echo/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" + "golang.org/x/oauth2" +) + +// These tests drive the real /auth/refresh handler against a real identity +// provider token endpoint over HTTP, and read the cookies off the wire, so the +// behaviour under test is the whole refresh round trip rather than a helper. +// +// The refresh path is where both reported bugs surface. It is the request that +// fails with a 401 when the refresh cookie was given the access token's lifetime +// (#3210), and it is the request that reissues user* cookies which can outlive the +// session boundary (#3223). + +// fakeIdP serves an OAuth2 token endpoint that returns a fixed token response. +type fakeIdP struct { + *httptest.Server + requests int +} + +// newFakeIdP starts a token endpoint returning the given response body. The +// caller supplies expires_in and refresh_token so each test can describe the +// provider it is standing in for. +func newFakeIdP(t *testing.T, tokenResponse map[string]any) *fakeIdP { + t.Helper() + + idp := &fakeIdP{} + idp.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + idp.requests++ + w.Header().Set("Content-Type", "application/json") + require.NoError(t, json.NewEncoder(w).Encode(tokenResponse)) + })) + t.Cleanup(idp.Close) + + return idp +} + +// signedLikeJWT builds a token with the three segment shape of a JWT carrying the +// given exp. Keycloak and other providers issue refresh tokens of this form. +func signedLikeJWT(t *testing.T, exp time.Time) string { + t.Helper() + enc := base64.RawURLEncoding.EncodeToString + return fmt.Sprintf("%s.%s.%s", + enc([]byte(`{"alg":"RS256","typ":"JWT"}`)), + enc([]byte(fmt.Sprintf(`{"sub":"test-user","typ":"Refresh","exp":%d}`, exp.Unix()))), + "c2lnbmF0dXJl", + ) +} + +// doRefresh runs the refresh handler and returns the cookies it set. +func doRefresh(t *testing.T, idp *fakeIdP, maxSessionDuration, refreshTokenDuration time.Duration, sessionStartedAt time.Time) map[string]*http.Cookie { + t.Helper() + + oauthCfg := &oauth2.Config{ + ClientID: "temporal-ui", + ClientSecret: "temporal-secret", + Endpoint: oauth2.Endpoint{TokenURL: idp.URL + "/token"}, + } + + req := httptest.NewRequest(http.MethodGet, "/auth/refresh", nil) + req.AddCookie(&http.Cookie{Name: "refresh", Value: "the-refresh-token-held-by-the-browser"}) + if !sessionStartedAt.IsZero() { + req.AddCookie(&http.Cookie{Name: "session_start", Value: strconv.FormatInt(sessionStartedAt.Unix(), 10)}) + } + + rec := httptest.NewRecorder() + c := echo.New().NewContext(req, rec) + + // provider is only dereferenced to verify an id_token, which these responses + // deliberately omit so the test needs no signing keys. + handler := refreshTokens(context.Background(), oauthCfg, nil, maxSessionDuration, refreshTokenDuration, false) + require.NoError(t, handler(c)) + require.Equal(t, http.StatusOK, rec.Code) + require.Equal(t, 1, idp.requests, "the handler should have exchanged the refresh token exactly once") + + got := map[string]*http.Cookie{} + for _, cookie := range rec.Result().Cookies() { + got[cookie.Name] = cookie + } + return got +} + +// TestRefreshCookieOutlivesTheAccessToken is the regression test for #3210. +// +// The provider reports expires_in=5, describing its access token, while issuing a +// refresh token good for seven days. Deriving the refresh cookie from expires_in +// dropped it after five seconds, so the next refresh had no cookie to send and +// came back 401. +func TestRefreshCookieOutlivesTheAccessToken(t *testing.T) { + const accessTokenTTL = 5 * time.Second + refreshTokenTTL := 7 * 24 * time.Hour + + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": signedLikeJWT(t, time.Now().Add(refreshTokenTTL)), + "token_type": "Bearer", + "expires_in": int(accessTokenTTL.Seconds()), + }) + + cookies := doRefresh(t, idp, 0, 0, time.Time{}) + + refresh := cookies["refresh"] + require.NotNil(t, refresh, "the handler must reissue the refresh cookie") + assert.Greater(t, refresh.MaxAge, int(accessTokenTTL.Seconds()), + "the refresh cookie must not expire with the access token") + assert.InDelta(t, refreshTokenTTL.Seconds(), refresh.MaxAge, 5, + "the refresh cookie should track the refresh token's own exp claim") + assert.True(t, refresh.HttpOnly, "the refresh token must stay out of reach of scripts") +} + +// TestRefreshCookieForOpaqueTokenUsesConfiguredDuration covers the providers that +// issue opaque refresh tokens, whose lifetime the server cannot read and which the +// operator therefore declares in config. +func TestRefreshCookieForOpaqueTokenUsesConfiguredDuration(t *testing.T) { + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": "an-entirely-opaque-refresh-token", + "token_type": "Bearer", + "expires_in": 5, + }) + + cookies := doRefresh(t, idp, 0, 24*time.Hour, time.Time{}) + + refresh := cookies["refresh"] + require.NotNil(t, refresh) + assert.Equal(t, int((24 * time.Hour).Seconds()), refresh.MaxAge) +} + +// TestRefreshCookieForOpaqueTokenFallsBackToDefault covers an opaque refresh token +// from a provider the operator has not configured a lifetime for. +func TestRefreshCookieForOpaqueTokenFallsBackToDefault(t *testing.T) { + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": "an-entirely-opaque-refresh-token", + "token_type": "Bearer", + "expires_in": 5, + }) + + cookies := doRefresh(t, idp, 0, 0, time.Time{}) + + refresh := cookies["refresh"] + require.NotNil(t, refresh) + assert.Equal(t, int((7 * 24 * time.Hour).Seconds()), refresh.MaxAge) +} + +// TestUserCookiesNeverOutliveTheSession is the regression test for the cookie half +// of #3223. +// +// This is the last refresh before the session boundary. The user* cookies used to +// be reissued with a flat sixty seconds, so the browser kept presenting a +// signed-in UI for another thirty seconds after the server had stopped honouring +// the session, and every API call in that window returned 401. +func TestUserCookiesNeverOutliveTheSession(t *testing.T) { + const maxSessionDuration = 90 * time.Second + sessionStartedAt := time.Now().Add(-60 * time.Second) // 30s of session left + + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": "an-entirely-opaque-refresh-token", + "token_type": "Bearer", + "expires_in": 5, + }) + + cookies := doRefresh(t, idp, maxSessionDuration, 0, sessionStartedAt) + + user := cookies["user0"] + require.NotNil(t, user, "the handler must reissue the user cookie") + assert.InDelta(t, 30, user.MaxAge, 2, "the user cookie should expire with the session") + assert.Less(t, user.MaxAge, 60, "the user cookie must not outlive the session boundary") + assert.Positive(t, user.MaxAge, "a MaxAge of 0 would make this a session cookie") +} + +// TestUserCookiesKeepDefaultWhenSessionHasRoom checks the clamp only bites near the +// boundary, and leaves the ordinary case alone. +func TestUserCookiesKeepDefaultWhenSessionHasRoom(t *testing.T) { + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": "an-entirely-opaque-refresh-token", + "token_type": "Bearer", + "expires_in": 5, + }) + + cookies := doRefresh(t, idp, 8*time.Hour, 0, time.Now()) + + user := cookies["user0"] + require.NotNil(t, user) + assert.Equal(t, 60, user.MaxAge) +} + +// TestUserCookiesUnaffectedWithoutMaxSessionDuration is the default deployment, +// where no session limit is configured and nothing should change. +func TestUserCookiesUnaffectedWithoutMaxSessionDuration(t *testing.T) { + idp := newFakeIdP(t, map[string]any{ + "access_token": "new-access-token", + "refresh_token": "an-entirely-opaque-refresh-token", + "token_type": "Bearer", + "expires_in": 5, + }) + + cookies := doRefresh(t, idp, 0, 0, time.Time{}) + + user := cookies["user0"] + require.NotNil(t, user) + assert.Equal(t, 60, user.MaxAge) +} diff --git a/tests/e2e/auth-cookie-lifetimes.spec.ts b/tests/e2e/auth-cookie-lifetimes.spec.ts new file mode 100644 index 0000000000..353c999d34 --- /dev/null +++ b/tests/e2e/auth-cookie-lifetimes.spec.ts @@ -0,0 +1,145 @@ +import { expect, type Page, type Response, test } from '@playwright/test'; + +import { + AUTH_UI_SERVER_URL, + E2E_AUTH_TTL, +} from '../../utilities/auth-e2e-stack'; + +/** + * End to end coverage for the auth cookie lifetimes, against a real identity + * provider and a real auth-enabled ui-server (see utilities/auth-e2e-stack.ts). + * + * These lifetimes are decided by the Go server and reach the browser only as + * Set-Cookie headers, so a test that mocks the auth endpoints — as + * tests/integration/oauth-flow.spec.ts does — cannot observe them. That gap is + * what this file covers. + * + * Regression tests for: + * #3210 — the refresh cookie was given the access token's lifetime, so it was + * gone at the moment a refresh needed it, and the refresh 401'd. + * #3223 — the user* cookies were issued for a flat 60s, so they outlived a + * shorter session, leaving a signed-in UI whose API calls all 401'd. + */ + +/** maxSessionDuration in server/config/e2e-auth.yaml, in seconds. */ +const MAX_SESSION_DURATION = 45; +/** refreshTokenDuration in server/config/e2e-auth.yaml, in seconds. */ +const REFRESH_TOKEN_DURATION = 60 * 60 * 24; +/** The lifetime SetUser gives user* cookies when the session has room to spare. */ +const DEFAULT_USER_COOKIE_MAX_AGE = 60; + +// This suite drives a real login, so it must not inherit the signed-in state +// the rest of the E2E suite shares. +test.use({ storageState: { cookies: [], origins: [] } }); + +type CookieMaxAges = Record; + +/** Reads the Max-Age the server sent, rather than a value derived in the browser. */ +const parseSetCookieMaxAges = (setCookieHeader: string): CookieMaxAges => { + const maxAges: CookieMaxAges = {}; + + for (const line of setCookieHeader.split('\n')) { + const name = line.split('=')[0]?.trim(); + const maxAge = /max-age=(-?\d+)/i.exec(line)?.[1]; + if (name && maxAge) maxAges[name] = Number(maxAge); + } + + return maxAges; +}; + +/** + * Signs in through the identity provider and returns the Max-Age of each cookie + * the ui-server set on the callback. + */ +const signIn = async (page: Page): Promise => { + const callback = page.waitForResponse( + (response: Response) => + response.url().includes('/auth/sso/callback') && response.status() < 400, + ); + + await page.goto(`${AUTH_UI_SERVER_URL}/auth/sso`); + + await page.locator('input[name="login"]').fill('e2e@temporal.io'); + await page.locator('input[name="password"]').fill('any-password'); + await page.locator('form[action$="/login"] button[type="submit"]').click(); + + // The provider asks for consent the first time a client is authorized. + const consent = page.locator( + 'form[action$="/confirm"] button[type="submit"]', + ); + await consent.click({ timeout: 5000 }).catch(() => { + // Already authorized, so it redirected straight back. + }); + + const response = await callback; + const setCookie = (await response.headersArray()) + .filter((header) => header.name.toLowerCase() === 'set-cookie') + .map((header) => header.value) + .join('\n'); + + return parseSetCookieMaxAges(setCookie); +}; + +/** Asks the ui-server to refresh, from a page on its own origin so the cookies ride along. */ +const refresh = (page: Page): Promise => + page.evaluate(async () => { + const response = await fetch('/auth/refresh', { credentials: 'include' }); + return response.status; + }); + +test('refresh cookie outlives the access token', async ({ page }) => { + test.setTimeout(30_000); + + const maxAges = await signIn(page); + + // The bug: this was the access token's lifetime, so the cookie expired at the + // very moment the refresh it exists for came due. + expect(maxAges.refresh).toBeGreaterThan(E2E_AUTH_TTL.ACCESS_TOKEN); + + // The mock provider issues opaque refresh tokens, so the configured + // refreshTokenDuration is the value that should apply. + expect(maxAges.refresh).toBe(REFRESH_TOKEN_DURATION); +}); + +test('user cookies do not outlive the session', async ({ page }) => { + test.setTimeout(30_000); + + const maxAges = await signIn(page); + + // The bug: a flat 60s, which outlives this 45s session and leaves the browser + // holding credentials the server has already stopped honouring. + expect(maxAges.user0).toBeLessThanOrEqual(MAX_SESSION_DURATION); + expect(maxAges.user0).toBeLessThan(DEFAULT_USER_COOKIE_MAX_AGE); + expect(maxAges.user0).toBeGreaterThan(0); +}); + +test('token refresh succeeds after the access token has expired', async ({ + page, +}) => { + test.setTimeout(40_000); + + await signIn(page); + + // Outlive the access token. This is the moment the refresh cookie has to + // still be in the browser, and only real elapsed time gets us there. + // eslint-disable-next-line playwright/no-wait-for-timeout + await page.waitForTimeout((E2E_AUTH_TTL.ACCESS_TOKEN + 2) * 1000); + + // The bug: 401, because the refresh cookie had already expired and so the + // browser sent nothing. + expect(await refresh(page)).toBe(200); +}); + +test('refresh is refused once the session has expired', async ({ page }) => { + test.setTimeout(90_000); + + await signIn(page); + + // As above, the session boundary is a wall clock deadline on the server. + // eslint-disable-next-line playwright/no-wait-for-timeout + await page.waitForTimeout((MAX_SESSION_DURATION + 2) * 1000); + + // Lengthening the cookies must not turn maxSessionDuration into a limit that + // no longer ends a session. + expect(await refresh(page)).toBe(401); +}); diff --git a/tests/global-setup.ts b/tests/global-setup.ts index a31478411a..c2481c6654 100644 --- a/tests/global-setup.ts +++ b/tests/global-setup.ts @@ -3,6 +3,7 @@ import { chromium, FullConfig } from '@playwright/test'; import { connect, startWorkflows } from '../temporal/client'; import { createCodecServer } from '../temporal/codec-server'; import { runWorker } from '../temporal/worker'; +import { startAuthStack } from '../utilities/auth-e2e-stack'; import { createTemporalServer } from '../utilities/temporal-server'; import { createUIServer } from '../utilities/ui-server'; @@ -23,6 +24,12 @@ const setupDependencies = async () => { const client = await connect(); await runWorker(); await startWorkflows(client, { waitForResult: false }); + + // A second, auth-enabled ui-server plus a real identity provider, for + // tests/e2e/auth-cookie-lifetimes.spec.ts. The rest of the suite is + // unaffected: this listens on its own port and the other tests never + // navigate to it. + await startAuthStack(); } catch (e) { console.log('Error setting up server: ', e); } diff --git a/tests/global-teardown.ts b/tests/global-teardown.ts index e61912d0cc..056d79b6c8 100644 --- a/tests/global-teardown.ts +++ b/tests/global-teardown.ts @@ -3,6 +3,7 @@ import { FullConfig } from '@playwright/test'; import { disconnect, stopWorkflows } from '../temporal/client'; import { getCodecServer } from '../temporal/codec-server'; import { stopWorker } from '../temporal/worker'; +import { stopAuthStack } from '../utilities/auth-e2e-stack'; import { getTemporalServer } from '../utilities/temporal-server'; import { getUIServer } from '../utilities/ui-server'; @@ -12,11 +13,12 @@ export default async function (config: FullConfig) { const codecServer = getCodecServer(); const uiServer = getUIServer(); + await stopAuthStack(); await stopWorkflows(); await stopWorker(); await disconnect(); await codecServer.stop(); - await uiServer.shutdown(); + await uiServer?.shutdown(); await temporal.shutdown(); } } diff --git a/utilities/auth-e2e-stack.ts b/utilities/auth-e2e-stack.ts new file mode 100644 index 0000000000..14696f0f95 --- /dev/null +++ b/utilities/auth-e2e-stack.ts @@ -0,0 +1,87 @@ +import waitForPort from 'wait-port'; + +import { + Account, + getConfig, + OIDCServer, + providerConfiguration, + routes, +} from './oidc-server'; +import { createUIServer, getUIServer, type UIServer } from './ui-server'; + +/** + * The auth stack for the E2E suite: a real identity provider and a real + * auth-enabled ui-server, alongside the unauthenticated one the rest of the + * suite uses. + * + * The cookie lifetimes under test are decided entirely by the Go server, so a + * test that mocks the auth endpoints cannot see them. This starts the real + * thing instead. + */ + +/** + * Token lifetimes for the suite, in seconds. + * + * These are much shorter than the defaults in the OIDC server's own + * configuration, which are tuned for a human clicking through + * `pnpm dev:with-auth`. A test needs to outlive an access token in a few + * seconds, not a minute. + * + * ACCESS_TOKEN must stay below the 45s maxSessionDuration in + * server/config/e2e-auth.yaml, so that a refresh happens inside the session. + */ +export const E2E_AUTH_TTL = { + ACCESS_TOKEN: 5, + ID_TOKEN: 5, + /** Well past the session, so the refresh cookie is never the reason a refresh fails. */ + REFRESH_TOKEN: 60 * 60 * 24, + SESSION: 120, +} as const; + +/** The auth-enabled ui-server's port, from server/config/e2e-auth.yaml. */ +export const AUTH_UI_SERVER_URL = 'http://localhost:8081'; + +let oidcServer: OIDCServer | undefined; + +export const startAuthStack = async (): Promise => { + const { PORT, ISSUER, VIEWS_PATH } = getConfig(); + + oidcServer = new OIDCServer({ + issuer: ISSUER, + port: PORT, + viewsPath: VIEWS_PATH, + // Overridden per instance rather than edited in place, so the shared + // configuration keeps serving `pnpm dev:with-auth` unchanged. + providerConfiguration: { + ...providerConfiguration, + ttl: { + ...(providerConfiguration.ttl ?? {}), + AccessToken: E2E_AUTH_TTL.ACCESS_TOKEN, + IdToken: E2E_AUTH_TTL.ID_TOKEN, + RefreshToken: E2E_AUTH_TTL.REFRESH_TOKEN, + Session: E2E_AUTH_TTL.SESSION, + }, + }, + accountModel: Account, + routes, + }); + + await oidcServer.start(); + await waitForPort({ port: PORT, output: 'silent' }); + console.log(`✨ OIDC server running on port ${PORT}`); + + const authUIServer = await createUIServer('e2e-auth'); + await authUIServer.ready(); + + return authUIServer; +}; + +export const stopAuthStack = async (): Promise => { + await getUIServer('e2e-auth')?.shutdown(); + + if (oidcServer) { + oidcServer.stop(); + oidcServer = undefined; + console.log('🔪 killed OIDC server'); + } +}; diff --git a/utilities/ui-server.ts b/utilities/ui-server.ts index 9d04b40b0c..50733212a2 100644 --- a/utilities/ui-server.ts +++ b/utilities/ui-server.ts @@ -8,12 +8,15 @@ export type UIServer = { ready: () => ReturnType; }; -export type ValidEnv = 'development' | 'e2e' | 'with-auth'; +export type ValidEnv = 'development' | 'e2e' | 'e2e-auth' | 'with-auth'; -let uiServer: UIServer; +// Keyed by env, because the E2E run needs two servers at once: the main one on +// 8080 and an auth-enabled one on 8081. A single slot would leave whichever +// started first without a handle to shut it down. +const uiServers = new Map(); -export const getUIServer = (): UIServer => { - return uiServer; +export const getUIServer = (env: ValidEnv = 'e2e'): UIServer | undefined => { + return uiServers.get(env); }; const portForEnv = (env: ValidEnv) => { @@ -71,10 +74,12 @@ export const createUIServer = async ( return waitForPort({ port: portForEnv(env), output: 'silent' }); }; - uiServer = { + const server: UIServer = { shutdown, ready, }; - return uiServer; + uiServers.set(env, server); + + return server; };