From d58dd7a7c6dbe8a2c7d3ba582c35b0fcfb5e0745 Mon Sep 17 00:00:00 2001 From: kolaente Date: Thu, 9 Apr 2026 13:33:28 +0200 Subject: [PATCH] fix(auth): enforce TOTP on OIDC callback for users with 2FA enabled The OIDC callback handler previously issued a JWT without ever checking TOTP state. For installations with EmailFallback (or UsernameFallback) enabled, this allowed an attacker who could authenticate at the IdP with a matching email to log in as a local user with TOTP enrolled, bypassing the second factor entirely. HandleCallback now runs enforceTOTPIfRequired after resolving the user and before any team sync writes, returning 412/1017 when the passcode is missing or invalid. Clients resubmit the OIDC flow with the totp_passcode field populated. Fixes GHSA-8jvc-mcx6-r4cg --- pkg/modules/auth/openid/openid.go | 37 ++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 6760b0d1c..381570f42 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -163,11 +163,12 @@ func enforceTOTPIfRequired(s *xorm.Session, u *user.User, totpPasscode string) e // @Param callback body openid.Callback true "The openid callback" // @Param provider path int true "The OpenID Connect provider key as returned by the /info endpoint" // @Success 200 {object} auth.Token +// @Failure 412 {object} models.Message "Invalid totp passcode." // @Failure 500 {object} models.Message "Internal error" // @Router /auth/openid/{provider}/callback [post] func HandleCallback(c *echo.Context) error { - provider, oauthToken, idToken, err := getProviderAndOidcTokens(c) + provider, cb, oauthToken, idToken, err := getProviderAndOidcTokens(c) if err != nil { var detailedErr *models.ErrOpenIDBadRequestWithDetails if errors.As(err, &detailedErr) { @@ -204,6 +205,20 @@ func HandleCallback(c *echo.Context) error { return &user.ErrAccountLocked{UserID: u.ID} } + // Must run before team sync so a failed 2FA attempt cannot mutate team + // membership. Commit before HandleFailedTOTPAuth so the getOrCreateUser + // writes persist and the SQLite write lock is released — its dedicated + // session needs to acquire its own. See GHSA-fgfv-pv97-6cmj. + if err := enforceTOTPIfRequired(s, u, cb.TOTPPasscode); err != nil { + if commitErr := s.Commit(); commitErr != nil { + log.Errorf("Error committing session after failed OIDC TOTP attempt for user %d: %v", u.ID, commitErr) + } + if user.IsErrInvalidTOTPPasscode(err) { + user.HandleFailedTOTPAuth(u) + } + return err + } + teamData := getTeamDataFromToken(cl.VikunjaGroups, provider) err = models.SyncExternalTeamsForUser(s, u, teamData, idToken.Issuer, provider.Name) @@ -492,21 +507,21 @@ func getClaims(provider *Provider, oauth2Token *oauth2.Token, idToken *oidc.IDTo return cl, nil } -func getProviderAndOidcTokens(c *echo.Context) (*Provider, *oauth2.Token, *oidc.IDToken, error) { +func getProviderAndOidcTokens(c *echo.Context) (*Provider, *Callback, *oauth2.Token, *oidc.IDToken, error) { cb := &Callback{} if err := c.Bind(cb); err != nil { - return nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Bad data"} + return nil, nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Bad data"} } // Check if the provider exists providerKey := c.Param("provider") provider, err := GetProvider(providerKey) if err != nil { - return nil, nil, nil, err + return nil, cb, nil, nil, err } if provider == nil { - return nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Provider does not exist"} + return nil, cb, nil, nil, &models.ErrOpenIDBadRequest{Message: "Provider does not exist"} } log.Debugf("Trying to authenticate user using provider: %s", provider.Key) @@ -522,25 +537,25 @@ func getProviderAndOidcTokens(c *echo.Context) (*Provider, *oauth2.Token, *oidc. if err := json.Unmarshal(rerr.Body, &details); err != nil { log.Errorf("Error unmarshalling token for provider %s: %v", provider.Name, err) log.Debugf("Raw token value is %s", rerr.Body) - return nil, nil, nil, err + return nil, cb, nil, nil, err } log.Errorf("Error retrieving token: %s", err) log.Debugf("Raw token value is %s", rerr.Body) - return nil, nil, nil, &models.ErrOpenIDBadRequestWithDetails{ + return nil, cb, nil, nil, &models.ErrOpenIDBadRequestWithDetails{ Message: "Could not authenticate against third party.", Details: details, } } - return nil, nil, nil, err + return nil, cb, nil, nil, err } // Extract the ID Token from OAuth2 token. rawIDToken, ok := oauth2Token.Extra("id_token").(string) if !ok { log.Debugf("Could not get id_token, raw token is %v", oauth2Token) - return nil, nil, nil, &models.ErrOpenIDBadRequest{Message: "Missing token"} + return nil, cb, nil, nil, &models.ErrOpenIDBadRequest{Message: "Missing token"} } verifier := provider.openIDProvider.Verifier(&oidc.Config{ClientID: provider.ClientID}) @@ -549,8 +564,8 @@ func getProviderAndOidcTokens(c *echo.Context) (*Provider, *oauth2.Token, *oidc. idToken, err := verifier.Verify(context.Background(), rawIDToken) if err != nil { log.Errorf("Error verifying token for provider %s: %v", provider.Name, err) - return nil, nil, nil, err + return nil, cb, nil, nil, err } - return provider, oauth2Token, idToken, nil + return provider, cb, oauth2Token, idToken, nil }