From 8a482a4e84c95bec1ad30709c19af1f668c8498d Mon Sep 17 00:00:00 2001 From: Ross Nelson Date: Thu, 17 Sep 2026 16:56:29 -0400 Subject: [PATCH 1/3] fix(auth): derive cookie lifetimes from the session and refresh token The refresh cookie was given the access token's lifetime, and the user* cookies a flat minute regardless of the session boundary. Both are fixed here, along with the missing Docker settings that made the second one impossible to configure. Refresh cookie (#3210). Its MaxAge came from oauth2.Token.Expiry, which is populated from the token response's expires_in. Per RFC 6749 5.1 and OIDC Core 3.2.2.5 that describes the access token, not the refresh token, so the cookie was dropped at the moment the access token expired and the refresh it existed to perform came back 401. The lifetime now comes from the refresh token's own exp claim where the provider issues a JWT, then from a new per-provider refreshTokenDuration for providers that issue opaque tokens, then a 7 day default. The existing 30 day cap is kept. User cookies (#3223). They were always issued for 60 seconds, so a refresh shortly before the session boundary left the browser holding credentials the server had already stopped honouring: a signed-in UI whose every API call returned 401. They are now clamped to whatever is left of the session. Docker config (#3223). maxSessionDuration was enforced but absent from docker.yaml, so it could not be set without a wholly custom config file. Both it and refreshTokenDuration are now exposed, and both default to unset so existing deployments are unaffected. Also corrects docs advising `maxSessionDuration: 0`, which yaml.v3 rejects as an int rather than a duration. The working spelling is `0s`. --- AUTHENTICATION.md | 105 ++++++-- server/config/docker.yaml | 2 + server/config/with-auth.yaml | 4 +- .../plugins/fs_config_provider/loader_test.go | 32 +++ server/server/auth/auth.go | 171 ++++++++++-- server/server/auth/auth_test.go | 4 +- server/server/auth/cookie_test.go | 198 ++++++++++++++ server/server/config/config.go | 6 + server/server/route/auth.go | 35 ++- server/server/route/auth_cookies_test.go | 243 ++++++++++++++++++ 10 files changed, 745 insertions(+), 55 deletions(-) create mode 100644 server/server/auth/cookie_test.go create mode 100644 server/server/route/auth_cookies_test.go 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/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) +} From 494ce8b02fba4b4bf8624882c256dcd6094a73e2 Mon Sep 17 00:00:00 2001 From: Ross Nelson Date: Thu, 17 Sep 2026 18:55:07 -0400 Subject: [PATCH 2/3] test(auth): cover cookie lifetimes end to end against a real provider The Go tests added alongside the fix drive the refresh handler in process. That covers the logic but stops short of the thing users actually hit: a browser, a real identity provider, and cookies the Go server writes as Set-Cookie headers. tests/integration/oauth-flow.spec.ts cannot fill the gap either, since it mocks /auth/sso and /auth/refresh outright, which is exactly what would need to be real. This starts a second, auth-enabled ui-server on 8081 alongside the existing E2E one, backed by the repo's own mock OIDC provider, and drives a real login against it. The e2e CI job already builds the Go server and runs the Playwright suite, so this needs no new job or infrastructure. Three of the four tests fail against the previous behaviour: the refresh cookie carries the access token's 5s lifetime rather than 24h, the user0 cookie is issued for 60s against a 45s session, and the refresh after the access token expires returns 401. The fourth asserts that session expiry still ends a session, and passes either way, so that lengthening the cookies cannot quietly disable maxSessionDuration. The ui-server test harness tracked one server in a module level variable, which two concurrent servers would clobber, leaving the first without a handle to shut down. It is now keyed by env. --- server/config/e2e-auth.yaml | 49 ++++++++ tests/e2e/auth-cookie-lifetimes.spec.ts | 145 ++++++++++++++++++++++++ tests/global-setup.ts | 7 ++ tests/global-teardown.ts | 2 + utilities/auth-e2e-stack.ts | 87 ++++++++++++++ utilities/ui-server.ts | 17 ++- 6 files changed, 301 insertions(+), 6 deletions(-) create mode 100644 server/config/e2e-auth.yaml create mode 100644 tests/e2e/auth-cookie-lifetimes.spec.ts create mode 100644 utilities/auth-e2e-stack.ts 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/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..e07a6d4f09 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,6 +13,7 @@ export default async function (config: FullConfig) { const codecServer = getCodecServer(); const uiServer = getUIServer(); + await stopAuthStack(); await stopWorkflows(); await stopWorker(); await disconnect(); 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..15659f780c 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 => { + 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; }; From 0c59d7ec5e9edfac1b527acdf39980c004d684fb Mon Sep 17 00:00:00 2001 From: Ross Nelson Date: Thu, 17 Sep 2026 19:03:20 -0400 Subject: [PATCH 3/3] fix(test): let getUIServer report a missing server Map.get returns UIServer | undefined. The previous module level variable was typed as UIServer and hid that, so keying the registry surfaced a strict mode error the old shape had been papering over. The return type now says what it returns, and the one caller that assumed a server handles its absence. --- tests/global-teardown.ts | 2 +- utilities/ui-server.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/global-teardown.ts b/tests/global-teardown.ts index e07a6d4f09..056d79b6c8 100644 --- a/tests/global-teardown.ts +++ b/tests/global-teardown.ts @@ -18,7 +18,7 @@ export default async function (config: FullConfig) { await stopWorker(); await disconnect(); await codecServer.stop(); - await uiServer.shutdown(); + await uiServer?.shutdown(); await temporal.shutdown(); } } diff --git a/utilities/ui-server.ts b/utilities/ui-server.ts index 15659f780c..50733212a2 100644 --- a/utilities/ui-server.ts +++ b/utilities/ui-server.ts @@ -15,7 +15,7 @@ export type ValidEnv = 'development' | 'e2e' | 'e2e-auth' | 'with-auth'; // started first without a handle to shut it down. const uiServers = new Map(); -export const getUIServer = (env: ValidEnv = 'e2e'): UIServer => { +export const getUIServer = (env: ValidEnv = 'e2e'): UIServer | undefined => { return uiServers.get(env); };