Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset Session ID on login #18018

Merged
merged 10 commits into from
Dec 20, 2021
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
gitea.com/go-chi/binding v0.0.0-20211013065440-d16dc407c2be
gitea.com/go-chi/cache v0.0.0-20211013020926-78790b11abf1
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
github.com/42wim/sshsig v0.0.0-20211121163825-841cf5bbc121
github.com/Microsoft/go-winio v0.5.0 // indirect
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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/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/session v0.0.0-20211013065435-7d334f340c09 h1:1+K+6uOXrnSfLHXvu8jpuoNEWA3/NULOlLSRZwaij+c=
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 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=
Expand Down
12 changes: 12 additions & 0 deletions modules/session/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
67 changes: 62 additions & 5 deletions routers/web/user/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -90,6 +91,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
Expand Down Expand Up @@ -256,6 +261,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 will need to use 2FA TOTP or U2F, save data
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
Expand Down Expand Up @@ -419,6 +429,9 @@ func TwoFactorScratchPost(ctx *context.Context) {
}

handleSignInFull(ctx, u, remember, false)
if ctx.Written() {
zeripath marked this conversation as resolved.
Show resolved Hide resolved
return
}
ctx.Flash.Info(ctx.Tr("auth.twofa_scratch_used"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
return
Expand Down Expand Up @@ -526,6 +539,9 @@ func U2FSign(ctx *context.Context) {
}
}
redirect := handleSignInFull(ctx, user, remember, false)
if ctx.Written() {
return
}
if redirect == "" {
redirect = setting.AppSubURL + "/"
}
Expand All @@ -538,7 +554,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 *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 {
Expand All @@ -549,6 +569,12 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
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")
Expand All @@ -572,7 +598,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
if len(u.Language) == 0 {
u.Language = ctx.Locale.Language()
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 + "/"
}
}
Expand Down Expand Up @@ -779,6 +805,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)
}
Expand Down Expand Up @@ -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,
// we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page.
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 {
log.Error("Error setting uid in session: %v", err)
}
Expand Down Expand Up @@ -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.
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
log.Error("Error setting twofaUid in session: %v", err)
Expand Down Expand Up @@ -1090,6 +1132,11 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r
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)
Expand Down Expand Up @@ -1227,7 +1274,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
Expand Down Expand Up @@ -1370,7 +1417,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
Expand Down Expand Up @@ -1591,6 +1638,13 @@ func handleAccountActivation(ctx *context.Context, user *user_model.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)
}
Expand Down Expand Up @@ -1862,11 +1916,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
Expand Down
6 changes: 6 additions & 0 deletions routers/web/user/auth_openid.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
}
Expand Down
11 changes: 10 additions & 1 deletion services/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"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"
)
Expand Down Expand Up @@ -106,6 +107,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 *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))
zeripath marked this conversation as resolved.
Show resolved Hide resolved
} else {
sess = newSess
}

_ = sess.Delete("openid_verified_uri")
_ = sess.Delete("openid_signin_remember")
_ = sess.Delete("openid_determined_email")
Expand All @@ -114,7 +123,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))
}
Expand Down
6 changes: 6 additions & 0 deletions services/auth/source/oauth2/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

return session, chiStore.Set(name, session)
Expand All @@ -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 {
chiStore := chiSession.GetSession(r)

if session.IsNew {
_, _ = chiSession.RegenerateSession(w, r)
session.IsNew = false
}

if err := chiStore.Set(session.Name(), session); err != nil {
return err
}
Expand Down
22 changes: 21 additions & 1 deletion vendor/gitea.com/go-chi/session/session.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ gitea.com/go-chi/cache/memcache
# gitea.com/go-chi/captcha v0.0.0-20211013065431-70641c1a35d5
## explicit
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
gitea.com/go-chi/session
gitea.com/go-chi/session/couchbase
Expand Down