summaryrefslogtreecommitdiffstats
path: root/routers
diff options
context:
space:
mode:
authorGusted <postmaster@gusted.xyz>2024-08-11 05:13:01 +0200
committerEarl Warren <contact@earl-warren.org>2024-11-15 10:59:36 +0100
commit1ce33aa38d1d258d14523ff2c7c2dbf339f22b74 (patch)
tree93298b9246f031cf684b126fc76b901e781fbbb4 /routers
parentMerge pull request 'ci: use oci mirror images' (#5963) from viceice/ci/oci-mi... (diff)
downloadforgejo-1ce33aa38d1d258d14523ff2c7c2dbf339f22b74.tar.xz
forgejo-1ce33aa38d1d258d14523ff2c7c2dbf339f22b74.zip
fix: extend `forgejo_auth_token` table
- Add a `purpose` column, this allows the `forgejo_auth_token` table to be used by other parts of Forgejo, while still enjoying the no-compromise architecture. - Remove the 'roll your own crypto' time limited code functions and migrate them to the `forgejo_auth_token` table. This migration ensures generated codes can only be used for their purpose and ensure they are invalidated after their usage by deleting it from the database, this also should help making auditing of the security code easier, as we're no longer trying to stuff a lot of data into a HMAC construction. -Helper functions are rewritten to ensure a safe-by-design approach to these tokens. - Add the `forgejo_auth_token` to dbconsistency doctor and add it to the `deleteUser` function. - TODO: Add cron job to delete expired authorization tokens. - Unit and integration tests added.
Diffstat (limited to 'routers')
-rw-r--r--routers/web/auth/auth.go96
-rw-r--r--routers/web/auth/password.go18
-rw-r--r--routers/web/user/setting/account.go15
3 files changed, 71 insertions, 58 deletions
diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
index 1f5ebef7d7..941586db72 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -5,8 +5,6 @@
package auth
import (
- "crypto/subtle"
- "encoding/hex"
"errors"
"fmt"
"net/http"
@@ -63,38 +61,11 @@ func autoSignIn(ctx *context.Context) (bool, error) {
return false, nil
}
- lookupKey, validator, found := strings.Cut(authCookie, ":")
- if !found {
- return false, nil
- }
-
- authToken, err := auth.FindAuthToken(ctx, lookupKey)
- if err != nil {
- if errors.Is(err, util.ErrNotExist) {
- return false, nil
- }
- return false, err
- }
-
- if authToken.IsExpired() {
- err = auth.DeleteAuthToken(ctx, authToken)
- return false, err
- }
-
- rawValidator, err := hex.DecodeString(validator)
+ u, err := user_model.VerifyUserAuthorizationToken(ctx, authCookie, auth.LongTermAuthorization, false)
if err != nil {
- return false, err
+ return false, fmt.Errorf("VerifyUserAuthorizationToken: %w", err)
}
-
- if subtle.ConstantTimeCompare([]byte(authToken.HashedValidator), []byte(auth.HashValidator(rawValidator))) == 0 {
- return false, nil
- }
-
- u, err := user_model.GetUserByID(ctx, authToken.UID)
- if err != nil {
- if !user_model.IsErrUserNotExist(err) {
- return false, fmt.Errorf("GetUserByID: %w", err)
- }
+ if u == nil {
return false, nil
}
@@ -633,7 +604,10 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.
return false
}
- mailer.SendActivateAccountMail(ctx.Locale, u)
+ if err := mailer.SendActivateAccountMail(ctx, u); err != nil {
+ ctx.ServerError("SendActivateAccountMail", err)
+ return false
+ }
ctx.Data["IsSendRegisterMail"] = true
ctx.Data["Email"] = u.Email
@@ -674,7 +648,10 @@ func Activate(ctx *context.Context) {
ctx.Data["ResendLimited"] = true
} else {
ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale)
- mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer)
+ if err := mailer.SendActivateAccountMail(ctx, ctx.Doer); err != nil {
+ ctx.ServerError("SendActivateAccountMail", err)
+ return
+ }
if err := ctx.Cache.Put(cacheKey+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil {
log.Error("Set cache(MailResendLimit) fail: %v", err)
@@ -687,7 +664,12 @@ func Activate(ctx *context.Context) {
return
}
- user := user_model.VerifyUserActiveCode(ctx, code)
+ user, err := user_model.VerifyUserAuthorizationToken(ctx, code, auth.UserActivation, false)
+ if err != nil {
+ ctx.ServerError("VerifyUserAuthorizationToken", err)
+ return
+ }
+
// if code is wrong
if user == nil {
ctx.Data["IsCodeInvalid"] = true
@@ -751,7 +733,12 @@ func ActivatePost(ctx *context.Context) {
return
}
- user := user_model.VerifyUserActiveCode(ctx, code)
+ user, err := user_model.VerifyUserAuthorizationToken(ctx, code, auth.UserActivation, true)
+ if err != nil {
+ ctx.ServerError("VerifyUserAuthorizationToken", err)
+ return
+ }
+
// if code is wrong
if user == nil {
ctx.Data["IsCodeInvalid"] = true
@@ -835,24 +822,33 @@ func ActivateEmail(ctx *context.Context) {
code := ctx.FormString("code")
emailStr := ctx.FormString("email")
- // Verify code.
- if email := user_model.VerifyActiveEmailCode(ctx, code, emailStr); email != nil {
- if err := user_model.ActivateEmail(ctx, email); err != nil {
- ctx.ServerError("ActivateEmail", err)
- return
- }
+ u, err := user_model.VerifyUserAuthorizationToken(ctx, code, auth.EmailActivation(emailStr), true)
+ if err != nil {
+ ctx.ServerError("VerifyUserAuthorizationToken", err)
+ return
+ }
+ if u == nil {
+ ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+ return
+ }
- log.Trace("Email activated: %s", email.Email)
- ctx.Flash.Success(ctx.Tr("settings.add_email_success"))
+ email, err := user_model.GetEmailAddressOfUser(ctx, emailStr, u.ID)
+ if err != nil {
+ ctx.ServerError("GetEmailAddressOfUser", err)
+ return
+ }
- if u, err := user_model.GetUserByID(ctx, email.UID); err != nil {
- log.Warn("GetUserByID: %d", email.UID)
- } else {
- // Allow user to validate more emails
- _ = ctx.Cache.Delete("MailResendLimit_" + u.LowerName)
- }
+ if err := user_model.ActivateEmail(ctx, email); err != nil {
+ ctx.ServerError("ActivateEmail", err)
+ return
}
+ log.Trace("Email activated: %s", email.Email)
+ ctx.Flash.Success(ctx.Tr("settings.add_email_success"))
+
+ // Allow user to validate more emails
+ _ = ctx.Cache.Delete("MailResendLimit_" + u.LowerName)
+
// FIXME: e-mail verification does not require the user to be logged in,
// so this could be redirecting to the login page.
// Should users be logged in automatically here? (consider 2FA requirements, etc.)
diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go
index d25bd682e2..363c01c6a8 100644
--- a/routers/web/auth/password.go
+++ b/routers/web/auth/password.go
@@ -86,7 +86,10 @@ func ForgotPasswdPost(ctx *context.Context) {
return
}
- mailer.SendResetPasswordMail(u)
+ if err := mailer.SendResetPasswordMail(ctx, u); err != nil {
+ ctx.ServerError("SendResetPasswordMail", err)
+ return
+ }
if err = ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil {
log.Error("Set cache(MailResendLimit) fail: %v", err)
@@ -97,7 +100,7 @@ func ForgotPasswdPost(ctx *context.Context) {
ctx.HTML(http.StatusOK, tplForgotPassword)
}
-func commonResetPassword(ctx *context.Context) (*user_model.User, *auth.TwoFactor) {
+func commonResetPassword(ctx *context.Context, shouldDeleteToken bool) (*user_model.User, *auth.TwoFactor) {
code := ctx.FormString("code")
ctx.Data["Title"] = ctx.Tr("auth.reset_password")
@@ -113,7 +116,12 @@ func commonResetPassword(ctx *context.Context) (*user_model.User, *auth.TwoFacto
}
// Fail early, don't frustrate the user
- u := user_model.VerifyUserActiveCode(ctx, code)
+ u, err := user_model.VerifyUserAuthorizationToken(ctx, code, auth.PasswordReset, shouldDeleteToken)
+ if err != nil {
+ ctx.ServerError("VerifyUserAuthorizationToken", err)
+ return nil, nil
+ }
+
if u == nil {
ctx.Flash.Error(ctx.Tr("auth.invalid_code_forgot_password", fmt.Sprintf("%s/user/forgot_password", setting.AppSubURL)), true)
return nil, nil
@@ -145,7 +153,7 @@ func commonResetPassword(ctx *context.Context) (*user_model.User, *auth.TwoFacto
func ResetPasswd(ctx *context.Context) {
ctx.Data["IsResetForm"] = true
- commonResetPassword(ctx)
+ commonResetPassword(ctx, false)
if ctx.Written() {
return
}
@@ -155,7 +163,7 @@ func ResetPasswd(ctx *context.Context) {
// ResetPasswdPost response from account recovery request
func ResetPasswdPost(ctx *context.Context) {
- u, twofa := commonResetPassword(ctx)
+ u, twofa := commonResetPassword(ctx, true)
if ctx.Written() {
return
}
diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go
index 7e4d45e991..6f40e39c8d 100644
--- a/routers/web/user/setting/account.go
+++ b/routers/web/user/setting/account.go
@@ -155,9 +155,15 @@ func EmailPost(ctx *context.Context) {
return
}
// Only fired when the primary email is inactive (Wrong state)
- mailer.SendActivateAccountMail(ctx.Locale, ctx.Doer)
+ if err := mailer.SendActivateAccountMail(ctx, ctx.Doer); err != nil {
+ ctx.ServerError("SendActivateAccountMail", err)
+ return
+ }
} else {
- mailer.SendActivateEmailMail(ctx.Doer, email.Email)
+ if err := mailer.SendActivateEmailMail(ctx, ctx.Doer, email.Email); err != nil {
+ ctx.ServerError("SendActivateEmailMail", err)
+ return
+ }
}
address = email.Email
@@ -218,7 +224,10 @@ func EmailPost(ctx *context.Context) {
// Send confirmation email
if setting.Service.RegisterEmailConfirm {
- mailer.SendActivateEmailMail(ctx.Doer, form.Email)
+ if err := mailer.SendActivateEmailMail(ctx, ctx.Doer, form.Email); err != nil {
+ ctx.ServerError("SendActivateEmailMail", err)
+ return
+ }
if err := ctx.Cache.Put("MailResendLimit_"+ctx.Doer.LowerName, ctx.Doer.LowerName, 180); err != nil {
log.Error("Set cache(MailResendLimit) fail: %v", err)
}