fix: harden oidc callback security
This commit is contained in:
@@ -127,30 +127,34 @@ func (h *AuthHandler) OIDCOAuthStart(c *gin.Context) {
|
||||
redirectTo = oidcOAuthDefaultRedirectTo
|
||||
}
|
||||
|
||||
browserSessionKey, err := generateOAuthPendingBrowserSession()
|
||||
if err != nil {
|
||||
response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_BROWSER_SESSION_GEN_FAILED", "failed to generate oauth browser session").WithCause(err))
|
||||
return
|
||||
}
|
||||
|
||||
secureCookie := isRequestHTTPS(c)
|
||||
oidcSetCookie(c, oidcOAuthStateCookieName, encodeCookieValue(state), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
oidcSetCookie(c, oidcOAuthRedirectCookie, encodeCookieValue(redirectTo), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
setOAuthPendingBrowserCookie(c, browserSessionKey, secureCookie)
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
|
||||
codeChallenge := ""
|
||||
if cfg.UsePKCE {
|
||||
verifier, genErr := oauth.GenerateCodeVerifier()
|
||||
if genErr != nil {
|
||||
response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(genErr))
|
||||
return
|
||||
}
|
||||
codeChallenge = oauth.GenerateCodeChallenge(verifier)
|
||||
oidcSetCookie(c, oidcOAuthVerifierCookie, encodeCookieValue(verifier), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
verifier, genErr := oauth.GenerateCodeVerifier()
|
||||
if genErr != nil {
|
||||
response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_PKCE_GEN_FAILED", "failed to generate pkce verifier").WithCause(genErr))
|
||||
return
|
||||
}
|
||||
codeChallenge = oauth.GenerateCodeChallenge(verifier)
|
||||
oidcSetCookie(c, oidcOAuthVerifierCookie, encodeCookieValue(verifier), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
|
||||
nonce := ""
|
||||
if cfg.ValidateIDToken {
|
||||
nonce, err = oauth.GenerateState()
|
||||
if err != nil {
|
||||
response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_NONCE_GEN_FAILED", "failed to generate oauth nonce").WithCause(err))
|
||||
return
|
||||
}
|
||||
oidcSetCookie(c, oidcOAuthNonceCookie, encodeCookieValue(nonce), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
nonce, err = oauth.GenerateState()
|
||||
if err != nil {
|
||||
response.ErrorFrom(c, infraerrors.InternalServer("OAUTH_NONCE_GEN_FAILED", "failed to generate oauth nonce").WithCause(err))
|
||||
return
|
||||
}
|
||||
oidcSetCookie(c, oidcOAuthNonceCookie, encodeCookieValue(nonce), oidcOAuthCookieMaxAgeSec, secureCookie)
|
||||
|
||||
redirectURI := strings.TrimSpace(cfg.RedirectURL)
|
||||
if redirectURI == "" {
|
||||
@@ -212,23 +216,24 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) {
|
||||
if redirectTo == "" {
|
||||
redirectTo = oidcOAuthDefaultRedirectTo
|
||||
}
|
||||
browserSessionKey, _ := readOAuthPendingBrowserCookie(c)
|
||||
if strings.TrimSpace(browserSessionKey) == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_browser_session", "missing oauth browser session", "")
|
||||
return
|
||||
}
|
||||
|
||||
codeVerifier := ""
|
||||
if cfg.UsePKCE {
|
||||
codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie)
|
||||
if codeVerifier == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "")
|
||||
return
|
||||
}
|
||||
codeVerifier, _ = readCookieDecoded(c, oidcOAuthVerifierCookie)
|
||||
if codeVerifier == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_verifier", "missing pkce verifier", "")
|
||||
return
|
||||
}
|
||||
|
||||
expectedNonce := ""
|
||||
if cfg.ValidateIDToken {
|
||||
expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie)
|
||||
if expectedNonce == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "")
|
||||
return
|
||||
}
|
||||
expectedNonce, _ = readCookieDecoded(c, oidcOAuthNonceCookie)
|
||||
if expectedNonce == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_nonce", "missing oauth nonce", "")
|
||||
return
|
||||
}
|
||||
|
||||
redirectURI := strings.TrimSpace(cfg.RedirectURL)
|
||||
@@ -258,7 +263,7 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
if cfg.ValidateIDToken && strings.TrimSpace(tokenResp.IDToken) == "" {
|
||||
if strings.TrimSpace(tokenResp.IDToken) == "" {
|
||||
redirectOAuthError(c, frontendCallback, "missing_id_token", "missing id_token", "")
|
||||
return
|
||||
}
|
||||
@@ -304,9 +309,13 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
}
|
||||
if userInfoClaims.Subject != "" && idClaims.Subject != "" && strings.TrimSpace(userInfoClaims.Subject) != strings.TrimSpace(idClaims.Subject) {
|
||||
redirectOAuthError(c, frontendCallback, "subject_mismatch", "userinfo subject does not match id_token", "")
|
||||
return
|
||||
}
|
||||
|
||||
identityKey := oidcIdentityKey(issuer, subject)
|
||||
email := oidcSelectLoginEmail(userInfoClaims.Email, idClaims.Email, identityKey)
|
||||
email := oidcSyntheticEmailFromIdentityKey(identityKey)
|
||||
username := firstNonEmpty(
|
||||
userInfoClaims.Username,
|
||||
idClaims.PreferredUsername,
|
||||
@@ -318,34 +327,73 @@ func (h *AuthHandler) OIDCOAuthCallback(c *gin.Context) {
|
||||
tokenPair, _, err := h.authService.LoginOrRegisterOAuthWithTokenPair(c.Request.Context(), email, username, "")
|
||||
if err != nil {
|
||||
if errors.Is(err, service.ErrOAuthInvitationRequired) {
|
||||
pendingToken, tokenErr := h.authService.CreatePendingOAuthToken(email, username)
|
||||
if tokenErr != nil {
|
||||
redirectOAuthError(c, frontendCallback, "login_failed", "service_error", "")
|
||||
if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{
|
||||
Intent: "login",
|
||||
Identity: service.PendingAuthIdentityKey{
|
||||
ProviderType: "oidc",
|
||||
ProviderKey: issuer,
|
||||
ProviderSubject: subject,
|
||||
},
|
||||
ResolvedEmail: email,
|
||||
RedirectTo: redirectTo,
|
||||
BrowserSessionKey: browserSessionKey,
|
||||
UpstreamIdentityClaims: map[string]any{
|
||||
"email": email,
|
||||
"username": username,
|
||||
"subject": subject,
|
||||
"issuer": issuer,
|
||||
"email_verified": emailVerified != nil && *emailVerified,
|
||||
"provider_fallback": strings.TrimSpace(cfg.ProviderName),
|
||||
},
|
||||
CompletionResponse: map[string]any{
|
||||
"error": "invitation_required",
|
||||
"redirect": redirectTo,
|
||||
},
|
||||
}); err != nil {
|
||||
redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "")
|
||||
return
|
||||
}
|
||||
fragment := url.Values{}
|
||||
fragment.Set("error", "invitation_required")
|
||||
fragment.Set("pending_oauth_token", pendingToken)
|
||||
fragment.Set("redirect", redirectTo)
|
||||
redirectWithFragment(c, frontendCallback, fragment)
|
||||
redirectToFrontendCallback(c, frontendCallback)
|
||||
return
|
||||
}
|
||||
redirectOAuthError(c, frontendCallback, "login_failed", infraerrors.Reason(err), infraerrors.Message(err))
|
||||
return
|
||||
}
|
||||
|
||||
fragment := url.Values{}
|
||||
fragment.Set("access_token", tokenPair.AccessToken)
|
||||
fragment.Set("refresh_token", tokenPair.RefreshToken)
|
||||
fragment.Set("expires_in", fmt.Sprintf("%d", tokenPair.ExpiresIn))
|
||||
fragment.Set("token_type", "Bearer")
|
||||
fragment.Set("redirect", redirectTo)
|
||||
redirectWithFragment(c, frontendCallback, fragment)
|
||||
if err := h.createOAuthPendingSession(c, oauthPendingSessionPayload{
|
||||
Intent: "login",
|
||||
Identity: service.PendingAuthIdentityKey{
|
||||
ProviderType: "oidc",
|
||||
ProviderKey: issuer,
|
||||
ProviderSubject: subject,
|
||||
},
|
||||
ResolvedEmail: email,
|
||||
RedirectTo: redirectTo,
|
||||
BrowserSessionKey: browserSessionKey,
|
||||
UpstreamIdentityClaims: map[string]any{
|
||||
"email": email,
|
||||
"username": username,
|
||||
"subject": subject,
|
||||
"issuer": issuer,
|
||||
"email_verified": emailVerified != nil && *emailVerified,
|
||||
"provider_fallback": strings.TrimSpace(cfg.ProviderName),
|
||||
},
|
||||
CompletionResponse: map[string]any{
|
||||
"access_token": tokenPair.AccessToken,
|
||||
"refresh_token": tokenPair.RefreshToken,
|
||||
"expires_in": tokenPair.ExpiresIn,
|
||||
"token_type": "Bearer",
|
||||
"redirect": redirectTo,
|
||||
},
|
||||
}); err != nil {
|
||||
redirectOAuthError(c, frontendCallback, "session_error", "failed to continue oauth login", "")
|
||||
return
|
||||
}
|
||||
redirectToFrontendCallback(c, frontendCallback)
|
||||
}
|
||||
|
||||
type completeOIDCOAuthRequest struct {
|
||||
PendingOAuthToken string `json:"pending_oauth_token" binding:"required"`
|
||||
InvitationCode string `json:"invitation_code" binding:"required"`
|
||||
InvitationCode string `json:"invitation_code" binding:"required"`
|
||||
}
|
||||
|
||||
// CompleteOIDCOAuthRegistration completes a pending OAuth registration by validating
|
||||
@@ -358,9 +406,38 @@ func (h *AuthHandler) CompleteOIDCOAuthRegistration(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
email, username, err := h.authService.VerifyPendingOAuthToken(req.PendingOAuthToken)
|
||||
secureCookie := isRequestHTTPS(c)
|
||||
sessionToken, err := readOAuthPendingSessionCookie(c)
|
||||
if err != nil {
|
||||
c.JSON(http.StatusUnauthorized, gin.H{"error": "INVALID_TOKEN", "message": "invalid or expired registration token"})
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
clearOAuthPendingBrowserCookie(c, secureCookie)
|
||||
response.ErrorFrom(c, service.ErrPendingAuthSessionNotFound)
|
||||
return
|
||||
}
|
||||
browserSessionKey, err := readOAuthPendingBrowserCookie(c)
|
||||
if err != nil {
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
clearOAuthPendingBrowserCookie(c, secureCookie)
|
||||
response.ErrorFrom(c, service.ErrPendingAuthBrowserMismatch)
|
||||
return
|
||||
}
|
||||
pendingSvc, err := h.pendingIdentityService()
|
||||
if err != nil {
|
||||
response.ErrorFrom(c, err)
|
||||
return
|
||||
}
|
||||
session, err := pendingSvc.GetBrowserSession(c.Request.Context(), sessionToken, browserSessionKey)
|
||||
if err != nil {
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
clearOAuthPendingBrowserCookie(c, secureCookie)
|
||||
response.ErrorFrom(c, err)
|
||||
return
|
||||
}
|
||||
|
||||
email := strings.TrimSpace(session.ResolvedEmail)
|
||||
username := pendingSessionStringValue(session.UpstreamIdentityClaims, "username")
|
||||
if email == "" || username == "" {
|
||||
response.ErrorFrom(c, infraerrors.BadRequest("PENDING_AUTH_SESSION_INVALID", "pending auth registration context is invalid"))
|
||||
return
|
||||
}
|
||||
|
||||
@@ -369,6 +446,14 @@ func (h *AuthHandler) CompleteOIDCOAuthRegistration(c *gin.Context) {
|
||||
response.ErrorFrom(c, err)
|
||||
return
|
||||
}
|
||||
if _, err := pendingSvc.ConsumeBrowserSession(c.Request.Context(), sessionToken, browserSessionKey); err != nil {
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
clearOAuthPendingBrowserCookie(c, secureCookie)
|
||||
response.ErrorFrom(c, err)
|
||||
return
|
||||
}
|
||||
clearOAuthPendingSessionCookie(c, secureCookie)
|
||||
clearOAuthPendingBrowserCookie(c, secureCookie)
|
||||
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"access_token": tokenPair.AccessToken,
|
||||
@@ -405,9 +490,7 @@ func oidcExchangeCode(
|
||||
form.Set("client_id", cfg.ClientID)
|
||||
form.Set("code", code)
|
||||
form.Set("redirect_uri", redirectURI)
|
||||
if cfg.UsePKCE {
|
||||
form.Set("code_verifier", codeVerifier)
|
||||
}
|
||||
form.Set("code_verifier", codeVerifier)
|
||||
|
||||
r := client.R().
|
||||
SetContext(ctx).
|
||||
@@ -592,13 +675,9 @@ func buildOIDCAuthorizeURL(cfg config.OIDCConnectConfig, state, nonce, codeChall
|
||||
q.Set("scope", cfg.Scopes)
|
||||
}
|
||||
q.Set("state", state)
|
||||
if strings.TrimSpace(nonce) != "" {
|
||||
q.Set("nonce", nonce)
|
||||
}
|
||||
if cfg.UsePKCE {
|
||||
q.Set("code_challenge", codeChallenge)
|
||||
q.Set("code_challenge_method", "S256")
|
||||
}
|
||||
q.Set("nonce", nonce)
|
||||
q.Set("code_challenge", codeChallenge)
|
||||
q.Set("code_challenge_method", "S256")
|
||||
|
||||
u.RawQuery = q.Encode()
|
||||
return u.String(), nil
|
||||
@@ -831,14 +910,6 @@ func oidcSyntheticEmailFromIdentityKey(identityKey string) string {
|
||||
return "oidc-" + hex.EncodeToString(sum[:16]) + service.OIDCConnectSyntheticEmailDomain
|
||||
}
|
||||
|
||||
func oidcSelectLoginEmail(userInfoEmail, idTokenEmail, identityKey string) string {
|
||||
email := strings.TrimSpace(firstNonEmpty(userInfoEmail, idTokenEmail))
|
||||
if email != "" {
|
||||
return email
|
||||
}
|
||||
return oidcSyntheticEmailFromIdentityKey(identityKey)
|
||||
}
|
||||
|
||||
func oidcFallbackUsername(subject string) string {
|
||||
subject = strings.TrimSpace(subject)
|
||||
if subject == "" {
|
||||
|
||||
Reference in New Issue
Block a user