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 <art27@cantab.net>

* with new session.RegenerateID function

Signed-off-by: Andrew Thornton <art27@cantab.net>

* update go-chi/session

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Ensure that session id is changed after oauth data is set and between account linking pages too

Signed-off-by: Andrew Thornton <art27@cantab.net>

* placate lint

Signed-off-by: Andrew Thornton <art27@cantab.net>

* as per review

Signed-off-by: Andrew Thornton <art27@cantab.net>
tokarchuk/v1.17
zeripath 3 years ago committed by GitHub
parent 2cd1479e77
commit bcc13f3889
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      go.mod
  2. 4
      go.sum
  3. 12
      modules/session/store.go
  4. 67
      routers/web/user/auth.go
  5. 6
      routers/web/user/auth_openid.go
  6. 11
      services/auth/auth.go
  7. 6
      services/auth/source/oauth2/store.go
  8. 22
      vendor/gitea.com/go-chi/session/session.go
  9. 2
      vendor/modules.txt

@ -9,7 +9,7 @@ require (
gitea.com/go-chi/binding v0.0.0-20211013065440-d16dc407c2be gitea.com/go-chi/binding v0.0.0-20211013065440-d16dc407c2be
gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1 gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09 gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
gitea.com/lunny/levelqueue v0.4.1 gitea.com/lunny/levelqueue v0.4.1
github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121 github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121
github.com/Microsoft/go-winio v0.5.0 // indirect github.com/Microsoft/go-winio v0.5.0 // indirect

@ -48,8 +48,8 @@ gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1 h1:Z7DcvTkxt8ovcENgPsQ
gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0= gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1/go.mod h1:k2V/gPDEtXGjjMGuBJiapffAXTv76H4snSmlJRLUhH0=
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 h1:J/1i8u40TbcLP/w2w2KCkgW2PelIqYkD5UOwu8IOvVU= gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 h1:J/1i8u40TbcLP/w2w2KCkgW2PelIqYkD5UOwu8IOvVU=
gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5/go.mod h1:hQ9SYHKdOX968wJglb/NMQ+UqpOKwW4L+EYdvkWjHSo= gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5/go.mod h1:hQ9SYHKdOX968wJglb/NMQ+UqpOKwW4L+EYdvkWjHSo=
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09 h1:1+K+6uOXrnSfLHXvu8jpuoNEWA3/NULOlLSRZwaij+c= gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8 h1:tJQRXgZigkLeeW9LPlps9G9aMoE6LAmqigLA+wxmd1Q=
gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09/go.mod h1:fc/pjt5EqNKgqQXYzcas1Z5L5whkZHyOvTA7OzWVJck= 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 h1:RZ+AFx5gBsZuyqCvofhAkPQ9uaVDPJnsULoJZIYaJNw=
gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU= gitea.com/lunny/levelqueue v0.4.1/go.mod h1:HBqmLbz56JWpfEGG0prskAV97ATNRoj5LDmPicD22hU=
gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s= gitea.com/xorm/sqlfiddle v0.0.0-20180821085327-62ce714f951a h1:lSA0F4e9A2NcQSqGqTOXqu2aRi/XEQxDCBwM8yJtE6s=

@ -4,9 +4,21 @@
package session package session
import (
"net/http"
"gitea.com/go-chi/session"
)
// Store represents a session store // Store represents a session store
type Store interface { type Store interface {
Get(interface{}) interface{} Get(interface{}) interface{}
Set(interface{}, interface{}) error Set(interface{}, interface{}) error
Delete(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
}

@ -22,6 +22,7 @@ import (
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/password"
"code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/recaptcha"
"code.gitea.io/gitea/modules/session"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
@ -90,6 +91,10 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
isSucceed = true 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 // Set session IDs
if err := ctx.Session.Set("uid", u.ID); err != nil { if err := ctx.Session.Set("uid", u.ID); err != nil {
return false, err return false, err
@ -256,6 +261,11 @@ func SignInPost(ctx *context.Context) {
return return
} }
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
ctx.ServerError("UserSignIn: Unable to set regenerate session", err)
return
}
// User will need to use 2FA TOTP or U2F, save data // User will need to use 2FA TOTP or U2F, save data
if err := ctx.Session.Set("twofaUid", u.ID); err != nil { if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err) ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
@ -419,6 +429,9 @@ func TwoFactorScratchPost(ctx *context.Context) {
} }
handleSignInFull(ctx, u, remember, false) handleSignInFull(ctx, u, remember, false)
if ctx.Written() {
return
}
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used")) ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security") ctx.Redirect(setting.AppSubURL + "/user/settings/security")
return return
@ -526,6 +539,9 @@ func U2FSign(ctx *context.Context) {
} }
} }
redirect := handleSignInFull(ctx, user, remember, false) redirect := handleSignInFull(ctx, user, remember, false)
if ctx.Written() {
return
}
if redirect == "" { if redirect == "" {
redirect = setting.AppSubURL + "/" redirect = setting.AppSubURL + "/"
} }
@ -538,7 +554,11 @@ func U2FSign(ctx *context.Context) {
// This handles the final part of the sign-in process of the user. // This handles the final part of the sign-in process of the user.
func handleSignIn(ctx *context.Context, u *user_model.User, remember bool) { func handleSignIn(ctx *context.Context, u *user_model.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 *user_model.User, remember, obeyRedirect bool) string { 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
setting.CookieRememberName, u.Name, days) 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_verified_uri")
_ = ctx.Session.Delete("openid_signin_remember") _ = ctx.Session.Delete("openid_signin_remember")
_ = ctx.Session.Delete("openid_determined_email") _ = ctx.Session.Delete("openid_determined_email")
@ -572,7 +598,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
if len(u.Language) == 0 { if len(u.Language) == 0 {
u.Language = ctx.Locale.Language() u.Language = ctx.Locale.Language()
if err := user_model.UpdateUserCols(db.DefaultContext, u, "language"); err != nil { if err := user_model.UpdateUserCols(db.DefaultContext, 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 + "/" return setting.AppSubURL + "/"
} }
} }
@ -779,6 +805,11 @@ func getUserName(gothUser *goth.User) string {
} }
func showLinkingLogin(ctx *context.Context, gothUser goth.User) { 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 { if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
log.Error("Error setting linkAccountGothUser in session: %v", err) log.Error("Error setting linkAccountGothUser in session: %v", err)
} }
@ -822,6 +853,12 @@ func handleOAuth2SignIn(ctx *context.Context, source *login.Source, u *user_mode
// If this user is enrolled in 2FA and this source doesn't override it, // If this user is enrolled in 2FA and this source doesn't override it,
// we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page. // we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page.
if !needs2FA { if !needs2FA {
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
ctx.ServerError("RegenerateSession", err)
return
}
// Set session IDs
if err := ctx.Session.Set("uid", u.ID); err != nil { if err := ctx.Session.Set("uid", u.ID); err != nil {
log.Error("Error setting uid in session: %v", err) log.Error("Error setting uid in session: %v", err)
} }
@ -878,6 +915,11 @@ func handleOAuth2SignIn(ctx *context.Context, source *login.Source, u *user_mode
} }
} }
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. // User needs to use 2FA, save data and redirect to 2FA page.
if err := ctx.Session.Set("twofaUid", u.ID); err != nil { if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
log.Error("Error setting twofaUid in session: %v", err) 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
return 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. // User needs to use 2FA, save data and redirect to 2FA page.
if err := ctx.Session.Set("twofaUid", u.ID); err != nil { if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
log.Error("Error setting twofaUid in session: %v", err) log.Error("Error setting twofaUid in session: %v", err)
@ -1227,7 +1274,7 @@ func LinkAccountPostRegister(ctx *context.Context) {
return return
} }
ctx.Redirect(setting.AppSubURL + "/user/login") handleSignIn(ctx, u, false)
} }
// HandleSignOut resets the session and sets the cookies // HandleSignOut resets the session and sets the cookies
@ -1370,7 +1417,7 @@ func SignUpPost(ctx *context.Context) {
} }
ctx.Flash.Success(ctx.Tr("auth.sign_up_successful")) ctx.Flash.Success(ctx.Tr("auth.sign_up_successful"))
handleSignInFull(ctx, u, false, true) handleSignIn(ctx, u, false)
} }
// createAndHandleCreatedUser calls createUserInContext and // createAndHandleCreatedUser calls createUserInContext and
@ -1591,6 +1638,13 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
log.Trace("User activated: %s", user.Name) 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 { if err := ctx.Session.Set("uid", user.ID); err != nil {
log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err) log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
} }
@ -1862,11 +1916,14 @@ func ResetPasswdPost(ctx *context.Context) {
handleSignInFull(ctx, u, remember, false) handleSignInFull(ctx, u, remember, false)
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used")) ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
if ctx.Written() {
return
}
ctx.Redirect(setting.AppSubURL + "/user/settings/security") ctx.Redirect(setting.AppSubURL + "/user/settings/security")
return return
} }
handleSignInFull(ctx, u, remember, true) handleSignIn(ctx, u, remember)
} }
// MustChangePassword renders the page to change a user's password // MustChangePassword renders the page to change a user's password

@ -16,6 +16,7 @@ import (
"code.gitea.io/gitea/modules/hcaptcha" "code.gitea.io/gitea/modules/hcaptcha"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/recaptcha"
"code.gitea.io/gitea/modules/session"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web"
@ -232,6 +233,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 { if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err) log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
} }

@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/session"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/modules/web/middleware"
) )
@ -106,6 +107,14 @@ func isGitRawReleaseOrLFSPath(req *http.Request) bool {
// handleSignIn clears existing session variables and stores new ones for the specified user object // 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 *user_model.User) { func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *user_model.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_verified_uri")
_ = sess.Delete("openid_signin_remember") _ = sess.Delete("openid_signin_remember")
_ = sess.Delete("openid_determined_email") _ = sess.Delete("openid_determined_email")
@ -114,7 +123,7 @@ func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore
_ = sess.Delete("twofaRemember") _ = sess.Delete("twofaRemember")
_ = sess.Delete("u2fChallenge") _ = sess.Delete("u2fChallenge")
_ = sess.Delete("linkAccount") _ = sess.Delete("linkAccount")
err := sess.Set("uid", user.ID) err = sess.Set("uid", user.ID)
if err != nil { if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err)) log.Error(fmt.Sprintf("Error setting session: %v", err))
} }

@ -55,6 +55,7 @@ func (st *SessionsStore) getOrNew(r *http.Request, name string, override bool) (
} }
} }
session.IsNew = override
session.ID = chiStore.ID() // Simply copy the session id from the chi store session.ID = chiStore.ID() // Simply copy the session id from the chi store
return session, chiStore.Set(name, session) return session, chiStore.Set(name, session)
@ -64,6 +65,11 @@ func (st *SessionsStore) getOrNew(r *http.Request, name string, override bool) (
func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *sessions.Session) error { func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *sessions.Session) error {
chiStore := chiSession.GetSession(r) chiStore := chiSession.GetSession(r)
if session.IsNew {
_, _ = chiSession.RegenerateSession(w, r)
session.IsNew = false
}
if err := chiStore.Set(session.Name(), session); err != nil { if err := chiStore.Set(session.Name(), session); err != nil {
return err return err
} }

@ -260,7 +260,7 @@ func Sessioner(options ...Options) func(next http.Handler) http.Handler {
return return
} }
if err = sess.Release(); err != nil { if err = s.RawStore.Release(); err != nil {
panic("session(release): " + err.Error()) panic("session(release): " + err.Error())
} }
}) })
@ -274,6 +274,26 @@ func GetSession(req *http.Request) Store {
return sess return sess
} }
// RegenerateSession
func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) {
sess, ok := GetSession(req).(*store)
if !ok {
return nil, fmt.Errorf("no session in request context")
}
oldRawStore := sess.RawStore
if err := oldRawStore.Release(); err != nil {
return nil, err
}
store, err := sess.RegenerateID(resp, req)
if err != nil {
return nil, err
}
sess.RawStore = store
return sess, nil
}
// Provider is the interface that provides session manipulations. // Provider is the interface that provides session manipulations.
type Provider interface { type Provider interface {
// Init initializes session provider. // Init initializes session provider.

@ -18,7 +18,7 @@ gitea.com/go-chi/cache/memcache
# gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5 # gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5
## explicit ## explicit
gitea.com/go-chi/captcha gitea.com/go-chi/captcha
# gitea.com/go-chi/session v0.0.0-20211013065435-7d334f340c09 # gitea.com/go-chi/session v0.0.0-20211218221615-e3605d8b28b8
## explicit ## explicit
gitea.com/go-chi/session gitea.com/go-chi/session
gitea.com/go-chi/session/couchbase gitea.com/go-chi/session/couchbase

Loading…
Cancel
Save