Skip to content

Commit bcc13f3

Browse files
authored
Reset Session ID on login (#18018)
* Reset Session ID on login When logging in the SessionID should be reset and the session cleaned up. Signed-off-by: Andrew Thornton <[email protected]> * with new session.RegenerateID function Signed-off-by: Andrew Thornton <[email protected]> * update go-chi/session Signed-off-by: Andrew Thornton <[email protected]> * Ensure that session id is changed after oauth data is set and between account linking pages too Signed-off-by: Andrew Thornton <[email protected]> * placate lint Signed-off-by: Andrew Thornton <[email protected]> * as per review Signed-off-by: Andrew Thornton <[email protected]>
1 parent 2cd1479 commit bcc13f3

File tree

9 files changed

+121
-11
lines changed

9 files changed

+121
-11
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-20211013065440-d16dc407c2be
1010
gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1
1111
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5
12-
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09
12+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
1313
gitea.com/lunny/levelqueue v0.4.1
1414
github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121
1515
github.com/Microsoft/go-winio v0.5.0 // indirect

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1 h1:Z7DcvTkxt8ovcENgPsQ
4848
gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0=
4949
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 h1:J/1i8u40TbcLP/w2w2KCkgW2PelIqYkD5UOwu8IOvVU=
5050
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5/go.mod h1:hQ9SYHKdOX968wJglb/NMQ+UqpOKwW4L+EYdvkWjHSo=
51-
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09 h1:1+K+6uOXrnSfLHXvu8jpuoNEWA3/NULOlLSRZwaij+c=
52-
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck=
51+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LPlps9G9aMoE6LAmqigLA+wxmd1Q=
52+
gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck=
5353
gitea.com/lunny/levelqueue v0.4.1 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw=
5454
gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU=
5555
gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s=

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: 62 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"
@@ -90,6 +91,10 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
9091

9192
isSucceed = true
9293

94+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
95+
return false, fmt.Errorf("unable to RegenerateSession: Error: %w", err)
96+
}
97+
9398
// Set session IDs
9499
if err := ctx.Session.Set("uid", u.ID); err != nil {
95100
return false, err
@@ -256,6 +261,11 @@ func SignInPost(ctx *context.Context) {
256261
return
257262
}
258263

264+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
265+
ctx.ServerError("UserSignIn: Unable to set regenerate session", err)
266+
return
267+
}
268+
259269
// User will need to use 2FA TOTP or U2F, save data
260270
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
261271
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
@@ -419,6 +429,9 @@ func TwoFactorScratchPost(ctx *context.Context) {
419429
}
420430

421431
handleSignInFull(ctx, u, remember, false)
432+
if ctx.Written() {
433+
return
434+
}
422435
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
423436
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
424437
return
@@ -526,6 +539,9 @@ func U2FSign(ctx *context.Context) {
526539
}
527540
}
528541
redirect := handleSignInFull(ctx, user, remember, false)
542+
if ctx.Written() {
543+
return
544+
}
529545
if redirect == "" {
530546
redirect = setting.AppSubURL + "/"
531547
}
@@ -538,7 +554,11 @@ func U2FSign(ctx *context.Context) {
538554

539555
// This handles the final part of the sign-in process of the user.
540556
func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) {
541-
handleSignInFull(ctx, u, remember, true)
557+
redirect := handleSignInFull(ctx, u, remember, true)
558+
if ctx.Written() {
559+
return
560+
}
561+
ctx.Redirect(redirect)
542562
}
543563

544564
func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRedirect bool) string {
@@ -549,6 +569,12 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
549569
setting.CookieRememberName, u.Name, days)
550570
}
551571

572+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
573+
ctx.ServerError("RegenerateSession", err)
574+
return setting.AppSubURL + "/"
575+
}
576+
577+
// Delete the openid, 2fa and linkaccount data
552578
_ = ctx.Session.Delete("openid_verified_uri")
553579
_ = ctx.Session.Delete("openid_signin_remember")
554580
_ = ctx.Session.Delete("openid_determined_email")
@@ -572,7 +598,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
572598
if len(u.Language) == 0 {
573599
u.Language = ctx.Locale.Language()
574600
if err := user_model.UpdateUserCols(db.DefaultContext, u, "language"); err != nil {
575-
log.Error(fmt.Sprintf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
601+
ctx.ServerError("UpdateUserCols Language", fmt.Errorf("Error updating user language [user: %d, locale: %s]", u.ID, u.Language))
576602
return setting.AppSubURL + "/"
577603
}
578604
}
@@ -779,6 +805,11 @@ func getUserName(gothUser *goth.User) string {
779805
}
780806

781807
func showLinkingLogin(ctx *context.Context, gothUser goth.User) {
808+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
809+
ctx.ServerError("RegenerateSession", err)
810+
return
811+
}
812+
782813
if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
783814
log.Error("Error setting linkAccountGothUser in session: %v", err)
784815
}
@@ -822,6 +853,12 @@ func handleOAuth2SignIn(ctx *context.Context, source *login.Source, u *user_mode
822853
// If this user is enrolled in 2FA and this source doesn't override it,
823854
// we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page.
824855
if !needs2FA {
856+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
857+
ctx.ServerError("RegenerateSession", err)
858+
return
859+
}
860+
861+
// Set session IDs
825862
if err := ctx.Session.Set("uid", u.ID); err != nil {
826863
log.Error("Error setting uid in session: %v", err)
827864
}
@@ -878,6 +915,11 @@ func handleOAuth2SignIn(ctx *context.Context, source *login.Source, u *user_mode
878915
}
879916
}
880917

918+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
919+
ctx.ServerError("RegenerateSession", err)
920+
return
921+
}
922+
881923
// User needs to use 2FA, save data and redirect to 2FA page.
882924
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
883925
log.Error("Error setting twofaUid in session: %v", err)
@@ -1090,6 +1132,11 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r
10901132
return
10911133
}
10921134

1135+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1136+
ctx.ServerError("RegenerateSession", err)
1137+
return
1138+
}
1139+
10931140
// User needs to use 2FA, save data and redirect to 2FA page.
10941141
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
10951142
log.Error("Error setting twofaUid in session: %v", err)
@@ -1227,7 +1274,7 @@ func LinkAccountPostRegister(ctx *context.Context) {
12271274
return
12281275
}
12291276

1230-
ctx.Redirect(setting.AppSubURL + "/user/login")
1277+
handleSignIn(ctx, u, false)
12311278
}
12321279

12331280
// HandleSignOut resets the session and sets the cookies
@@ -1370,7 +1417,7 @@ func SignUpPost(ctx *context.Context) {
13701417
}
13711418

13721419
ctx.Flash.Success(ctx.Tr("auth.sign_up_successful"))
1373-
handleSignInFull(ctx, u, false, true)
1420+
handleSignIn(ctx, u, false)
13741421
}
13751422

13761423
// createAndHandleCreatedUser calls createUserInContext and
@@ -1591,6 +1638,13 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
15911638

15921639
log.Trace("User activated: %s", user.Name)
15931640

1641+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1642+
log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err)
1643+
ctx.ServerError("ActivateUserEmail", err)
1644+
return
1645+
}
1646+
1647+
// Set session IDs
15941648
if err := ctx.Session.Set("uid", user.ID); err != nil {
15951649
log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
15961650
}
@@ -1862,11 +1916,14 @@ func ResetPasswdPost(ctx *context.Context) {
18621916

18631917
handleSignInFull(ctx, u, remember, false)
18641918
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
1919+
if ctx.Written() {
1920+
return
1921+
}
18651922
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
18661923
return
18671924
}
18681925

1869-
handleSignInFull(ctx, u, remember, true)
1926+
handleSignIn(ctx, u, remember)
18701927
}
18711928

18721929
// 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"
@@ -232,6 +233,11 @@ func signInOpenIDVerify(ctx *context.Context) {
232233
}
233234
}
234235

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

services/auth/auth.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"code.gitea.io/gitea/models/db"
1616
user_model "code.gitea.io/gitea/models/user"
1717
"code.gitea.io/gitea/modules/log"
18+
"code.gitea.io/gitea/modules/session"
1819
"code.gitea.io/gitea/modules/setting"
1920
"code.gitea.io/gitea/modules/web/middleware"
2021
)
@@ -106,6 +107,14 @@ func isGitRawReleaseOrLFSPath(req *http.Request) bool {
106107

107108
// handleSignIn clears existing session variables and stores new ones for the specified user object
108109
func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *user_model.User) {
110+
// We need to regenerate the session...
111+
newSess, err := session.RegenerateSession(resp, req)
112+
if err != nil {
113+
log.Error(fmt.Sprintf("Error regenerating session: %v", err))
114+
} else {
115+
sess = newSess
116+
}
117+
109118
_ = sess.Delete("openid_verified_uri")
110119
_ = sess.Delete("openid_signin_remember")
111120
_ = sess.Delete("openid_determined_email")
@@ -114,7 +123,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
114123
_ = sess.Delete("twofaRemember")
115124
_ = sess.Delete("u2fChallenge")
116125
_ = sess.Delete("linkAccount")
117-
err := sess.Set("uid", user.ID)
126+
err = sess.Set("uid", user.ID)
118127
if err != nil {
119128
log.Error(fmt.Sprintf("Error setting session: %v", err))
120129
}

services/auth/source/oauth2/store.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ func (st *SessionsStore) getOrNew(r *http.Request, name string, override bool) (
5555
}
5656
}
5757

58+
session.IsNew = override
5859
session.ID = chiStore.ID() // Simply copy the session id from the chi store
5960

6061
return session, chiStore.Set(name, session)
@@ -64,6 +65,11 @@ func (st *SessionsStore) getOrNew(r *http.Request, name string, override bool) (
6465
func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *sessions.Session) error {
6566
chiStore := chiSession.GetSession(r)
6667

68+
if session.IsNew {
69+
_, _ = chiSession.RegenerateSession(w, r)
70+
session.IsNew = false
71+
}
72+
6773
if err := chiStore.Set(session.Name(), session); err != nil {
6874
return err
6975
}

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

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

vendor/modules.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ gitea.com/go-chi/cache/memcache
1818
# gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5
1919
## explicit
2020
gitea.com/go-chi/captcha
21-
# gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09
21+
# gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
2222
## explicit
2323
gitea.com/go-chi/session
2424
gitea.com/go-chi/session/couchbase

0 commit comments

Comments
 (0)