fix: do 2FA on OpenID connect

models/user/openid.go

@@ -40,8 +40,8 @@ func GetUserOpenIDs(ctx context.Context, uid int64) ([]*UserOpenID, error) {
 	return openids, nil
 }
 
-// isOpenIDUsed returns true if the openid has been used.
-func isOpenIDUsed(ctx context.Context, uri string) (bool, error) {
+// IsOpenIDUsed returns true if the openid has been used.
+func IsOpenIDUsed(ctx context.Context, uri string) (bool, error) {
 	if len(uri) == 0 {
 		return true, nil
 	}
@@ -71,7 +71,7 @@ func (err ErrOpenIDAlreadyUsed) Unwrap() error {
 // AddUserOpenID adds an pre-verified/normalized OpenID URI to given user.
 // NOTE: make sure openid.URI is normalized already
 func AddUserOpenID(ctx context.Context, openid *UserOpenID) error {
-	used, err := isOpenIDUsed(ctx, openid.URI)
+	used, err := IsOpenIDUsed(ctx, openid.URI)
 	if err != nil {
 		return err
 	} else if used {

routers/web/auth/2fa.go

@@ -81,6 +81,14 @@ func TwoFactorPost(ctx *context.Context) {
 			}
 		}
 
+		// Handle OpenID linking to user.
+		if oid, ok := ctx.Session.Get("twofaOpenID").(string); ok {
+			if err := user_model.AddUserOpenID(ctx, &user_model.UserOpenID{UID: u.ID, URI: oid}); err != nil {
+				ctx.ServerError("AddUserOpenID", err)
+				return
+			}
+		}
+
 		twofa.LastUsedPasscode = form.Passcode
 		if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
 			ctx.ServerError("UserSignIn", err)
@@ -146,6 +154,14 @@ func TwoFactorScratchPost(ctx *context.Context) {
 			return
 		}
 
+		// Handle OpenID linking to user.
+		if oid, ok := ctx.Session.Get("twofaOpenID").(string); ok {
+			if err := user_model.AddUserOpenID(ctx, &user_model.UserOpenID{UID: u.ID, URI: oid}); err != nil {
+				ctx.ServerError("AddUserOpenID", err)
+				return
+			}
+		}
+
 		handleSignInFull(ctx, u, remember, false)
 		if ctx.Written() {
 			return

routers/web/auth/auth.go

@@ -310,6 +310,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
 		"openid_determined_username",
 		"twofaUid",
 		"twofaRemember",
+		"twofaOpenID",
 		"linkAccount",
 	}, map[string]any{
 		"uid": u.ID,

routers/web/auth/openid.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/url"
 
+	auth_model "forgejo.org/models/auth"
 	user_model "forgejo.org/models/user"
 	"forgejo.org/modules/auth/openid"
 	"forgejo.org/modules/base"
@@ -253,6 +254,7 @@ func ConnectOpenID(ctx *context.Context) {
 // ConnectOpenIDPost handles submission of a form to connect an OpenID URI to an existing account
 func ConnectOpenIDPost(ctx *context.Context) {
 	form := web.GetForm(ctx).(*forms.ConnectOpenIDForm)
+	remember, _ := ctx.Session.Get("openid_signin_remember").(bool)
 	oid, _ := ctx.Session.Get("openid_verified_uri").(string)
 	if oid == "" {
 		ctx.Redirect(setting.AppSubURL + "/user/login/openid")
@@ -264,28 +266,63 @@ func ConnectOpenIDPost(ctx *context.Context) {
 	ctx.Data["EnableOpenIDSignUp"] = setting.Service.EnableOpenIDSignUp
 	ctx.Data["OpenID"] = oid
 
-	u, _, err := auth.UserSignIn(ctx, form.UserName, form.Password)
+	u, source, err := auth.UserSignIn(ctx, form.UserName, form.Password)
 	if err != nil {
 		handleSignInError(ctx, form.UserName, &form, tplConnectOID, "ConnectOpenIDPost", err)
 		return
 	}
 
-	// add OpenID for the user
-	userOID := &user_model.UserOpenID{UID: u.ID, URI: oid}
-	if err = user_model.AddUserOpenID(ctx, userOID); err != nil {
-		if user_model.IsErrOpenIDAlreadyUsed(err) {
-			ctx.RenderWithErr(ctx.Tr("form.openid_been_used", oid), tplConnectOID, &form)
-			return
-		}
-		ctx.ServerError("AddUserOpenID", err)
+	// Check if OID is already in use.
+	if used, err := user_model.IsOpenIDUsed(ctx, oid); err != nil {
+		ctx.ServerError("IsOpenIDUsed", err)
+		return
+	} else if used {
+		ctx.RenderWithErr(ctx.Tr("form.openid_been_used", oid), tplConnectOID, &form)
 		return
 	}
 
-	ctx.Flash.Success(ctx.Tr("settings.add_openid_success"))
+	// Check if 2FA needs to be done.
+	has2FA, err := auth_model.HasTwoFactorByUID(ctx, u.ID)
+	if err != nil {
+		ctx.ServerError("HasTwoFactorByUID", err)
+		return
+	}
+	if skipper, ok := source.Cfg.(auth.LocalTwoFASkipper); !has2FA || (ok && skipper.IsSkipLocalTwoFA()) {
+		// Link this OID to the user.
+		if err := user_model.AddUserOpenID(ctx, &user_model.UserOpenID{UID: u.ID, URI: oid}); err != nil {
+			ctx.ServerError("AddUserOpenID", err)
+			return
+		}
 
-	remember, _ := ctx.Session.Get("openid_signin_remember").(bool)
-	log.Trace("Session stored openid-remember: %t", remember)
-	handleSignIn(ctx, u, remember)
+		ctx.Flash.Success(ctx.Tr("settings.add_openid_success"))
+		handleSignIn(ctx, u, remember)
+		return
+	}
+
+	// Check if the user has webauthn registration.
+	hasWebAuthnTwofa, err := auth_model.HasWebAuthnRegistrationsByUID(ctx, u.ID)
+	if err != nil {
+		ctx.ServerError("HasWebAuthnRegistrationsByUID", err)
+		return
+	}
+
+	if err := updateSession(ctx, nil, map[string]any{
+		"twofaUid":      u.ID,
+		"twofaRemember": remember,
+		"twofaOpenID":   oid,
+	}); err != nil {
+		ctx.ServerError("Unable to update session", err)
+		return
+	}
+
+	// If we have WebAuthn, redirect there first.
+	if hasWebAuthnTwofa {
+		ctx.Redirect(setting.AppSubURL + "/user/webauthn")
+		return
+	}
+
+	// Fallback to TOTP.
+	ctx.Redirect(setting.AppSubURL + "/user/two_factor")
 }
 
 // RegisterOpenID shows a form to create a new user authenticated via an OpenID URI

routers/web/auth/webauthn.go

@@ -166,6 +166,14 @@ func WebAuthnLoginAssertionPost(ctx *context.Context) {
 		}
 	}
 
+	// Handle OpenID linking to user.
+	if oid, ok := ctx.Session.Get("twofaOpenID").(string); ok {
+		if err := user_model.AddUserOpenID(ctx, &user_model.UserOpenID{UID: user.ID, URI: oid}); err != nil {
+			ctx.ServerError("AddUserOpenID", err)
+			return
+		}
+	}
+
 	remember := ctx.Session.Get("twofaRemember").(bool)
 	redirect := handleSignInFull(ctx, user, remember, false)
 	if redirect == "" {

services/auth/auth.go

@@ -77,6 +77,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
 	_ = sess.Delete("openid_determined_username")
 	_ = sess.Delete("twofaUid")
 	_ = sess.Delete("twofaRemember")
+	_ = sess.Delete("twofaOpenID")
 	_ = sess.Delete("webauthnAssertion")
 	_ = sess.Delete("linkAccount")
 	err = sess.Set("uid", user.ID)