fix: 为失效 OIDC 事务提供安全重试
细分回调事务、状态和授权响应错误,提供脱敏诊断编号与显式重新登录入口,避免失效事务形成重试循环。
This commit is contained in:
@@ -2,6 +2,8 @@ package httpapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/rand"
|
||||
"encoding/base64"
|
||||
"errors"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -21,6 +23,11 @@ const (
|
||||
errorCodeOIDCSessionCSRF = "OIDC_SESSION_CSRF_REJECTED"
|
||||
errorCodeOIDCLoginInvalid = "OIDC_LOGIN_INVALID"
|
||||
errorCodeOIDCTokenExchangeFailed = "OIDC_TOKEN_EXCHANGE_FAILED"
|
||||
|
||||
oidcLoginFailureCookieMissing = "COOKIE_MISSING"
|
||||
oidcLoginFailureTransactionInvalid = "TRANSACTION_INVALID"
|
||||
oidcLoginFailureStateMismatch = "STATE_MISMATCH"
|
||||
oidcLoginFailureAuthorizationResponseMissing = "AUTHORIZATION_RESPONSE_MISSING"
|
||||
)
|
||||
|
||||
// startOIDCLogin godoc
|
||||
@@ -84,12 +91,20 @@ func (s *Server) completeOIDCLogin(w http.ResponseWriter, r *http.Request) {
|
||||
s.clearOIDCLoginCookie(w)
|
||||
cookie, err := r.Cookie(oidcsession.LoginTransactionCookieName)
|
||||
if err != nil {
|
||||
s.writeOIDCCallbackError(w, r, http.StatusBadRequest, "登录事务无效或已过期", errorCodeOIDCLoginInvalid)
|
||||
s.writeOIDCLoginTransactionError(w, r, "登录事务无效或已过期", oidcLoginFailureCookieMissing)
|
||||
return
|
||||
}
|
||||
transaction, err := s.oidcSessionCipher.DecodeLoginTransaction(cookie.Value, time.Now())
|
||||
if err != nil || r.URL.Query().Get("state") == "" || r.URL.Query().Get("state") != transaction.State || r.URL.Query().Get("code") == "" {
|
||||
s.writeOIDCCallbackError(w, r, http.StatusBadRequest, "登录事务校验失败,请重新登录", errorCodeOIDCLoginInvalid)
|
||||
if err != nil {
|
||||
s.writeOIDCLoginTransactionError(w, r, "登录事务校验失败,请重新登录", oidcLoginFailureTransactionInvalid)
|
||||
return
|
||||
}
|
||||
if r.URL.Query().Get("state") == "" || r.URL.Query().Get("state") != transaction.State {
|
||||
s.writeOIDCLoginTransactionError(w, r, "登录事务校验失败,请重新登录", oidcLoginFailureStateMismatch)
|
||||
return
|
||||
}
|
||||
if r.URL.Query().Get("code") == "" {
|
||||
s.writeOIDCLoginTransactionError(w, r, "认证中心回调缺少必要参数,请重新登录", oidcLoginFailureAuthorizationResponseMissing)
|
||||
return
|
||||
}
|
||||
tokens, err := s.oidcClient.ExchangeCode(r.Context(), r.URL.Query().Get("code"), transaction.PKCEVerifier)
|
||||
@@ -224,17 +239,53 @@ func (s *Server) oidcReturnLocation(returnTo string) string {
|
||||
}
|
||||
|
||||
func (s *Server) writeOIDCCallbackError(w http.ResponseWriter, r *http.Request, status int, message, code string) {
|
||||
s.writeOIDCCallbackErrorWithDiagnostics(w, r, status, message, code, "", "")
|
||||
}
|
||||
|
||||
func (s *Server) writeOIDCLoginTransactionError(w http.ResponseWriter, r *http.Request, message, reason string) {
|
||||
diagnosticID := newOIDCDiagnosticID()
|
||||
if s.logger != nil {
|
||||
s.logger.WarnContext(r.Context(), "OIDC login transaction rejected",
|
||||
"event", "oidc_login_transaction_rejected",
|
||||
"reason", reason,
|
||||
"diagnosticId", diagnosticID,
|
||||
)
|
||||
}
|
||||
s.writeOIDCCallbackErrorWithDiagnostics(w, r, http.StatusBadRequest, message, errorCodeOIDCLoginInvalid, reason, diagnosticID)
|
||||
}
|
||||
|
||||
func (s *Server) writeOIDCCallbackErrorWithDiagnostics(w http.ResponseWriter, r *http.Request, status int, message, code, reason, diagnosticID string) {
|
||||
base := strings.TrimRight(strings.TrimSpace(s.cfg.WebBaseURL), "/")
|
||||
if parsed, err := url.Parse(base); base != "" && err == nil && parsed.Host != "" && (parsed.Scheme == "https" || parsed.Scheme == "http" && (parsed.Hostname() == "localhost" || parsed.Hostname() == "127.0.0.1")) {
|
||||
query := parsed.Query()
|
||||
query.Set("oidcError", code)
|
||||
if reason != "" {
|
||||
query.Set("oidcErrorReason", reason)
|
||||
}
|
||||
if diagnosticID != "" {
|
||||
query.Set("oidcDiagnosticId", diagnosticID)
|
||||
}
|
||||
parsed.RawQuery = query.Encode()
|
||||
http.Redirect(w, r, parsed.String(), http.StatusSeeOther)
|
||||
return
|
||||
}
|
||||
if reason != "" || diagnosticID != "" {
|
||||
writeErrorWithDetails(w, status, message, map[string]any{
|
||||
"reason": reason, "diagnosticId": diagnosticID,
|
||||
}, code)
|
||||
return
|
||||
}
|
||||
writeError(w, status, message, code)
|
||||
}
|
||||
|
||||
func newOIDCDiagnosticID() string {
|
||||
var value [12]byte
|
||||
if _, err := rand.Read(value[:]); err == nil {
|
||||
return base64.RawURLEncoding.EncodeToString(value[:])
|
||||
}
|
||||
return base64.RawURLEncoding.EncodeToString([]byte(time.Now().UTC().Format(time.RFC3339Nano)))
|
||||
}
|
||||
|
||||
func (s *Server) writeOIDCCallbackProjectionError(w http.ResponseWriter, r *http.Request, err error) {
|
||||
switch {
|
||||
case errors.Is(err, store.ErrOIDCUserNotProvisioned):
|
||||
|
||||
@@ -7,6 +7,8 @@ import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -59,6 +61,77 @@ func TestStartOIDCLoginRejectsOpenRedirect(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCompleteOIDCLoginReportsSafeTransactionFailureReason(t *testing.T) {
|
||||
cipher, err := oidcsession.NewCipher(bytes.Repeat([]byte{3}, 32))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
transaction, _, err := oidcsession.NewLoginTransaction("/", time.Now())
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
encodedTransaction, err := cipher.EncodeLoginTransaction(transaction)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
for _, test := range []struct {
|
||||
name, cookie, state, code, wantReason string
|
||||
}{
|
||||
{name: "cookie missing", state: "sensitive-state-marker", code: "sensitive-code-marker", wantReason: "COOKIE_MISSING"},
|
||||
{name: "transaction invalid", cookie: "invalid-encrypted-cookie", state: "sensitive-state-marker", code: "sensitive-code-marker", wantReason: "TRANSACTION_INVALID"},
|
||||
{name: "state mismatch", cookie: encodedTransaction, state: "sensitive-state-marker", code: "sensitive-code-marker", wantReason: "STATE_MISMATCH"},
|
||||
{name: "authorization response missing", cookie: encodedTransaction, state: transaction.State, wantReason: "AUTHORIZATION_RESPONSE_MISSING"},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
var logs bytes.Buffer
|
||||
server := &Server{
|
||||
cfg: config.Config{
|
||||
OIDCEnabled: true, OIDCBrowserSessionEnabled: true,
|
||||
WebBaseURL: "http://localhost:5178",
|
||||
},
|
||||
auth: &auth.Authenticator{OIDCVerifier: &auth.OIDCVerifier{}}, oidcClient: &fakeOIDCClient{},
|
||||
oidcSessions: &fakeOIDCSessions{}, oidcSessionCipher: cipher,
|
||||
logger: slog.New(slog.NewJSONHandler(&logs, nil)),
|
||||
}
|
||||
query := url.Values{}
|
||||
if test.state != "" {
|
||||
query.Set("state", test.state)
|
||||
}
|
||||
if test.code != "" {
|
||||
query.Set("code", test.code)
|
||||
}
|
||||
request := httptest.NewRequest(http.MethodGet, "/api/v1/auth/oidc/callback?"+query.Encode(), nil)
|
||||
if test.cookie != "" {
|
||||
request.AddCookie(&http.Cookie{Name: oidcsession.LoginTransactionCookieName, Value: test.cookie})
|
||||
}
|
||||
recorder := httptest.NewRecorder()
|
||||
|
||||
server.completeOIDCLogin(recorder, request)
|
||||
|
||||
if recorder.Code != http.StatusSeeOther {
|
||||
t.Fatalf("callback status=%d, want 303", recorder.Code)
|
||||
}
|
||||
location, err := url.Parse(recorder.Header().Get("Location"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if location.Query().Get("oidcError") != errorCodeOIDCLoginInvalid || location.Query().Get("oidcErrorReason") != test.wantReason {
|
||||
t.Fatalf("callback error=%q reason=%q", location.Query().Get("oidcError"), location.Query().Get("oidcErrorReason"))
|
||||
}
|
||||
diagnosticID := location.Query().Get("oidcDiagnosticId")
|
||||
if diagnosticID == "" || !strings.Contains(logs.String(), `"diagnosticId":"`+diagnosticID+`"`) || !strings.Contains(logs.String(), `"reason":"`+test.wantReason+`"`) {
|
||||
t.Fatalf("missing correlated safe diagnostic: location=%q logs=%s", location.RawQuery, logs.String())
|
||||
}
|
||||
for _, secretMarker := range []string{"sensitive-state-marker", "sensitive-code-marker", "invalid-encrypted-cookie"} {
|
||||
if strings.Contains(location.RawQuery, secretMarker) || strings.Contains(logs.String(), secretMarker) {
|
||||
t.Fatalf("OIDC diagnostic leaked callback security material")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeleteOIDCBrowserSessionIsIdempotentAndExpiresCookie(t *testing.T) {
|
||||
sessions := &fakeOIDCSessions{}
|
||||
server := &Server{cfg: config.Config{OIDCSessionCookieSecure: true}, oidcSessions: sessions}
|
||||
|
||||
Reference in New Issue
Block a user