Skip to content

Commit 76e1c13

Browse files
authored
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]>
1 parent 148a417 commit 76e1c13

File tree

11 files changed

+148
-31
lines changed

11 files changed

+148
-31
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ require (
99
gitea.com/go-chi/binding v0.0.0-20210301195521-1fe1c9a555e7
1010
gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e
1111
gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e
12-
gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee
12+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
1313
gitea.com/lunny/levelqueue v0.4.1
1414
github.com/Microsoft/go-winio v0.5.0 // indirect
1515
github.com/NYTimes/gziphandler v1.1.1

go.sum

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e h1:zgPGaf3kXP0cVm9J0l8
4747
gitea.com/go-chi/cache v0.0.0-20210110083709-82c4c9ce2d5e/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0=
4848
gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e h1:YjaQU6XFicdhPN+MlGolcXO8seYY2+EY5g7vZPB17CQ=
4949
gitea.com/go-chi/captcha v0.0.0-20210110083842-e7696c336a1e/go.mod h1:nfA7JaGv3hbGQ1ktdhAsZhdS84qKffI8NMlHr+Opsog=
50-
gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee h1:9U6HuKUBt/cGK6T/64dEuz0r7Yp97WAAEJvXHDlY3ws=
51-
gitea.com/go-chi/session v0.0.0-20210108030337-0cb48c5ba8ee/go.mod h1:Ozg8IchVNb/Udg+ui39iHRYqVHSvf3C99ixdpLR8Vu0=
50+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LPlps9G9aMoE6LAmqigLA+wxmd1Q=
51+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck=
5252
gitea.com/lunny/levelqueue v0.4.1 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw=
5353
gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU=
5454
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
325325
github.com/go-chi/chi v1.5.1/go.mod h1:REp24E+25iKvxgeTfHmdUoL5x15kBiDBlnIl5bCwe2k=
326326
github.com/go-chi/chi v1.5.4 h1:QHdzF2szwjqVV4wmByUnTcsbIg7UGaQ0tPF2t5GcAIs=
327327
github.com/go-chi/chi v1.5.4/go.mod h1:uaf8YgoFazUOkPBG7fxPftUylNumIev9awIWOENIuEg=
328-
github.com/go-chi/chi/v5 v5.0.1 h1:ALxjCrTf1aflOlkhMnCUP86MubbWFrzB3gkRPReLpTo=
329328
github.com/go-chi/chi/v5 v5.0.1/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
329+
github.com/go-chi/chi/v5 v5.0.4 h1:5e494iHzsYBiyXQAHHuI4tyJS9M3V84OuX3ufIIGHFo=
330+
github.com/go-chi/chi/v5 v5.0.4/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8=
330331
github.com/go-chi/cors v1.2.0 h1:tV1g1XENQ8ku4Bq3K9ub2AtgG+p16SmzeMSGTwrOKdE=
331332
github.com/go-chi/cors v1.2.0/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
332333
github.com/go-enry/go-enry/v2 v2.7.1 h1:WCqtfyteIz61GYk9lRVy8HblvIv4cP9GIiwm/6txCbU=

modules/session/store.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,21 @@
44

55
package session
66

7+
import (
8+
"net/http"
9+
10+
"gitea.com/go-chi/session"
11+
)
12+
713
// Store represents a session store
814
type Store interface {
915
Get(interface{}) interface{}
1016
Set(interface{}, interface{}) error
1117
Delete(interface{}) error
1218
}
19+
20+
// RegenerateSession regenerates the underlying session and returns the new store
21+
func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) {
22+
s, err := session.RegenerateSession(resp, req)
23+
return s, err
24+
}

routers/web/user/auth.go

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"code.gitea.io/gitea/modules/log"
2323
"code.gitea.io/gitea/modules/password"
2424
"code.gitea.io/gitea/modules/recaptcha"
25+
"code.gitea.io/gitea/modules/session"
2526
"code.gitea.io/gitea/modules/setting"
2627
"code.gitea.io/gitea/modules/timeutil"
2728
"code.gitea.io/gitea/modules/web"
@@ -87,6 +88,10 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
8788

8889
isSucceed = true
8990

91+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
92+
return false, fmt.Errorf("unable to RegenerateSession: Error: %w", err)
93+
}
94+
9095
// Set session IDs
9196
if err := ctx.Session.Set("uid", u.ID); err != nil {
9297
return false, err
@@ -235,6 +240,11 @@ func SignInPost(ctx *context.Context) {
235240
return
236241
}
237242

243+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
244+
ctx.ServerError("UserSignIn: Unable to set regenerate session", err)
245+
return
246+
}
247+
238248
// User needs to use 2FA, save data and redirect to 2FA page.
239249
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
240250
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
@@ -395,6 +405,9 @@ func TwoFactorScratchPost(ctx *context.Context) {
395405
}
396406

397407
handleSignInFull(ctx, u, remember, false)
408+
if ctx.Written() {
409+
return
410+
}
398411
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
399412
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
400413
return
@@ -505,6 +518,9 @@ func U2FSign(ctx *context.Context) {
505518
}
506519
}
507520
redirect := handleSignInFull(ctx, user, remember, false)
521+
if ctx.Written() {
522+
return
523+
}
508524
if redirect == "" {
509525
redirect = setting.AppSubURL + "/"
510526
}
@@ -517,7 +533,11 @@ func U2FSign(ctx *context.Context) {
517533

518534
// This handles the final part of the sign-in process of the user.
519535
func handleSignIn(ctx *context.Context, u *models.User, remember bool) {
520-
handleSignInFull(ctx, u, remember, true)
536+
redirect := handleSignInFull(ctx, u, remember, true)
537+
if ctx.Written() {
538+
return
539+
}
540+
ctx.Redirect(redirect)
521541
}
522542

523543
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
528548
setting.CookieRememberName, u.Name, days)
529549
}
530550

551+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
552+
ctx.ServerError("RegenerateSession", err)
553+
return setting.AppSubURL + "/"
554+
}
555+
556+
// Delete the openid, 2fa and linkaccount data
531557
_ = ctx.Session.Delete("openid_verified_uri")
532558
_ = ctx.Session.Delete("openid_signin_remember")
533559
_ = ctx.Session.Delete("openid_determined_email")
@@ -551,7 +577,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR
551577
if len(u.Language) == 0 {
552578
u.Language = ctx.Locale.Language()
553579
if err := models.UpdateUserCols(u, "language"); err != nil {
554-
log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
580+
ctx.ServerError("UpdateUserCols Language", fmt.Errorf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
555581
return setting.AppSubURL + "/"
556582
}
557583
}
@@ -697,6 +723,11 @@ func getUserName(gothUser *goth.User) string {
697723
}
698724

699725
func showLinkingLogin(ctx *context.Context, gothUser goth.User) {
726+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
727+
ctx.ServerError("RegenerateSession", err)
728+
return
729+
}
730+
700731
if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
701732
log.Error("Error setting linkAccountGothUser in session: %v", err)
702733
}
@@ -736,6 +767,11 @@ func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User
736767
return
737768
}
738769

770+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
771+
ctx.ServerError("RegenerateSession", err)
772+
return
773+
}
774+
739775
if err := ctx.Session.Set("uid", u.ID); err != nil {
740776
log.Error("Error setting uid in session: %v", err)
741777
}
@@ -776,6 +812,11 @@ func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User
776812
return
777813
}
778814

815+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
816+
ctx.ServerError("RegenerateSession", err)
817+
return
818+
}
819+
779820
// User needs to use 2FA, save data and redirect to 2FA page.
780821
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
781822
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
9651006
return
9661007
}
9671008

1009+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1010+
ctx.ServerError("RegenerateSession", err)
1011+
return
1012+
}
1013+
9681014
// User needs to use 2FA, save data and redirect to 2FA page.
9691015
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
9701016
log.Error("Error setting twofaUid in session: %v", err)
@@ -1102,7 +1148,7 @@ func LinkAccountPostRegister(ctx *context.Context) {
11021148
return
11031149
}
11041150

1105-
ctx.Redirect(setting.AppSubURL + "/user/login")
1151+
handleSignIn(ctx, u, false)
11061152
}
11071153

11081154
// HandleSignOut resets the session and sets the cookies
@@ -1244,7 +1290,7 @@ func SignUpPost(ctx *context.Context) {
12441290
}
12451291

12461292
ctx.Flash.Success(ctx.Tr("auth.sign_up_successful"))
1247-
handleSignInFull(ctx, u, false, true)
1293+
handleSignIn(ctx, u, false)
12481294
}
12491295

12501296
// createAndHandleCreatedUser calls createUserInContext and
@@ -1465,6 +1511,13 @@ func handleAccountActivation(ctx *context.Context, user *models.User) {
14651511

14661512
log.Trace("User activated: %s", user.Name)
14671513

1514+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1515+
log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err)
1516+
ctx.ServerError("ActivateUserEmail", err)
1517+
return
1518+
}
1519+
1520+
// Set session IDs
14681521
if err := ctx.Session.Set("uid", user.ID); err != nil {
14691522
log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
14701523
}
@@ -1737,11 +1790,14 @@ func ResetPasswdPost(ctx *context.Context) {
17371790

17381791
handleSignInFull(ctx, u, remember, false)
17391792
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
1793+
if ctx.Written() {
1794+
return
1795+
}
17401796
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
17411797
return
17421798
}
17431799

1744-
handleSignInFull(ctx, u, remember, true)
1800+
handleSignIn(ctx, u, remember)
17451801
}
17461802

17471803
// MustChangePassword renders the page to change a user's password

routers/web/user/auth_openid.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"code.gitea.io/gitea/modules/hcaptcha"
1717
"code.gitea.io/gitea/modules/log"
1818
"code.gitea.io/gitea/modules/recaptcha"
19+
"code.gitea.io/gitea/modules/session"
1920
"code.gitea.io/gitea/modules/setting"
2021
"code.gitea.io/gitea/modules/util"
2122
"code.gitea.io/gitea/modules/web"
@@ -231,6 +232,11 @@ func signInOpenIDVerify(ctx *context.Context) {
231232
}
232233
}
233234

235+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
236+
ctx.ServerError("RegenerateSession", err)
237+
return
238+
}
239+
234240
if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
235241
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
236242
}

services/auth/auth.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"code.gitea.io/gitea/models"
1616
"code.gitea.io/gitea/modules/log"
17+
"code.gitea.io/gitea/modules/session"
1718
"code.gitea.io/gitea/modules/setting"
1819
"code.gitea.io/gitea/modules/web/middleware"
1920
)
@@ -95,6 +96,14 @@ func isGitRawReleaseOrLFSPath(req *http.Request) bool {
9596

9697
// handleSignIn clears existing session variables and stores new ones for the specified user object
9798
func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *models.User) {
99+
// We need to regenerate the session...
100+
newSess, err := session.RegenerateSession(resp, req)
101+
if err != nil {
102+
log.Error(fmt.Sprintf("Error regenerating session: %v", err))
103+
} else {
104+
sess = newSess
105+
}
106+
98107
_ = sess.Delete("openid_verified_uri")
99108
_ = sess.Delete("openid_signin_remember")
100109
_ = sess.Delete("openid_determined_email")
@@ -103,7 +112,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
103112
_ = sess.Delete("twofaRemember")
104113
_ = sess.Delete("u2fChallenge")
105114
_ = sess.Delete("linkAccount")
106-
err := sess.Set("uid", user.ID)
115+
err = sess.Set("uid", user.ID)
107116
if err != nil {
108117
log.Error(fmt.Sprintf("Error setting session: %v", err))
109118
}

vendor/gitea.com/go-chi/session/README.md

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/gitea.com/go-chi/session/go.mod

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/gitea.com/go-chi/session/go.sum

Lines changed: 2 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)