From 3eaf36aae7ce815bb249208407f5c72bad3b85ef Mon Sep 17 00:00:00 2001 From: Elias Schneider Date: Fri, 12 Dec 2025 14:51:07 +0100 Subject: [PATCH] fix: restrict email one time sign in token to same browser (#1144) --- .../internal/cmds/one_time_access_token.go | 2 +- backend/internal/common/errors.go | 7 ++ .../internal/controller/user_controller.go | 6 +- backend/internal/model/user.go | 5 +- backend/internal/service/user_service.go | 76 ++++++++++++------- backend/internal/utils/cookie/add_cookie.go | 6 ++ backend/internal/utils/cookie/cookie_names.go | 2 + ...0000_one_time_access_device_token.down.sql | 1 + ...000000_one_time_access_device_token.up.sql | 1 + ...0000_one_time_access_device_token.down.sql | 1 + ...000000_one_time_access_device_token.up.sql | 1 + 11 files changed, 76 insertions(+), 32 deletions(-) create mode 100644 backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.down.sql create mode 100644 backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.up.sql create mode 100644 backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.down.sql create mode 100644 backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.up.sql diff --git a/backend/internal/cmds/one_time_access_token.go b/backend/internal/cmds/one_time_access_token.go index 736ae733..7aa5b56b 100644 --- a/backend/internal/cmds/one_time_access_token.go +++ b/backend/internal/cmds/one_time_access_token.go @@ -51,7 +51,7 @@ var oneTimeAccessTokenCmd = &cobra.Command{ } // Create a new access token that expires in 1 hour - oneTimeAccessToken, txErr = service.NewOneTimeAccessToken(user.ID, time.Hour) + oneTimeAccessToken, txErr = service.NewOneTimeAccessToken(user.ID, time.Hour, false) if txErr != nil { return fmt.Errorf("failed to generate access token: %w", txErr) } diff --git a/backend/internal/common/errors.go b/backend/internal/common/errors.go index 9d35c0de..9647dbd4 100644 --- a/backend/internal/common/errors.go +++ b/backend/internal/common/errors.go @@ -38,6 +38,13 @@ type TokenInvalidOrExpiredError struct{} func (e *TokenInvalidOrExpiredError) Error() string { return "token is invalid or expired" } func (e *TokenInvalidOrExpiredError) HttpStatusCode() int { return 400 } +type DeviceCodeInvalid struct{} + +func (e *DeviceCodeInvalid) Error() string { + return "one time access code must be used on the device it was generated for" +} +func (e *DeviceCodeInvalid) HttpStatusCode() int { return 400 } + type TokenInvalidError struct{} func (e *TokenInvalidError) Error() string { diff --git a/backend/internal/controller/user_controller.go b/backend/internal/controller/user_controller.go index 2c10eb3a..cf87720e 100644 --- a/backend/internal/controller/user_controller.go +++ b/backend/internal/controller/user_controller.go @@ -391,12 +391,13 @@ func (uc *UserController) RequestOneTimeAccessEmailAsUnauthenticatedUserHandler( return } - err := uc.userService.RequestOneTimeAccessEmailAsUnauthenticatedUser(c.Request.Context(), input.Email, input.RedirectPath) + deviceToken, err := uc.userService.RequestOneTimeAccessEmailAsUnauthenticatedUser(c.Request.Context(), input.Email, input.RedirectPath) if err != nil { _ = c.Error(err) return } + cookie.AddDeviceTokenCookie(c, deviceToken) c.Status(http.StatusNoContent) } @@ -440,7 +441,8 @@ func (uc *UserController) RequestOneTimeAccessEmailAsAdminHandler(c *gin.Context // @Success 200 {object} dto.UserDto // @Router /api/one-time-access-token/{token} [post] func (uc *UserController) exchangeOneTimeAccessTokenHandler(c *gin.Context) { - user, token, err := uc.userService.ExchangeOneTimeAccessToken(c.Request.Context(), c.Param("token"), c.ClientIP(), c.Request.UserAgent()) + deviceToken, _ := c.Cookie(cookie.DeviceTokenCookieName) + user, token, err := uc.userService.ExchangeOneTimeAccessToken(c.Request.Context(), c.Param("token"), deviceToken, c.ClientIP(), c.Request.UserAgent()) if err != nil { _ = c.Error(err) return diff --git a/backend/internal/model/user.go b/backend/internal/model/user.go index c596a7d7..5e1bfd87 100644 --- a/backend/internal/model/user.go +++ b/backend/internal/model/user.go @@ -87,8 +87,9 @@ func (u User) Initials() string { type OneTimeAccessToken struct { Base - Token string - ExpiresAt datatype.DateTime + Token string + DeviceToken *string + ExpiresAt datatype.DateTime UserID string User User diff --git a/backend/internal/service/user_service.go b/backend/internal/service/user_service.go index fa92a27d..260e9778 100644 --- a/backend/internal/service/user_service.go +++ b/backend/internal/service/user_service.go @@ -432,28 +432,36 @@ func (s *UserService) RequestOneTimeAccessEmailAsAdmin(ctx context.Context, user return &common.OneTimeAccessDisabledError{} } - return s.requestOneTimeAccessEmailInternal(ctx, userID, "", ttl) + _, err := s.requestOneTimeAccessEmailInternal(ctx, userID, "", ttl, true) + return err } -func (s *UserService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ctx context.Context, userID, redirectPath string) error { +func (s *UserService) RequestOneTimeAccessEmailAsUnauthenticatedUser(ctx context.Context, userID, redirectPath string) (string, error) { isDisabled := !s.appConfigService.GetDbConfig().EmailOneTimeAccessAsUnauthenticatedEnabled.IsTrue() if isDisabled { - return &common.OneTimeAccessDisabledError{} + return "", &common.OneTimeAccessDisabledError{} } var userId string err := s.db.Model(&model.User{}).Select("id").Where("email = ?", userID).First(&userId).Error if errors.Is(err, gorm.ErrRecordNotFound) { // Do not return error if user not found to prevent email enumeration - return nil + return "", nil } else if err != nil { - return err + return "", err } - return s.requestOneTimeAccessEmailInternal(ctx, userId, redirectPath, 15*time.Minute) + deviceToken, err := s.requestOneTimeAccessEmailInternal(ctx, userId, redirectPath, 15*time.Minute, true) + if err != nil { + return "", err + } else if deviceToken == nil { + return "", errors.New("device token expected but not returned") + } + + return *deviceToken, nil } -func (s *UserService) requestOneTimeAccessEmailInternal(ctx context.Context, userID, redirectPath string, ttl time.Duration) error { +func (s *UserService) requestOneTimeAccessEmailInternal(ctx context.Context, userID, redirectPath string, ttl time.Duration, withDeviceToken bool) (*string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -461,21 +469,20 @@ func (s *UserService) requestOneTimeAccessEmailInternal(ctx context.Context, use user, err := s.GetUser(ctx, userID) if err != nil { - return err + return nil, err } if user.Email == nil { - return &common.UserEmailNotSetError{} + return nil, &common.UserEmailNotSetError{} } - oneTimeAccessToken, err := s.createOneTimeAccessTokenInternal(ctx, user.ID, ttl, tx) + oneTimeAccessToken, deviceToken, err := s.createOneTimeAccessTokenInternal(ctx, user.ID, ttl, withDeviceToken, tx) if err != nil { - return err + return nil, err } - err = tx.Commit().Error if err != nil { - return err + return nil, err } // We use a background context here as this is running in a goroutine @@ -508,28 +515,29 @@ func (s *UserService) requestOneTimeAccessEmailInternal(ctx context.Context, use } }() - return nil + return deviceToken, nil } -func (s *UserService) CreateOneTimeAccessToken(ctx context.Context, userID string, ttl time.Duration) (string, error) { - return s.createOneTimeAccessTokenInternal(ctx, userID, ttl, s.db) +func (s *UserService) CreateOneTimeAccessToken(ctx context.Context, userID string, ttl time.Duration) (token string, err error) { + token, _, err = s.createOneTimeAccessTokenInternal(ctx, userID, ttl, false, s.db) + return token, err } -func (s *UserService) createOneTimeAccessTokenInternal(ctx context.Context, userID string, ttl time.Duration, tx *gorm.DB) (string, error) { - oneTimeAccessToken, err := NewOneTimeAccessToken(userID, ttl) +func (s *UserService) createOneTimeAccessTokenInternal(ctx context.Context, userID string, ttl time.Duration, withDeviceToken bool, tx *gorm.DB) (token string, deviceToken *string, err error) { + oneTimeAccessToken, err := NewOneTimeAccessToken(userID, ttl, withDeviceToken) if err != nil { - return "", err + return "", nil, err } err = tx.WithContext(ctx).Create(oneTimeAccessToken).Error if err != nil { - return "", err + return "", nil, err } - return oneTimeAccessToken.Token, nil + return oneTimeAccessToken.Token, oneTimeAccessToken.DeviceToken, nil } -func (s *UserService) ExchangeOneTimeAccessToken(ctx context.Context, token string, ipAddress, userAgent string) (model.User, string, error) { +func (s *UserService) ExchangeOneTimeAccessToken(ctx context.Context, token, deviceToken, ipAddress, userAgent string) (model.User, string, error) { tx := s.db.Begin() defer func() { tx.Rollback() @@ -549,6 +557,10 @@ func (s *UserService) ExchangeOneTimeAccessToken(ctx context.Context, token stri } return model.User{}, "", err } + if oneTimeAccessToken.DeviceToken != nil && deviceToken != *oneTimeAccessToken.DeviceToken { + return model.User{}, "", &common.DeviceCodeInvalid{} + } + accessToken, err := s.jwtService.GenerateAccessToken(oneTimeAccessToken.User) if err != nil { return model.User{}, "", err @@ -818,23 +830,33 @@ func (s *UserService) DeleteSignupToken(ctx context.Context, tokenID string) err return s.db.WithContext(ctx).Delete(&model.SignupToken{}, "id = ?", tokenID).Error } -func NewOneTimeAccessToken(userID string, ttl time.Duration) (*model.OneTimeAccessToken, error) { +func NewOneTimeAccessToken(userID string, ttl time.Duration, withDeviceToken bool) (*model.OneTimeAccessToken, error) { // If expires at is less than 15 minutes, use a 6-character token instead of 16 tokenLength := 16 if ttl <= 15*time.Minute { tokenLength = 6 } - randomString, err := utils.GenerateRandomAlphanumericString(tokenLength) + token, err := utils.GenerateRandomAlphanumericString(tokenLength) if err != nil { return nil, err } + var deviceToken *string + if withDeviceToken { + dt, err := utils.GenerateRandomAlphanumericString(16) + if err != nil { + return nil, err + } + deviceToken = &dt + } + now := time.Now().Round(time.Second) o := &model.OneTimeAccessToken{ - UserID: userID, - ExpiresAt: datatype.DateTime(now.Add(ttl)), - Token: randomString, + UserID: userID, + ExpiresAt: datatype.DateTime(now.Add(ttl)), + Token: token, + DeviceToken: deviceToken, } return o, nil diff --git a/backend/internal/utils/cookie/add_cookie.go b/backend/internal/utils/cookie/add_cookie.go index b4591e13..8c33d781 100644 --- a/backend/internal/utils/cookie/add_cookie.go +++ b/backend/internal/utils/cookie/add_cookie.go @@ -1,6 +1,8 @@ package cookie import ( + "time" + "github.com/gin-gonic/gin" ) @@ -11,3 +13,7 @@ func AddAccessTokenCookie(c *gin.Context, maxAgeInSeconds int, token string) { func AddSessionIdCookie(c *gin.Context, maxAgeInSeconds int, sessionID string) { c.SetCookie(SessionIdCookieName, sessionID, maxAgeInSeconds, "/", "", true, true) } + +func AddDeviceTokenCookie(c *gin.Context, deviceToken string) { + c.SetCookie(DeviceTokenCookieName, deviceToken, int(15*time.Minute.Seconds()), "/api/one-time-access-token", "", true, true) +} diff --git a/backend/internal/utils/cookie/cookie_names.go b/backend/internal/utils/cookie/cookie_names.go index 8d10067d..a0799911 100644 --- a/backend/internal/utils/cookie/cookie_names.go +++ b/backend/internal/utils/cookie/cookie_names.go @@ -8,10 +8,12 @@ import ( var AccessTokenCookieName = "__Host-access_token" var SessionIdCookieName = "__Host-session" +var DeviceTokenCookieName = "__Host-device_token" //nolint:gosec func init() { if strings.HasPrefix(common.EnvConfig.AppURL, "http://") { AccessTokenCookieName = "access_token" SessionIdCookieName = "session" + DeviceTokenCookieName = "device_token" } } diff --git a/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.down.sql b/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.down.sql new file mode 100644 index 00000000..e8dba6e4 --- /dev/null +++ b/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.down.sql @@ -0,0 +1 @@ +ALTER TABLE one_time_access_tokens DROP COLUMN device_token; \ No newline at end of file diff --git a/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.up.sql b/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.up.sql new file mode 100644 index 00000000..70e6efa4 --- /dev/null +++ b/backend/resources/migrations/postgres/20251210000000_one_time_access_device_token.up.sql @@ -0,0 +1 @@ +ALTER TABLE one_time_access_tokens ADD COLUMN device_token VARCHAR(16); \ No newline at end of file diff --git a/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.down.sql b/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.down.sql new file mode 100644 index 00000000..e8dba6e4 --- /dev/null +++ b/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.down.sql @@ -0,0 +1 @@ +ALTER TABLE one_time_access_tokens DROP COLUMN device_token; \ No newline at end of file diff --git a/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.up.sql b/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.up.sql new file mode 100644 index 00000000..1aabc368 --- /dev/null +++ b/backend/resources/migrations/sqlite/20251210000000_one_time_access_device_token.up.sql @@ -0,0 +1 @@ +ALTER TABLE one_time_access_tokens ADD COLUMN device_token TEXT; \ No newline at end of file