From f739682e9a689988e4a6b839780813732ae6f357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=B4=8A=E1=B4=8F=E1=B4=87=20=E1=B4=84=CA=9C=E1=B4=87?= =?UTF-8?q?=C9=B4?= Date: Fri, 22 May 2026 15:33:06 -0400 Subject: [PATCH] Move sign-in MFA step to React with /api/web/user/mfa (#8288) --- .moon/tasks.yml | 4 + AGENTS.md | 4 + cmd/gogs/internal/web/web.go | 6 +- cmd/gogs/internal/web/webapi.go | 218 ++++++++++--- conf/locale/locale_en-US.ini | 33 +- docs/dev/local_development.md | 5 +- internal/auth/smtp/config.go | 17 +- internal/context/auth.go | 1 + internal/route/user/auth.go | 128 +------- moon.yml | 40 +++ templates/user/auth/two_factor.tmpl | 28 -- .../user/auth/two_factor_recovery_code.tmpl | 28 -- web/DESIGN.md | 8 + web/scripts/extract-locales.mjs | 12 + web/src/locales/en-US.json | 18 +- web/src/pages/MFA.tsx | 213 +++++++++++++ web/src/pages/SignIn.tsx | 291 ++++++++++-------- web/src/router.tsx | 20 +- 18 files changed, 710 insertions(+), 364 deletions(-) create mode 100644 .moon/tasks.yml delete mode 100644 templates/user/auth/two_factor.tmpl delete mode 100644 templates/user/auth/two_factor_recovery_code.tmpl create mode 100644 web/src/pages/MFA.tsx diff --git a/.moon/tasks.yml b/.moon/tasks.yml new file mode 100644 index 000000000..01c1edaff --- /dev/null +++ b/.moon/tasks.yml @@ -0,0 +1,4 @@ +$schema: "https://moonrepo.dev/schemas/tasks.json" + +taskOptions: + outputStyle: "stream" diff --git a/AGENTS.md b/AGENTS.md index f787d06bc..a3b1592ed 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,10 @@ This applies to all texts, including but not limited to UI, documentation, code - Use `github.com/cockroachdb/errors` for error handling. - Use `github.com/stretchr/testify` for assertions in tests. Be mindful about the choice of `require` and `assert`, the former should be used when the test cannot proceed meaningfully after a failed assertion. +## Localization + +- Only edit `conf/locale/locale_en-US.ini`. The other `locale_*.ini` files are community-maintained translations. Do not add, remove, or rewrite keys in them, even when removing keys that are dead on the Go/template side. + ## UI guidelines - Design mobile-friendly. Every UI must look and work well on narrow viewports before adding desktop refinements via responsive breakpoints. Test at ~375px width before considering a UI done. diff --git a/cmd/gogs/internal/web/web.go b/cmd/gogs/internal/web/web.go index a4ba0621e..b91270a9a 100644 --- a/cmd/gogs/internal/web/web.go +++ b/cmd/gogs/internal/web/web.go @@ -87,11 +87,6 @@ func Run(configPath string, portOverride int) error { // ***** START: User ***** m.Group("/user", func() { - m.Group("/login", func() { - m.Combo("/two_factor").Get(user.LoginTwoFactor).Post(user.LoginTwoFactorPost) - m.Combo("/two_factor_recovery_code").Get(user.LoginTwoFactorRecoveryCode).Post(user.LoginTwoFactorRecoveryCodePost) - }) - m.Get("/sign_up", user.SignUp) m.Post("/sign_up", bindIgnErr(form.Register{}), user.SignUpPost) m.Get("/reset_password", user.ResetPasswd) @@ -530,6 +525,7 @@ func Run(configPath string, portOverride int) error { }, ignSignIn) m.Any("/api/web/*", bridgeToWebAPI(webHandler)) + m.Get("/redirect", bridgeToWebAPI(webHandler)) m.Any("/*", func(c *context.Context) { c.ServeWeb() }) }, session.Sessioner(session.Options{ diff --git a/cmd/gogs/internal/web/webapi.go b/cmd/gogs/internal/web/webapi.go index c375dba1c..50a31ccc1 100644 --- a/cmd/gogs/internal/web/webapi.go +++ b/cmd/gogs/internal/web/webapi.go @@ -4,12 +4,14 @@ import ( stdctx "context" "encoding/json" "net/http" + "reflect" "strings" "github.com/cockroachdb/errors" "github.com/flamego/binding" "github.com/flamego/flamego" "github.com/flamego/validator" + "github.com/go-macaron/cache" "github.com/go-macaron/i18n" "github.com/go-macaron/session" "gopkg.in/macaron.v1" @@ -20,6 +22,7 @@ import ( "gogs.io/gogs/internal/context" "gogs.io/gogs/internal/database" "gogs.io/gogs/internal/urlx" + "gogs.io/gogs/internal/userx" ) type ( @@ -27,15 +30,17 @@ type ( webAPISessionKey struct{} webAPIMacaronKey struct{} webAPILocaleKey struct{} + webAPICacheKey struct{} ) -func bridgeToWebAPI(webHandler http.Handler) func(c *context.Context, l i18n.Locale) { - return func(c *context.Context, l i18n.Locale) { +func bridgeToWebAPI(webHandler http.Handler) func(c *context.Context, l i18n.Locale, ca cache.Cache) { + return func(c *context.Context, l i18n.Locale, ca cache.Cache) { ctx := c.Req.Context() ctx = stdctx.WithValue(ctx, webAPIUserKey{}, c.User) ctx = stdctx.WithValue(ctx, webAPISessionKey{}, c.Session) ctx = stdctx.WithValue(ctx, webAPIMacaronKey{}, c.Context) ctx = stdctx.WithValue(ctx, webAPILocaleKey{}, l) + ctx = stdctx.WithValue(ctx, webAPICacheKey{}, ca) webHandler.ServeHTTP(c.Resp, c.Req.WithContext(ctx)) } } @@ -46,7 +51,8 @@ func webAPIInjector(c flamego.Context) { sess, _ := ctx.Value(webAPISessionKey{}).(session.Store) mc, _ := ctx.Value(webAPIMacaronKey{}).(*macaron.Context) l, _ := ctx.Value(webAPILocaleKey{}).(i18n.Locale) - c.Map(user, sess, mc, l) + ca, _ := ctx.Value(webAPICacheKey{}).(cache.Cache) + c.Map(user, sess, mc, l, ca) } func webAPIBodyLimiter(c flamego.Context) { @@ -54,6 +60,39 @@ func webAPIBodyLimiter(c flamego.Context) { r.Body = http.MaxBytesReader(c.ResponseWriter(), r.Body, 4*1024) // 4 KiB } +// webAPIValidator is the shared validator instance used by every webapi +// binding. Registering the json-tag name function makes validation errors +// carry the wire field name (e.g. "recoveryCode") via ve.Field(), so the +// 400 payload keys match what the React client sends and reads. +var webAPIValidator = func() *validator.Validate { + v := validator.New() + v.RegisterTagNameFunc(func(fld reflect.StructField) string { + name := strings.SplitN(fld.Tag.Get("json"), ",", 2)[0] + if name == "-" { + return "" + } + return name + }) + return v +}() + +// bindJSON binds the request body to T. On binding or validation failure it +// short-circuits with a 400 carrying the standard renderBindingErrors payload, +// so downstream handlers can drop the `if len(bindErrs) > 0` boilerplate and +// the binding.Errors parameter entirely. +func bindJSON(model any) flamego.Handler { + return binding.JSON(model, binding.Options{ + Validator: webAPIValidator, + ErrorHandler: func(c flamego.Context, l i18n.Locale, errs binding.Errors) { + w := c.ResponseWriter() + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(renderBindingErrors(l, errs)) + }, + }) +} + func mountWebAPIRoutes(f *flamego.Flame) { f.ReturnHandler(func(c flamego.Context, statusCode int, resp any, err error) { w := c.ResponseWriter() @@ -79,22 +118,41 @@ func mountWebAPIRoutes(f *flamego.Flame) { f.Get("/info", getUserInfo) f.Combo("/sign-in"). Get(getUserSignIn). - Post(binding.JSON(userSignInRequest{}), postUserSignIn) + Post(bindJSON(userSignInRequest{}), postUserSignIn) + f.Group("/mfa", func() { + f.Combo(""). + Get(getUserMFA). + Post(bindJSON(userMFARequest{}), postUserMFA) + f.Post("/recovery", bindJSON(userMFARecoveryRequest{}), postUserMFARecovery) + }) f.Post("/sign-out", postUserSignOut) }) }, webAPIBodyLimiter, webAPIInjector) + + f.Get("/redirect", getRedirect) } +func getRedirect(c flamego.Context) { + to := c.Request().URL.Query().Get("to") + if !urlx.IsSameSite(to) { + to = conf.Server.Subpath + "/" + } + c.Redirect(to, http.StatusSeeOther) +} + +// fieldErrors maps JSON field names to per-field localized messages. A non-nil +// value renders inline under the input. A nil value marks the input as +// invalid (highlight + focus eligibility) without duplicating text. Used in +// concert with bindingErrorResponse.Error to surface one banner message while +// highlighting multiple inputs. +type fieldErrors map[string]*string + // bindingErrorResponse carries form-validation failures. Error is the top-level -// message shown as a banner above the form (used when the failure is not tied to -// a specific input, e.g. malformed body, bad credentials). Fields maps JSON -// field names to per-field localized messages. A non-nil value renders inline -// under the input. nil marks the input as invalid (highlight + focus -// eligibility) without duplicating text. Pair Error with nil entries in Fields -// to surface one banner message while highlighting multiple inputs. +// message shown as a banner above the form (used when the failure is not tied +// to a specific input, e.g. malformed body, bad credentials). type bindingErrorResponse struct { - Error string `json:"error,omitempty"` - Fields map[string]*string `json:"fields,omitempty"` + Error string `json:"error,omitempty"` + Fields fieldErrors `json:"fields,omitempty"` } // ruleSuffixKeys maps a validator tag to the shared "form.*_error" suffix key @@ -121,7 +179,7 @@ func renderBindingErrors(l i18n.Locale, errs binding.Errors) *bindingErrorRespon } } - out := make(map[string]*string) + out := make(fieldErrors) for _, e := range errs { var ves validator.ValidationErrors ok := errors.As(e.Err, &ves) @@ -129,7 +187,7 @@ func renderBindingErrors(l i18n.Locale, errs binding.Errors) *bindingErrorRespon continue } for _, ve := range ves { - field := strings.ToLower(ve.StructField()) + field := ve.Field() if _, exists := out[field]; exists { // Keep the first rule that failed for a given field so the client renders one // message per input. Subsequent rules surface only after the first is fixed. @@ -165,7 +223,7 @@ type userSignInPageResponse struct { func getUserSignIn(r *http.Request) (statusCode int, resp *userSignInPageResponse, err error) { sources, err := database.Handle.LoginSources().List(r.Context(), database.ListLoginSourceOptions{OnlyActivated: true}) if err != nil { - log.Error("getUserSignIn: list activated login sources: %+v", err) + log.Error("getUserSignIn: list activated login sources: %v", err) return http.StatusInternalServerError, nil, errors.Wrap(err, "list activated login sources") } loginSources := make([]loginSource, 0, len(sources)) @@ -180,42 +238,48 @@ type userSignInRequest struct { Password string `json:"password" validate:"required,max=255"` LoginSource int64 `json:"loginSource"` Remember bool `json:"remember"` - RedirectTo string `json:"redirectTo"` } type userSignInResponse struct { - TwoFactor bool `json:"twoFactor,omitempty"` - RedirectTo string `json:"redirectTo,omitempty"` + // MFA is true when the account has MFA enabled and the password step + // succeeded but a second factor is still required. The client should + // navigate to /user/mfa to complete the challenge. + MFA bool `json:"mfa,omitempty"` } -func postUserSignIn(r *http.Request, sess session.Store, mc *macaron.Context, l i18n.Locale, req userSignInRequest, bindErrs binding.Errors) (statusCode int, resp any, err error) { - if len(bindErrs) > 0 { - return http.StatusBadRequest, renderBindingErrors(l, bindErrs), nil - } - +func postUserSignIn(r *http.Request, sess session.Store, mc *macaron.Context, l i18n.Locale, req userSignInRequest) (statusCode int, resp any, err error) { u, err := database.Handle.Users().Authenticate(r.Context(), req.Username, req.Password, req.LoginSource) if err != nil { switch { case auth.IsErrBadCredentials(err): return http.StatusUnauthorized, &bindingErrorResponse{ Error: l.Tr("form.username_password_incorrect"), - Fields: map[string]*string{"username": nil, "password": nil}, + Fields: fieldErrors{"username": nil, "password": nil}, }, nil case database.IsErrLoginSourceMismatch(err): return http.StatusUnprocessableEntity, nil, errors.New(l.Tr("form.auth_source_mismatch")) default: - log.Error("postUserSignIn: authenticate user %q: %+v", req.Username, err) + log.Error("postUserSignIn: authenticate user %q: %v", req.Username, err) return http.StatusInternalServerError, nil, errors.Wrap(err, "authenticate user") } } if database.Handle.TwoFactors().IsEnabled(r.Context(), u.ID) { - _ = sess.Set("twoFactorRemember", req.Remember) - _ = sess.Set("twoFactorUserID", u.ID) - return http.StatusOK, &userSignInResponse{TwoFactor: true}, nil + _ = sess.Set("mfaRemember", req.Remember) + _ = sess.Set("mfaUserID", u.ID) + return http.StatusOK, &userSignInResponse{MFA: true}, nil } - if req.Remember { + completeSignIn(sess, mc, u, req.Remember) + return http.StatusOK, &userSignInResponse{}, nil +} + +// completeSignIn finalizes the sign-in session for u: writes the auth session, +// clears any in-flight MFA session, and sets remember-me / login-status +// cookies. The caller is responsible for navigating to a post-login +// destination via /redirect?to=. +func completeSignIn(sess session.Store, mc *macaron.Context, u *database.User, remember bool) { + if remember { days := 86400 * conf.Security.LoginRememberDays mc.SetCookie(conf.Security.CookieUsername, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true) mc.SetSuperSecureCookie(u.Rands+u.Password, conf.Security.CookieRememberName, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true) @@ -223,19 +287,103 @@ func postUserSignIn(r *http.Request, sess session.Store, mc *macaron.Context, l _ = sess.Set("uid", u.ID) _ = sess.Set("uname", u.Name) - _ = sess.Delete("twoFactorRemember") - _ = sess.Delete("twoFactorUserID") + _ = sess.Delete("mfaRemember") + _ = sess.Delete("mfaUserID") mc.SetCookie(conf.Session.CSRFCookieName, "", -1, conf.Server.Subpath) if conf.Security.EnableLoginStatusCookie { mc.SetCookie(conf.Security.LoginStatusCookieName, "true", 0, conf.Server.Subpath) } +} - redirectTo := req.RedirectTo - if !urlx.IsSameSite(redirectTo) { - redirectTo = conf.Server.Subpath + "/" +func getUserMFA(sess session.Store) (statusCode int, resp any, err error) { + if _, ok := sess.Get("mfaUserID").(int64); !ok { + return http.StatusNotFound, nil, nil } - return http.StatusOK, &userSignInResponse{RedirectTo: redirectTo}, nil + return http.StatusNoContent, nil, nil +} + +type userMFARequest struct { + Passcode string `json:"passcode" validate:"required,len=6"` +} + +type userMFAResponse struct{} + +func postUserMFA(r *http.Request, sess session.Store, mc *macaron.Context, ca cache.Cache, l i18n.Locale, req userMFARequest) (statusCode int, resp any, err error) { + userID, ok := sess.Get("mfaUserID").(int64) + if !ok { + return http.StatusUnauthorized, &bindingErrorResponse{Error: l.Tr("auth.mfa_session_expired")}, nil + } + + t, err := database.Handle.TwoFactors().GetByUserID(r.Context(), userID) + if err != nil { + log.Error("postUserMFA: get two factor by user ID %d: %v", userID, err) + return http.StatusInternalServerError, nil, errors.Wrap(err, "get two factor by user ID") + } + + valid, err := t.ValidateTOTP(req.Passcode) + if err != nil { + log.Error("postUserMFA: validate TOTP for user %d: %v", userID, err) + return http.StatusInternalServerError, nil, errors.Wrap(err, "validate TOTP") + } + if !valid { + msg := l.Tr("auth.mfa_invalid_passcode") + return http.StatusUnauthorized, &bindingErrorResponse{ + Fields: fieldErrors{"passcode": &msg}, + }, nil + } + + if ca.IsExist(userx.TwoFactorCacheKey(userID, req.Passcode)) { + msg := l.Tr("auth.mfa_reused_passcode") + return http.StatusUnauthorized, &bindingErrorResponse{ + Fields: fieldErrors{"passcode": &msg}, + }, nil + } + if err = ca.Put(userx.TwoFactorCacheKey(userID, req.Passcode), 1, 60); err != nil { + log.Error("postUserMFA: cache two factor passcode for user %d: %v", userID, err) + } + + u, err := database.Handle.Users().GetByID(r.Context(), userID) + if err != nil { + log.Error("postUserMFA: get user by ID %d: %v", userID, err) + return http.StatusInternalServerError, nil, errors.Wrap(err, "get user by ID") + } + + remember, _ := sess.Get("mfaRemember").(bool) + completeSignIn(sess, mc, u, remember) + return http.StatusOK, &userMFAResponse{}, nil +} + +type userMFARecoveryRequest struct { + RecoveryCode string `json:"recoveryCode" validate:"required,len=11"` +} + +func postUserMFARecovery(r *http.Request, sess session.Store, mc *macaron.Context, l i18n.Locale, req userMFARecoveryRequest) (statusCode int, resp any, err error) { + userID, ok := sess.Get("mfaUserID").(int64) + if !ok { + return http.StatusUnauthorized, &bindingErrorResponse{Error: l.Tr("auth.mfa_session_expired")}, nil + } + + if err := database.Handle.TwoFactors().UseRecoveryCode(r.Context(), userID, req.RecoveryCode); err != nil { + if database.IsTwoFactorRecoveryCodeNotFound(err) { + msg := l.Tr("auth.mfa_invalid_recovery_code") + return http.StatusUnauthorized, &bindingErrorResponse{ + Fields: fieldErrors{"recoveryCode": &msg}, + }, nil + } + log.Error("postUserMFARecovery: use recovery code for user %d: %v", userID, err) + return http.StatusInternalServerError, nil, errors.Wrap(err, "use recovery code") + } + + u, err := database.Handle.Users().GetByID(r.Context(), userID) + if err != nil { + log.Error("postUserMFARecovery: get user by ID %d: %v", userID, err) + return http.StatusInternalServerError, nil, errors.Wrap(err, "get user by ID") + } + + remember, _ := sess.Get("mfaRemember").(bool) + completeSignIn(sess, mc, u, remember) + return http.StatusOK, &userMFAResponse{}, nil } type userInfo struct { diff --git a/conf/locale/locale_en-US.ini b/conf/locale/locale_en-US.ini index 0c06ceaf5..fbe5bfe0a 100644 --- a/conf/locale/locale_en-US.ini +++ b/conf/locale/locale_en-US.ini @@ -159,14 +159,29 @@ search = Search [auth] create_new_account = Create New Account sign_in_submitting = Signing in... -sign_in_failed = Sign-in failed. Please try again. +sign_in_failed = Could not sign in, please try again. show_password = Show password hide_password = Hide password +back_to_sign_in = Back to sign in +mfa_title = Multi-factor authentication +mfa_passcode = Passcode +mfa_passcode_placeholder = Enter the 6-digit code from your authenticator +mfa_recovery_code = Recovery code +mfa_recovery_code_placeholder = Enter a recovery code +mfa_use_recovery_code = Use a recovery code instead +mfa_use_passcode = Use a passcode instead +mfa_verify = Verify +mfa_verifying = Verifying... +mfa_session_expired = Your sign-in session has expired. Please sign in again. +mfa_verify_failed = Verification failed. Please try again. +mfa_invalid_passcode = The passcode you entered is not valid. +mfa_reused_passcode = The passcode you entered has already been used, please try another one. +mfa_invalid_recovery_code = Recovery code already used or invalid. register_hepler_msg = Already have an account? Sign in now! social_register_hepler_msg = Already have an account? Bind now! disable_register_prompt = Sorry, registration has been disabled. Please contact the site administrator. disable_register_mail = Sorry, email services are disabled. Please contact the site administrator. -auth_source = Authentication Source +auth_source = Authentication source local = Local remember_me = Remember me forgot_password= Forgot Password @@ -186,14 +201,6 @@ reset_password_helper = Click here to reset your password password_too_short = Password length must be at least 6 characters. non_local_account = Non-local accounts cannot change passwords through Gogs. -login_two_factor = Two-factor Authentication -login_two_factor_passcode = Authentication Passcode -login_two_factor_enter_recovery_code = Enter a two-factor recovery code -login_two_factor_recovery = Two-factor Recovery -login_two_factor_recovery_code = Recovery Code -login_two_factor_enter_passcode = Enter a two-factor passcode -login_two_factor_invalid_recovery_code = Recovery code already used or invalid. - [mail] activate_account = Please activate your account activate_email = Verify your email address @@ -220,6 +227,8 @@ PayloadUrl = Payload URL TeamName = Team name AuthName = Authorization name AdminEmail = Admin email +Passcode = Passcode +RecoveryCode = Recovery code NewBranchName = New branch name CommitSummary = Commit summary @@ -1097,7 +1106,7 @@ users.created = Created users.send_register_notify = Send Registration Notification To User users.new_success = New account '%s' has been created successfully. users.edit = Edit -users.auth_source = Authentication Source +users.auth_source = Authentication source users.local = Local users.auth_login_name = Authentication Login Name users.password_helper = Leave it empty to remain unchanged. @@ -1130,7 +1139,7 @@ repos.stars = Stars repos.issues = Issues repos.size = Size -auths.auth_sources = Authentication Sources +auths.auth_sources = Authentication sources auths.new = Add New Source auths.name = Name auths.type = Type diff --git a/docs/dev/local_development.md b/docs/dev/local_development.md index 4e0ec7d43..2e0098db6 100644 --- a/docs/dev/local_development.md +++ b/docs/dev/local_development.md @@ -38,13 +38,16 @@ Gogs has the following dependencies: 1. Install dependencies: ```bash - brew install go postgresql git npm moon + brew install go postgresql git npm moon portless + portless trust npm install -g less npm install -g less-plugin-clean-css go install github.com/derision-test/go-mockgen/cmd/go-mockgen@v1.3.3 go install golang.org/x/tools/cmd/goimports@latest ``` + `portless trust` adds the local CA to your system trust store so `https://gogs.localhost` works without browser warnings. The `moon run gogs:dev` task will start the proxy and register the route automatically. + 1. Configure PostgreSQL to start automatically: ```bash diff --git a/internal/auth/smtp/config.go b/internal/auth/smtp/config.go index 3b31250c4..f3a239ce7 100644 --- a/internal/auth/smtp/config.go +++ b/internal/auth/smtp/config.go @@ -2,12 +2,19 @@ package smtp import ( "crypto/tls" - "fmt" + "net" "net/smtp" + "strconv" + "time" "github.com/cockroachdb/errors" ) +// dialTimeout bounds how long the SMTP authentication flow waits on the +// underlying TCP connect. Without it, an unreachable or misspelled host hangs +// the sign-in request until the OS-level connect timeout (minutes). +const dialTimeout = 10 * time.Second + // Config contains configuration for SMTP authentication. // // ⚠️ WARNING: Change to the field name must preserve the INI key name for backward compatibility. @@ -21,10 +28,16 @@ type Config struct { } func (c *Config) doAuth(auth smtp.Auth) error { - client, err := smtp.Dial(fmt.Sprintf("%s:%d", c.Host, c.Port)) + addr := net.JoinHostPort(c.Host, strconv.Itoa(c.Port)) + conn, err := net.DialTimeout("tcp", addr, dialTimeout) if err != nil { return err } + client, err := smtp.NewClient(conn, c.Host) + if err != nil { + _ = conn.Close() + return err + } defer client.Close() if err = client.Hello("gogs"); err != nil { diff --git a/internal/context/auth.go b/internal/context/auth.go index b5469a7be..7d4825750 100644 --- a/internal/context/auth.go +++ b/internal/context/auth.go @@ -116,6 +116,7 @@ func isWebPath(p string) bool { p = strings.TrimPrefix(p, conf.Server.Subpath) switch { case p == "/user/sign-in", + p == "/user/mfa", strings.HasPrefix(p, "/assets/"), strings.HasPrefix(p, "/src/"), strings.HasPrefix(p, "/node_modules/"), diff --git a/internal/route/user/auth.go b/internal/route/user/auth.go index c1506ea71..0503cef36 100644 --- a/internal/route/user/auth.go +++ b/internal/route/user/auth.go @@ -4,7 +4,6 @@ import ( gocontext "context" "encoding/hex" "net/http" - "net/url" "strconv" "github.com/go-macaron/captcha" @@ -16,135 +15,16 @@ import ( "gogs.io/gogs/internal/email" "gogs.io/gogs/internal/form" "gogs.io/gogs/internal/tool" - "gogs.io/gogs/internal/urlx" "gogs.io/gogs/internal/userx" ) const ( - tmplUserAuthTwoFactor = "user/auth/two_factor" - tmplUserAuthTwoFactorRecoveryCode = "user/auth/two_factor_recovery_code" - tmplUserAuthSignup = "user/auth/signup" - TmplUserAuthActivate = "user/auth/activate" - tmplUserAuthForgotPassword = "user/auth/forgot_passwd" - tmplUserAuthResetPassword = "user/auth/reset_passwd" + tmplUserAuthSignup = "user/auth/signup" + TmplUserAuthActivate = "user/auth/activate" + tmplUserAuthForgotPassword = "user/auth/forgot_passwd" + tmplUserAuthResetPassword = "user/auth/reset_passwd" ) -func afterLogin(c *context.Context, u *database.User, remember bool) { - if remember { - days := 86400 * conf.Security.LoginRememberDays - c.SetCookie(conf.Security.CookieUsername, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true) - c.SetSuperSecureCookie(u.Rands+u.Password, conf.Security.CookieRememberName, u.Name, days, conf.Server.Subpath, "", conf.Security.CookieSecure, true) - } - - _ = c.Session.Set("uid", u.ID) - _ = c.Session.Set("uname", u.Name) - _ = c.Session.Delete("twoFactorRemember") - _ = c.Session.Delete("twoFactorUserID") - - // Clear whatever CSRF has right now, force to generate a new one - c.SetCookie(conf.Session.CSRFCookieName, "", -1, conf.Server.Subpath) - if conf.Security.EnableLoginStatusCookie { - c.SetCookie(conf.Security.LoginStatusCookieName, "true", 0, conf.Server.Subpath) - } - - redirectTo, _ := url.QueryUnescape(c.GetCookie("redirect_to")) - c.SetCookie("redirect_to", "", -1, conf.Server.Subpath) - if urlx.IsSameSite(redirectTo) { - c.Redirect(redirectTo) - return - } - - c.RedirectSubpath("/") -} - -func LoginTwoFactor(c *context.Context) { - _, ok := c.Session.Get("twoFactorUserID").(int64) - if !ok { - c.NotFound() - return - } - - c.Success(tmplUserAuthTwoFactor) -} - -func LoginTwoFactorPost(c *context.Context) { - userID, ok := c.Session.Get("twoFactorUserID").(int64) - if !ok { - c.NotFound() - return - } - - t, err := database.Handle.TwoFactors().GetByUserID(c.Req.Context(), userID) - if err != nil { - c.Error(err, "get two factor by user ID") - return - } - - passcode := c.Query("passcode") - valid, err := t.ValidateTOTP(passcode) - if err != nil { - c.Error(err, "validate TOTP") - return - } else if !valid { - c.Flash.Error(c.Tr("settings.two_factor_invalid_passcode")) - c.RedirectSubpath("/user/login/two_factor") - return - } - - u, err := database.Handle.Users().GetByID(c.Req.Context(), userID) - if err != nil { - c.Error(err, "get user by ID") - return - } - - // Prevent same passcode from being reused - if c.Cache.IsExist(userx.TwoFactorCacheKey(u.ID, passcode)) { - c.Flash.Error(c.Tr("settings.two_factor_reused_passcode")) - c.RedirectSubpath("/user/login/two_factor") - return - } - if err = c.Cache.Put(userx.TwoFactorCacheKey(u.ID, passcode), 1, 60); err != nil { - log.Error("Failed to put cache 'two factor passcode': %v", err) - } - - afterLogin(c, u, c.Session.Get("twoFactorRemember").(bool)) -} - -func LoginTwoFactorRecoveryCode(c *context.Context) { - _, ok := c.Session.Get("twoFactorUserID").(int64) - if !ok { - c.NotFound() - return - } - - c.Success(tmplUserAuthTwoFactorRecoveryCode) -} - -func LoginTwoFactorRecoveryCodePost(c *context.Context) { - userID, ok := c.Session.Get("twoFactorUserID").(int64) - if !ok { - c.NotFound() - return - } - - if err := database.Handle.TwoFactors().UseRecoveryCode(c.Req.Context(), userID, c.Query("recovery_code")); err != nil { - if database.IsTwoFactorRecoveryCodeNotFound(err) { - c.Flash.Error(c.Tr("auth.login_two_factor_invalid_recovery_code")) - c.RedirectSubpath("/user/login/two_factor_recovery_code") - } else { - c.Error(err, "use recovery code") - } - return - } - - u, err := database.Handle.Users().GetByID(c.Req.Context(), userID) - if err != nil { - c.Error(err, "get user by ID") - return - } - afterLogin(c, u, c.Session.Get("twoFactorRemember").(bool)) -} - func SignOut(c *context.Context) { _ = c.Session.Flush() _ = c.Session.Destory(c.Context) diff --git a/moon.yml b/moon.yml index 09645f8a7..2354b876c 100644 --- a/moon.yml +++ b/moon.yml @@ -88,6 +88,44 @@ tasks: - "install" - "web:build" + portless: + script: | + portless alias gogs 3000 --force >/dev/null + portless proxy start >/dev/null 2>&1 || true + mkdir -p .bin/custom/conf + touch .bin/custom/conf/app.ini + awk ' + BEGIN { in_server=0; saw_server=0; set_domain=0; set_url=0 } + /^\[server\]/ { in_server=1; saw_server=1; print; next } + /^\[/ { + if (in_server) { + if (!set_domain) print "DOMAIN = gogs.localhost" + if (!set_url) print "EXTERNAL_URL = https://gogs.localhost/" + in_server=0 + } + print; next + } + in_server && /^[[:space:]]*DOMAIN[[:space:]]*=/ { + print "DOMAIN = gogs.localhost"; set_domain=1; next + } + in_server && /^[[:space:]]*EXTERNAL_URL[[:space:]]*=/ { + print "EXTERNAL_URL = https://gogs.localhost/"; set_url=1; next + } + { print } + END { + if (in_server) { + if (!set_domain) print "DOMAIN = gogs.localhost" + if (!set_url) print "EXTERNAL_URL = https://gogs.localhost/" + } else if (!saw_server) { + print "" + print "[server]" + print "DOMAIN = gogs.localhost" + print "EXTERNAL_URL = https://gogs.localhost/" + } + } + ' .bin/custom/conf/app.ini > .bin/custom/conf/app.ini.tmp \ + && mv .bin/custom/conf/app.ini.tmp .bin/custom/conf/app.ini + dev: command: ".bin/gogs web" preset: "server" @@ -96,6 +134,7 @@ tasks: deps: - "build" - "web:dev" + - "portless" prod: command: ".bin/gogs web" @@ -104,3 +143,4 @@ tasks: TTY_FORCE: "1" deps: - "build-prod" + - "portless" diff --git a/templates/user/auth/two_factor.tmpl b/templates/user/auth/two_factor.tmpl deleted file mode 100644 index fc47d8f33..000000000 --- a/templates/user/auth/two_factor.tmpl +++ /dev/null @@ -1,28 +0,0 @@ -{{template "base/head" .}} -
-
-
-
- {{.CSRFTokenHTML}} -

- {{.i18n.Tr "auth.login_two_factor"}} -

-
- {{template "base/alert" .}} -
- -
- -
-
- - -
-

- {{.i18n.Tr "auth.login_two_factor_enter_recovery_code"}} -

-
-
-
-
-{{template "base/footer" .}} diff --git a/templates/user/auth/two_factor_recovery_code.tmpl b/templates/user/auth/two_factor_recovery_code.tmpl deleted file mode 100644 index 929806292..000000000 --- a/templates/user/auth/two_factor_recovery_code.tmpl +++ /dev/null @@ -1,28 +0,0 @@ -{{template "base/head" .}} -
-
-
-
- {{.CSRFTokenHTML}} -

- {{.i18n.Tr "auth.login_two_factor_recovery"}} -

-
- {{template "base/alert" .}} -
- -
- -
-
- - -
-

- {{.i18n.Tr "auth.login_two_factor_enter_passcode"}} -

-
-
-
-
-{{template "base/footer" .}} diff --git a/web/DESIGN.md b/web/DESIGN.md index f942cece0..37e8af8df 100644 --- a/web/DESIGN.md +++ b/web/DESIGN.md @@ -67,6 +67,14 @@ Two conventions coexist in `web/src/`: Library modules in `lib/` are plain `.ts` files in lowercase (`i18n.ts`, `theme.ts`, `utils.ts`). +## Forms + +Disable the entire form while a submit is in flight, not just the submit button. Wrap the body in `
` — native `disabled` propagates to every nested input and button. + +Anchor links inside the form aren't covered by `disabled`. For each, set `tabIndex={submitting ? -1 : N}`, `aria-disabled={submitting || undefined}`, `className={submitting ? "pointer-events-none opacity-50" : undefined}`, and an `onClick` that calls `e.preventDefault()` when submitting. + +Swap the submit label to a present-continuous string ("Signing in…", "Verifying…") while submitting. Keep idle and active strings as separate locale keys. + ## Accessibility WCAG 2.2 AA is the floor. Apply these patterns in components: diff --git a/web/scripts/extract-locales.mjs b/web/scripts/extract-locales.mjs index 94c2fc27b..f9b5d8557 100644 --- a/web/scripts/extract-locales.mjs +++ b/web/scripts/extract-locales.mjs @@ -52,6 +52,18 @@ const REUSED_KEYS = [ "sign_in_failed", "show_password", "hide_password", + "back_to_sign_in", + "mfa_title", + "mfa_passcode", + "mfa_passcode_placeholder", + "mfa_recovery_code", + "mfa_recovery_code_placeholder", + "mfa_use_recovery_code", + "mfa_use_passcode", + "mfa_verify", + "mfa_verifying", + "mfa_session_expired", + "mfa_verify_failed", ]; // Lightweight INI parser: handles `key = value` and `key=value`, ignores diff --git a/web/src/locales/en-US.json b/web/src/locales/en-US.json index 1c8c07aa2..80d31b579 100644 --- a/web/src/locales/en-US.json +++ b/web/src/locales/en-US.json @@ -29,13 +29,25 @@ "username_placeholder": "Enter your username or email", "password": "Password", "password_placeholder": "Enter your password", - "auth_source": "Authentication Source", + "auth_source": "Authentication source", "local": "Local", "remember_me": "Remember me", "forget_password": "Forgot password?", "sign_up_now": "Create a new account", "sign_in_submitting": "Signing in...", - "sign_in_failed": "Sign-in failed. Please try again.", + "sign_in_failed": "Could not sign in, please try again.", "show_password": "Show password", - "hide_password": "Hide password" + "hide_password": "Hide password", + "back_to_sign_in": "Back to sign in", + "mfa_title": "Multi-factor authentication", + "mfa_passcode": "Passcode", + "mfa_passcode_placeholder": "Enter the 6-digit code from your authenticator", + "mfa_recovery_code": "Recovery code", + "mfa_recovery_code_placeholder": "Enter a recovery code", + "mfa_use_recovery_code": "Use a recovery code instead", + "mfa_use_passcode": "Use a passcode instead", + "mfa_verify": "Verify", + "mfa_verifying": "Verifying...", + "mfa_session_expired": "Your sign-in session has expired. Please sign in again.", + "mfa_verify_failed": "Verification failed. Please try again." } diff --git a/web/src/pages/MFA.tsx b/web/src/pages/MFA.tsx new file mode 100644 index 000000000..b20cc00a5 --- /dev/null +++ b/web/src/pages/MFA.tsx @@ -0,0 +1,213 @@ +import { Link, getRouteApi, useNavigate } from "@tanstack/react-router"; +import { useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; + +import { Button } from "@/components/ui/button"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { usePageTitle } from "@/lib/page-title"; +import { subUrl } from "@/lib/url"; + +interface MFAErrorResponse { + error?: string; + fields?: Record; +} + +type Mode = "passcode" | "recovery"; + +const route = getRouteApi("/user/mfa"); + +export function MFA() { + const { t } = useTranslation(); + usePageTitle(t("mfa_title")); + const navigate = useNavigate(); + // When no challenge is pending the loader has already kicked off a full + // navigation away; the early return keeps this page from flashing. + const { pending } = route.useLoaderData(); + + const [mode, setMode] = useState("passcode"); + const [passcode, setPasscode] = useState(""); + const [recoveryCode, setRecoveryCode] = useState(""); + const [submitting, setSubmitting] = useState(false); + const [formError, setFormError] = useState(null); + const [fieldErrors, setFieldErrors] = useState>({}); + const passcodeRef = useRef(null); + const recoveryRef = useRef(null); + + // Focus the active input on initial render and on every mode swap. + // `autoFocus` on the JSX doesn't reliably fire when the route mounts via + // a TanStack navigate or when the conditional swaps which input is in the + // tree, so we drive focus explicitly off the mode. + useEffect(() => { + if (!pending) return; + if (mode === "passcode") passcodeRef.current?.focus(); + else recoveryRef.current?.focus(); + }, [mode, pending]); + + if (!pending) { + return null; + } + + function switchMode(next: Mode) { + setMode(next); + setFormError(null); + setFieldErrors({}); + } + + function onSubmit(event: React.FormEvent) { + event.preventDefault(); + setFormError(null); + setFieldErrors({}); + setSubmitting(true); + void (async () => { + try { + const url = mode === "passcode" ? subUrl("/api/web/user/mfa") : subUrl("/api/web/user/mfa/recovery"); + const body = mode === "passcode" ? JSON.stringify({ passcode }) : JSON.stringify({ recoveryCode }); + const res = await fetch(url, { + method: "POST", + credentials: "same-origin", + headers: { "Content-Type": "application/json" }, + body, + }); + if (!res.ok) { + const errBody = (await res.json().catch(() => ({}))) as MFAErrorResponse; + if (res.status === 401 && !errBody.fields) { + // Session-expired or missing 2FA session: send the user back to start. + await navigate({ to: "/user/sign-in" }); + return; + } + if (errBody.error) setFormError(errBody.error); + if (errBody.fields) setFieldErrors(errBody.fields); + if (!errBody.error && !errBody.fields) { + setFormError(t("mfa_verify_failed")); + } + setSubmitting(false); + // Focus after React re-enables the fieldset; .focus() is a no-op + // while the input is still inside a disabled fieldset, so we defer + // past the commit with rAF. + requestAnimationFrame(() => { + if (mode === "passcode") passcodeRef.current?.focus(); + else recoveryRef.current?.focus(); + }); + return; + } + const to = new URLSearchParams(window.location.search).get("redirect_to") ?? ""; + window.location.assign(subUrl("/redirect") + "?to=" + encodeURIComponent(to)); + } catch { + setFormError(t("mfa_verify_failed")); + setSubmitting(false); + } + })(); + } + + const isPasscode = mode === "passcode"; + const inputId = isPasscode ? "passcode" : "recovery_code"; + const inputErrorKey = isPasscode ? "passcode" : "recoveryCode"; + + return ( +
+ + + {t("mfa_title")} + + +
+
+ {formError && ( +
+ {formError} +
+ )} + +
+ {isPasscode ? ( +
+ + setPasscode(e.target.value)} + aria-invalid={inputErrorKey in fieldErrors ? true : undefined} + aria-describedby={fieldErrors[inputErrorKey] ? `${inputId}-error` : undefined} + /> + {fieldErrors[inputErrorKey] && ( +

+ {fieldErrors[inputErrorKey]} +

+ )} +
+ ) : ( +
+ + setRecoveryCode(e.target.value)} + aria-invalid={inputErrorKey in fieldErrors ? true : undefined} + aria-describedby={fieldErrors[inputErrorKey] ? `${inputId}-error` : undefined} + /> + {fieldErrors[inputErrorKey] && ( +

+ {fieldErrors[inputErrorKey]} +

+ )} +
+ )} + +
+ + + +
+
+
+
+ + +
+ ); +} diff --git a/web/src/pages/SignIn.tsx b/web/src/pages/SignIn.tsx index 5027a07e4..742becf84 100644 --- a/web/src/pages/SignIn.tsx +++ b/web/src/pages/SignIn.tsx @@ -1,4 +1,4 @@ -import { getRouteApi } from "@tanstack/react-router"; +import { getRouteApi, useNavigate } from "@tanstack/react-router"; import { Eye, EyeOff } from "lucide-react"; import { useRef, useState } from "react"; import { useTranslation } from "react-i18next"; @@ -23,8 +23,7 @@ export interface SignInPage { } interface SignInResponse { - twoFactor?: boolean; - redirectTo?: string; + mfa?: boolean; } interface SignInErrorResponse { @@ -40,6 +39,7 @@ const route = getRouteApi("/user/sign-in"); export function SignIn() { const { t } = useTranslation(); usePageTitle(t("sign_in")); + const navigate = useNavigate(); const { loginSources } = route.useLoaderData(); const defaultSource = loginSources.find((s) => s.isDefault); @@ -61,35 +61,47 @@ export function SignIn() { setSubmitting(true); void (async () => { try { - const redirectTo = new URLSearchParams(window.location.search).get("redirect_to") ?? ""; const res = await fetch(subUrl("/api/web/user/sign-in"), { method: "POST", credentials: "same-origin", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ username, password, loginSource, remember, redirectTo }), + body: JSON.stringify({ username, password, loginSource, remember }), }); if (!res.ok) { const body = (await res.json().catch(() => ({}))) as SignInErrorResponse; if (body.error) setFormError(body.error); else setFormError(null); + let focusField: (typeof FIELD_ORDER)[number] | undefined; if (body.fields) { setFieldErrors(body.fields); - const first = FIELD_ORDER.find((f) => f in (body.fields ?? {})); - if (first === "username") usernameRef.current?.focus(); - else if (first === "password") passwordRef.current?.focus(); + focusField = FIELD_ORDER.find((f) => f in (body.fields ?? {})); } if (!body.error && !body.fields) { setFormError(t("sign_in_failed")); } setSubmitting(false); + // Defer focus past the React commit so the fieldset is re-enabled + // (.focus() is a no-op while the field is inside a disabled fieldset). + requestAnimationFrame(() => { + if (focusField === "username") usernameRef.current?.focus(); + else if (focusField === "password") passwordRef.current?.focus(); + }); return; } const data = (await res.json()) as SignInResponse; - if (data.twoFactor) { - window.location.assign(subUrl("/user/login/two_factor")); + if (data.mfa) { + // Preserve ?redirect_to= so the MFA step can finalize the same target. + const search = new URLSearchParams(window.location.search); + const redirectTo = search.get("redirect_to"); + await navigate({ + to: "/user/mfa", + search: redirectTo ? { redirect_to: redirectTo } : {}, + }); return; } - window.location.assign(data.redirectTo || subUrl("/")); + const to = new URLSearchParams(window.location.search).get("redirect_to") ?? ""; + // /redirect is a server endpoint (303), must be a full navigation. + window.location.assign(subUrl("/redirect") + "?to=" + encodeURIComponent(to)); } catch { setFormError(t("sign_in_failed")); setSubmitting(false); @@ -104,124 +116,153 @@ export function SignIn() { {t("sign_in")} -
- {formError && ( -
- {formError} -
- )} - -
- - setUsername(e.target.value)} - aria-invalid={"username" in fieldErrors ? true : undefined} - aria-describedby={fieldErrors.username ? "username-error" : undefined} - /> - {fieldErrors.username && ( -

- {fieldErrors.username} -

- )} -
- -
-
- - -
-
- setPassword(e.target.value)} - aria-invalid={"password" in fieldErrors ? true : undefined} - aria-describedby={fieldErrors.password ? "password-error" : undefined} - className="pr-10" - /> - -
- {fieldErrors.password && ( -

- {fieldErrors.password} -

+ {formError} +
)} - - {loginSources.length > 0 && ( -
- - +
+
+ + setUsername(e.target.value)} + aria-invalid={"username" in fieldErrors ? true : undefined} + aria-describedby={fieldErrors.username ? "username-error" : undefined} + /> + {fieldErrors.username && ( +

+ {fieldErrors.username} +

+ )} +
+ +
+ +
+ setPassword(e.target.value)} + aria-invalid={"password" in fieldErrors ? true : undefined} + aria-describedby={fieldErrors.password ? "password-error" : undefined} + className="pr-10" + /> + +
+ {fieldErrors.password && ( +

+ {fieldErrors.password} +

+ )} +
+ + {loginSources.length > 0 && ( +
+ + +
+ )} + +
+ setRemember(v === true)} + /> + +
+ +
+ + +
- )} - -
- setRemember(v === true)} - /> - -
- -
- - -
+
diff --git a/web/src/router.tsx b/web/src/router.tsx index c3140f63b..7b0b3302b 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -13,6 +13,7 @@ import { webContext } from "@/lib/context"; import { subUrl } from "@/lib/url"; import type { UserInfo } from "@/lib/user-info"; import { Landing } from "@/pages/Landing"; +import { MFA } from "@/pages/MFA"; import { NotFound } from "@/pages/NotFound"; import { SignIn, type SignInPage } from "@/pages/SignIn"; @@ -67,7 +68,24 @@ const signInRoute = createRoute({ component: SignIn, }); -const routeTree = rootRoute.addChildren([landingRoute, signInRoute]); +const mfaRoute = createRoute({ + getParentRoute: () => rootRoute, + path: "/user/mfa", + loader: async (): Promise<{ pending: boolean }> => { + const res = await fetch(subUrl("/api/web/user/mfa"), { credentials: "same-origin" }); + if (res.status === 404) { + // No pending MFA challenge — there is nothing to verify here, so fall + // through to the server-rendered home, which will redirect to sign-in + // for anonymous visitors and to the dashboard for signed-in ones. + window.location.assign(subUrl("/")); + return { pending: false }; + } + return { pending: res.ok }; + }, + component: MFA, +}); + +const routeTree = rootRoute.addChildren([landingRoute, signInRoute, mfaRoute]); function makeRouter(context: RouterContext) { return createRouter({