%s
` - if body == nil { - if err != nil { - body = fmt.Appendf(nil, tmpl, statusCode, http.StatusText(statusCode), html.EscapeString(err.Error())) - } - } - _, _ = w.Write(body) -} - // SetHeaders writes the HTTP response headers for all OAuth endpoint responses. // // It defaults to DefaultSetHeaders and may be reassigned at startup to customize @@ -157,16 +307,20 @@ func DefaultSetHeaders(hw http.ResponseWriter, ishttps bool) { hw.Header().Set("Cache-Control", "no-store") } -func (srv *Server) writeResult(hw http.ResponseWriter, statusCode int, err error, body []byte) { +func (srv *Server) writeResult(hw http.ResponseWriter, statusCode int, err error) { SetHeaders(hw, srv.ishttps) hw.WriteHeader(statusCode) - writeBody(hw, statusCode, err, body) + if err != nil { + const tmpl = `%s
` + body := fmt.Appendf(nil, tmpl, statusCode, http.StatusText(statusCode), html.EscapeString(err.Error())) + _, _ = hw.Write(body) // #nosec G705 -- error text is HTML-escaped + } } // ErrOAuth2NotConfigured means OAuth2/OIDC is not configured on the Server. var ErrOAuth2NotConfigured = errors.New("oauth2 not configured") -// ErrOAuth2MissingSession means no jaws session was available to carry the OAuth2 flow state. +// ErrOAuth2MissingSession means the session or its pending flow is no longer usable. var ErrOAuth2MissingSession = errors.New("oauth2 missing session") // ErrOAuth2MissingState means the callback session did not contain the expected state value. @@ -175,17 +329,32 @@ var ErrOAuth2MissingState = errors.New("oauth2 missing state") // ErrOAuth2WrongState means the callback state value did not match the stored session state. var ErrOAuth2WrongState = errors.New("oauth2 wrong state") +// ErrOAuth2ExpiredState means the provider returned after the flow lifetime. +var ErrOAuth2ExpiredState = errors.New("oauth2 expired state") + // ErrOAuth2MissingPKCEVerifier means the callback session did not contain the required PKCE verifier. var ErrOAuth2MissingPKCEVerifier = errors.New("oauth2 missing pkce verifier") +// ErrOAuth2LoginInProgress means a callback has consumed the pending login state. +var ErrOAuth2LoginInProgress = errors.New("oauth2 login already in progress") + +// ErrOAuth2InvalidOrigin means a link start request failed the same-origin check. +var ErrOAuth2InvalidOrigin = errors.New("oauth2 invalid origin") + // ErrUserInfoStatus means the UserInfo endpoint returned a non-200 HTTP status. var ErrUserInfoStatus = errors.New("userinfo status") -func randomHexString() string { - b := [32]byte{} - _, _ = rand.Read(b[:]) // never returns an error, always fills all of b - return hex.EncodeToString(b[:]) -} +// ErrUserInfoTooLarge means the authenticated profile exceeded the response limit. +var ErrUserInfoTooLarge = errors.New("userinfo response too large") + +// ErrUserInfoInvalidResponse means the UserInfo response was not one JSON object. +var ErrUserInfoInvalidResponse = errors.New("userinfo invalid response") + +// ErrOAuth2InvalidProfile means the profile or access token could not prove a stable subject. +var ErrOAuth2InvalidProfile = errors.New("oauth2 invalid profile") + +// ErrOAuth2LinkRejected means the application did not accept a proven link. +var ErrOAuth2LinkRejected = errors.New("oauth2 link rejected") // mergeUserInfoClaims accepts fallback claims only for the verified subject. // Email verification stays paired with the email from the same response. @@ -227,7 +396,7 @@ func (srv *Server) extractEmail(claims map[string]any) (sessEmailValue any, veri } } if l := srv.Jaws.Logger; l != nil { - l.Warn("jawsauth: no email found", "userinfo", claims) + l.Warn("jawsauth: no email found") } return } @@ -241,6 +410,10 @@ func extractEmailVerified(claims map[string]any) (verified bool) { verified, _ = strconv.ParseBool(value) case float64: verified = value != 0 + case json.Number: + if number, err := value.Float64(); err == nil { + verified = number != 0 + } } } return @@ -249,19 +422,41 @@ func extractEmailVerified(claims map[string]any) (verified bool) { func (srv *Server) fetchUserInfo(ctx context.Context, userinfoURL string, tokenSource oauth2.TokenSource) (userinfo map[string]any, err error) { if userinfoURL != "" && tokenSource != nil { client := oauth2.NewClient(ctx, tokenSource) + client.CheckRedirect = func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + timeout := boundedProviderClient(client).Timeout + requestCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + // oauth2.Transport logs from its deprecated CancelRequest; use the request context. + client.Timeout = 0 + var request *http.Request + if request, err = http.NewRequestWithContext(requestCtx, http.MethodGet, userinfoURL, nil); err != nil { + return nil, err + } var resp *http.Response - if resp, err = client.Get(userinfoURL); /*#nosec G704*/ err == nil { + if resp, err = client.Do(request); /*#nosec G704*/ err == nil { defer func() { if closeErr := resp.Body.Close(); err == nil && closeErr != nil { err = closeErr } }() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("%w %s", ErrUserInfoStatus, resp.Status) + } var body []byte - if body, err = io.ReadAll(io.LimitReader(resp.Body, 32768)); err == nil { - if resp.StatusCode == http.StatusOK { - err = json.Unmarshal(body, &userinfo) + if body, err = io.ReadAll(io.LimitReader(resp.Body, maxUserInfoBytes+1)); err == nil { + if len(body) > maxUserInfoBytes { + err = ErrUserInfoTooLarge + } else if !json.Valid(body) { + err = ErrUserInfoInvalidResponse } else { - err = fmt.Errorf("%w %s", ErrUserInfoStatus, resp.Status) + decoder := json.NewDecoder(bytes.NewReader(body)) + if srv.profileSubject != nil { + decoder.UseNumber() + } + err = decoder.Decode(&userinfo) + if err != nil || userinfo == nil { + userinfo, err = nil, ErrUserInfoInvalidResponse + } } } } @@ -275,130 +470,231 @@ func (srv *Server) addUserInfoClaims(ctx context.Context, claims map[string]any, } } -// HandleAuthResponse handles the OIDC redirect/callback endpoint. +func (srv *Server) verifyOAuthIdentity(ctx context.Context, token *oauth2.Token, nonce string) (claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time, status int, err error) { + status = http.StatusUnauthorized + if srv.idTokenVerifier != nil { + var raw string + if token != nil { + raw, _ = token.Extra("id_token").(string) + } + idToken, verifiedClaims, verifyErr := srv.verifyIDToken(ctx, raw) + if verifyErr != nil { + return nil, nil, time.Time{}, status, verifyErr + } + if nonce == "" { + return nil, nil, time.Time{}, status, ErrOIDCMissingNonce + } + if idToken.Nonce != nonce { + return nil, nil, time.Time{}, status, ErrOIDCNonceMismatch + } + claims = verifiedClaims + tokenSource = srv.oauth2cfg.TokenSource(srv.oauth2Context(context.Background()), token) + srv.addUserInfoClaims(ctx, claims, oauth2.StaticTokenSource(token)) + if err = srv.identityExpiryError(idToken.Expiry); err != nil { + return nil, nil, time.Time{}, status, err + } + return claims, tokenSource, idToken.Expiry, 0, nil + } + if srv.profileSubject == nil || !profileTokenUnexpired(token) { + return nil, nil, time.Time{}, status, ErrOAuth2InvalidProfile + } + claims, err = srv.fetchUserInfo(ctx, srv.userinfoUrl, oauth2.StaticTokenSource(token)) + if err != nil { + return nil, nil, time.Time{}, http.StatusBadGateway, err + } + // UserInfo can outlive the token; reject it before calling application code. + if !profileTokenUnexpired(token) { + return nil, nil, time.Time{}, http.StatusUnauthorized, ErrOAuth2InvalidProfile + } + var subject string + subject, err = srv.profileSubject(claims) + if srv.Jaws.Log(err) != nil || subject == "" { + return nil, nil, time.Time{}, status, ErrOAuth2InvalidProfile + } + claims["iss"] = srv.issuer + claims["sub"] = subject + expiry = time.Now().Add(profileAuthLifetime) + if !token.Expiry.IsZero() && token.Expiry.Before(expiry) { + expiry = token.Expiry + } + if err = srv.identityExpiryError(expiry); err != nil { + return nil, nil, time.Time{}, status, err + } + return claims, nil, expiry, 0, nil +} + +func profileTokenUnexpired(token *oauth2.Token) bool { + if token == nil || token.AccessToken == "" { + return false + } + if token.Expiry.IsZero() { + // oauth2 uses zero Expiry for both omitted and explicit zero expires_in. + switch expiresIn := token.Extra("expires_in").(type) { + case nil: + return true + case string: + return expiresIn == "" // omitted in a form-encoded token response + } + return false + } + return token.Expiry.After(time.Now()) +} + +func (srv *Server) rotateOAuthSession(hw http.ResponseWriter, hr *http.Request, old *jaws.Session, flow pendingOAuthFlow) (rotated *jaws.Session, hadTimer, stale bool) { + rotationLock := srv.SessionRotationLock + if rotationLock != nil { + rotationLock.Lock() + defer rotationLock.Unlock() + } + if flow.revoked() || srv.Jaws.GetSession(hr) != old { + return nil, false, true + } + if rotated = srv.Jaws.NewSession(hw, hr); rotated != nil { + srv.mu.Lock() + hadTimer = srv.authTimers[old.ID()] != nil + srv.stopSessionAuthTimerLocked(old, nil) + srv.mu.Unlock() + } + return +} + +func (srv *Server) completeLink(sess *jaws.Session, hr *http.Request, claims map[string]any, expiry time.Time, flow pendingOAuthFlow) (status int, err error) { + rotationLock := srv.SessionRotationLock + if rotationLock != nil { + rotationLock.Lock() + defer rotationLock.Unlock() + } + if flow.revoked() || srv.Jaws.GetSession(hr) != sess { + return http.StatusForbidden, ErrOAuth2MissingSession + } + if err = srv.identityExpiryError(expiry); err != nil { + return http.StatusUnauthorized, err + } + status, err = srv.LinkEvent(sess, hr, claims) + if srv.Jaws.Log(err) != nil || status != 0 { + if status < 400 || status > 599 { + status = http.StatusForbidden + } + return status, ErrOAuth2LinkRejected + } + return http.StatusFound, nil +} + +func (srv *Server) completeLogin(hw http.ResponseWriter, hr *http.Request, old *jaws.Session, flow pendingOAuthFlow, claims map[string]any, tokenSource oauth2.TokenSource, expiry time.Time) (sess *jaws.Session, status int, err error) { + sess, status, err = old, http.StatusServiceUnavailable, ErrOAuth2MissingSession + rotated, hadTimer, stale := srv.rotateOAuthSession(hw, hr, old, flow) + if rotated == nil { + if stale { + status = http.StatusForbidden + } else { + hw.Header().Set("Retry-After", "60") + } + return + } + if hadTimer && srv.LogoutEvent != nil { + srv.LogoutEvent(old, hr) + } + sess = rotated + err = srv.storeSessionAuthClaimsIfActive(sess, claims, tokenSource, expiry, nil, flow.canceled) + if err != nil { + http.SetCookie(hw, rotated.Close()) + if errors.Is(err, ErrOAuth2MissingSession) { + status = http.StatusForbidden + } else { + status = http.StatusUnauthorized + } + return + } + return sess, http.StatusFound, nil +} + +func (srv *Server) completeOAuthCallback(ctx context.Context, hw http.ResponseWriter, hr *http.Request, sess *jaws.Session, flow pendingOAuthFlow) (*jaws.Session, int, error) { + status, err := oauth2CallbackError(http.StatusBadRequest, hr) + if err != nil { + return sess, status, err + } + if flow.verifier == "" { + return sess, http.StatusBadRequest, ErrOAuth2MissingPKCEVerifier + } + token, err := srv.oauth2cfg.Exchange(ctx, hr.FormValue("code") /* #nosec G120 */, oauth2.VerifierOption(flow.verifier)) + if srv.Jaws.Log(err) != nil { + status := http.StatusBadGateway + var retrieveErr *oauth2.RetrieveError + if errors.As(err, &retrieveErr) && retrieveErr.Response != nil && + retrieveErr.Response.StatusCode == http.StatusBadRequest { + status = http.StatusBadRequest + } + return sess, status, err + } + claims, tokenSource, expiry, status, err := srv.verifyOAuthIdentity(ctx, token, flow.nonce) + if err != nil { + return sess, status, err + } + if flow.link { + status, err = srv.completeLink(sess, hr, claims, expiry, flow) + } else { + sess, status, err = srv.completeLogin(hw, hr, sess, flow, claims, tokenSource, expiry) + } + return sess, status, err +} + +// HandleAuthResponse handles the OIDC or OAuth2 profile callback endpoint. // // For GET requests it validates the state, exchanges the authorization code using the -// stored PKCE verifier, verifies the id_token and its nonce, fetches UserInfo, and stores -// the claims in a new session. It invokes [Server.LoginEvent] on success or -// [Server.LoginFailed] on failure. Non-GET requests receive 405. +// stored PKCE verifier, verifies the ID token or authenticated profile, and stores +// claims in a new session. A link flow instead invokes [Server.LinkEvent] +// and keeps the original session. It invokes [Server.LoginEvent] after a +// new sign-in or [Server.LoginFailed] on login or link failure. Non-GET requests receive 405. // // Rotation discards pre-login application data and requires a free JaWS session slot. // If no slot is available, existing authentication is preserved and the default // response is 503. The user must restart login after capacity becomes available. func (srv *Server) HandleAuthResponse(hw http.ResponseWriter, hr *http.Request) { - statusCode := http.StatusMethodNotAllowed - err := ErrOAuth2Callback - - if hr.Method == http.MethodGet { - oauth2Config, location := srv.begin(hr) - var sessValue any - var sessEmail string - authctx := srv.oauth2Context(hr.Context()) - sess := srv.Jaws.GetSession(hr) + if hr.Method != http.MethodGet { + hw.Header().Set("Allow", http.MethodGet) + srv.writeResult(hw, http.StatusMethodNotAllowed, ErrOAuth2Callback) + return + } + providerCtx, cancel := context.WithTimeout(hr.Context(), oauth2ProviderTimeout) + defer cancel() + authctx := srv.oauth2Context(providerCtx) + sess := srv.Jaws.GetSession(hr) + var sessEmail string + if sess != nil { + sessEmail, _ = sess.Get(srv.SessionEmailKey).(string) + } + statusCode, err := http.StatusInternalServerError, ErrOAuth2NotConfigured + var flow pendingOAuthFlow + if srv.oauth2cfg != nil { + statusCode, err = http.StatusBadRequest, ErrOAuth2MissingSession if sess != nil { - sessEmail, _ = sess.Get(srv.SessionEmailKey).(string) - } - err = ErrOAuth2NotConfigured - statusCode = http.StatusInternalServerError - - if oauth2Config != nil { - err = ErrOAuth2MissingSession - statusCode = http.StatusBadRequest - if sess != nil { - gotState := hr.FormValue("state") // #nosec G120 - wantState, _ := sess.Get(oauth2StateKey).(string) - verifier, _ := sess.Get(oauth2PKCEVerifierKey).(string) - wantNonce, _ := sess.Get(oauth2NonceKey).(string) - sess.Set(oauth2StateKey, nil) - sess.Set(oauth2PKCEVerifierKey, nil) - sess.Set(oauth2NonceKey, nil) - err = ErrOAuth2MissingState - if wantState != "" { - err = ErrOAuth2WrongState - if wantState == gotState { - if statusCode, err = oauth2CallbackError(statusCode, hr); err == nil { - err = ErrOAuth2MissingPKCEVerifier - if verifier != "" { - var token *oauth2.Token - exchangeOptions := []oauth2.AuthCodeOption{ - oauth2.AccessTypeOffline, - oauth2.VerifierOption(verifier), - } - if token, err = oauth2Config.Exchange(authctx, hr.FormValue("code") /* #nosec G120 */, exchangeOptions...); srv.Jaws.Log(err) == nil { - err = ErrOAuth2NotConfigured - statusCode = http.StatusInternalServerError - if srv.idTokenVerifier != nil { - rawIDToken, _ := token.Extra("id_token").(string) - statusCode = http.StatusUnauthorized - err = ErrOIDCMissingIDToken - if rawIDToken != "" { - var idToken *oidc.IDToken - if idToken, err = srv.idTokenVerifier.Verify(authctx, rawIDToken); wrapOIDC(ErrOIDCInvalidIDToken, &err) == nil { - err = ErrOIDCMissingNonce - if wantNonce != "" { - err = ErrOIDCNonceMismatch - if idToken.Nonce == wantNonce { - var claims map[string]any - if err = idToken.Claims(&claims); wrapOIDC(ErrOIDCInvalidIDToken, &err) == nil { - tokenSource := oauth2Config.TokenSource(srv.oauth2Context(context.Background()), token) - // Finish network work before starting the new session's idle lifetime. - srv.addUserInfoClaims(authctx, claims, tokenSource) - referrer := sess.Get(oauth2ReferrerKey) - requests := sess.Requests() - _, hadAuth := srv.sessionAuthStatus(sess, nil) - err = ErrOAuth2MissingSession - statusCode = http.StatusServiceUnavailable - if rotated := srv.Jaws.NewSession(hw, hr); rotated != nil { - srv.mu.Lock() - hadTimer := srv.authTimers[sess.ID()] != nil - srv.stopSessionAuthTimerLocked(sess, nil) - srv.mu.Unlock() - reloadAuthRequests(requests) - if hadAuth && hadTimer && srv.LogoutEvent != nil { - srv.LogoutEvent(sess, hr) - } - sess = rotated - sess.Set(oauth2ReferrerKey, referrer) - err = srv.storeSessionAuthClaims(sess, claims, tokenSource, idToken.Expiry, nil) - } else { - hw.Header().Set("Retry-After", "60") - } - if err == nil { - sessValue = claims - sessEmail, _ = sess.Get(srv.SessionEmailKey).(string) - if s, ok := sess.Get(oauth2ReferrerKey).(string); ok { - location = sanitizeRedirectTarget(hr.Host, s) - } - sess.Set(oauth2ReferrerKey, nil) - hw.Header().Set("Location", location) - statusCode = http.StatusFound - } - } - } - } - } - } - } - } - } - } - } + flow, err = srv.takeOAuthFlow(sess, hr.FormValue("state") /* #nosec G120 */) + if err == nil { + defer srv.finishOAuthFlow(sess, flow) + sess, statusCode, err = srv.completeOAuthCallback(authctx, hw, hr, sess, flow) + if err == nil && !flow.link { + sessEmail, _ = sess.Get(srv.SessionEmailKey).(string) } } } - if sess != nil { - if sessValue == nil { - clearSessionOAuthFlow(sess) - srv.Jaws.Dirty(sess) - } else if srv.LoginEvent != nil { - srv.LoginEvent(sess, hr) - } + } + if err == nil && !flow.link { + srv.emitLoginEvent(sess, hr) + current, _ := srv.sessionAuthStatus(sess, time.Now) + if flow.revoked() || !current || !sessionAlive(sess) { + srv.clearSessionAuth(sess, hr, nil, false) + http.SetCookie(hw, sess.Close()) + err = ErrOAuth2MissingSession + statusCode = http.StatusForbidden } - if err != nil && srv.LoginFailed != nil { - if srv.LoginFailed(hw, hr, statusCode, err, sessEmail) { - return - } + } + if err == nil { + hw.Header().Set("Location", flow.referrer) + } + if err != nil && srv.LoginFailed != nil { + if srv.LoginFailed(hw, hr, statusCode, err, sessEmail) { + return } } - srv.writeResult(hw, statusCode, err, nil) + srv.writeResult(hw, statusCode, err) } diff --git a/oauth2_callback_edge_test.go b/oauth2_callback_edge_test.go new file mode 100644 index 0000000..fbc1ecf --- /dev/null +++ b/oauth2_callback_edge_test.go @@ -0,0 +1,262 @@ +package jawsauth + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/linkdata/jaws" + "golang.org/x/oauth2" +) + +func TestOIDCCallbackRejectsEmptySubject(t *testing.T) { + for _, link := range []bool{false, true} { + name := "sign-in" + if link { + name = "link" + } + t.Run(name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + idToken := makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "client", "sub": "", "nonce": "nonce", + "exp": time.Now().Add(time.Hour).Unix(), + }) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(`{"access_token":"access","token_type":"Bearer","id_token":"` + idToken + `"}`), nil + })} + logins, links := 0, 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + links++ + return 0, nil + } + req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state&code=code", nil) + w := httptest.NewRecorder() + sess := jw.NewSession(w, req) + setTestOAuthFlow(sess, srv, pendingOAuthFlow{ + state: "state", expiry: time.Now().Add(oauth2FlowLifetime), + verifier: oauth2.GenerateVerifier(), nonce: "nonce", link: link, + }) + if link { + sess.Set(srv.SessionKey, map[string]any{"sub": "existing"}) + } + + srv.HandleAuthResponse(w, req) + if w.Code != http.StatusUnauthorized || !strings.Contains(w.Body.String(), ErrOIDCInvalidIDToken.Error()) || + jw.GetSession(req) != sess || logins != 0 || links != 0 || len(w.Result().Cookies()) != 1 { + t.Fatalf("empty subject: status=%d body=%q logins=%d links=%d cookies=%v", w.Code, w.Body.String(), logins, links, w.Result().Cookies()) + } + if link && sess.Get(srv.SessionKey).(map[string]any)["sub"] != "existing" { + t.Fatal("link callback changed original authentication") + } + if !link && sess.Get(srv.SessionKey) != nil { + t.Fatal("empty subject authenticated pre-auth session") + } + if !link { + refreshReq := httptest.NewRequest(http.MethodGet, "http://example.com/refresh", nil) + refreshSess := jw.NewSession(httptest.NewRecorder(), refreshReq) + err := srv.setSessionAuthFromToken(t.Context(), refreshSess, nil, makeOAuth2Token("access", idToken, ""), time.Time{}, nil) + if !errors.Is(err, ErrOIDCInvalidIDToken) || refreshSess.Get(srv.SessionKey) != nil { + t.Fatalf("empty subject refresh: claims=%v err=%v", refreshSess.Get(srv.SessionKey), err) + } + } + }) + } +} + +func TestOIDCExpiryDuringUserInfoRejected(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + srv.userinfoUrl = "https://provider.example/userinfo" + idToken := makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "client", "sub": "person", "nonce": "nonce", + "exp": time.Now().Add(5 * time.Second).Unix(), + }) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/userinfo" { + time.Sleep(6 * time.Second) + return jsonResponse(`{"sub":"person","email":"user@example.com"}`), nil + } + return jsonResponse(`{"access_token":"access","token_type":"Bearer","id_token":"` + idToken + `"}`), nil + })} + req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state&code=code", nil) + w := httptest.NewRecorder() + sess := jw.NewSession(w, req) + setTestOAuthFlow(sess, srv, pendingOAuthFlow{ + state: "state", expiry: time.Now().Add(oauth2FlowLifetime), + verifier: oauth2.GenerateVerifier(), nonce: "nonce", + }) + + srv.HandleAuthResponse(w, req) + if w.Code != http.StatusUnauthorized || !strings.Contains(w.Body.String(), ErrOIDCInvalidIDToken.Error()) || + jw.GetSession(req) != sess || sess.Get(srv.SessionKey) != nil || len(w.Result().Cookies()) != 1 { + t.Fatalf("expired during UserInfo: status=%d body=%q cookies=%v", w.Code, w.Body.String(), w.Result().Cookies()) + } + idToken = makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "client", "sub": "person", "nonce": "nonce", + "exp": time.Now().Add(5 * time.Second).Unix(), + }) + refreshReq := httptest.NewRequest(http.MethodGet, "http://example.com/refresh", nil) + refreshSess := jw.NewSession(httptest.NewRecorder(), refreshReq) + refreshToken := makeOAuth2Token("access", idToken, "") + err := srv.setSessionAuthFromToken(srv.oauth2Context(t.Context()), refreshSess, + oauth2.StaticTokenSource(refreshToken), refreshToken, time.Time{}, nil) + if !errors.Is(err, ErrOIDCInvalidIDToken) || refreshSess.Get(srv.SessionKey) != nil { + t.Fatalf("expired refresh during UserInfo: claims=%v err=%v", refreshSess.Get(srv.SessionKey), err) + } + }) +} + +func TestProfileExpiryDuringSubjectRejected(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := &Server{Jaws: jw, userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { + time.Sleep(12 * time.Second) + return "person", nil + }} + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(`{"id":42}`), nil + })}) + token := &oauth2.Token{AccessToken: "access", TokenType: "Bearer", Expiry: time.Now().Add(11 * time.Second)} + claims, _, _, status, err := srv.verifyOAuthIdentity(ctx, token, "") + if !errors.Is(err, ErrOAuth2InvalidProfile) || claims != nil || status != http.StatusUnauthorized { + t.Fatalf("expiry during subject: claims=%v status=%d err=%v", claims, status, err) + } + }) +} + +func TestOAuthLogoutDuringLoginEventClosesRotatedSession(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + entered := make(chan *jaws.Session, 1) + release := make(chan struct{}) + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { + entered <- sess + <-release + } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + req := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + req.AddCookie(cookie) + preauth := jw.GetSession(req) + if preauth == nil { + t.Fatal("missing pre-auth session") + } + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "first") }() + rotated := waitFor(t, entered, "LoginEvent was not reached") + if rotated == preauth || rotated.Get(srv.SessionKey) == nil { + t.Fatal("LoginEvent did not receive an authenticated rotated session") + } + if !srv.Logout(preauth, nil) { + t.Fatal("logout failed") + } + releaseOnce.Do(func() { close(release) }) + w := waitFor(t, result, "canceled callback did not finish") + cookies := w.Result().Cookies() + if w.Code != http.StatusForbidden || w.Header().Get("Location") != "" || len(cookies) != 2 || cookies[1].MaxAge >= 0 || + cookies[1].Name != cookies[0].Name || cookies[1].Value != cookies[0].Value || + rotated.Cookie().MaxAge >= 0 || rotated.Get(srv.SessionKey) != nil || srv.authTimers[rotated.ID()] != nil { + t.Fatalf("superseded login: status=%d location=%q cookies=%v claims=%v", w.Code, w.Header().Get("Location"), cookies, rotated.Get(srv.SessionKey)) + } +} + +func TestLoginClearedBeforeEventSkipsLifecycleEvents(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + req := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + req.AddCookie(cookie) + preauth := jw.GetSession(req) + var events []string + srv.LoginEvent = func(*jaws.Session, *http.Request) { events = append(events, "login") } + srv.LogoutEvent = func(*jaws.Session, *http.Request) { events = append(events, "logout") } + cleared := false + jw.Debug = true + jw.Logger = infoHookLogger(func(msg string) { + if msg == "jawsauth: scheduled auth timer" { + for _, sess := range jw.Sessions() { + if sess != preauth { + cleared = srv.Logout(sess, nil) + } + } + } + }) + response := profileCallback(srv, cookie, state, "first") + if !cleared || response.Code != http.StatusForbidden || len(events) != 0 { + t.Fatalf("cleared=%t status=%d events=%v", cleared, response.Code, events) + } + srv.mu.Lock() + pending := len(srv.loginEvents) + srv.mu.Unlock() + if pending != 0 { + t.Fatalf("pending login events = %d", pending) + } +} + +func TestLoginEventCanLogoutItsSession(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + var events []string + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { + events = append(events, "login") + if !srv.Logout(sess, nil) { + t.Fatal("logout failed") + } + } + srv.LogoutEvent = func(*jaws.Session, *http.Request) { events = append(events, "logout") } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "first") + if response.Code != http.StatusForbidden || strings.Join(events, ",") != "login,logout" { + t.Fatalf("status=%d events=%v", response.Code, events) + } +} + +func TestLogoutDuringLoginEventDefersLogoutEvent(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + entered := make(chan *jaws.Session, 1) + release := make(chan struct{}) + releaseOnce := sync.OnceFunc(func() { close(release) }) + defer releaseOnce() + events := make(chan string, 2) + logoutRequests := make(chan *http.Request, 1) + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { + events <- "login" + entered <- sess + <-release + } + srv.LogoutEvent = func(_ *jaws.Session, request *http.Request) { + events <- "logout" + logoutRequests <- request + } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "first") }() + rotated := waitFor(t, entered, "LoginEvent did not start") + loggedOut := make(chan bool, 1) + logoutRequest := httptest.NewRequest(http.MethodGet, "https://app.example/logout", nil) + go func() { loggedOut <- srv.Logout(rotated, logoutRequest) }() + if !waitFor(t, loggedOut, "concurrent logout blocked") || len(events) != 1 { + t.Fatalf("logout before LoginEvent returned: events=%d", len(events)) + } + releaseOnce() + response := waitFor(t, result, "callback did not finish") + first := waitFor(t, events, "missing LoginEvent") + second := waitFor(t, events, "missing LogoutEvent") + deferredRequest := waitFor(t, logoutRequests, "missing LogoutEvent request") + if response.Code != http.StatusForbidden || first != "login" || second != "logout" || deferredRequest != nil || srv.CurrentClaims(rotated) != nil { + t.Fatalf("status=%d events=%s,%s request=%v claims=%v", response.Code, first, second, deferredRequest, srv.CurrentClaims(rotated)) + } +} diff --git a/oauth2_flow_guards_test.go b/oauth2_flow_guards_test.go new file mode 100644 index 0000000..a8394b0 --- /dev/null +++ b/oauth2_flow_guards_test.go @@ -0,0 +1,437 @@ +package jawsauth + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "testing/synctest" + "time" + + "github.com/linkdata/jaws" + "golang.org/x/oauth2" +) + +type infoHookLogger func(string) + +func (hook infoHookLogger) Info(msg string, _ ...any) { hook(msg) } +func (infoHookLogger) Warn(string, ...any) {} +func (infoHookLogger) Error(string, ...any) {} + +type rotationHeaderGate struct { + *httptest.ResponseRecorder + once sync.Once + entered chan struct{} + release chan struct{} +} + +func (w *rotationHeaderGate) Header() http.Header { + w.once.Do(func() { + close(w.entered) + <-w.release + }) + return w.ResponseRecorder.Header() +} + +func TestLoginStartDuringRotationKeepsFirstCallback(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + lock := &observedRotationLock{second: make(chan struct{})} + srv.SessionRotationLock = lock + gate := &rotationHeaderGate{ResponseRecorder: httptest.NewRecorder(), entered: make(chan struct{}), release: make(chan struct{})} + callbackDone := make(chan struct{}) + go func() { + r := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback?state="+state+"&code=first", nil) + r.AddCookie(cookie) + srv.HandleAuthResponse(gate, r) + close(callbackDone) + }() + waitFor(t, gate.entered, "callback did not reach session rotation") + startDone := make(chan *httptest.ResponseRecorder, 1) + go func() { + r := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + r.AddCookie(cookie) + w := httptest.NewRecorder() + srv.HandleLogin(w, r) + startDone <- w + }() + release := sync.OnceFunc(func() { close(gate.release) }) + defer release() + waitFor(t, lock.second, "second login did not wait for rotation") + release() + waitFor(t, callbackDone, "first callback did not finish") + second := waitFor(t, startDone, "second login did not finish") + if gate.Code != http.StatusFound || len(gate.Result().Cookies()) != 1 || second.Code != http.StatusFound || len(second.Result().Cookies()) != 1 { + t.Fatalf("overlapping logins: first=%d cookies=%v second=%d cookies=%v", gate.Code, gate.Result().Cookies(), second.Code, second.Result().Cookies()) + } +} + +func TestCallbackRevokedAfterStoreBalancesEvents(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + old := jw.GetSession(r) + if old == nil { + t.Fatal("pending session missing") + } + var rotated *jaws.Session + logins, logouts := 0, 0 + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { rotated = sess; logins++ } + srv.LogoutEvent = func(sess *jaws.Session, _ *http.Request) { + if sess != old { + logouts++ + } + } + revoked := false + jw.Debug = true + jw.Logger = infoHookLogger(func(msg string) { + if msg == "jawsauth: scheduled auth timer" { + revoked = srv.Logout(old, nil) + } + }) + w := profileCallback(srv, cookie, state, "proof") + if !revoked || w.Code != http.StatusForbidden || logins != 1 || logouts != 1 || rotated == nil || sessionAlive(rotated) || len(srv.authTimers) != 0 { + t.Fatalf("post-store revocation: revoked=%t status=%d logins=%d logouts=%d rotated=%v timers=%d", revoked, w.Code, logins, logouts, rotated, len(srv.authTimers)) + } +} + +func TestCallbackRejectsAuthInvalidatedByLoginEvent(t *testing.T) { + for _, action := range []string{"expired auth", "closed session"} { + t.Run(action, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + called := false + srv.LoginEvent = func(sess *jaws.Session, _ *http.Request) { + called = true + if action == "expired auth" { + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(-time.Second)) + } else { + sess.Close() + } + } + response := profileCallback(srv, cookie, state, "first") + cookies := response.Result().Cookies() + if !called || response.Code != http.StatusForbidden || response.Header().Get("Location") != "" || len(cookies) != 2 || cookies[1].MaxAge >= 0 { + t.Fatalf("invalidated login: called=%t status=%d location=%q cookies=%v", called, response.Code, response.Header().Get("Location"), cookies) + } + }) + } +} + +func TestProfileTokenExpiresBetweenRotationAndStore(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + first := profileCallback(srv, cookie, state, "first") + if first.Code != http.StatusFound { + t.Fatalf("initial login: %d", first.Code) + } + cookie = first.Result().Cookies()[0] + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + old := jw.GetSession(r) + if old == nil { + t.Fatal("old session missing") + } + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + if req.URL.Path == "/token" { + return jsonResponse(`{"access_token":"short","token_type":"Bearer","expires_in":30}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + logins := 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + srv.LogoutEvent = func(sess *jaws.Session, _ *http.Request) { + if sess == old { + time.Sleep(31 * time.Second) + } + } + w := profileCallback(srv, cookie, state, "second") + cookies := w.Result().Cookies() + if w.Code != http.StatusUnauthorized || w.Header().Get("Location") != "" || + len(cookies) != 2 || cookies[1].MaxAge >= 0 || cookies[1].Value != cookies[0].Value || + logins != 0 || len(srv.authTimers) != 0 { + t.Fatalf("expired before store: status=%d location=%q cookies=%v logins=%d timers=%d", w.Code, w.Header().Get("Location"), cookies, logins, len(srv.authTimers)) + } + }) +} + +func TestLinkWithoutEventDoesNotStartFlow(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + srv.LinkEvent = nil + r := httptest.NewRequest(http.MethodPost, "https://app.example/auth/link/github", nil) + if jw.NewSession(httptest.NewRecorder(), r) == nil { + t.Fatal("missing session") + } + w := httptest.NewRecorder() + srv.HandleLink(w, r) + if w.Code != http.StatusInternalServerError || w.Header().Get("Location") != "" || + !strings.Contains(w.Body.String(), ErrOAuth2NotConfigured.Error()) || calls != 0 { + t.Fatalf("missing LinkEvent: status=%d location=%q body=%q calls=%d", w.Code, w.Header().Get("Location"), w.Body.String(), calls) + } +} + +func TestLinkRejectsGETBeforeStartingFlow(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { return 0, nil } + r := httptest.NewRequest(http.MethodGet, "https://app.example/auth/link/github", nil) + sess := jw.NewSession(httptest.NewRecorder(), r) + if sess == nil { + t.Fatal("missing session") + } + w := httptest.NewRecorder() + srv.HandleLink(w, r) + if w.Code != http.StatusMethodNotAllowed || w.Header().Get("Allow") != http.MethodPost || w.Header().Get("Location") != "" || sess.Get(srv.sessionKey(oauth2FlowKey)) != nil || calls != 0 { + t.Fatalf("GET link: status=%d allow=%q location=%q flow=%v calls=%d", w.Code, w.Header().Get("Allow"), w.Header().Get("Location"), sess.Get(srv.sessionKey(oauth2FlowKey)), calls) + } +} + +func TestLinkRejectsInvalidEventResults(t *testing.T) { + for _, tc := range []struct { + name string + status int + err error + want int + }{ + {"error without status", 0, errors.New("secret persistence detail"), http.StatusForbidden}, + {"success status without acceptance", http.StatusOK, nil, http.StatusForbidden}, + {"conflict status", http.StatusConflict, errors.New("secret persistence detail"), http.StatusConflict}, + {"out of range status", 600, errors.New("secret persistence detail"), http.StatusForbidden}, + } { + t.Run(tc.name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + first := profileCallback(srv, cookie, state, "first") + if first.Code != http.StatusFound { + t.Fatalf("sign-in: %d", first.Code) + } + cookie = first.Result().Cookies()[0] + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + return tc.status, tc.err + } + _, state = profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + w := profileCallback(srv, cookie, state, "second") + if w.Code != tc.want || w.Header().Get("Location") != "" || + len(w.Result().Cookies()) != 0 || !strings.Contains(w.Body.String(), ErrOAuth2LinkRejected.Error()) || + strings.Contains(w.Body.String(), "secret persistence detail") { + t.Fatalf("invalid LinkEvent result: status=%d location=%q body=%q cookies=%v", w.Code, w.Header().Get("Location"), w.Body.String(), w.Result().Cookies()) + } + }) + } +} + +func TestLinkRejectsIdentityThatExpiresWhileWaitingForLock(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + lock := &holdFirstRotationLock{first: make(chan struct{}), releaseFirst: make(chan struct{})} + srv.SessionRotationLock = lock + release := sync.OnceFunc(func() { close(lock.releaseFirst) }) + defer release() + writes := 0 + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + writes++ + return 0, nil + } + request := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback", nil) + sess := jw.NewSession(httptest.NewRecorder(), request) + if sess == nil || jw.GetSession(request) != sess { + t.Fatal("missing session") + } + type completion struct { + status int + err error + } + result := make(chan completion, 1) + go func() { + status, err := srv.completeLink(sess, request, map[string]any{"sub": "42"}, time.Now().Add(time.Second), pendingOAuthFlow{}) + result <- completion{status, err} + }() + <-lock.first + time.Sleep(2 * time.Second) // synctest advances time while completeLink waits. + release() + got := <-result + if got.status != http.StatusUnauthorized || !errors.Is(got.err, ErrOAuth2InvalidProfile) || writes != 0 { + t.Fatalf("expired link: status=%d err=%v writes=%d", got.status, got.err, writes) + } + }) +} + +func TestUserInfoRequiresOneJSONObject(t *testing.T) { + for _, profile := range []bool{false, true} { + t.Run(map[bool]string{false: "OIDC", true: "profile"}[profile], func(t *testing.T) { + for _, tc := range []struct{ name, body string }{ + {"trailing", `{"id":42} {"id":43}`}, + {"null", `null`}, + {"array", `[1]`}, + {"string", `"x"`}, + {"number", `1`}, + } { + t.Run(tc.name, func(t *testing.T) { + srv := &Server{} + if profile { + srv.profileSubject = func(map[string]any) (string, error) { return "42", nil } + } + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(tc.body), nil + })}) + claims, err := srv.fetchUserInfo(ctx, "https://provider.example/user", oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "proof"})) + if !errors.Is(err, ErrUserInfoInvalidResponse) || claims != nil { + t.Fatalf("invalid UserInfo accepted: claims=%v err=%v", claims, err) + } + }) + } + }) + } +} + +func TestLinkWithoutRefererReturnsHome(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + first := profileCallback(srv, cookie, state, "first") + if first.Code != http.StatusFound { + t.Fatalf("sign-in: %d", first.Code) + } + cookie = first.Result().Cookies()[0] + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { return 0, nil } + _, state = profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + w := profileCallback(srv, cookie, state, "second") + if w.Code != http.StatusFound || w.Header().Get("Location") != "/" || len(w.Result().Cookies()) != 0 { + t.Fatalf("link return: status=%d location=%q cookies=%v", w.Code, w.Header().Get("Location"), w.Result().Cookies()) + } +} + +func TestProfileUserInfoNullDoesNotCallSubject(t *testing.T) { + called := false + srv := &Server{ + userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { + called = true + return "42", nil + }, + } + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(`null`), nil + })}) + claims, source, expiry, status, err := srv.verifyOAuthIdentity(ctx, &oauth2.Token{AccessToken: "proof", TokenType: "Bearer"}, "") + if !errors.Is(err, ErrUserInfoInvalidResponse) || claims != nil || source != nil || !expiry.IsZero() || status != http.StatusBadGateway || called { + t.Fatalf("null profile: claims=%v source=%t expiry=%v status=%d err=%v subject_called=%t", claims, source != nil, expiry, status, err, called) + } +} + +func TestCallbackClosesRotatedSessionOnStoreFailure(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + first := profileCallback(srv, cookie, state, "first") + if first.Code != http.StatusFound { + t.Fatalf("sign-in: %d", first.Code) + } + cookie = first.Result().Cookies()[0] + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + closedRotated := false + srv.LogoutEvent = func(old *jaws.Session, r *http.Request) { + if rotated := jw.GetSession(r); rotated != nil && rotated != old { + rotated.Close() + closedRotated = true + } + } + w := profileCallback(srv, cookie, state, "second") + cookies := w.Result().Cookies() + if !closedRotated || w.Code != http.StatusForbidden || w.Header().Get("Location") != "" || + len(cookies) != 2 || cookies[1].MaxAge >= 0 || cookies[1].Value != cookies[0].Value { + t.Fatalf("store after close: closed=%t status=%d location=%q cookies=%v", closedRotated, w.Code, w.Header().Get("Location"), cookies) + } +} + +type revokeAfterRotationLock struct { + sync.Mutex + logout func() +} + +func (lock *revokeAfterRotationLock) Unlock() { + lock.Mutex.Unlock() + if lock.logout != nil { + logout := lock.logout + lock.logout = nil + logout() + } +} + +func TestReloginStopsOldTimerBeforeRotationUnlock(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie := profileLogin(t, srv) + oldRequest := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + oldRequest.AddCookie(cookie) + old := jw.GetSession(oldRequest) + if old == nil { + t.Fatal("old session missing") + } + _, state := profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + timer := factory.timer(0) + var logoutRequest *http.Request + logouts := 0 + srv.LogoutEvent = func(sess *jaws.Session, hr *http.Request) { + if sess == old { + logouts++ + logoutRequest = hr + } + } + var stoppedBeforeFire bool + srv.SessionRotationLock = &revokeAfterRotationLock{logout: func() { + stoppedBeforeFire = timer.isStopped() + timer.fire() + }} + response := profileCallback(srv, cookie, state, "second") + if response.Code != http.StatusFound || logouts != 1 || logoutRequest == nil || !stoppedBeforeFire { + t.Fatalf("re-login: status=%d logouts=%d request=%v timer_stopped_before_fire=%t", response.Code, logouts, logoutRequest != nil, stoppedBeforeFire) + } +} + +func TestCallbackRevokedAfterRotationClosesNewSession(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + first := profileCallback(srv, cookie, state, "first") + if first.Code != http.StatusFound { + t.Fatalf("sign-in: %d", first.Code) + } + cookie = first.Result().Cookies()[0] + oldRequest := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + oldRequest.AddCookie(cookie) + old := jw.GetSession(oldRequest) + if old == nil { + t.Fatal("old session missing") + } + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + logins := 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + revoked := false + srv.SessionRotationLock = &revokeAfterRotationLock{logout: func() { + revoked = srv.Logout(old, nil) + }} + w := profileCallback(srv, cookie, state, "second") + cookies := w.Result().Cookies() + if !revoked || logins != 0 || w.Code != http.StatusForbidden || w.Header().Get("Location") != "" || + len(cookies) != 2 || cookies[1].MaxAge >= 0 || cookies[1].Value != cookies[0].Value { + t.Fatalf("revoked after rotation: revoked=%t logins=%d status=%d location=%q cookies=%v", revoked, logins, w.Code, w.Header().Get("Location"), cookies) + } +} diff --git a/oauth2_redirect_test.go b/oauth2_redirect_test.go new file mode 100644 index 0000000..2c5bf58 --- /dev/null +++ b/oauth2_redirect_test.go @@ -0,0 +1,90 @@ +package jawsauth + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/linkdata/jaws" + "golang.org/x/oauth2" +) + +func TestOAuthTokenRequestsDoNotFollowRedirect(t *testing.T) { + for _, grant := range []string{"exchange", "refresh"} { + for _, status := range []int{http.StatusTemporaryRedirect, http.StatusPermanentRedirect} { + t.Run(fmt.Sprintf("%s/%d", grant, status), func(t *testing.T) { + var secondHop atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + secondHop.Add(1) + })) + defer target.Close() + var firstHop atomic.Int32 + provider := httptest.NewTLSServer(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { + firstHop.Add(1) + if hr.URL.Path != "/token" || hr.Method != http.MethodPost { + t.Errorf("token request: %s %s", hr.Method, hr.URL.Path) + } + if err := hr.ParseForm(); err != nil { + t.Error(err) + } + if hr.PostForm.Get("client_secret") != "client-secret" { + t.Errorf("first hop client secret = %q", hr.PostForm.Get("client_secret")) + } + if grant == "exchange" { + if hr.PostForm.Get("grant_type") != "authorization_code" || + hr.PostForm.Get("code") != "auth-code" || hr.PostForm.Get("code_verifier") != "pkce-verifier" { + t.Errorf("first hop exchange form = %v", hr.PostForm) + } + } else if hr.PostForm.Get("grant_type") != "refresh_token" || hr.PostForm.Get("refresh_token") != "refresh-secret" { + t.Errorf("first hop refresh form = %v", hr.PostForm) + } + hw.Header().Set("Location", target.URL+"/leak") + hw.WriteHeader(status) + })) + defer provider.Close() + + client := provider.Client() + client.Timeout = 3 * time.Second + client.CheckRedirect = func(*http.Request, []*http.Request) error { + t.Error("original redirect policy was used") + return nil + } + logger := &testAuthDebugLogger{} + srv := &Server{httpClient: client} + if grant == "refresh" && status == http.StatusPermanentRedirect { + srv.Jaws = &jaws.Jaws{Debug: true, Logger: logger} + } + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + if grant == "exchange" { + srv.httpClient = nil + ctx = context.WithValue(ctx, oauth2.HTTPClient, client) + } + authctx := srv.oauth2Context(ctx) + cfg := oauth2.Config{ClientID: "client", ClientSecret: "client-secret", + Endpoint: oauth2.Endpoint{TokenURL: provider.URL + "/token", AuthStyle: oauth2.AuthStyleInParams}} + var err error + if grant == "exchange" { + _, err = cfg.Exchange(authctx, "auth-code", oauth2.SetAuthURLParam("code_verifier", "pkce-verifier")) + } else { + _, err = cfg.TokenSource(authctx, &oauth2.Token{RefreshToken: "refresh-secret"}).Token() + } + var retrieve *oauth2.RetrieveError + if !errors.As(err, &retrieve) || retrieve.Response == nil || retrieve.Response.StatusCode != status { + t.Fatalf("redirect response = %v, want HTTP %d", err, status) + } + if firstHop.Load() != 1 || secondHop.Load() != 0 { + t.Fatalf("token request hops: first=%d second=%d", firstHop.Load(), secondHop.Load()) + } + if srv.Jaws != nil && !logger.hasInfo("jawsauth: oauth2 request") { + t.Fatal("debug wrapper did not observe token request") + } + }) + } + } +} diff --git a/oauth2_test.go b/oauth2_test.go index 61708cc..162102f 100644 --- a/oauth2_test.go +++ b/oauth2_test.go @@ -3,7 +3,6 @@ package jawsauth import ( "context" "encoding/base64" - "encoding/hex" "encoding/json" "errors" "fmt" @@ -69,10 +68,10 @@ func makeIDToken(t *testing.T, claims map[string]any) string { return header + "." + payload + "." + signature } -func Test_errtext(t *testing.T) { - var sb strings.Builder - writeBody(&sb, http.StatusForbidden, ErrOAuth2WrongState, nil) - s := sb.String() +func TestWriteResultError(t *testing.T) { + hw := httptest.NewRecorder() + (&Server{}).writeResult(hw, http.StatusForbidden, ErrOAuth2WrongState) + s := hw.Body.String() if !strings.Contains(s, "403 Forbidden") { t.Fatal() } @@ -102,6 +101,18 @@ func Test_sanitizeRedirectTarget(t *testing.T) { if s := sanitizeRedirectTarget("example.com:8443", "https://example.com/dashboard"); s != "/dashboard" { t.Fatal(s) } + if s := sanitizeRedirectTarget("example.com", "https://example.com.:443/dashboard"); s != "/dashboard" { + t.Fatal(s) + } + if s := sanitizeRedirectTarget("[::1]", "https://[::1]:443/dashboard"); s != "/dashboard" { + t.Fatal(s) + } + if s := sanitizeRedirectTarget("[::1]:443", "https://[::1]/dashboard"); s != "/dashboard" { + t.Fatal(s) + } + if s := sanitizeRedirectTarget("::1:443", "https://[::1]/dashboard"); s != "/" { + t.Fatalf("malformed host accepted: %q", s) + } if s := sanitizeRedirectTarget("example.com", "section/page"); s != "/section/page" { t.Fatal(s) } @@ -117,25 +128,80 @@ func Test_beginSanitizesReferrer(t *testing.T) { srv := &Server{HandledPaths: make(map[string]struct{})} hr := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/login", nil) hr.Header.Set("Referer", "https://evil.com/wrong") - _, location := srv.begin(hr) + location := srv.begin(hr) if location != "/" { t.Fatal(location) } } +func TestProtectedRequestReturnsToRequestedPage(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + hr := httptest.NewRequest(http.MethodGet, "http://example.com/protected?tab=one", nil) + hr.Header.Set("Referer", "http://example.com/home") + hw := httptest.NewRecorder() + srv.Wrap(testStatusHandler{statusCode: http.StatusOK}).ServeHTTP(hw, hr) + if hw.Code != http.StatusFound { + t.Fatalf("protected request status = %d", hw.Code) + } + if got := testOAuthFlow(jw.GetSession(hr), srv).referrer; got != "/protected?tab=one" { + t.Fatalf("OAuth return target = %q", got) + } +} + func Test_beginReferrerHandling(t *testing.T) { srv := &Server{HandledPaths: map[string]struct{}{"/oauth2/login": {}}} hr := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/login", nil) hr.Header.Set("Referer", "https://example.com/oauth2/login") - _, location := srv.begin(hr) + location := srv.begin(hr) if location != "/" { t.Fatal(location) } hr.Header.Set("Referer", "https://example.com/app/home") - _, location = srv.begin(hr) + location = srv.begin(hr) if location != "/app/home" { t.Fatal(location) } + hr = httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/login?next=x", nil) + location = srv.begin(hr) + if location != "/" { + t.Fatalf("query-bearing login loops to %q", location) + } + hr.Header.Set("Referer", "https://example.com/oauth2/login?next=x") + location = srv.begin(hr) + if location != "/" { + t.Fatalf("query-bearing referrer loops to %q", location) + } + hr.Header.Set("Referer", "https://example.com/app/home?next=x") + location = srv.begin(hr) + if location != "/app/home?next=x" { + t.Fatalf("unhandled target changed to %q", location) + } + for _, referer := range []string{ + "https://example.com/oauth2//login", "https://example.com/oauth2/./login", + } { + hr.Header.Set("Referer", referer) + location = srv.begin(hr) + if location != "/" { + t.Fatalf("handled path %q returned to %q", referer, location) + } + } +} + +func TestBeginExcludesTrailingSlashCallback(t *testing.T) { + srv := &Server{HandledPaths: map[string]struct{}{"/oauth2/callback/": {}}} + hr := httptest.NewRequest(http.MethodGet, "https://example.com/oauth2/login", nil) + for _, referer := range []string{ + "https://example.com/oauth2/callback/", + "https://example.com/oauth2//callback/", + "https://example.com/oauth2/./callback/", + } { + hr.Header.Set("Referer", referer) + location := srv.begin(hr) + if location != "/" { + t.Fatalf("callback return target %q kept as %q", referer, location) + } + } } // Test_beginPathBoundary verifies that handled paths only rewrite exact @@ -146,7 +212,7 @@ func Test_beginPathBoundary(t *testing.T) { hr := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/login", nil) hr.Header.Set("Referer", "https://example.com/admin/login") for range 50 { - _, location := srv.begin(hr) + location := srv.begin(hr) if location != "/" { t.Fatal(location) } @@ -154,18 +220,14 @@ func Test_beginPathBoundary(t *testing.T) { srv = &Server{HandledPaths: map[string]struct{}{"/login": {}}} hr.Header.Set("Referer", "https://example.com/admin/login") - _, location := srv.begin(hr) + location := srv.begin(hr) if location != "/admin/login" { t.Fatal(location) } } func Test_handleLoginGeneratesOpaqueState(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, HandledPaths: map[string]struct{}{"/oauth2/login": {}}, @@ -194,14 +256,12 @@ func Test_handleLoginGeneratesOpaqueState(t *testing.T) { if sess == nil { t.Fatal("missing session") } - state, _ := sess.Get(oauth2StateKey).(string) - if len(state) != 64 { - t.Fatal(state) + flow := testOAuthFlow(sess, nil) + state := flow.state + if len(state) != 26 { + t.Fatalf("state length = %d, want 26", len(state)) } - if _, err = hex.DecodeString(state); err != nil { - t.Fatal(err) - } - verifier, _ := sess.Get(oauth2PKCEVerifierKey).(string) + verifier := flow.verifier if verifier == "" { t.Fatal("missing pkce verifier") } @@ -216,25 +276,21 @@ func Test_handleLoginGeneratesOpaqueState(t *testing.T) { if gotMethod := values.Get("code_challenge_method"); gotMethod != "S256" { t.Fatal(gotMethod) } - nonce, _ := sess.Get(oauth2NonceKey).(string) - if len(nonce) != 64 { - t.Fatal(nonce) + nonce := flow.nonce + if len(nonce) != 26 || nonce == state { + t.Fatalf("nonce %q is missing or matches state", nonce) } if gotNonce := values.Get("nonce"); gotNonce != nonce { t.Fatal(gotNonce) } - referrer, _ := sess.Get(oauth2ReferrerKey).(string) + referrer := flow.referrer if referrer != "/secure" { t.Fatal(referrer) } } func Test_handleLoginCreatesSessionAndRotatesState(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, HandledPaths: map[string]struct{}{"/oauth2/login": {}}, @@ -263,14 +319,14 @@ func Test_handleLoginCreatesSessionAndRotatesState(t *testing.T) { if sess == nil { t.Fatal("missing session") } - firstState, _ := sess.Get(oauth2StateKey).(string) + firstState := testOAuthFlow(sess, nil).state if firstState == "" { t.Fatal("missing state") } rec = httptest.NewRecorder() srv.HandleLogin(rec, req) - secondState, _ := sess.Get(oauth2StateKey).(string) + secondState := testOAuthFlow(sess, nil).state if secondState == "" { t.Fatal("missing replacement state") } @@ -280,11 +336,7 @@ func Test_handleLoginCreatesSessionAndRotatesState(t *testing.T) { } func Test_handleLogoutClearsPendingOAuthState(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, SessionKey: "oauth2userinfo", @@ -295,13 +347,10 @@ func Test_handleLogoutClearsPendingOAuthState(t *testing.T) { } req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/logout", nil) sess := jw.NewSession(httptest.NewRecorder(), req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, "verifier123") - sess.Set(oauth2NonceKey, "nonce123") - sess.Set(oauth2ReferrerKey, "/protected") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: "verifier123", nonce: "nonce123", referrer: "/protected"}) sess.Set(srv.SessionKey, map[string]any{"email": "user@example.com"}) sess.Set(srv.SessionTokenKey, oauth2.StaticTokenSource(makeOAuth2Token("access", "", ""))) - sess.Set(oauth2IDTokenExpiryKey, time.Now().Add(time.Hour)) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(time.Hour)) sess.Set(srv.SessionEmailKey, "user@example.com") sess.Set(srv.SessionEmailVerifiedKey, true) @@ -314,29 +363,16 @@ func Test_handleLogoutClearsPendingOAuthState(t *testing.T) { t.Fatal(resp.Status) } closeResponseBody(t, resp) - if value := sess.Get(oauth2StateKey); value != nil { - t.Fatal(value) - } - if value := sess.Get(oauth2PKCEVerifierKey); value != nil { + if value := sess.Get(srv.sessionKey(oauth2FlowKey)); value != nil { t.Fatal(value) } - if value := sess.Get(oauth2NonceKey); value != nil { - t.Fatal(value) - } - if value := sess.Get(oauth2ReferrerKey); value != nil { - t.Fatal(value) - } - if value := sess.Get(oauth2IDTokenExpiryKey); value != nil { + if value := sess.Get(srv.sessionKey(oauth2AuthExpiryKey)); value != nil { t.Fatal(value) } } func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) var mu sync.Mutex var gotVerifier string @@ -425,10 +461,7 @@ func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state="+wantState+"&code="+wantCode, nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, wantState) - sess.Set(oauth2PKCEVerifierKey, verifier) - sess.Set(oauth2NonceKey, wantNonce) - sess.Set(oauth2ReferrerKey, "/secure") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: wantState, expiry: time.Now().Add(oauth2FlowLifetime), verifier: verifier, nonce: wantNonce, referrer: "/secure"}) oldSession := sess srv.HandleAuthResponse(rec, req) @@ -470,14 +503,8 @@ func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { if receivedProviderErr != nil { t.Fatal(receivedProviderErr) } - if gotState, _ := sess.Get(oauth2StateKey).(string); gotState != "" { - t.Fatal(gotState) - } - if gotVerifier, _ := sess.Get(oauth2PKCEVerifierKey).(string); gotVerifier != "" { - t.Fatal(gotVerifier) - } - if gotNonce, _ := sess.Get(oauth2NonceKey).(string); gotNonce != "" { - t.Fatal(gotNonce) + if value := sess.Get(srv.sessionKey(oauth2FlowKey)); value != nil { + t.Fatal(value) } if gotEmail, _ := sess.Get(srv.SessionEmailKey).(string); gotEmail != "idtoken@example.com" { t.Fatal(gotEmail) @@ -503,11 +530,7 @@ func Test_handleAuthResponseUsesPKCEVerifier(t *testing.T) { // the TokenSource stored in the session uses a request-independent context. // Scheduled refreshes can run after the callback request finishes. func Test_handleAuthResponseStoredTokenSourceSurvivesRequestCancel(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" const wantNonce = "nonce123" @@ -555,9 +578,7 @@ func Test_handleAuthResponseStoredTokenSourceSurvivesRequestCancel(t *testing.T) rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state&code=code", nil).WithContext(ctx) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, wantNonce) + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: wantNonce}) oldSession := sess srv.HandleAuthResponse(rec, req) @@ -585,11 +606,7 @@ func Test_handleAuthResponseStoredTokenSourceSurvivesRequestCancel(t *testing.T) } func Test_handleAuthResponseMissingPKCEVerifier(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) var tokenRequests int provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { @@ -622,8 +639,7 @@ func Test_handleAuthResponseMissingPKCEVerifier(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), nonce: "nonce123"}) srv.HandleAuthResponse(rec, req) @@ -645,11 +661,7 @@ func Test_handleAuthResponseMissingPKCEVerifier(t *testing.T) { } func Test_handleAuthResponseWrongStatePreservesCurrentAuth(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := newWrapperTestServer(jw, "https://issuer.example") rec := httptest.NewRecorder() @@ -662,13 +674,10 @@ func Test_handleAuthResponseWrongStatePreservesCurrentAuth(t *testing.T) { expiry := time.Now().Add(time.Hour).Truncate(time.Second) sess.Set(srv.SessionKey, claims) sess.Set(srv.SessionTokenKey, tokenSource) - sess.Set(oauth2IDTokenExpiryKey, expiry) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), expiry) sess.Set(srv.SessionEmailKey, "user@example.com") sess.Set(srv.SessionEmailVerifiedKey, true) - sess.Set(oauth2StateKey, "legit") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, "nonce") - sess.Set(oauth2ReferrerKey, "/secure") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "legit", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: "nonce", referrer: "/secure"}) srv.HandleAuthResponse(rec, req) @@ -690,7 +699,7 @@ func Test_handleAuthResponseWrongStatePreservesCurrentAuth(t *testing.T) { if got := sess.Get(srv.SessionTokenKey); got != tokenSource { t.Fatal(got) } - if got, _ := sess.Get(oauth2IDTokenExpiryKey).(time.Time); !got.Equal(expiry) { + if got, _ := sess.Get(srv.sessionKey(oauth2AuthExpiryKey)).(time.Time); !got.Equal(expiry) { t.Fatal(got) } if got, _ := sess.Get(srv.SessionEmailKey).(string); got != "user@example.com" { @@ -699,16 +708,17 @@ func Test_handleAuthResponseWrongStatePreservesCurrentAuth(t *testing.T) { if got, _ := sess.Get(srv.SessionEmailVerifiedKey).(bool); !got { t.Fatal(got) } - if got := sess.Get(oauth2StateKey); got != nil { + flow := testOAuthFlow(sess, nil) + if got := flow.state; got != "legit" { t.Fatal(got) } - if got := sess.Get(oauth2PKCEVerifierKey); got != nil { + if got := flow.verifier; got == "" { t.Fatal(got) } - if got := sess.Get(oauth2NonceKey); got != nil { + if got := flow.nonce; got != "nonce" { t.Fatal(got) } - if got := sess.Get(oauth2ReferrerKey); got != nil { + if got := flow.referrer; got != "/secure" { t.Fatal(got) } } @@ -729,11 +739,21 @@ func Test_extractEmailVerified(t *testing.T) { claims: map[string]any{"email_verified": "true"}, want: true, }, + { + name: "profile numeric", + claims: map[string]any{"email_verified": json.Number("1.0")}, + want: true, + }, { name: "float", claims: map[string]any{"email_verified": float64(1)}, want: true, }, + {name: "numeric zero", claims: map[string]any{"email_verified": json.Number("0")}}, + {name: "invalid number", claims: map[string]any{"email_verified": json.Number("x")}}, + {name: "false string", claims: map[string]any{"email_verified": "false"}}, + {name: "invalid string", claims: map[string]any{"email_verified": "junk"}}, + {name: "float zero", claims: map[string]any{"email_verified": float64(0)}}, } for _, tc := range testCases { @@ -746,8 +766,7 @@ func Test_extractEmailVerified(t *testing.T) { } func TestServer_fetchUserInfoStatusError(t *testing.T) { - provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { - _ = hr + provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, _ *http.Request) { hw.WriteHeader(http.StatusUnauthorized) })) defer provider.Close() @@ -770,6 +789,218 @@ func TestServer_fetchUserInfoStatusError(t *testing.T) { } } +func TestTokenExchangeFailureStatus(t *testing.T) { + for _, tc := range []struct { + name string + providerStatus int + transportError error + want int + }{ + {"network failure", 0, errors.New("provider unavailable"), http.StatusBadGateway}, + {"timeout", 0, context.DeadlineExceeded, http.StatusBadGateway}, + {"provider bad request", http.StatusBadRequest, nil, http.StatusBadRequest}, + {"invalid client", http.StatusUnauthorized, nil, http.StatusBadGateway}, + {"provider unavailable", http.StatusServiceUnavailable, nil, http.StatusBadGateway}, + } { + t.Run(tc.name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path != "/token" { + t.Errorf("unexpected provider request %s", r.URL.Path) + return nil, errors.New("unexpected provider request") + } + if tc.transportError != nil { + return nil, tc.transportError + } + w := httptest.NewRecorder() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.providerStatus) + errorCode := "invalid_grant" + if tc.providerStatus == http.StatusUnauthorized { + errorCode = "invalid_client" + } + _, _ = fmt.Fprintf(w, "{\"error\":%q}", errorCode) + return w.Result(), nil + })} + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "code") + if response.Code != tc.want { + t.Fatalf("token exchange status = %d, want %d", response.Code, tc.want) + } + }) + } +} + +func TestServer_fetchUserInfoRespectsRequestCancellation(t *testing.T) { + started := make(chan struct{}) + release := make(chan struct{}) + defer close(release) + client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + close(started) + select { + case <-r.Context().Done(): + return nil, r.Context().Err() + case <-release: + return nil, errors.New("transport released without cancellation") + } + })} + ctx, cancel := context.WithCancel(context.WithValue(t.Context(), oauth2.HTTPClient, client)) + defer cancel() + result := make(chan error, 1) + go func() { + _, err := (&Server{}).fetchUserInfo(ctx, "https://provider.example/userinfo", + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "access"})) + result <- err + }() + waitFor(t, started, "UserInfo request did not start") + cancel() + if err := waitFor(t, result, "UserInfo request ignored cancellation"); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled UserInfo request: %v", err) + } +} + +func TestServer_fetchUserInfoUsesClientTimeoutAsRequestDeadline(t *testing.T) { + const timeout = 20 * time.Millisecond + var requestDeadline time.Time + client := &http.Client{ + Timeout: timeout, + Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + requestDeadline, _ = r.Context().Deadline() + select { + case <-r.Context().Done(): + case <-time.After(time.Second): + return nil, errors.New("UserInfo request did not time out") + } + return nil, r.Context().Err() + }), + } + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, client) + started := time.Now() + _, err := (&Server{}).fetchUserInfo(ctx, "https://provider.example/userinfo", + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "access"})) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("UserInfo timeout: %v", err) + } + if requestDeadline.Before(started) || requestDeadline.After(started.Add(time.Second)) { + t.Fatalf("UserInfo request deadline %s, want a short bounded deadline", requestDeadline) + } + if client.Timeout != timeout { + t.Fatalf("configured client timeout changed to %s", client.Timeout) + } +} + +func TestServer_fetchUserInfoOversizedStatusError(t *testing.T) { + provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, _ *http.Request) { + hw.WriteHeader(http.StatusServiceUnavailable) + _, _ = io.WriteString(hw, strings.Repeat("x", maxUserInfoBytes+1)) + })) + defer provider.Close() + + _, err := (&Server{}).fetchUserInfo(t.Context(), provider.URL, + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "token", TokenType: "Bearer"})) + if !errors.Is(err, ErrUserInfoStatus) || errors.Is(err, ErrUserInfoTooLarge) || !strings.Contains(err.Error(), "503 Service Unavailable") { + t.Fatalf("oversized error response = %v, want status error", err) + } +} + +func TestServer_fetchUserInfoDoesNotFollowBearerRedirect(t *testing.T) { + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Error("UserInfo redirect target received the bearer token request") + })) + defer target.Close() + provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { + if hr.Header.Get("Authorization") != "Bearer secret" { + t.Errorf("original UserInfo authorization = %q", hr.Header.Get("Authorization")) + } + http.Redirect(hw, hr, target.URL+"/profile", http.StatusFound) + })) + defer provider.Close() + + _, err := (&Server{}).fetchUserInfo(t.Context(), provider.URL+"/userinfo", + oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "secret", TokenType: "Bearer"})) + if !errors.Is(err, ErrUserInfoStatus) || !strings.Contains(err.Error(), "302 Found") { + t.Fatalf("redirect response = %v, want 302 without following", err) + } +} + +func TestOAuthOfflineOptionUsesAuthorizationOnly(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + srv.Options = []oauth2.AuthCodeOption{oauth2.AccessTypeOffline} + var idToken string + var tokenAccessType, tokenVerifier, tokenCode string + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path != "/token" { + t.Fatalf("unexpected provider request %s", r.URL) + } + if err := r.ParseForm(); err != nil { + return nil, err + } + tokenAccessType = r.PostForm.Get("access_type") + tokenVerifier = r.PostForm.Get("code_verifier") + tokenCode = r.PostForm.Get("code") + return jsonResponse(`{"access_token":"access","token_type":"Bearer","id_token":"` + idToken + `"}`), nil + })} + start := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/login", nil) + startResult := httptest.NewRecorder() + srv.HandleLogin(startResult, start) + if startResult.Code != http.StatusFound || len(startResult.Result().Cookies()) != 1 { + t.Fatalf("login start: status=%d cookies=%v", startResult.Code, startResult.Result().Cookies()) + } + location, err := url.Parse(startResult.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + if location.Query().Get("access_type") != "offline" || location.Query().Get("state") == "" || location.Query().Get("nonce") == "" { + t.Fatalf("authorization location = %s", location) + } + idToken = makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "client", "sub": "person", "nonce": location.Query().Get("nonce"), + "exp": time.Now().Add(time.Hour).Unix(), + }) + callback := httptest.NewRequest(http.MethodGet, + "http://example.com/oauth2/callback?state="+url.QueryEscape(location.Query().Get("state"))+"&code=example-code", nil) + callback.AddCookie(startResult.Result().Cookies()[0]) + result := httptest.NewRecorder() + srv.HandleAuthResponse(result, callback) + if result.Code != http.StatusFound || tokenAccessType != "" || tokenVerifier == "" || tokenCode != "example-code" { + t.Fatalf("exchange: status=%d access_type=%q verifier=%q code=%q body=%q", result.Code, tokenAccessType, tokenVerifier, tokenCode, result.Body.String()) + } +} + +func TestVerifyOIDCUserInfoDoesNotRefreshDuringCallback(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + srv.userinfoUrl = "https://provider.example/userinfo" + userinfoCalls, refreshCalls := 0, 0 + client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + switch req.URL.Path { + case "/userinfo": + userinfoCalls++ + return jsonResponse(`{"sub":"one","email":"user@example.com"}`), nil + case "/token": + refreshCalls++ + return jsonResponse(`{"access_token":"refreshed","token_type":"Bearer"}`), nil + default: + t.Fatalf("unexpected provider request %s", req.URL) + return nil, nil + } + })} + srv.httpClient = client + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, client) + raw := makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "client", "sub": "one", "nonce": "nonce", + "exp": time.Now().Add(time.Hour).Unix(), + }) + token := makeOAuth2Token("access", raw, "refresh") + token.Expiry = time.Now().Add(time.Second) // Inside oauth2's refresh skew. + claims, source, _, _, err := srv.verifyOAuthIdentity(ctx, token, "nonce") + if err != nil || source == nil || claims["email"] != "user@example.com" || userinfoCalls != 1 || refreshCalls != 0 { + t.Fatalf("claims=%v source=%t userinfo=%d refresh=%d err=%v", claims, source != nil, userinfoCalls, refreshCalls, err) + } +} + func TestServer_fetchUserInfoCloseError(t *testing.T) { wantErr := errors.New("close response body") client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { @@ -796,11 +1027,7 @@ func TestServer_fetchUserInfoCloseError(t *testing.T) { } func Test_handleAuthResponseMissingIDToken(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" provider := httptest.NewServer(http.HandlerFunc(func(hw http.ResponseWriter, hr *http.Request) { @@ -832,9 +1059,7 @@ func Test_handleAuthResponseMissingIDToken(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: "nonce123"}) srv.HandleAuthResponse(rec, req) @@ -853,11 +1078,7 @@ func Test_handleAuthResponseMissingIDToken(t *testing.T) { } func Test_handleAuthResponseInvalidIDToken(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" idToken := makeIDToken(t, map[string]any{ @@ -898,9 +1119,7 @@ func Test_handleAuthResponseInvalidIDToken(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: "nonce123"}) srv.HandleAuthResponse(rec, req) @@ -919,11 +1138,7 @@ func Test_handleAuthResponseInvalidIDToken(t *testing.T) { } func Test_handleAuthResponseAudienceMismatch(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" idToken := makeIDToken(t, map[string]any{ @@ -964,9 +1179,7 @@ func Test_handleAuthResponseAudienceMismatch(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: "nonce123"}) srv.HandleAuthResponse(rec, req) @@ -985,11 +1198,7 @@ func Test_handleAuthResponseAudienceMismatch(t *testing.T) { } func Test_handleAuthResponseNonceMismatch(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" idToken := makeIDToken(t, map[string]any{ @@ -1030,9 +1239,7 @@ func Test_handleAuthResponseNonceMismatch(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier(), nonce: "nonce123"}) srv.HandleAuthResponse(rec, req) @@ -1051,11 +1258,7 @@ func Test_handleAuthResponseNonceMismatch(t *testing.T) { } func Test_handleAuthResponseWithoutSession(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, HandledPaths: map[string]struct{}{}, @@ -1085,11 +1288,7 @@ func Test_handleAuthResponseWithoutSession(t *testing.T) { } func Test_handleAuthResponseLoginFailedFallsBack(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{Jaws: jw} var callbackCount int var callbackStatus int @@ -1133,11 +1332,7 @@ func Test_handleAuthResponseLoginFailedFallsBack(t *testing.T) { } func Test_handleAuthResponseLoginFailedHandlesResponse(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{Jaws: jw} const customBody = "handled by callback" srv.LoginFailed = func(hw http.ResponseWriter, hr *http.Request, httpCode int, callErr error, email string) bool { @@ -1175,11 +1370,7 @@ func Test_handleAuthResponseLoginFailedHandlesResponse(t *testing.T) { } func TestServerExtractEmail(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) testCases := []struct { name string @@ -1265,17 +1456,8 @@ func TestServerExtractEmail(t *testing.T) { if handler.message != "jawsauth: no email found" { t.Fatalf("unexpected log message: %s", handler.message) } - var attrFound bool - for _, attr := range handler.attrs { - if attr.Key == "userinfo" { - attrFound = true - if !reflect.DeepEqual(attr.Value.Any(), tc.userinfo) { - t.Fatalf("logged userinfo mismatch: %#v", attr.Value.Any()) - } - } - } - if !attrFound { - t.Fatal("userinfo attribute missing from log entry") + if len(handler.attrs) != 0 { + t.Fatalf("profile claims leaked to log: %#v", handler.attrs) } } else { if handler.called { @@ -1319,11 +1501,7 @@ func (h *recordingHandler) WithGroup(string) slog.Handler { } func Test_handleAuthResponseMissingState(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, HandledPaths: map[string]struct{}{}, @@ -1354,11 +1532,7 @@ func Test_handleAuthResponseMissingState(t *testing.T) { } func Test_handleAuthResponseMissingNonce(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) const issuer = "https://issuer.example" idToken := makeIDToken(t, map[string]any{ @@ -1399,9 +1573,8 @@ func Test_handleAuthResponseMissingNonce(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "http://example.com/oauth2/callback?state=state123&code=authcode123", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) - // deliberately leave oauth2NonceKey unset to exercise the missing-nonce branch + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: oauth2.GenerateVerifier()}) + // deliberately leave nonce unset to exercise the missing-nonce branch srv.HandleAuthResponse(rec, req) @@ -1419,12 +1592,23 @@ func Test_handleAuthResponseMissingNonce(t *testing.T) { } } -func Test_handlersRejectNonGet(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) +func TestHandleLinkWithoutOAuthConfigReturnsError(t *testing.T) { + jw := newTestJaws(t) + srv := &Server{Jaws: jw, LinkEvent: func(*jaws.Session, *http.Request, map[string]any) (int, error) { return 0, nil }} + req := httptest.NewRequest(http.MethodPost, "https://app.example/auth/link/provider", nil) + if jw.NewSession(httptest.NewRecorder(), req) == nil { + t.Fatal("missing session") + } + w := httptest.NewRecorder() + srv.HandleLink(w, req) + if w.Code != http.StatusInternalServerError || w.Header().Get("Location") != "" || + !strings.Contains(w.Body.String(), ErrOAuth2NotConfigured.Error()) { + t.Fatalf("unconfigured link: status=%d location=%q body=%q", w.Code, w.Header().Get("Location"), w.Body.String()) } - defer jw.Close() +} + +func Test_handlersRejectNonGet(t *testing.T) { + jw := newTestJaws(t) newServer := func() *Server { return &Server{ @@ -1454,9 +1638,7 @@ func Test_handlersRejectNonGet(t *testing.T) { rec := httptest.NewRecorder() req := httptest.NewRequest(method, "http://example.com/oauth2/callback", nil) sess := jw.NewSession(rec, req) - sess.Set(oauth2StateKey, "state123") - sess.Set(oauth2PKCEVerifierKey, "verifier123") - sess.Set(oauth2NonceKey, "nonce123") + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state123", expiry: time.Now().Add(oauth2FlowLifetime), verifier: "verifier123", nonce: "nonce123"}) h.handler(srv)(rec, req) @@ -1465,16 +1647,20 @@ func Test_handlersRejectNonGet(t *testing.T) { if resp.StatusCode != http.StatusMethodNotAllowed { t.Fatal(resp.Status) } + if allow := resp.Header.Get("Allow"); allow != http.MethodGet { + t.Fatalf("Allow = %q, want GET", allow) + } if loc := resp.Header.Get("Location"); loc != "" { t.Fatalf("unexpected Location header: %q", loc) } - if got, _ := sess.Get(oauth2StateKey).(string); got != "state123" { + flow := testOAuthFlow(sess, nil) + if got := flow.state; got != "state123" { t.Fatal(got) } - if got, _ := sess.Get(oauth2PKCEVerifierKey).(string); got != "verifier123" { + if got := flow.verifier; got != "verifier123" { t.Fatal(got) } - if got, _ := sess.Get(oauth2NonceKey).(string); got != "nonce123" { + if got := flow.nonce; got != "nonce123" { t.Fatal(got) } }) @@ -1532,7 +1718,7 @@ func TestRedirectTargetBounds(t *testing.T) { if !tc.accepted { want = "/" } - if got := jw.GetSession(req).Get(oauth2ReferrerKey); got != want { + if got := testOAuthFlow(jw.GetSession(req), nil).referrer; got != want { t.Errorf("referer=%v length=%d: unexpected stored target", referer, tc.length) } jw.Close() @@ -1543,11 +1729,7 @@ func TestRedirectTargetBounds(t *testing.T) { func TestLoginSessionUnavailable(t *testing.T) { for _, limit := range []string{"global", "perIP", "closed"} { t.Run(limit, func(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) switch limit { case "global": jw.MaxSessions = 1 diff --git a/oauth_flow_grace_test.go b/oauth_flow_grace_test.go new file mode 100644 index 0000000..d6cbdc1 --- /dev/null +++ b/oauth_flow_grace_test.go @@ -0,0 +1,277 @@ +package jawsauth + +import ( + "errors" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "testing/synctest" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "github.com/linkdata/jaws" + "golang.org/x/oauth2" +) + +func TestOAuthProviderVisitKeepsOnlyPendingSessionAlive(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := &Server{Jaws: jw, oauth2cfg: &oauth2.Config{ + ClientID: "test", RedirectURL: "https://app.example/auth/callback", + Endpoint: oauth2.Endpoint{AuthURL: "https://provider.example/auth", TokenURL: "https://provider.example/token"}, + }} + start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + response := httptest.NewRecorder() + srv.HandleLogin(response, start) + if response.Code != http.StatusFound { + t.Fatalf("start: %d", response.Code) + } + cookies := response.Result().Cookies() + if len(cookies) != 1 { + t.Fatalf("cookies: %d", len(cookies)) + } + callback := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback", nil) + callback.AddCookie(cookies[0]) + unrelated := httptest.NewRequest(http.MethodGet, "https://app.example/other", nil) + unrelated.RemoteAddr = "192.0.2.2:1234" + if jw.NewSession(httptest.NewRecorder(), unrelated) == nil { + t.Fatal("unrelated session missing") + } + time.Sleep(time.Minute + time.Second) + if jw.GetSession(callback) == nil { + t.Fatal("pending preauth session expired during provider visit") + } + if jw.GetSession(unrelated) != nil { + t.Fatal("unrelated idle session gained OAuth grace") + } + time.Sleep(oauth2FlowLifetime) + if jw.GetSession(callback) != nil { + t.Fatal("pending preauth session survived its bounded grace") + } + }) +} + +func TestPendingLoginProtectedRequestDoesNotRestartOrRenewGrace(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + protected := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) + protected.AddCookie(cookie) + time.Sleep(7 * time.Minute) + response := httptest.NewRecorder() + srv.Wrap(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("pending login reached protected handler") + })).ServeHTTP(response, protected) + if response.Code != http.StatusFound || response.Header().Get("Location") == "" { + t.Fatalf("pending login: status=%d location=%q body=%q", response.Code, response.Header().Get("Location"), response.Body.String()) + } + stored := testOAuthFlow(jw.GetSession(protected), srv) + if stored.state != state { + t.Fatalf("pending login replaced state: want %q got %q", state, stored.state) + } + time.Sleep(3*time.Minute + oauth2ProviderTimeout + 2*time.Second) + if jw.GetSession(protected) != nil { + t.Fatal("protected request renewed pending session grace") + } + }) +} + +func TestPendingLoginProtectedRequestRenewsNearExpiry(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, oldState := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + protected := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) + protected.AddCookie(cookie) + time.Sleep(oauth2FlowLifetime - time.Second) + response := httptest.NewRecorder() + srv.Wrap(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("pending login reached protected handler") + })).ServeHTTP(response, protected) + location, err := url.Parse(response.Header().Get("Location")) + if err != nil || response.Code != http.StatusFound { + t.Fatalf("new login: status=%d location=%q err=%v", response.Code, response.Header().Get("Location"), err) + } + newState := location.Query().Get("state") + if newState == "" || newState == oldState || testOAuthFlow(jw.GetSession(protected), srv).state != newState { + t.Fatalf("near-expiry flow was reused: old=%q new=%q", oldState, newState) + } + // Complete the renewed flow after the original session's grace deadline. + time.Sleep(oauth2ProviderTimeout + 3*time.Second) + if result := profileCallback(srv, cookie, newState, "first"); result.Code != http.StatusFound { + t.Fatalf("renewed login callback: status=%d body=%q", result.Code, result.Body.String()) + } + }) +} + +func TestExpiredPendingLoginStartsFreshFlow(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie, oldState := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + request := httptest.NewRequest(http.MethodGet, "https://app.example/protected", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + if sess == nil || jw.NewRequest(httptest.NewRecorder(), request).Session() != sess { + t.Fatal("pending session not attached to live page") + } + time.Sleep(oauth2FlowLifetime + time.Second) + response := httptest.NewRecorder() + srv.Wrap(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("expired pending flow reached protected handler") + })).ServeHTTP(response, request) + if response.Code != http.StatusFound || jw.GetSession(request) != sess { + t.Fatalf("fresh login: status=%d session=%v", response.Code, jw.GetSession(request)) + } + location, err := url.Parse(response.Header().Get("Location")) + if err != nil || location.Query().Get("state") == "" || location.Query().Get("state") == oldState || testOAuthFlow(sess, srv).state != location.Query().Get("state") { + t.Fatalf("expired flow was reused: location=%q err=%v", response.Header().Get("Location"), err) + } + }) +} + +func TestOAuthFlowExpiresEvenWithLiveSession(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + providerCalls := 0 + srv := &Server{Jaws: jw, oauth2cfg: &oauth2.Config{ + ClientID: "test", RedirectURL: "https://app.example/auth/callback", + Endpoint: oauth2.Endpoint{AuthURL: "https://provider.example/auth", TokenURL: "https://provider.example/token"}, + }, httpClient: &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + providerCalls++ + return nil, errors.New("provider should not be called") + })}} + start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + response := httptest.NewRecorder() + srv.HandleLogin(response, start) + if response.Code != http.StatusFound { + t.Fatalf("start: %d", response.Code) + } + cookies := response.Result().Cookies() + callback := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback", nil) + callback.AddCookie(cookies[0]) + sess := jw.GetSession(callback) + if sess == nil { + t.Fatal("session missing") + } + state := testOAuthFlow(sess, nil).state + callback.URL.RawQuery = "state=" + state + "&code=unused" + page := jw.NewRequest(httptest.NewRecorder(), callback) + if page.Session() != sess { + t.Fatal("page did not attach") + } + time.Sleep(oauth2FlowLifetime + time.Second) + if jw.GetSession(callback) == nil { + t.Fatal("live page should hold session") + } + result := httptest.NewRecorder() + srv.HandleAuthResponse(result, callback) + if result.Code != http.StatusBadRequest || !strings.Contains(result.Body.String(), ErrOAuth2ExpiredState.Error()) || sess.Get(srv.sessionKey(oauth2FlowKey)) != nil || providerCalls != 0 { + t.Fatalf("expired flow: status=%d state=%v provider_calls=%d body=%q", result.Code, sess.Get(srv.sessionKey(oauth2FlowKey)), providerCalls, result.Body.String()) + } + }) +} + +func TestOIDCInitialCallbackAfterProviderVisit(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + var idToken string + exchanges := 0 + srv := &Server{Jaws: jw, + SessionKey: "claims", SessionTokenKey: "token", SessionEmailKey: "email", SessionEmailVerifiedKey: "email_verified", + oauth2cfg: &oauth2.Config{ClientID: "app", RedirectURL: "https://app.example/auth/callback", + Endpoint: oauth2.Endpoint{AuthURL: "https://issuer.example/authorize", TokenURL: "https://issuer.example/token"}}, + idTokenVerifier: oidc.NewVerifier("https://issuer.example", passthroughKeySet{}, &oidc.Config{ClientID: "app"}), + httpClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + exchanges++ + return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/json"}}, + Body: io.NopCloser(strings.NewReader(`{"access_token":"access","token_type":"Bearer","id_token":"` + idToken + `"}`))}, nil + })}, + } + start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + response := httptest.NewRecorder() + srv.HandleLogin(response, start) + if response.Code != http.StatusFound { + t.Fatalf("start: %d", response.Code) + } + cookie := response.Result().Cookies()[0] + preauth := jw.GetSession(start) + flow := testOAuthFlow(preauth, nil) + state, nonce := flow.state, flow.nonce + idToken = makeIDToken(t, map[string]any{ + "iss": "https://issuer.example", "aud": "app", "sub": "subject-1", "nonce": nonce, + "exp": time.Now().Add(time.Hour).Unix(), "iat": time.Now().Unix(), + }) + time.Sleep(time.Minute + time.Second) + callback := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback?state="+state+"&code=code", nil) + callback.AddCookie(cookie) + result := httptest.NewRecorder() + srv.HandleAuthResponse(result, callback) + if result.Code != http.StatusFound || exchanges != 1 || jw.GetSession(callback) == preauth { + t.Fatalf("delayed OIDC callback: status=%d exchanges=%d body=%q", result.Code, exchanges, result.Body.String()) + } + rotated := result.Result().Cookies() + if len(rotated) != 1 { + t.Fatalf("rotated cookies: %d", len(rotated)) + } + authorized := httptest.NewRequest(http.MethodGet, "https://app.example/", nil) + authorized.AddCookie(rotated[0]) + sess := jw.GetSession(authorized) + if sess == nil || sess.Get(srv.SessionKey).(map[string]any)["sub"] != "subject-1" { + t.Fatal("delayed OIDC sign-in missing verified subject") + } + }) +} + +func TestOAuthLinkCallbackNearDeadlineCanFinishProviderExchange(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + linked := false + srv := &Server{Jaws: jw, SessionKey: "claims", issuer: "https://provider.example", + profileSubject: func(map[string]any) (string, error) { return "42", nil }, + userinfoUrl: "https://provider.example/user", + oauth2cfg: &oauth2.Config{ClientID: "app", RedirectURL: "https://app.example/auth/callback", + Endpoint: oauth2.Endpoint{AuthURL: "https://provider.example/auth", TokenURL: "https://provider.example/token"}}, + httpClient: &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + time.Sleep(10 * time.Second) + return jsonResponse(`{"access_token":"access","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })}, + } + start := httptest.NewRequest(http.MethodPost, "https://app.example/auth/link", nil) + created := httptest.NewRecorder() + sess := jw.NewSession(created, start) + if sess == nil { + t.Fatal("original session missing") + } + sess.Set(srv.SessionKey, map[string]any{"sub": "original"}) + srv.LinkEvent = func(current *jaws.Session, _ *http.Request, claims map[string]any) (int, error) { + if current != sess || current.Get(srv.SessionKey).(map[string]any)["sub"] != "original" || claims["sub"] != "42" { + return http.StatusForbidden, ErrOAuth2LinkRejected + } + linked = true + return 0, nil + } + startResult := httptest.NewRecorder() + srv.HandleLink(startResult, start) + if startResult.Code != http.StatusSeeOther { + t.Fatalf("link start: %d", startResult.Code) + } + state := testOAuthFlow(sess, nil).state + cookie := created.Result().Cookies()[0] + time.Sleep(oauth2FlowLifetime - time.Second) + callback := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback?state="+state+"&code=code", nil) + callback.AddCookie(cookie) + result := httptest.NewRecorder() + srv.HandleAuthResponse(result, callback) + if result.Code != http.StatusFound || !linked || jw.GetSession(callback) != sess { + t.Fatalf("near-deadline link: status=%d linked=%t body=%q", result.Code, linked, result.Body.String()) + } + }) +} diff --git a/profile_link_test.go b/profile_link_test.go new file mode 100644 index 0000000..8ce42cd --- /dev/null +++ b/profile_link_test.go @@ -0,0 +1,1154 @@ +package jawsauth + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "testing/synctest" + "time" + + "github.com/linkdata/jaws" + "golang.org/x/oauth2" +) + +func jsonResponse(body string) *http.Response { + return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/json"}}, Body: io.NopCloser(strings.NewReader(body))} +} + +func TestOAuthProfileConfigRequiresExplicitSecureEndpoints(t *testing.T) { + cfg := &Config{RedirectURL: "https://app.example/auth/callback", Issuer: "https://provider.example", + AuthURL: "https://provider.example/authorize", TokenURL: "https://provider.example/token", + UserInfoURL: "https://provider.example/user", ClientID: "app", + ProfileSubject: func(map[string]any) (string, error) { return "42", nil }} + if err := cfg.Validate(); err != nil { + t.Fatal(err) + } + cfg.UserInfoURL = "" + if err := cfg.Validate(); !errors.Is(err, ErrConfigMissingValue) { + t.Fatalf("missing profile URL: %v", err) + } + cfg.UserInfoURL = "http://provider.example/user" + if err := cfg.Validate(); !errors.Is(err, ErrConfigEndpointMustBeHTTPS) { + t.Fatalf("insecure profile URL: %v", err) + } + cfg.AllowInsecureIssuer = true + cfg.Issuer = "ftp://provider.example" + if err := cfg.Validate(); !errors.Is(err, ErrConfigIssuerMustBeHTTPS) { + t.Fatalf("non-HTTP dev issuer: %v", err) + } + cfg.Issuer = "http://provider.example" + cfg.AuthURL = "http://provider.example/authorize" + cfg.TokenURL = "http://provider.example/token" + if err := cfg.Validate(); err != nil { + t.Fatalf("HTTP dev endpoints: %v", err) + } +} + +func TestOAuthProfileAuthLifetimeCapsLongProviderToken(t *testing.T) { + srv := &Server{issuer: "https://provider.example", userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { return "42", nil }} + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(`{"id":42}`), nil + })}) + for _, duration := range []time.Duration{30 * time.Minute, 72 * time.Hour} { + token := &oauth2.Token{AccessToken: "secret", TokenType: "Bearer", Expiry: time.Now().Add(duration)} + before := time.Now() + _, _, expiry, status, err := srv.verifyOAuthIdentity(ctx, token, "") + after := time.Now() + if err != nil || status != 0 { + t.Fatalf("duration %s: status=%d error=%v", duration, status, err) + } + if duration < 24*time.Hour { + if !expiry.Equal(token.Expiry) { + t.Fatalf("duration %s: expiry=%s token expiry=%s", duration, expiry, token.Expiry) + } + } else if expiry.Before(before.Add(24*time.Hour)) || expiry.After(after.Add(24*time.Hour)) { + t.Fatalf("duration %s: cap expiry=%s, callback bounds=%s..%s", duration, expiry, before, after) + } + } +} + +func TestOAuthProfileCallbackStateIsSingleUseConcurrently(t *testing.T) { + jw := newTestJaws(t) + var calls atomic.Int32 + client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + calls.Add(1) + if r.URL.Path == "/token" { + return jsonResponse(`{"access_token":"secret","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + srv, err := New(jw, &Config{Namespace: "one", RedirectURL: "https://app.example/auth/callback", + Issuer: "https://provider.example", AuthURL: "https://provider.example/authorize", + TokenURL: "https://provider.example/token", UserInfoURL: "https://provider.example/user", + ClientID: "app", HTTPClient: client, + ProfileSubject: func(map[string]any) (string, error) { return "42", nil }}, func(string, http.Handler) {}) + if err != nil { + t.Fatal(err) + } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + responses := make(chan int, 2) + ready := make(chan struct{}, 2) + start := make(chan struct{}) + var workers sync.WaitGroup + for range 2 { + workers.Add(1) + go func() { + defer workers.Done() + ready <- struct{}{} + <-start + responses <- profileCallback(srv, cookie, state, "first").Code + }() + } + <-ready + <-ready + close(start) + workers.Wait() + close(responses) + seen := map[int]int{} + for code := range responses { + seen[code]++ + } + if seen[http.StatusFound] != 1 || seen[http.StatusBadRequest] != 1 || calls.Load() != 2 { + t.Fatalf("concurrent replay: statuses=%v provider_calls=%d", seen, calls.Load()) + } +} + +type rotationNoticeLock struct { + mu sync.Mutex + entered chan struct{} + once sync.Once +} + +func (l *rotationNoticeLock) Lock() { + l.once.Do(func() { close(l.entered) }) + l.mu.Lock() +} + +func (l *rotationNoticeLock) Unlock() { l.mu.Unlock() } + +func TestOAuthProfileRotationLockDoesNotBlockProvider(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + providerReached := make(chan struct{}) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + close(providerReached) + return jsonResponse(`{"access_token":"proof","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + lock := &rotationNoticeLock{entered: make(chan struct{})} + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + srv.SessionRotationLock = lock + oldRequest := httptest.NewRequest(http.MethodGet, "https://app.example/", nil) + oldRequest.AddCookie(cookie) + old := jw.GetSession(oldRequest) + lock.mu.Lock() + release := sync.OnceFunc(func() { lock.mu.Unlock() }) + defer release() + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "proof") }() + waitFor(t, providerReached, "provider request blocked by session rotation lock") + waitFor(t, lock.entered, "callback did not reach session rotation") + if jw.GetSession(oldRequest) != old { + t.Fatal("session rotated before acquiring lock") + } + release() + response := waitFor(t, result, "callback did not finish after rotation lock released") + if response.Code != http.StatusFound || len(response.Result().Cookies()) != 1 || jw.GetSession(oldRequest) != nil { + t.Fatalf("callback after rotation lock: status=%d cookies=%v", response.Code, response.Result().Cookies()) + } +} + +func TestOAuthFailedCallbackDoesNotEraseNewCallback(t *testing.T) { + jw := newTestJaws(t) + entered := make(chan struct{}) + secondEntered := make(chan struct{}) + release := make(chan struct{}) + secondRelease := make(chan struct{}) + var enterOnce sync.Once + var releaseOnce sync.Once + var secondReleaseOnce sync.Once + releaseCallback := func() { releaseOnce.Do(func() { close(release) }) } + releaseSecond := func() { secondReleaseOnce.Do(func() { close(secondRelease) }) } + defer releaseCallback() + defer releaseSecond() + client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + if err := r.ParseForm(); err != nil { + return nil, err + } + if r.PostForm.Get("code") == "first" { + enterOnce.Do(func() { close(entered) }) + <-release + return nil, errors.New("provider unavailable") + } + close(secondEntered) + <-secondRelease + return jsonResponse(`{"access_token":"secret","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + srv, err := New(jw, &Config{Namespace: "one", RedirectURL: "https://app.example/auth/callback", + Issuer: "https://provider.example", AuthURL: "https://provider.example/authorize", + TokenURL: "https://provider.example/token", UserInfoURL: "https://provider.example/user", + ClientID: "app", HTTPClient: client, + ProfileSubject: func(map[string]any) (string, error) { return "42", nil }}, func(string, http.Handler) {}) + if err != nil { + t.Fatal(err) + } + cookie, firstState := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + firstResult := make(chan int, 1) + go func() { firstResult <- profileCallback(srv, cookie, firstState, "first").Code }() + waitFor(t, entered, "first callback did not reach provider") + r := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + r.Header.Set("Referer", "https://app.example/second") + r.AddCookie(cookie) + w := httptest.NewRecorder() + srv.HandleLogin(w, r) + if w.Code != http.StatusFound { + t.Fatalf("second start: %d", w.Code) + } + location, err := url.Parse(w.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + secondState := location.Query().Get("state") + secondResult := make(chan *httptest.ResponseRecorder, 1) + go func() { secondResult <- profileCallback(srv, cookie, secondState, "second") }() + waitFor(t, secondEntered, "second callback did not reach provider") + releaseCallback() + if code := waitFor(t, firstResult, "failed callback did not finish"); code != http.StatusBadGateway { + t.Fatalf("failed callback status = %d", code) + } + sess := jw.GetSession(r) + if sess == nil { + t.Fatal("pending session missing") + } + srv.mu.Lock() + active := srv.oauthCallbacks[sess] + srv.mu.Unlock() + if active.canceled == nil || active.referrer != "/second" || active.revoked() { + t.Fatal("failed callback erased the newer callback") + } + if !srv.Logout(sess, nil) || !active.revoked() { + t.Fatal("logout did not revoke the newer callback") + } + releaseSecond() + second := waitFor(t, secondResult, "revoked second callback did not finish") + if second.Code != http.StatusForbidden || second.Header().Get("Location") != "" || len(second.Result().Cookies()) != 0 { + t.Fatalf("second callback: status=%d location=%q cookies=%v", second.Code, second.Header().Get("Location"), second.Result().Cookies()) + } +} + +func TestOAuthProfileCallbackCanceledBeforeRotation(t *testing.T) { + for _, action := range []string{"logout", "new flow"} { + t.Run(action, func(t *testing.T) { + jw := newTestJaws(t) + entered := make(chan struct{}) + release := make(chan struct{}) + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + srv := newProfileTestServer(t, jw, nil) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + close(entered) + <-release + return jsonResponse(`{"access_token":"first-token","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42,"email":"first@example.com"}`), nil + })} + logins := 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + if sess == nil { + t.Fatal("missing pre-auth session") + } + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "first") }() + waitFor(t, entered, "callback did not reach provider") + + var nextState string + if action == "logout" { + if !srv.Logout(sess, nil) { + t.Fatal("logout did not clear session") + } + } else { + start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + start.AddCookie(cookie) + w := httptest.NewRecorder() + srv.HandleLogin(w, start) + if w.Code != http.StatusFound { + t.Fatalf("new flow start: %d", w.Code) + } + location, err := url.Parse(w.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + nextState = location.Query().Get("state") + if nextState == "" || nextState == state { + t.Fatalf("new flow state = %q", nextState) + } + } + + releaseOnce.Do(func() { close(release) }) + w := waitFor(t, result, "canceled callback did not finish") + if w.Code != http.StatusForbidden || len(w.Result().Cookies()) != 0 || logins != 0 { + t.Fatalf("canceled callback: status=%d cookies=%v logins=%d", w.Code, w.Result().Cookies(), logins) + } + storedState := testOAuthFlow(sess, srv).state + if sess.Get(srv.SessionKey) != nil || storedState != nextState { + t.Fatalf("canceled callback changed session or new flow: claims=%v state=%v", sess.Get(srv.SessionKey), storedState) + } + }) + } +} + +func TestOAuthProfileCallbackCanceledBeforeFinalStore(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie := profileLogin(t, srv) + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + old := jw.GetSession(request) + if old == nil { + t.Fatal("missing original session") + } + _, state := profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + canceled := false + logins := 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + srv.LogoutEvent = func(sess *jaws.Session, _ *http.Request) { + if sess == old && !canceled { + canceled = true + srv.Logout(old, nil) + } + } + w := profileCallback(srv, cookie, state, "second") + cookies := w.Result().Cookies() + if !canceled || logins != 0 || w.Code != http.StatusForbidden || w.Header().Get("Location") != "" || + len(cookies) != 2 || cookies[1].MaxAge >= 0 || cookies[1].Value != cookies[0].Value { + t.Fatalf("canceled before store: canceled=%t logins=%d status=%d location=%q cookies=%v", canceled, logins, w.Code, w.Header().Get("Location"), cookies) + } + request = httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookies[0]) + if jw.GetSession(request) != nil || old.Get(srv.SessionKey) != nil || len(srv.authTimers) != 0 { + t.Fatalf("canceled login retained auth or timer: old=%v timers=%d", old.Get(srv.SessionKey), len(srv.authTimers)) + } +} + +func TestOAuthProfileLinkCanceledByLogout(t *testing.T) { + jw := newTestJaws(t) + entered := make(chan struct{}) + release := make(chan struct{}) + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + srv := newProfileTestServer(t, jw, nil) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + if err := r.ParseForm(); err != nil { + return nil, err + } + if r.PostForm.Get("code") == "second" { + close(entered) + <-release + } + return jsonResponse(`{"access_token":"first-token","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42,"email":"first@example.com"}`), nil + })} + cookie := profileLogin(t, srv) + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + if sess == nil || sess.Get(srv.SessionKey) == nil { + t.Fatal("missing authenticated session") + } + links := 0 + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + links++ + return 0, nil + } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, linkState, "second") }() + waitFor(t, entered, "link callback did not reach provider") + if !srv.Logout(sess, nil) { + t.Fatal("logout did not clear linked session") + } + releaseOnce.Do(func() { close(release) }) + w := waitFor(t, result, "canceled link callback did not finish") + if w.Code != http.StatusForbidden || len(w.Result().Cookies()) != 0 || links != 0 || sess.Get(srv.SessionKey) != nil { + t.Fatalf("canceled link: status=%d cookies=%v links=%d claims=%v", w.Code, w.Result().Cookies(), links, sess.Get(srv.SessionKey)) + } +} + +func TestOAuthProfileAuthExpiryCancelsRunningLink(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie := profileLogin(t, srv) + entered := make(chan struct{}) + release := make(chan struct{}) + releaseProvider := sync.OnceFunc(func() { close(release) }) + defer releaseProvider() + transport := srv.httpClient.Transport + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + close(entered) + <-release + } + return transport.RoundTrip(r) + })} + links := 0 + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { links++; return 0, nil } + _, state := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "second") }() + waitFor(t, entered, "link callback did not reach provider") + req := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + req.AddCookie(cookie) + sess := jw.GetSession(req) + if sess == nil { + t.Fatal("authenticated session missing") + } + srv.mu.Lock() + srv.authTimers[sess.ID()].expiry = time.Now().Add(-time.Second) + srv.mu.Unlock() + factory.timer(0).fire() + releaseProvider() + response := waitFor(t, result, "expired link callback did not finish") + if response.Code != http.StatusForbidden || links != 0 || sess.Get(srv.SessionKey) != nil { + t.Fatalf("expired link: status=%d links=%d claims=%v", response.Code, links, sess.Get(srv.SessionKey)) + } +} + +func TestOAuthProfileLinkRejectsReplacedSession(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + cookie := profileLogin(t, srv) + entered := make(chan struct{}) + release := make(chan struct{}) + releaseProvider := sync.OnceFunc(func() { close(release) }) + defer releaseProvider() + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + close(entered) + <-release + return jsonResponse(`{"access_token":"proof","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + links := 0 + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { links++; return 0, nil } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + result := make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, linkState, "second") }() + waitFor(t, entered, "link callback did not reach provider") + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + old := jw.GetSession(request) + if old == nil { + t.Fatal("original session missing") + } + if replacement := jw.NewSession(httptest.NewRecorder(), request); replacement == nil || replacement == old || jw.GetSession(request) == old { + t.Fatal("JaWS did not replace the session") + } + releaseProvider() + response := waitFor(t, result, "replaced link callback did not finish") + if response.Code != http.StatusForbidden || links != 0 || len(response.Result().Cookies()) != 0 { + t.Fatalf("replaced link: status=%d links=%d cookies=%v", response.Code, links, response.Result().Cookies()) + } +} + +type observedRotationLock struct { + sync.Mutex + attempts atomic.Int32 + second chan struct{} +} + +func (lock *observedRotationLock) Lock() { + if lock.attempts.Add(1) == 2 { + close(lock.second) + } + lock.Mutex.Lock() +} + +func TestOAuthProfileLinkSerializesManagedRevocation(t *testing.T) { + for _, revocation := range []string{"timer", "wrapper", "handler logout"} { + for _, revocationFirst := range []bool{false, true} { + name := revocation + "/event first" + if revocationFirst { + name = revocation + "/revocation first" + } + t.Run(name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie := profileLogin(t, srv) + req := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + req.AddCookie(cookie) + sess := jw.GetSession(req) + lock := &observedRotationLock{second: make(chan struct{})} + var logoutEvents atomic.Int32 + srv.LogoutEvent = func(*jaws.Session, *http.Request) { + lock.Lock() + logoutEvents.Add(1) + lock.Unlock() + } + checked := make(chan struct{}) + release := make(chan struct{}) + releaseEvent := sync.OnceFunc(func() { close(release) }) + defer releaseEvent() + writes := 0 + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + if current, _ := srv.sessionAuthStatus(sess, time.Now); !current { + return http.StatusForbidden, ErrOAuth2MissingSession + } + close(checked) + <-release + if sess.Get(srv.SessionKey) == nil { + return http.StatusForbidden, ErrOAuth2MissingSession + } + writes++ // durable application write + return 0, nil + } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + srv.SessionRotationLock = lock + revoke := func() { + switch revocation { + case "timer": + srv.mu.Lock() + srv.authTimers[sess.ID()].expiry = time.Now().Add(-time.Second) + srv.mu.Unlock() + factory.timer(0).fire() + case "wrapper": + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(-time.Second)) + wrapper{server: srv}.ServeHTTP(httptest.NewRecorder(), req) + case "handler logout": + srv.HandleLogout(httptest.NewRecorder(), req) + } + } + if revocationFirst { + revoke() + releaseEvent() + response := profileCallback(srv, cookie, linkState, "second") + if response.Code != http.StatusBadRequest || writes != 0 || logoutEvents.Load() != 1 { + t.Fatalf("revocation won: status=%d writes=%d logout events=%d", response.Code, writes, logoutEvents.Load()) + } + return + } + callbackDone := make(chan *httptest.ResponseRecorder, 1) + go func() { callbackDone <- profileCallback(srv, cookie, linkState, "second") }() + waitFor(t, checked, "link did not check current auth") + revoked := make(chan struct{}) + go func() { revoke(); close(revoked) }() + waitFor(t, lock.second, "revocation did not reach rotation lock") + select { + case <-revoked: + t.Fatal("revocation completed before durable link write") + default: + } + releaseEvent() + waitFor(t, revoked, "revocation did not finish after link write") + response := waitFor(t, callbackDone, "link callback did not finish") + if response.Code != http.StatusFound || writes != 1 || logoutEvents.Load() != 1 { + t.Fatalf("callback status = %d, durable link writes = %d, logout events = %d", response.Code, writes, logoutEvents.Load()) + } + }) + } + } +} + +func TestOAuthProfileAuthExpiryPreservesOrdinaryLogin(t *testing.T) { + for _, phase := range []string{"pending", "callback"} { + t.Run(phase, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie := profileLogin(t, srv) + start := httptest.NewRequest(http.MethodGet, "https://app.example/auth/login", nil) + start.AddCookie(cookie) + sess := jw.GetSession(start) + if sess == nil { + t.Fatal("missing original session") + } + redirect := httptest.NewRecorder() + srv.HandleLogin(redirect, start) + if redirect.Code != http.StatusFound { + t.Fatalf("new login start: %d", redirect.Code) + } + location, err := url.Parse(redirect.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + state := location.Query().Get("state") + if state == "" { + t.Fatal("missing new login state") + } + var result chan *httptest.ResponseRecorder + var releaseOnce sync.Once + var release chan struct{} + if phase == "callback" { + entered := make(chan struct{}) + release = make(chan struct{}) + defer releaseOnce.Do(func() { close(release) }) + transport := srv.httpClient.Transport + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + if err := r.ParseForm(); err != nil { + return nil, err + } + if r.PostForm.Get("code") == "second" { + close(entered) + <-release + } + } + return transport.RoundTrip(r) + })} + result = make(chan *httptest.ResponseRecorder, 1) + go func() { result <- profileCallback(srv, cookie, state, "second") }() + waitFor(t, entered, "new login callback did not reach token exchange") + } + srv.mu.Lock() + srv.authTimers[sess.ID()].expiry = time.Now().Add(-time.Second) + srv.mu.Unlock() + factory.timer(0).fire() + if sess.Get(srv.SessionKey) != nil { + t.Fatal("expired original auth remained") + } + if phase == "pending" && testOAuthFlow(sess, srv).state != state { + t.Fatal("auth expiry erased pending login state") + } + if phase == "callback" { + protected := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + protected.AddCookie(cookie) + failedCalls := 0 + handleFailure := false + srv.LoginFailed = func(hw http.ResponseWriter, _ *http.Request, status int, failure error, _ string) bool { + failedCalls++ + if status != http.StatusConflict || !errors.Is(failure, ErrOAuth2LoginInProgress) || !srv.mu.TryLock() { + t.Fatalf("LoginFailed called under lock or with wrong failure: status=%d err=%v", status, failure) + } + srv.mu.Unlock() + if handleFailure { + hw.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(hw, "custom response") + return true + } + return false + } + defer func() { srv.LoginFailed = nil }() + conflict := httptest.NewRecorder() + srv.Wrap(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("pending callback reached protected handler") + })).ServeHTTP(conflict, protected) + if conflict.Code != http.StatusConflict || !strings.Contains(conflict.Body.String(), ErrOAuth2LoginInProgress.Error()) || failedCalls != 1 { + t.Fatalf("pending callback: status=%d body=%q failed_calls=%d", conflict.Code, conflict.Body.String(), failedCalls) + } + handleFailure = true + handled := httptest.NewRecorder() + srv.Wrap(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + t.Fatal("pending callback reached protected handler") + })).ServeHTTP(handled, protected) + if handled.Code != http.StatusAccepted || handled.Body.String() != "custom response" || failedCalls != 2 { + t.Fatalf("handled callback: status=%d body=%q failed_calls=%d", handled.Code, handled.Body.String(), failedCalls) + } + srv.LoginFailed = nil + } + var response *httptest.ResponseRecorder + if phase == "callback" { + releaseOnce.Do(func() { close(release) }) + response = waitFor(t, result, "new login callback did not finish") + } else { + response = profileCallback(srv, cookie, state, "second") + } + if response.Code != http.StatusFound || len(response.Result().Cookies()) != 1 { + t.Fatalf("new login: status=%d cookies=%v body=%q", response.Code, response.Result().Cookies(), response.Body.String()) + } + check := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + check.AddCookie(response.Result().Cookies()[0]) + rotated := jw.GetSession(check) + if rotated == nil || rotated == sess || rotated.Get(srv.SessionKey).(map[string]any)["sub"] != "99" { + t.Fatal("new login did not authenticate a fresh session") + } + }) + } +} + +func newProfileTestServer(t *testing.T, jw *jaws.Jaws, calls *int) *Server { + t.Helper() + client := &http.Client{Timeout: time.Second, Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if calls != nil { + *calls++ + } + switch r.URL.Path { + case "/token": + if err := r.ParseForm(); err != nil || r.PostForm.Get("code_verifier") == "" { + return nil, errors.New("missing PKCE verifier") + } + if r.PostForm.Get("code") == "second" { + return jsonResponse(`{"access_token":"second-token","token_type":"Bearer"}`), nil + } + return jsonResponse(`{"access_token":"first-token","token_type":"Bearer"}`), nil + case "/user": + switch r.Header.Get("Authorization") { + case "Bearer first-token": + return jsonResponse(`{"id":9007199254740993,"login":"first","email":"first@example.com"}`), nil + case "Bearer second-token": + return jsonResponse(`{"id":99,"login":"second"}`), nil + } + } + return nil, errors.New("unexpected provider request") + })} + srv, err := New(jw, &Config{ + Namespace: "github", RedirectURL: "https://app.example/auth/callback", Issuer: "https://provider.example", + AuthURL: "https://provider.example/authorize", TokenURL: "https://provider.example/token", + UserInfoURL: "https://provider.example/user", ClientID: "app", ClientSecret: "secret", + Scopes: []string{"read:user"}, HTTPClient: client, + ProfileSubject: func(claims map[string]any) (string, error) { + id, ok := claims["id"].(json.Number) + if !ok { + return "", ErrOAuth2InvalidProfile + } + value, err := strconv.ParseUint(string(id), 10, 64) + if err != nil || value == 0 { + return "", ErrOAuth2InvalidProfile + } + return string(id), nil + }, + }, func(string, http.Handler) {}) + if err != nil || !srv.Valid() { + t.Fatalf("profile server: %v", err) + } + return srv +} + +func profileStart(t *testing.T, srv *Server, method, path string, cookie *http.Cookie) (*http.Cookie, string) { + t.Helper() + r := httptest.NewRequest(method, "https://app.example"+path, nil) + if cookie != nil { + r.AddCookie(cookie) + } + w := httptest.NewRecorder() + if method == http.MethodPost { + srv.HandleLink(w, r) + } else { + srv.HandleLogin(w, r) + } + wantStatus := http.StatusFound + if method == http.MethodPost { + wantStatus = http.StatusSeeOther + } + if w.Code != wantStatus { + t.Fatalf("start %s: %d %s", path, w.Code, w.Body.String()) + } + location, err := url.Parse(w.Header().Get("Location")) + if err != nil || location.Query().Get("state") == "" { + t.Fatalf("provider redirect: %v %q", err, w.Header().Get("Location")) + } + if method == http.MethodGet { + if strings.Contains(location.RawQuery, "openid") || location.Query().Has("nonce") { + t.Fatalf("profile flow sent OIDC-only parameters: %s", location) + } + cookies := w.Result().Cookies() + if cookie == nil { + if len(cookies) != 1 { + t.Fatalf("new session cookies: %d", len(cookies)) + } + cookie = cookies[0] + } else if len(cookies) != 0 { + t.Fatalf("existing session cookies: %d", len(cookies)) + } + } + return cookie, location.Query().Get("state") +} + +func profileCallback(srv *Server, cookie *http.Cookie, state, code string) *httptest.ResponseRecorder { + r := httptest.NewRequest(http.MethodGet, "https://app.example/auth/callback?state="+url.QueryEscape(state)+"&code="+code, nil) + r.AddCookie(cookie) + w := httptest.NewRecorder() + srv.HandleAuthResponse(w, r) + return w +} + +func profileLogin(t *testing.T, srv *Server) *http.Cookie { + t.Helper() + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "first") + cookies := response.Result().Cookies() + if response.Code != http.StatusFound || len(cookies) != 1 { + t.Fatalf("profile login: status=%d cookies=%v", response.Code, cookies) + } + return cookies[0] +} + +func TestOAuthProfileSignInAndDualProofLink(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + logins, links := 0, 0 + srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } + oldCookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + wrong := profileCallback(srv, oldCookie, "wrong", "first") + if wrong.Code != http.StatusBadRequest || calls != 0 { + t.Fatalf("wrong state consumed login: status=%d calls=%d", wrong.Code, calls) + } + // Let the one-minute initial JaWS grace period pass before callback. + time.Sleep(time.Minute + time.Second) + first := profileCallback(srv, oldCookie, state, "first") + if first.Code != http.StatusFound || logins != 1 || calls != 2 { + t.Fatalf("first sign-in: status=%d logins=%d calls=%d", first.Code, logins, calls) + } + newCookies := first.Result().Cookies() + if len(newCookies) != 1 { + t.Fatalf("rotated cookies: %d", len(newCookies)) + } + cookie := newCookies[0] + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + claims, _ := sess.Get(srv.SessionKey).(map[string]any) + if claims["sub"] != "9007199254740993" || claims["iss"] != "https://provider.example" || claims["email"] != "first@example.com" { + t.Fatalf("verified profile: %#v", claims) + } + srv.LinkEvent = func(current *jaws.Session, _ *http.Request, newClaims map[string]any) (int, error) { + links++ + if current != sess || current.Get(srv.SessionKey).(map[string]any)["sub"] != "9007199254740993" || newClaims["sub"] != "99" { + return http.StatusForbidden, errors.New("proof mismatch") + } + return 0, nil + } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + // Check that a pending link also survives the initial grace period. + time.Sleep(time.Minute + time.Second) + linked := profileCallback(srv, cookie, linkState, "second") + if linked.Code != http.StatusFound || links != 1 || logins != 1 || calls != 4 || len(linked.Result().Cookies()) != 0 { + t.Fatalf("link: status=%d links=%d logins=%d calls=%d", linked.Code, links, logins, calls) + } + if jw.GetSession(request) != sess || sess.Get(srv.SessionKey).(map[string]any)["sub"] != "9007199254740993" { + t.Fatal("link replaced the original proof") + } + replayed := profileCallback(srv, cookie, linkState, "second") + if replayed.Code != http.StatusBadRequest || calls != 4 || links != 1 { + t.Fatalf("replay: status=%d calls=%d links=%d", replayed.Code, calls, links) + } + }) +} + +func TestOAuthProfileExpiredLinkStateNeverCallsProvider(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + cookie := profileLogin(t, srv) + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + t.Fatal("expired link reached link event") + return 0, nil + } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + sess := jw.GetSession(r) + flow := testOAuthFlow(sess, srv) + flow.expiry = time.Now().Add(-time.Second) + setTestOAuthFlow(sess, srv, flow) + before := calls + response := profileCallback(srv, cookie, linkState, "second") + if response.Code != http.StatusBadRequest || !strings.Contains(response.Body.String(), ErrOAuth2ExpiredState.Error()) || calls != before { + t.Fatalf("expired: status=%d calls=%d body=%q", response.Code, calls, response.Body.String()) + } +} + +func TestOAuthLinkRequiresSessionAndSameOriginRequestAndRejectsDeclinedLink(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + missing := httptest.NewRequest(http.MethodPost, "https://app.example/auth/link/github", nil) + w := httptest.NewRecorder() + srv.LinkEvent = func(*jaws.Session, *http.Request, map[string]any) (int, error) { + return http.StatusForbidden, errors.New("secret persistence detail") + } + srv.HandleLink(w, missing) + if w.Code != http.StatusForbidden || calls != 0 { + t.Fatalf("anonymous link: status=%d calls=%d", w.Code, calls) + } + cookie := profileLogin(t, srv) + cross := httptest.NewRequest(http.MethodPost, "https://app.example/auth/link/github", nil) + cross.Header.Set("Origin", "https://evil.example") + cross.AddCookie(cookie) + w = httptest.NewRecorder() + srv.HandleLink(w, cross) + if w.Code != http.StatusForbidden || calls != 2 { + t.Fatalf("cross-origin link: status=%d calls=%d", w.Code, calls) + } + _, linkState := profileStart(t, srv, http.MethodPost, "/auth/link/github", cookie) + declined := profileCallback(srv, cookie, linkState, "second") + if declined.Code != http.StatusForbidden || declined.Header().Get("Location") != "" || + !strings.Contains(declined.Body.String(), ErrOAuth2LinkRejected.Error()) || + strings.Contains(declined.Body.String(), "secret persistence detail") || len(declined.Result().Cookies()) != 0 { + t.Fatalf("declined link: status=%d body=%q", declined.Code, declined.Body.String()) + } + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookie) + sess := jw.GetSession(r) + if sess == nil { + t.Fatal("declined link removed original session") + } + claims, ok := sess.Get(srv.SessionKey).(map[string]any) + if !ok || claims["sub"] != "9007199254740993" { + t.Fatal("declined link changed original proof") + } +} + +func TestOAuthProfileRejectsInvalidSubjectAndOversizedResponse(t *testing.T) { + jw := newTestJaws(t) + calls := 0 + srv := newProfileTestServer(t, jw, &calls) + srv.profileSubject = func(map[string]any) (string, error) { return "", nil } + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "first") + if response.Code != http.StatusUnauthorized || calls != 2 || len(response.Result().Cookies()) != 0 { + t.Fatalf("invalid subject: status=%d calls=%d", response.Code, calls) + } + request := httptest.NewRequest(http.MethodGet, "https://app.example/", nil) + request.AddCookie(cookie) + if sess := jw.GetSession(request); sess == nil || sess.Get(srv.SessionKey) != nil { + t.Fatal("invalid profile authenticated the preauth session") + } + srv.profileSubject = func(map[string]any) (string, error) { return "42", ErrOAuth2InvalidProfile } + _, state = profileStart(t, srv, http.MethodGet, "/auth/login", cookie) + response = profileCallback(srv, cookie, state, "second") + if response.Code != http.StatusUnauthorized || len(response.Result().Cookies()) != 0 { + t.Fatalf("subject with error: status=%d cookies=%v", response.Code, response.Result().Cookies()) + } + if sess := jw.GetSession(request); sess == nil || sess.Get(srv.SessionKey) != nil { + t.Fatal("subject error authenticated the preauth session") + } + oversized := &Server{profileSubject: srv.profileSubject} + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + return jsonResponse(`{"id":42}` + strings.Repeat(" ", maxUserInfoBytes+1)), nil + })}) + _, err := oversized.fetchUserInfo(ctx, "https://provider.example/user", oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "secret"})) + if !errors.Is(err, ErrUserInfoTooLarge) { + t.Fatalf("oversized profile: %v", err) + } +} + +func TestOAuthProfileRejectsTokenExpiredDuringUserInfo(t *testing.T) { + token := &oauth2.Token{AccessToken: "access", TokenType: "Bearer", Expiry: time.Now().Add(11 * time.Second)} + called := false + srv := &Server{ + userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { called = true; return "42", nil }, + } + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + token.Expiry = time.Now().Add(-time.Second) + return jsonResponse(`{"id":42}`), nil + })}) + claims, source, expiry, status, err := srv.verifyOAuthIdentity(ctx, token, "") + if !errors.Is(err, ErrOAuth2InvalidProfile) || claims != nil || source != nil || !expiry.IsZero() || status != http.StatusUnauthorized || called { + t.Fatalf("expired profile: claims=%v source=%t expiry=%v status=%d err=%v subject_called=%t", claims, source != nil, expiry, status, err, called) + } +} + +func TestOAuthProfileRejectsEmptyAccessTokenBeforeUserInfo(t *testing.T) { + calls := 0 + srv := &Server{ + userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { t.Fatal("empty token reached subject extractor"); return "", nil }, + } + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + calls++ + return jsonResponse(`{"id":42}`), nil + })}) + claims, source, expiry, status, err := srv.verifyOAuthIdentity(ctx, &oauth2.Token{TokenType: "Bearer"}, "") + if !errors.Is(err, ErrOAuth2InvalidProfile) || claims != nil || source != nil || !expiry.IsZero() || status != http.StatusUnauthorized || calls != 0 { + t.Fatalf("empty access token: claims=%v source=%t expiry=%v status=%d calls=%d err=%v", claims, source != nil, expiry, status, calls, err) + } +} + +func TestOAuthProfileUserInfoCallbackDeadline(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + var userinfoErr error + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + time.Sleep(10 * time.Second) + return jsonResponse(`{"access_token":"access","token_type":"Bearer"}`), nil + } + select { + case <-r.Context().Done(): + userinfoErr = r.Context().Err() + return nil, userinfoErr + case <-time.After(6 * time.Second): + userinfoErr = errors.New("UserInfo missed callback deadline") + return nil, userinfoErr + } + })} + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "code") + request := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + request.AddCookie(cookie) + sess := jw.GetSession(request) + var claims any + if sess != nil { + claims = sess.Get(srv.SessionKey) + } + if response.Code != http.StatusBadGateway || !errors.Is(userinfoErr, context.DeadlineExceeded) || + len(response.Result().Cookies()) != 0 || sess == nil || claims != nil { + t.Fatalf("profile deadline: status=%d err=%v cookies=%v claims=%v", response.Code, userinfoErr, response.Result().Cookies(), claims) + } + }) +} + +func TestOAuthProfileAcceptsShortUnexpiredToken(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + srv := &Server{Jaws: jw, issuer: "https://provider.example", userinfoUrl: "https://provider.example/user", + profileSubject: func(map[string]any) (string, error) { return "42", nil }} + calls := 0 + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + calls++ + if got := r.Header.Get("Authorization"); got != "Bearer access" { + t.Fatalf("userinfo authorization = %q", got) + } + return jsonResponse(`{"id":42}`), nil + })}) + token := &oauth2.Token{AccessToken: "access", TokenType: "Bearer", Expiry: time.Now().Add(5 * time.Second)} + if token.Valid() { + t.Fatal("test token should be inside oauth2's refresh skew") + } + claims, source, expiry, status, err := srv.verifyOAuthIdentity(ctx, token, "") + if err != nil || status != 0 || calls != 1 || source != nil || claims["sub"] != "42" || + !expiry.Equal(token.Expiry) { + t.Fatalf("short token: claims=%v source=%v expiry=%v status=%d calls=%d err=%v", claims, source, expiry, status, calls, err) + } + }) +} + +func TestOAuthProfileExplicitZeroExpiryRejectedButMissingExpiryCapped(t *testing.T) { + for _, tc := range []struct { + name, expiryField string + form bool + wantStatus int + }{ + {"omitted", "", false, http.StatusFound}, + {"null", `,"expires_in":null`, false, http.StatusFound}, + {"zero", `,"expires_in":0`, false, http.StatusUnauthorized}, + {"zero string", `,"expires_in":"0"`, false, http.StatusUnauthorized}, + {"form omitted", "", true, http.StatusFound}, + {"form zero", "&expires_in=0", true, http.StatusUnauthorized}, + } { + t.Run(tc.name, func(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/token" { + if tc.form { + return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/x-www-form-urlencoded"}}, + Body: io.NopCloser(strings.NewReader("access_token=access&token_type=Bearer" + tc.expiryField))}, nil + } + return jsonResponse(`{"access_token":"access","token_type":"Bearer"` + tc.expiryField + `}`), nil + } + return jsonResponse(`{"id":42}`), nil + })} + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "code") + if response.Code != tc.wantStatus { + t.Fatalf("status=%d body=%q", response.Code, response.Body.String()) + } + if tc.wantStatus == http.StatusUnauthorized { + if len(response.Result().Cookies()) != 0 || !strings.Contains(response.Body.String(), ErrOAuth2InvalidProfile.Error()) { + t.Fatalf("zero expiry authenticated: cookies=%v body=%q", response.Result().Cookies(), response.Body.String()) + } + return + } + cookies := response.Result().Cookies() + if len(cookies) != 1 { + t.Fatalf("rotated cookies: %v", cookies) + } + r := httptest.NewRequest(http.MethodGet, "https://app.example/account", nil) + r.AddCookie(cookies[0]) + sess := jw.GetSession(r) + if sess == nil || sess.Get(srv.SessionKey) == nil { + t.Fatal("missing expiry did not authenticate") + } + expiry, _ := sess.Get(srv.sessionKey(oauth2AuthExpiryKey)).(time.Time) + if remaining := time.Until(expiry); remaining < 23*time.Hour || remaining > 24*time.Hour { + t.Fatalf("omitted expiry cap = %s", remaining) + } + }) + } +} + +func TestOAuthProfileExpiresWithoutRefresh(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "first") + if response.Code != http.StatusFound || factory.len() != 1 { + t.Fatalf("profile sign-in: status=%d timers=%d", response.Code, factory.len()) + } + if delay := factory.timer(0).delay; delay <= 0 || delay > authSessionCheckInterval { + t.Fatalf("profile auth cleanup interval: %s", delay) + } + r := httptest.NewRequest(http.MethodGet, "https://app.example/", nil) + r.AddCookie(response.Result().Cookies()[0]) + sess := jw.GetSession(r) + factory.timer(0).fire() + if sess.Get(srv.SessionKey) == nil || factory.len() != 2 { + t.Fatal("profile auth cleared before its expiry") + } + if delay := factory.timer(1).delay; delay <= 0 || delay > authSessionCheckInterval { + t.Fatalf("profile auth rearm interval: %s", delay) + } + srv.mu.Lock() + srv.authTimers[sess.ID()].expiry = time.Now().Add(-time.Second) + srv.mu.Unlock() + factory.timer(1).fire() + if sess.Get(srv.SessionKey) != nil { + t.Fatal("profile auth remained after expiry timer") + } +} + +func TestOAuthProfileTimerReleasesClosedSession(t *testing.T) { + jw := newTestJaws(t) + srv := newProfileTestServer(t, jw, nil) + factory := &testAuthTimerFactory{} + srv.authTimerAfterFunc = factory.after + cookie, state := profileStart(t, srv, http.MethodGet, "/auth/login", nil) + response := profileCallback(srv, cookie, state, "first") + if response.Code != http.StatusFound || factory.len() != 1 { + t.Fatalf("profile sign-in: status=%d timers=%d", response.Code, factory.len()) + } + r := httptest.NewRequest(http.MethodGet, "https://app.example/", nil) + r.AddCookie(response.Result().Cookies()[0]) + sess := jw.GetSession(r) + sess.Close() + factory.timer(0).fire() + if sess.Get(srv.SessionKey) != nil || len(srv.authTimers) != 0 { + t.Fatal("closed profile session remained pinned by auth timer") + } +} diff --git a/server.go b/server.go index 874d8ea..d40fa6e 100644 --- a/server.go +++ b/server.go @@ -61,51 +61,85 @@ type HandleFunc func(uri string, handler http.Handler) // EventFunc is called for login and logout lifecycle events. // -// For [Server.LogoutEvent], hr may be nil. +// jawsauth calls [Server.LogoutEvent] after releasing SessionRotationLock. +// When LoginEvent is set, clearing auth before it begins suppresses both +// events; clearing auth during it defers LogoutEvent until it returns. +// LogoutEvent receives a nil request for timer cleanup, deferred delivery, or +// a call to [Server.Logout] with a nil request. +// When re-login replaces existing auth, its LogoutEvent runs on the old session +// before the new sign-in is stored; calling Logout there cancels the new sign-in. +// For cross-provider cleanup, check the other Server's CurrentClaims before +// calling Logout on the old session; that call can revoke a running sign-in. type EventFunc func(sess *jaws.Session, hr *http.Request) -// FailedFunc is called when a login attempt fails. +// FailedFunc is called on callback failures and when a protected request meets +// a running login callback before session rotation. Other flow-start failures +// write their own response. // // It returns true if it wrote the HTTP response itself, in which case jawsauth writes // nothing further. Errors can contain untrusted browser-supplied text. Quote errors // when logging them and escape them for the output format when rendering them. type FailedFunc func(hw http.ResponseWriter, hr *http.Request, httpCode int, err error, email string) (wroteresponse bool) -// Server provides OIDC-verified authentication for JaWS sessions. +// LinkFunc receives the current session and a newly authenticated identity. Recheck +// the existing authentication before a durable link write; with multiple +// providers, use the original provider's CurrentClaims. Logout and flow +// cancellation affect only their own Server. jawsauth holds +// SessionRotationLock while calling LinkFunc; do not acquire it again or call +// Server.Logout in the callback. Claims contain raw provider fields, so their +// types (including email_verified) may differ from stored session values. +// Return zero and nil on success, or an HTTP error status. +type LinkFunc func(sess *jaws.Session, hr *http.Request, claims map[string]any) (statusCode int, err error) + +// Server provides OIDC or OAuth2 profile authentication for JaWS sessions. // // Create one with New or NewDebug. Its methods are safe for concurrent use; the // exported configuration fields and callbacks should be set before serving requests. type Server struct { - Jaws *jaws.Jaws - SessionKey string // default is "oidc_claims", value will be of type map[string]any - SessionTokenKey string // default is "oauth2_tokensource", value will be of type oauth2.TokenSource - SessionEmailKey string // default is "email", value is a parsed string address with ASCII letters lowercased - SessionEmailVerifiedKey string // default is "email_verified", value will be of type bool - HandledPaths map[string]struct{} // URI paths we have registered handlers for - LoginEvent EventFunc // if not nil, called after a successful login - LogoutEvent EventFunc // if not nil, called after auth is cleared; hr may be nil - LoginFailed FailedFunc // if not nil, called on failed login - Options []oauth2.AuthCodeOption // options to use, see https://pkg.go.dev/golang.org/x/oauth2#AuthCodeOption + Jaws *jaws.Jaws + // New appends Config.Namespace to these four session keys when configured. + SessionKey string // default is "oidc_claims", value will be of type map[string]any + SessionTokenKey string // default is "oauth2_tokensource"; an oauth2.TokenSource in OIDC mode, unset in profile mode + SessionEmailKey string // default is "email", value is a parsed string address with ASCII letters lowercased + SessionEmailVerifiedKey string // default is "email_verified", value will be of type bool + HandledPaths map[string]struct{} // URI paths excluded from OAuth return targets + LoginEvent EventFunc // called after claims are stored; revocation may suppress or follow it + LogoutEvent EventFunc // auth-clear callback; see EventFunc for ordering and nil requests + LoginFailed FailedFunc // callback failures and protected requests during a login callback before rotation + LinkEvent LinkFunc // required by HandleLink; register its POST route separately + // SessionRotationLock is held during flow start, rotation, auth clearing, and LinkEvent. + // New supplies a shared package lock; set the same custom lock on servers using + // one JaWS instance when coordinating with other authentication methods. + SessionRotationLock sync.Locker + Options []oauth2.AuthCodeOption // authorization URL options, e.g. oauth2.AccessTypeOffline; never sent to token exchange // RequireVerifiedAdminEmail requires email_verified for a non-empty admin list. // - // It defaults to false. When true, providers that omit email_verified cannot - // grant admin access through a non-empty list. Set before serving requests. + // It defaults to true in profile mode and false in OIDC mode. When true, + // providers that omit email_verified cannot grant admin access through a + // non-empty list. Set before serving requests. // Ordinary login is unaffected. RequireVerifiedAdminEmail bool oauth2cfg *oauth2.Config idTokenVerifier *oidc.IDTokenVerifier userinfoUrl string + profileSubject func(map[string]any) (string, error) + issuer string httpClient *http.Client ishttps bool mu sync.Mutex // protects following fields and session email/verification pairs admins map[string]struct{} // if not empty, emails of admins handle403 http.Handler // handler for 403 Forbidden authTimers map[uint64]*authTimerState + loginEvents map[*jaws.Session]*pendingLoginEvent + oauthCallbacks map[*jaws.Session]pendingOAuthFlow authTimerAfterFunc authTimerAfterFunc + namespace string } +var defaultSessionRotationLock sync.Mutex + // NewDebug behaves like New but can override the scheme and host of cfg.RedirectURL. // // overrideUrl supplies the replacement scheme and host (an empty overrideUrl disables @@ -113,9 +147,9 @@ type Server struct { // development. // // A nil jw returns ErrServerNilJaws and a nil Server. Otherwise a non-nil Server is -// always returned. OIDC is configured, and the login, logout and callback handlers -// registered via handleFn, only when cfg, handleFn and cfg.RedirectURL are all provided; -// any error from OIDC discovery is returned alongside the not-yet-Valid Server. +// always returned. Authentication is configured, and the login, logout and callback +// handlers registered via handleFn, only when cfg, handleFn and cfg.RedirectURL are +// all provided; setup errors are returned with the Server. func NewDebug(jw *jaws.Jaws, cfg *Config, handleFn HandleFunc, overrideUrl string) (srv *Server, err error) { if jw == nil { err = ErrServerNilJaws @@ -128,14 +162,27 @@ func NewDebug(jw *jaws.Jaws, cfg *Config, handleFn HandleFunc, overrideUrl strin SessionEmailKey: "email", SessionEmailVerifiedKey: "email_verified", HandledPaths: make(map[string]struct{}), + SessionRotationLock: &defaultSessionRotationLock, admins: make(map[string]struct{}), handle403: default403handler{}, authTimers: make(map[uint64]*authTimerState), authTimerAfterFunc: realAuthTimerAfterFunc, } // #nosec G101 + if cfg != nil { + srv.namespace = cfg.Namespace + srv.issuer = cfg.Issuer + srv.SessionKey = srv.sessionKey(srv.SessionKey) + srv.SessionTokenKey = srv.sessionKey(srv.SessionTokenKey) + srv.SessionEmailKey = srv.sessionKey(srv.SessionEmailKey) + srv.SessionEmailVerifiedKey = srv.sessionKey(srv.SessionEmailVerifiedKey) + } if cfg != nil && handleFn != nil && cfg.RedirectURL != "" { - if srv.oauth2cfg, srv.userinfoUrl, srv.idTokenVerifier, err = cfg.buildContext(context.Background(), overrideUrl); err == nil { + setupCtx, cancel := context.WithTimeout(context.Background(), oauth2ProviderTimeout) + defer cancel() + if srv.oauth2cfg, srv.userinfoUrl, srv.idTokenVerifier, err = cfg.buildContext(setupCtx, overrideUrl); err == nil { srv.httpClient = cfg.HTTPClient + srv.profileSubject = cfg.ProfileSubject + srv.RequireVerifiedAdminEmail = srv.profileSubject != nil var u *url.URL if u, err = url.Parse(srv.oauth2cfg.RedirectURL); err == nil { srv.ishttps = (u.Scheme == "https") @@ -151,11 +198,12 @@ func NewDebug(jw *jaws.Jaws, cfg *Config, handleFn HandleFunc, overrideUrl strin return } -// New creates a Server providing OIDC-verified authentication for JaWS sessions. +// New creates a Server providing OIDC or OAuth2 profile authentication for JaWS sessions. // // It configures the Server from cfg and registers the login, logout and OAuth2 -// callback endpoints via handleFn. A nil jw returns ErrServerNilJaws. Use Valid to -// test whether OIDC authentication was successfully configured. +// callback endpoints via handleFn. A nil jw returns ErrServerNilJaws. Check both +// err and Valid before serving protected routes; an invalid Server leaves its +// wrapped handlers unprotected. func New(jw *jaws.Jaws, cfg *Config, handleFn HandleFunc) (srv *Server, err error) { return NewDebug(jw, cfg, handleFn, "") } @@ -164,6 +212,13 @@ func (srv *Server) makeAuth(rq *jaws.Request) jaws.Auth { return &JawsAuth{server: srv, sess: srv.Jaws.GetSession(rq.Initial())} } +func (srv *Server) sessionKey(key string) string { + if srv.namespace != "" { + return key + "." + srv.namespace + } + return key +} + func (srv *Server) handlePath(p string, handleFn HandleFunc, h http.Handler) { if _, ok := srv.HandledPaths[p]; !ok { srv.HandledPaths[p] = struct{}{} @@ -214,7 +269,8 @@ func (srv *Server) sessionIsAdminLocked(sess *jaws.Session, admins map[string]st // SetAdmins sets the emails of administrators. // -// Addresses are parsed with [mail.ParseAddress] and ASCII letters are lowercased. +// Valid addresses are parsed with [mail.ParseAddress]; other nonempty entries +// are retained. ASCII letters are lowercased. // Non-ASCII characters are matched exactly. If empty, everyone is considered an // administrator. Session checks also apply [Server.RequireVerifiedAdminEmail]. // @@ -284,18 +340,15 @@ func (srv *Server) get403Handler() (h http.Handler) { return } -// Valid returns true if OIDC authentication is configured. +// Valid returns true if OIDC or OAuth2 profile authentication is configured. func (srv *Server) Valid() bool { - return srv != nil && srv.oauth2cfg != nil && srv.idTokenVerifier != nil + return srv != nil && srv.oauth2cfg != nil && (srv.idTokenVerifier != nil || srv.profileSubject != nil) } // wrap returns a http.Handler that requires an authenticated user before invoking h. // -// When admin is true the user must additionally be an administrator (see SetAdmins), -// otherwise the 403 handler is served. Unauthenticated requests are redirected into the -// OIDC login flow (HandleLogin), which verifies the id_token and stores the claims in -// srv.SessionKey (with optional UserInfo fallback) before the user returns. If the -// Server is not Valid, returns h. +// When admin is true, non-admins receive 403. Unauthenticated GET begins login; +// other unauthenticated methods receive 403. An invalid Server returns h. func (srv *Server) wrap(h http.Handler, admin bool) (rh http.Handler) { rh = h if srv.Valid() { @@ -307,19 +360,19 @@ func (srv *Server) wrap(h http.Handler, admin bool) (rh http.Handler) { // WrapAdmin returns a http.Handler that requires an authenticated administrator // before invoking h. // -// Unauthenticated requests are redirected into the OIDC login flow (HandleLogin); -// authenticated users whose email is not an admin (see SetAdmins and IsAdmin) are -// served the 403 handler instead of h. [Server.RequireVerifiedAdminEmail] also -// applies. If the Server is not Valid, returns h. +// Unauthenticated GET begins login; other unauthenticated methods receive 403. +// Authenticated non-admins receive 403. [Server.RequireVerifiedAdminEmail] +// applies. A GET request during a running login callback may return 409. +// An invalid Server returns h. func (srv *Server) WrapAdmin(h http.Handler) (rh http.Handler) { return srv.wrap(h, true) } // Wrap returns a http.Handler that requires an authenticated user before invoking h. // -// Unauthenticated requests are redirected into the OIDC login flow (HandleLogin), which -// verifies the id_token and stores the claims in srv.SessionKey (with optional UserInfo -// fallback) before the user returns. If the Server is not Valid, returns h. +// Unauthenticated GET begins login; other unauthenticated methods receive 403. +// A GET request during a running login callback may return 409. +// An invalid Server returns h. func (srv *Server) Wrap(h http.Handler) (rh http.Handler) { return srv.wrap(h, false) } @@ -327,11 +380,10 @@ func (srv *Server) Wrap(h http.Handler) (rh http.Handler) { // HandlerAdmin returns a http.Handler that renders the named jaws.Template with dot // and requires an authenticated administrator. // -// Unauthenticated requests are redirected into the OIDC login flow (HandleLogin); -// authenticated non-admins (see SetAdmins and IsAdmin) are served the 403 handler. -// [Server.RequireVerifiedAdminEmail] also applies. -// If the Server is not Valid, the template handler is returned without the -// authentication requirement. +// Unauthenticated GET begins login; other unauthenticated methods receive 403. +// Authenticated non-admins receive 403. [Server.RequireVerifiedAdminEmail] +// applies. A GET request during a running login callback may return 409. +// An invalid, non-nil Server returns the unprotected template handler. func (srv *Server) HandlerAdmin(name string, dot any) http.Handler { return srv.wrap(ui.Handler(srv.Jaws, name, dot), true) } @@ -339,10 +391,9 @@ func (srv *Server) HandlerAdmin(name string, dot any) http.Handler { // Handler returns a http.Handler that renders the named jaws.Template with dot // and requires an authenticated user. // -// Unauthenticated requests are redirected into the OIDC login flow (HandleLogin), -// which verifies the id_token and stores the claims in srv.SessionKey (with optional -// UserInfo fallback) before the user returns. If the Server is not Valid, the template -// handler is returned without the authentication requirement. +// Unauthenticated GET begins login; other unauthenticated methods receive 403. +// A GET request during a running login callback may return 409. +// An invalid, non-nil Server returns the unprotected template handler. func (srv *Server) Handler(name string, dot any) http.Handler { return srv.wrap(ui.Handler(srv.Jaws, name, dot), false) } diff --git a/sessionlifecycle_test.go b/sessionlifecycle_test.go index fb3c39f..ade716a 100644 --- a/sessionlifecycle_test.go +++ b/sessionlifecycle_test.go @@ -24,23 +24,19 @@ func (l logoutOnWarnLogger) Error(string, ...any) {} func (l logoutOnWarnLogger) Warn(string, ...any) { l.onWarn() } func TestLogoutDuringClaimPreparation(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) factory := &testAuthTimerFactory{} srv := newTimerTestServer(t, jw, "https://issuer.example", factory) req := httptest.NewRequest(http.MethodGet, "/", nil) sess := jw.NewSession(httptest.NewRecorder(), req) expiry := time.Now().Add(time.Hour) - if err = srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, nil, expiry, nil); err != nil { + if err := srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, nil, expiry, nil); err != nil { t.Fatal(err) } entry := srv.authTimers[sess.ID()] jw.Logger = logoutOnWarnLogger{onWarn: func() { srv.Logout(sess, req) }} // Claim preparation may invoke application logging while logout completes. - err = srv.storeSessionAuthClaims(sess, map[string]any{"sub": "one"}, nil, expiry, entry) + err := srv.storeSessionAuthClaims(sess, map[string]any{"sub": "one"}, nil, expiry, entry) if !errors.Is(err, errAuthTimerStale) { t.Fatalf("got %v, want stale refresh", err) } @@ -51,16 +47,17 @@ func TestLogoutDuringClaimPreparation(t *testing.T) { } func TestAuthTimerDiscardsClosedSession(t *testing.T) { + synctest.Test(t, testAuthTimerDiscardsClosedSession) +} + +func testAuthTimerDiscardsClosedSession(t *testing.T) { for _, duringRefresh := range []bool{false, true} { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } + jw := newTestJaws(t) factory := &testAuthTimerFactory{} srv := newTimerTestServer(t, jw, "https://issuer.example", factory) req := httptest.NewRequest(http.MethodGet, "/", nil) sess := jw.NewSession(httptest.NewRecorder(), req) - expiry := time.Now().Add(time.Hour) + expiry := time.Now().Add(authRefreshSkew - time.Second) raw := makeIDToken(t, map[string]any{"iss": "https://issuer.example", "aud": "client", "sub": "one", "exp": expiry.Add(time.Hour).Unix(), "email": "user@example.com"}) calls := 0 source := tokenSourceFunc(func() (*oauth2.Token, error) { @@ -68,7 +65,7 @@ func TestAuthTimerDiscardsClosedSession(t *testing.T) { sess.Close() return makeOAuth2Token("access", raw, "refresh"), nil }) - if err = srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, source, expiry, nil); err != nil { + if err := srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, source, expiry, nil); err != nil { t.Fatal(err) } if !duringRefresh { @@ -83,10 +80,88 @@ func TestAuthTimerDiscardsClosedSession(t *testing.T) { t.Fatalf("calls=%d timers=%d scheduled=%d", calls, len(srv.authTimers), factory.len()) } assertWrapperAuthCleared(t, srv, sess) - jw.Close() } } +func TestOIDCTimerReleasesRotatedSession(t *testing.T) { + jw := newTestJaws(t) + factory := &testAuthTimerFactory{} + srv := newTimerTestServer(t, jw, "https://issuer.example", factory) + req := httptest.NewRequest(http.MethodGet, "/", nil) + first := httptest.NewRecorder() + old := jw.NewSession(first, req) + if old == nil { + t.Fatal("initial session missing") + } + if err := srv.storeSessionAuthClaims(old, map[string]any{"sub": "one"}, nil, time.Now().Add(4*time.Hour), nil); err != nil { + t.Fatal(err) + } + if delay := factory.timer(0).delay; delay <= 0 || delay > time.Minute { + t.Fatalf("old session cleanup delay = %s", delay) + } + factory.timer(0).fire() + if factory.len() != 2 || !srv.sessionAuthTimerCurrent(old, srv.authTimers[old.ID()]) { + t.Fatal("live OIDC session did not schedule its next liveness check") + } + replace := httptest.NewRequest(http.MethodGet, "/", nil) + replace.AddCookie(first.Result().Cookies()[0]) + if next := jw.NewSession(httptest.NewRecorder(), replace); next == nil || next == old || sessionAlive(old) { + t.Fatal("session was not rotated") + } + factory.timer(1).fire() + if len(srv.authTimers) != 0 { + t.Fatal("OIDC timer retained the rotated session") + } +} + +func TestOIDCRefreshTimeoutClearsExpiredAuth(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + jw := newTestJaws(t) + factory := &testAuthTimerFactory{} + srv := newTimerTestServer(t, jw, "https://issuer.example", factory) + started := make(chan struct{}, 1) + srv.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + select { + case started <- struct{}{}: + default: + } + <-req.Context().Done() + return nil, req.Context().Err() + })} + req := httptest.NewRequest(http.MethodGet, "/", nil) + sess := jw.NewSession(httptest.NewRecorder(), req) + if sess == nil { + t.Fatal("initial session missing") + } + source := srv.oauth2cfg.TokenSource(srv.oauth2Context(context.Background()), &oauth2.Token{RefreshToken: "refresh"}) + if err := srv.storeSessionAuthClaims(sess, map[string]any{"sub": "one"}, source, time.Now().Add(5*time.Second), nil); err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { + factory.timer(0).fire() + close(done) + }() + <-started + time.Sleep(oauth2ProviderTimeout - time.Second) + select { + case <-done: + t.Fatal("refresh completed before the provider timeout") + default: + } + time.Sleep(2 * time.Second) + synctest.Wait() + select { + case <-done: + default: + t.Fatal("stalled refresh did not time out") + } + if len(srv.authTimers) != 0 || sess.Get(srv.SessionKey) != nil { + t.Fatal("stalled refresh retained expired auth") + } + }) +} + func TestAuthorizationWithdrawalCancelsRequests(t *testing.T) { for _, action := range []string{"logout", "httpLogout", "expiry", "demotion", "restrictEveryone"} { t.Run(action, func(t *testing.T) { @@ -104,12 +179,16 @@ func TestAuthorizationWithdrawalCancelsRequests(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", nil) sess := jw.NewSession(httptest.NewRecorder(), req) expiry := time.Now().Add(time.Hour) - if action == "expiry" { - expiry = time.Now().Add(-time.Second) - } if err = srv.storeSessionAuthClaims(sess, map[string]any{"email": "user@example.com"}, nil, expiry, nil); err != nil { t.Fatal(err) } + if action == "expiry" { + expired := time.Now().Add(-time.Second) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), expired) + srv.mu.Lock() + srv.authTimers[sess.ID()].expiry = expired + srv.mu.Unlock() + } rq := newLiveAuthRequest(t, jw, req) ctx := rq.Context() if rq.Session() != sess || len(sess.Requests()) != 1 { @@ -172,13 +251,11 @@ func TestLoginRotationExistingAuth(t *testing.T) { t.Fatal("login reused old session") } } - sess.Set(oauth2StateKey, "state") - sess.Set(oauth2NonceKey, "nonce") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state", expiry: time.Now().Add(oauth2FlowLifetime), nonce: "nonce", verifier: oauth2.GenerateVerifier()}) rec := httptest.NewRecorder() srv.HandleAuthResponse(rec, req) if limit == 1 { - if rec.Code != http.StatusServiceUnavailable || rec.Header().Get("Location") != "" { + if rec.Code != http.StatusServiceUnavailable || rec.Header().Get("Location") != "" || rec.Header().Get("Retry-After") != "60" { t.Fatalf("status=%d headers=%v", rec.Code, rec.Header()) } if jw.GetSession(req) != sess || !srv.sessionAuthTimerCurrent(sess, entry) || sess.Get(srv.SessionEmailKey) != "existing@example.com" { @@ -200,11 +277,7 @@ func TestLoginRotationExistingAuth(t *testing.T) { } func TestSetAdminsLeavesAnonymousRequestsOpen(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := newWrapperTestServer(jw, "https://issuer.example") req := httptest.NewRequest(http.MethodGet, "/public", nil) jw.NewSession(httptest.NewRecorder(), req) @@ -300,21 +373,16 @@ func TestSetAdminsUsesSessionPolicy(t *testing.T) { } } -func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { - // UserInfo latency can exceed a new session's one-minute idle lifetime. - // synctest covers that interval without a wall-clock delay. +func TestLoginUserInfoDeadlineKeepsOIDCFallbackOptional(t *testing.T) { + // A slow UserInfo fallback must stop at the callback deadline before rotation. synctest.Test(t, func(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } + jw := newTestJaws(t) go jw.ServeWithTimeout(2 * time.Minute) - t.Cleanup(jw.Close) srv := newWrapperTestServer(jw, "https://issuer.example") expiry := time.Now().Add(time.Hour) hr := httptest.NewRequest(http.MethodGet, "/callback?state=state&code=code", nil) sess := jw.NewSession(httptest.NewRecorder(), hr) - if err = srv.storeSessionAuthClaims(sess, map[string]any{"sub": "user", "email": "user@example.com"}, nil, expiry, nil); err != nil { + if err := srv.storeSessionAuthClaims(sess, map[string]any{"sub": "user", "email": "user@example.com"}, nil, expiry, nil); err != nil { t.Fatal(err) } defer srv.Logout(sess, nil) @@ -324,37 +392,43 @@ func TestLoginPreparesUserInfoBeforeRotation(t *testing.T) { })).ServeHTTP(httptest.NewRecorder(), hr) raw := makeIDToken(t, map[string]any{"iss": "https://issuer.example", "aud": "client", "sub": "user", "exp": expiry.Unix(), "nonce": "nonce", "email": "user@example.com"}) userinfoCalls := 0 + var userinfoErr error srv.userinfoUrl = "https://provider.example/userinfo" srv.httpClient = &http.Client{Transport: roundTripFunc(func(hr *http.Request) (*http.Response, error) { body := `{"access_token":"dummy","token_type":"Bearer","expires_in":3600,"id_token":"` + raw + `"}` if hr.URL.Path == "/userinfo" { userinfoCalls++ - time.Sleep(61 * time.Second) - body = `{"sub":"user","email":"user@example.com","email_verified":true}` + select { + case <-hr.Context().Done(): + userinfoErr = hr.Context().Err() + return nil, userinfoErr + case <-time.After(oauth2ProviderTimeout + time.Second): + userinfoErr = errors.New("UserInfo missed callback deadline") + return nil, userinfoErr + } } return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/json"}}, Body: io.NopCloser(strings.NewReader(body))}, nil })} logins, logouts := 0, 0 srv.LoginEvent = func(*jaws.Session, *http.Request) { logins++ } srv.LogoutEvent = func(*jaws.Session, *http.Request) { logouts++ } - sess.Set(oauth2StateKey, "state") - sess.Set(oauth2NonceKey, "nonce") - sess.Set(oauth2PKCEVerifierKey, oauth2.GenerateVerifier()) + setTestOAuthFlow(sess, srv, pendingOAuthFlow{state: "state", expiry: time.Now().Add(oauth2FlowLifetime), nonce: "nonce", verifier: oauth2.GenerateVerifier()}) hw := httptest.NewRecorder() srv.HandleAuthResponse(hw, hr) current := jw.GetSession(hr) if current != nil { defer srv.Logout(current, nil) } - if hw.Code != http.StatusFound || logins != 1 || logouts != 1 || userinfoCalls != 1 { + if hw.Code != http.StatusFound || logins != 1 || logouts != 1 || userinfoCalls != 1 || + !errors.Is(userinfoErr, context.DeadlineExceeded) { t.Fatalf("status=%d logins=%d logouts=%d userinfo=%d", hw.Code, logins, logouts, userinfoCalls) } waitAuthRequestCancelled(t, ctx) if current == nil || current == sess || sess.Cookie().MaxAge >= 0 || !errors.Is(ctx.Err(), context.Canceled) { t.Fatal("login did not retire the old session and request") } - if current.Get(srv.SessionEmailVerifiedKey) != true { - t.Fatal("login did not store prepared UserInfo claims") + if current.Get(srv.SessionEmailVerifiedKey) == true { + t.Fatal("expired UserInfo fallback supplied verification") } }) } diff --git a/testwait_test.go b/testwait_test.go new file mode 100644 index 0000000..e8c345b --- /dev/null +++ b/testwait_test.go @@ -0,0 +1,30 @@ +package jawsauth + +import ( + "testing" + "time" + + "github.com/linkdata/jaws" +) + +func newTestJaws(t *testing.T) *jaws.Jaws { + t.Helper() + jw, err := jaws.New() + if err != nil { + t.Fatal(err) + } + t.Cleanup(jw.Close) + return jw +} + +func waitFor[T any](t *testing.T, ch <-chan T, msg string) T { + t.Helper() + select { + case value := <-ch: + return value + case <-time.After(30 * time.Second): + t.Fatal(msg) + } + var zero T + return zero +} diff --git a/wrapper.go b/wrapper.go index e29625c..005573b 100644 --- a/wrapper.go +++ b/wrapper.go @@ -15,9 +15,13 @@ func (w wrapper) ServeHTTP(hw http.ResponseWriter, hr *http.Request) { sess := w.server.Jaws.GetSession(hr) if current, present := w.server.sessionAuthStatus(sess, time.Now); !current { if present { - w.server.clearSessionAuth(sess, hr, nil) + w.server.clearSessionAuth(sess, hr, nil, true) } - w.server.HandleLogin(hw, hr) + if hr.Method != http.MethodGet { + w.server.writeResult(hw, http.StatusForbidden, nil) + return + } + w.server.startOAuth(hw, hr, false, true) return } diff --git a/wrapper_test.go b/wrapper_test.go index d9facd7..49ae748 100644 --- a/wrapper_test.go +++ b/wrapper_test.go @@ -24,11 +24,7 @@ func (h testStatusHandler) ServeHTTP(hw http.ResponseWriter, _ *http.Request) { // TestWrapperServeHTTPSet403HandlerConcurrent verifies that Set403Handler and // ServeHTTP can run concurrently under the race detector. func TestWrapperServeHTTPSet403HandlerConcurrent(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{ Jaws: jw, @@ -45,7 +41,7 @@ func TestWrapperServeHTTPSet403HandlerConcurrent(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), "ok": true, }) - sess.Set(oauth2IDTokenExpiryKey, time.Now().Add(time.Hour)) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(time.Hour)) sess.Set(srv.SessionEmailKey, "user@example.com") w := wrapper{ @@ -79,11 +75,7 @@ func TestWrapperServeHTTPSet403HandlerConcurrent(t *testing.T) { } func TestWrapperServeHTTPAllowsPresentAuthData(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := newWrapperTestServer(jw, "https://issuer.example") req := httptest.NewRequest(http.MethodGet, "http://example.com/protected", nil) @@ -92,7 +84,7 @@ func TestWrapperServeHTTPAllowsPresentAuthData(t *testing.T) { "exp": time.Now().Add(time.Hour).Unix(), "email": "user@example.com", }) - sess.Set(oauth2IDTokenExpiryKey, time.Now().Add(time.Hour)) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(time.Hour)) w := wrapper{ server: srv, @@ -107,6 +99,25 @@ func TestWrapperServeHTTPAllowsPresentAuthData(t *testing.T) { } } +func TestWrapperUnauthenticatedPostReturnsForbidden(t *testing.T) { + jw := newTestJaws(t) + srv := newWrapperTestServer(jw, "https://issuer.example") + req := httptest.NewRequest(http.MethodPost, "http://example.com/protected", nil) + rec := httptest.NewRecorder() + (wrapper{server: srv, handler: testStatusHandler{statusCode: http.StatusOK}}).ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden || rec.Header().Get("Allow") != "" { + t.Fatalf("protected POST: status=%d Allow=%q", rec.Code, rec.Header().Get("Allow")) + } + sess := jw.NewSession(httptest.NewRecorder(), req) + sess.Set(srv.SessionKey, map[string]any{"sub": "user"}) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), time.Now().Add(time.Hour)) + rec = httptest.NewRecorder() + (wrapper{server: srv, handler: testStatusHandler{statusCode: http.StatusOK}}).ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("authenticated POST: status=%d", rec.Code) + } +} + func TestWrapperServeHTTPClearsInvalidAuthData(t *testing.T) { testCases := []struct { name string @@ -130,17 +141,13 @@ func TestWrapperServeHTTPClearsInvalidAuthData(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := newWrapperTestServer(jw, "https://issuer.example") req := httptest.NewRequest(http.MethodGet, "http://example.com/protected", nil) sess := jw.NewSession(httptest.NewRecorder(), req) sess.Set(srv.SessionKey, tc.authValue) - sess.Set(oauth2IDTokenExpiryKey, tc.expiry) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), tc.expiry) w := wrapper{ server: srv, @@ -212,17 +219,13 @@ func TestSessionAuthStatus(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := &Server{SessionKey: "claims"} req := httptest.NewRequest(http.MethodGet, "http://example.com/protected", nil) sess := jw.NewSession(httptest.NewRecorder(), req) sess.Set(srv.SessionKey, tc.authValue) - sess.Set(oauth2IDTokenExpiryKey, tc.expiryValue) + sess.Set(srv.sessionKey(oauth2AuthExpiryKey), tc.expiryValue) gotCurrent, gotPresent := srv.sessionAuthStatus(sess, func() time.Time { return now }) if gotCurrent != tc.wantCurrent { @@ -236,11 +239,7 @@ func TestSessionAuthStatus(t *testing.T) { } func TestWrapperServeHTTPRedirectsWhenAuthDataMissing(t *testing.T) { - jw, err := jaws.New() - if err != nil { - t.Fatal(err) - } - defer jw.Close() + jw := newTestJaws(t) srv := newWrapperTestServer(jw, "https://issuer.example") tests := []struct { @@ -269,20 +268,37 @@ func TestWrapperServeHTTPRedirectsWhenAuthDataMissing(t *testing.T) { } func TestOAuth2ContextUsesServerHTTPClient(t *testing.T) { - client := &http.Client{} + client := &http.Client{Transport: &http.Transport{}, Timeout: time.Second} srv := &Server{httpClient: client} ctx := srv.oauth2Context(t.Context()) - if got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client); got != client { + if got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client); got == nil || got == client || + got.Transport != client.Transport || got.Timeout != client.Timeout || got.CheckRedirect == nil { t.Fatal(got) } - otherClient := &http.Client{} + otherClient := &http.Client{Transport: &http.Transport{}, Timeout: 2 * time.Second} base := context.WithValue(t.Context(), oauth2.HTTPClient, otherClient) ctx = srv.oauth2Context(base) - if got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client); got != otherClient { + if got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client); got == nil || got == otherClient || + got.Transport != otherClient.Transport || got.Timeout != otherClient.Timeout || got.CheckRedirect == nil { t.Fatal(got) } + + for _, timeout := range []time.Duration{0, 2 * oauth2ProviderTimeout} { + client := &http.Client{Timeout: timeout} + srv.httpClient = client + ctx = srv.oauth2Context(t.Context()) + got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client) + if got == nil || got.Timeout != oauth2ProviderTimeout || client.Timeout != timeout { + t.Fatalf("configured=%s copied=%v", timeout, got) + } + } + srv.httpClient = nil + ctx = srv.oauth2Context(t.Context()) + if got, _ := ctx.Value(oauth2.HTTPClient).(*http.Client); got == nil || got.Timeout != oauth2ProviderTimeout { + t.Fatalf("default OAuth client: %v", got) + } } func newWrapperTestServer(jw *jaws.Jaws, issuer string) *Server { @@ -316,7 +332,7 @@ func assertWrapperAuthCleared(t *testing.T, srv *Server, sess *jaws.Session) { if value := sess.Get(srv.SessionTokenKey); value != nil { t.Fatal(value) } - if value := sess.Get(oauth2IDTokenExpiryKey); value != nil { + if value := sess.Get(srv.sessionKey(oauth2AuthExpiryKey)); value != nil { t.Fatal(value) } if value := sess.Get(srv.SessionEmailKey); value != nil {