Reset Session ID on login (#18018) (#18041)

Backport #18018

When logging in the SessionID should be reset and the session cleaned up.

Also logs the user in on completion of linking account

Signed-off-by: Andrew Thornton <[email protected]>

go.mod

@@ -9,7 +9,7 @@ require (
 	gitea.com/go-chi/binding v0.0.0-20210301195521-1fe1c9a555e7
 	gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e
 	gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e
-	gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee
+	gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
 	gitea.com/lunny/levelqueue v0.4.1
 	github.com/Microsoft/go-winio v0.5.0 // indirect
 	github.com/NYTimes/gziphandler v1.1.1

go.sum

@@ -47,8 +47,8 @@ gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e h1:zgPGaf3kXP0cVm9J0l8
 gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0=
 gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e h1:YjaQU6XFicdhPN+MlGolcXO8seYY2+EY5g7vZPB17CQ=
 gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e/go.mod h1:nfA7JaGv3hbGQ1ktdhAsZhdS84qKffI8NMlHr+Opsog=
-gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee h1:9U6HuKUBt/cGK6T/64dEuz0r7Yp97WAAEJvXHDlY3ws=
-gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee/go.mod h1:Ozg8IchVNb/Udg+ui39iHRYqVHSvf3C99ixdpLR8Vu0=
+gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LPlps9G9aMoE6LAmqigLA+wxmd1Q=
+gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck=
 gitea.com/lunny/levelqueue v0.4.1 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw=
 gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU=
 gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s=
@@ -325,8 +325,9 @@ github.com/go-asn1-ber/asn1-ber v1.5.3/go.mod h1:hEBeB/ic+5LoWskz+yKT7vGhhPYkPro
 github.com/go-chi/chi v1.5.1/go.mod h1:REp24E+25iKvxgeTfHmdUoL5x15kBiDBlnIl5bCwe2k=
 github.com/go-chi/chi v1.5.4 h1:QHdzF2szwjqVV4wmByUnTcsbIg7UGaQ0tPF2t5GcAIs=
 github.com/go-chi/chi v1.5.4/go.mod h1:uaf8YgoFazUOkPBG7fxPftUylNumIev9awIWOENIuEg=
-github.com/go-chi/chi/v5 v5.0.1 h1:ALxjCrTf1aflOlkhMnCUP86MubbWFrzB3gkRPReLpTo=
 github.com/go-chi/chi/v5 v5.0.1/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
+github.com/go-chi/chi/v5 v5.0.4 h1:5e494iHzsYBiyXQAHHuI4tyJS9M3V84OuX3ufIIGHFo=
+github.com/go-chi/chi/v5 v5.0.4/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
 github.com/go-chi/cors v1.2.0 h1:tV1g1XENQ8ku4Bq3K9ub2AtgG+p16SmzeMSGTwrOKdE=
 github.com/go-chi/cors v1.2.0/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
 github.com/go-enry/go-enry/v2 v2.7.1 h1:WCqtfyteIz61GYk9lRVy8HblvIv4cP9GIiwm/6txCbU=

modules/session/store.go

@@ -4,9 +4,21 @@
 
 package session
 
+import (
+	"net/http"
+
+	"gitea.com/go-chi/session"
+)
+
 // Store represents a session store
 type Store interface {
 	Get(interface{}) interface{}
 	Set(interface{}, interface{}) error
 	Delete(interface{}) error
 }
+
+// RegenerateSession regenerates the underlying session and returns the new store
+func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) {
+	s, err := session.RegenerateSession(resp, req)
+	return s, err
+}

routers/web/user/auth.go

@@ -22,6 +22,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/password"
 	"code.gitea.io/gitea/modules/recaptcha"
+	"code.gitea.io/gitea/modules/session"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/timeutil"
 	"code.gitea.io/gitea/modules/web"
@@ -87,6 +88,10 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
 
 	isSucceed = true
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		return false, fmt.Errorf("unable to RegenerateSession: Error: %w", err)
+	}
+
 	// Set session IDs
 	if err := ctx.Session.Set("uid", u.ID); err != nil {
 		return false, err
@@ -235,6 +240,11 @@ func SignInPost(ctx *context.Context) {
 		return
 	}
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("UserSignIn: Unable to set regenerate session", err)
+		return
+	}
+
 	// User needs to use 2FA, save data and redirect to 2FA page.
 	if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
 		ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
@@ -395,6 +405,9 @@ func TwoFactorScratchPost(ctx *context.Context) {
 		}
 
 		handleSignInFull(ctx, u, remember, false)
+		if ctx.Written() {
+			return
+		}
 		ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
 		ctx.Redirect(setting.AppSubURL + "/user/settings/security")
 		return
@@ -505,6 +518,9 @@ func U2FSign(ctx *context.Context) {
 				}
 			}
 			redirect := handleSignInFull(ctx, user, remember, false)
+			if ctx.Written() {
+				return
+			}
 			if redirect == "" {
 				redirect = setting.AppSubURL + "/"
 			}
@@ -517,7 +533,11 @@ func U2FSign(ctx *context.Context) {
 
 // This handles the final part of the sign-in process of the user.
 func handleSignIn(ctx *context.Context, u *models.User, remember bool) {
-	handleSignInFull(ctx, u, remember, true)
+	redirect := handleSignInFull(ctx, u, remember, true)
+	if ctx.Written() {
+		return
+	}
+	ctx.Redirect(redirect)
 }
 
 func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyRedirect bool) string {
@@ -528,6 +548,12 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR
 			setting.CookieRememberName, u.Name, days)
 	}
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("RegenerateSession", err)
+		return setting.AppSubURL + "/"
+	}
+
+	// Delete the openid, 2fa and linkaccount data
 	_ = ctx.Session.Delete("openid_verified_uri")
 	_ = ctx.Session.Delete("openid_signin_remember")
 	_ = ctx.Session.Delete("openid_determined_email")
@@ -551,7 +577,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR
 	if len(u.Language) == 0 {
 		u.Language = ctx.Locale.Language()
 		if err := models.UpdateUserCols(u, "language"); err != nil {
-			log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
+			ctx.ServerError("UpdateUserCols Language", fmt.Errorf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
 			return setting.AppSubURL + "/"
 		}
 	}
@@ -697,6 +723,11 @@ func getUserName(gothUser *goth.User) string {
 }
 
 func showLinkingLogin(ctx *context.Context, gothUser goth.User) {
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("RegenerateSession", err)
+		return
+	}
+
 	if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
 		log.Error("Error setting linkAccountGothUser in session: %v", err)
 	}
@@ -736,6 +767,11 @@ func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User
 			return
 		}
 
+		if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+			ctx.ServerError("RegenerateSession", err)
+			return
+		}
+
 		if err := ctx.Session.Set("uid", u.ID); err != nil {
 			log.Error("Error setting uid in session: %v", err)
 		}
@@ -776,6 +812,11 @@ func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User
 		return
 	}
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("RegenerateSession", err)
+		return
+	}
+
 	// User needs to use 2FA, save data and redirect to 2FA page.
 	if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
 		log.Error("Error setting twofaUid in session: %v", err)
@@ -965,6 +1006,11 @@ func linkAccount(ctx *context.Context, u *models.User, gothUser goth.User, remem
 		return
 	}
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("RegenerateSession", err)
+		return
+	}
+
 	// User needs to use 2FA, save data and redirect to 2FA page.
 	if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
 		log.Error("Error setting twofaUid in session: %v", err)
@@ -1102,7 +1148,7 @@ func LinkAccountPostRegister(ctx *context.Context) {
 		return
 	}
 
-	ctx.Redirect(setting.AppSubURL + "/user/login")
+	handleSignIn(ctx, u, false)
 }
 
 // HandleSignOut resets the session and sets the cookies
@@ -1244,7 +1290,7 @@ func SignUpPost(ctx *context.Context) {
 	}
 
 	ctx.Flash.Success(ctx.Tr("auth.sign_up_successful"))
-	handleSignInFull(ctx, u, false, true)
+	handleSignIn(ctx, u, false)
 }
 
 // createAndHandleCreatedUser calls createUserInContext and
@@ -1465,6 +1511,13 @@ func handleAccountActivation(ctx *context.Context, user *models.User) {
 
 	log.Trace("User activated: %s", user.Name)
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err)
+		ctx.ServerError("ActivateUserEmail", err)
+		return
+	}
+
+	// Set session IDs
 	if err := ctx.Session.Set("uid", user.ID); err != nil {
 		log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
 	}
@@ -1737,11 +1790,14 @@ func ResetPasswdPost(ctx *context.Context) {
 
 		handleSignInFull(ctx, u, remember, false)
 		ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
+		if ctx.Written() {
+			return
+		}
 		ctx.Redirect(setting.AppSubURL + "/user/settings/security")
 		return
 	}
 
-	handleSignInFull(ctx, u, remember, true)
+	handleSignIn(ctx, u, remember)
 }
 
 // MustChangePassword renders the page to change a user's password

routers/web/user/auth_openid.go

@@ -16,6 +16,7 @@ import (
 	"code.gitea.io/gitea/modules/hcaptcha"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/recaptcha"
+	"code.gitea.io/gitea/modules/session"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
@@ -231,6 +232,11 @@ func signInOpenIDVerify(ctx *context.Context) {
 		}
 	}
 
+	if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
+		ctx.ServerError("RegenerateSession", err)
+		return
+	}
+
 	if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
 		log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
 	}

services/auth/auth.go

@@ -14,6 +14,7 @@ import (
 
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/session"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/web/middleware"
 )
@@ -95,6 +96,14 @@ func isGitRawReleaseOrLFSPath(req *http.Request) bool {
 
 // handleSignIn clears existing session variables and stores new ones for the specified user object
 func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *models.User) {
+	// We need to regenerate the session...
+	newSess, err := session.RegenerateSession(resp, req)
+	if err != nil {
+		log.Error(fmt.Sprintf("Error regenerating session: %v", err))
+	} else {
+		sess = newSess
+	}
+
 	_ = sess.Delete("openid_verified_uri")
 	_ = sess.Delete("openid_signin_remember")
 	_ = sess.Delete("openid_determined_email")
@@ -103,7 +112,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
 	_ = sess.Delete("twofaRemember")
 	_ = sess.Delete("u2fChallenge")
 	_ = sess.Delete("linkAccount")
-	err := sess.Set("uid", user.ID)
+	err = sess.Set("uid", user.ID)
 	if err != nil {
 		log.Error(fmt.Sprintf("Error setting session: %v", err))
 	}