From 71fb03d3247c01e3e0dd09950b91e4febafdf5d3 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 9 Nov 2022 14:44:05 +0800 Subject: [PATCH 1/5] refactor: updateSession --- routers/web/auth/auth.go | 114 ++++++++++++++------------------ routers/web/auth/linkaccount.go | 21 ++---- routers/web/auth/oauth.go | 44 ++++-------- routers/web/auth/openid.go | 26 ++------ 4 files changed, 73 insertions(+), 132 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 25d70d7c47818..e67afcacc068d 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -82,21 +82,13 @@ func AutoSignIn(ctx *context.Context) (bool, error) { isSucceed = true - if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "uid": u.ID, + "uname": u.Name, + }); 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 - } - if err := ctx.Session.Set("uname", u.Name); err != nil { - return false, err - } - if err := ctx.Session.Release(); err != nil { - return false, err - } - if err := resetLocale(ctx, u); err != nil { return false, err } @@ -252,32 +244,15 @@ 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 + updates := map[interface{}]interface{}{ + "twofaUid": u.ID, + "twofaRemember": form.Remember, } - - // User will need to use 2FA TOTP or WebAuthn, save data - if err := ctx.Session.Set("twofaUid", u.ID); err != nil { - ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err) - return - } - - if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil { - ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err) - return - } - if hasTOTPtwofa { - // User will need to use WebAuthn, save data - if err := ctx.Session.Set("totpEnrolled", u.ID); err != nil { - ctx.ServerError("UserSignIn: Unable to set WebAuthn Enrolled in session", err) - return - } + updates["totpEnrolled"] = u.ID } - - if err := ctx.Session.Release(); err != nil { - ctx.ServerError("UserSignIn: Unable to save session", err) + if err := regenerateSession(ctx, nil, updates); err != nil { + ctx.ServerError("UserSignIn: Unable to set regenerate session", err) return } @@ -308,29 +283,23 @@ 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 { + if err := regenerateSession(ctx, []interface{}{ + // Delete the openid, 2fa and linkaccount data + "openid_verified_uri", + "openid_signin_remember", + "openid_determined_email", + "openid_determined_username", + "twofaUid", + "twofaRemember", + "linkAccount", + }, map[interface{}]interface{}{ + "uid": u.ID, + "uname": u.Name, + }); 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") - _ = ctx.Session.Delete("openid_determined_username") - _ = ctx.Session.Delete("twofaUid") - _ = ctx.Session.Delete("twofaRemember") - _ = ctx.Session.Delete("linkAccount") - if err := ctx.Session.Set("uid", u.ID); err != nil { - log.Error("Error setting uid %d in session: %v", u.ID, err) - } - if err := ctx.Session.Set("uname", u.Name); err != nil { - log.Error("Error setting uname %s session: %v", u.Name, err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Unable to store session: %v", err) - } - // Language setting of the user overwrites the one previously set // If the user does not have a locale set, we save the current one. if len(u.Language) == 0 { @@ -762,22 +731,15 @@ 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 { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "uid": user.ID, + "uname": user.Name, + }); err != nil { log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err) ctx.ServerError("ActivateUserEmail", err) return } - if err := ctx.Session.Set("uid", user.ID); err != nil { - log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err) - } - if err := ctx.Session.Set("uname", user.Name); err != nil { - log.Error("Error setting uname in session[%s]: %v", ctx.Session.ID(), err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session[%s]: %v", ctx.Session.ID(), err) - } - if err := resetLocale(ctx, user); err != nil { ctx.ServerError("resetLocale", err) return @@ -814,3 +776,25 @@ func ActivateEmail(ctx *context.Context) { // Should users be logged in automatically here? (consider 2FA requirements, etc.) ctx.Redirect(setting.AppSubURL + "/user/settings/account") } + +func regenerateSession(ctx *context.Context, deletes []interface{}, updates map[interface{}]interface{}) error { + if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { + return fmt.Errorf("regenerate session: %w", err) + } + sess := ctx.Session + sessID := sess.ID() + for _, v := range deletes { + if err := sess.Delete(v); err != nil { + return fmt.Errorf("delete %v in session[%s]: %v", k, sessID, err) + } + } + for k, v := range updates { + if err := sess.Set(k, v); err != nil { + return fmt.Errorf("set %v in session[%s]: %v", k, sessID, err) + } + } + if err := sess.Release(); err != nil { + return fmt.Errorf("store session[%s]: %w", sessID, err) + } + return nil +} diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index 4f3f2062b6896..af6809125db94 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/mcaptcha" "code.gitea.io/gitea/modules/recaptcha" - "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/web" auth_service "code.gitea.io/gitea/services/auth" @@ -156,25 +155,15 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r return } - if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "twofaUid": u.ID, + "twofaRemember": remember, + "linkAccount": true, + }); 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) - } - if err := ctx.Session.Set("twofaRemember", remember); err != nil { - log.Error("Error setting twofaRemember in session: %v", err) - } - if err := ctx.Session.Set("linkAccount", true); err != nil { - log.Error("Error setting linkAccount in session: %v", err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session: %v", err) - } - // If WebAuthn is enrolled -> Redirect to WebAuthn instead regs, err := auth.GetWebAuthnCredentialsByUID(u.ID) if err == nil && len(regs) > 0 { diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 4fba8d8e8c31a..2d59a915311ea 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -22,7 +22,6 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" @@ -1027,17 +1026,12 @@ func setUserGroupClaims(loginSource *auth.Source, u *user_model.User, gothUser * } func showLinkingLogin(ctx *context.Context, gothUser goth.User) { - if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "linkAccountGothUser": gothUser, + }); 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) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session: %v", err) - } ctx.Redirect(setting.AppSubURL + "/user/link_account") } @@ -1075,21 +1069,14 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // 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 { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "uid": u.ID, + "uname": u.Name, + }); 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) - } - if err := ctx.Session.Set("uname", u.Name); err != nil { - log.Error("Error setting uname in session: %v", err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session: %v", err) - } - // Clear whatever CSRF cookie has right now, force to generate a new one middleware.DeleteCSRFCookie(ctx.Resp) @@ -1138,22 +1125,15 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model } } - if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + // User needs to use 2FA, save data and redirect to 2FA page. + "twofaUid": u.ID, + "twofaRemember": false, + }); 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) - } - if err := ctx.Session.Set("twofaRemember", false); err != nil { - log.Error("Error setting twofaRemember in session: %v", err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session: %v", err) - } - // If WebAuthn is enrolled -> Redirect to WebAuthn instead regs, err := auth.GetWebAuthnCredentialsByUID(u.ID) if err == nil && len(regs) > 0 { diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index 3b1065189d9dc..add3c3ca6dd06 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/mcaptcha" "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" @@ -232,27 +231,16 @@ 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) - } - if err := ctx.Session.Set("openid_determined_email", email); err != nil { - log.Error("signInOpenIDVerify: Could not set openid_determined_email in session: %v", err) - } - if u != nil { nickname = u.LowerName } - - if err := ctx.Session.Set("openid_determined_username", nickname); err != nil { - log.Error("signInOpenIDVerify: Could not set openid_determined_username in session: %v", err) - } - if err := ctx.Session.Release(); err != nil { - log.Error("signInOpenIDVerify: Unable to save changes to the session: %v", err) + if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + "openid_verified_uri": id, + "openid_determined_email": email, + "openid_determined_username": nickname, + }); err != nil { + ctx.ServerError("RegenerateSession", err) + return } if u != nil || !setting.Service.EnableOpenIDSignUp || setting.Service.AllowOnlyInternalRegistration { From e2e570a137637c827484fa594dd6e32cc381f582 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 9 Nov 2022 15:09:09 +0800 Subject: [PATCH 2/5] fix: rename to updateSession --- routers/web/auth/auth.go | 14 +++++++------- routers/web/auth/linkaccount.go | 2 +- routers/web/auth/oauth.go | 6 +++--- routers/web/auth/openid.go | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index e67afcacc068d..1e6df2b239e49 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -82,7 +82,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { isSucceed = true - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "uid": u.ID, "uname": u.Name, }); err != nil { @@ -251,7 +251,7 @@ func SignInPost(ctx *context.Context) { if hasTOTPtwofa { updates["totpEnrolled"] = u.ID } - if err := regenerateSession(ctx, nil, updates); err != nil { + if err := updateSession(ctx, nil, updates); err != nil { ctx.ServerError("UserSignIn: Unable to set regenerate session", err) return } @@ -283,7 +283,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe setting.CookieRememberName, u.Name, days) } - if err := regenerateSession(ctx, []interface{}{ + if err := updateSession(ctx, []interface{}{ // Delete the openid, 2fa and linkaccount data "openid_verified_uri", "openid_signin_remember", @@ -731,7 +731,7 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) { log.Trace("User activated: %s", user.Name) - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "uid": user.ID, "uname": user.Name, }); err != nil { @@ -777,14 +777,14 @@ func ActivateEmail(ctx *context.Context) { ctx.Redirect(setting.AppSubURL + "/user/settings/account") } -func regenerateSession(ctx *context.Context, deletes []interface{}, updates map[interface{}]interface{}) error { +func updateSession(ctx *context.Context, deletes []interface{}, updates map[interface{}]interface{}) error { if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { return fmt.Errorf("regenerate session: %w", err) } sess := ctx.Session sessID := sess.ID() - for _, v := range deletes { - if err := sess.Delete(v); err != nil { + for _, k := range deletes { + if err := sess.Delete(k); err != nil { return fmt.Errorf("delete %v in session[%s]: %v", k, sessID, err) } } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index af6809125db94..e7cbc91a8fdde 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -155,7 +155,7 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r return } - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "twofaUid": u.ID, "twofaRemember": remember, "linkAccount": true, diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 2d59a915311ea..30272d27cab38 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1026,7 +1026,7 @@ func setUserGroupClaims(loginSource *auth.Source, u *user_model.User, gothUser * } func showLinkingLogin(ctx *context.Context, gothUser goth.User) { - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "linkAccountGothUser": gothUser, }); err != nil { ctx.ServerError("RegenerateSession", err) @@ -1069,7 +1069,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // 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 := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "uid": u.ID, "uname": u.Name, }); err != nil { @@ -1125,7 +1125,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model } } - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, "twofaRemember": false, diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index add3c3ca6dd06..24e09f2f7da8a 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -234,7 +234,7 @@ func signInOpenIDVerify(ctx *context.Context) { if u != nil { nickname = u.LowerName } - if err := regenerateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[interface{}]interface{}{ "openid_verified_uri": id, "openid_determined_email": email, "openid_determined_username": nickname, From 589e973e417025eb867b443431993c880cdf1e41 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 9 Nov 2022 15:19:06 +0800 Subject: [PATCH 3/5] fix: keep comments --- routers/web/auth/auth.go | 3 +++ routers/web/auth/linkaccount.go | 1 + 2 files changed, 4 insertions(+) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 1e6df2b239e49..405a382229a4f 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -83,6 +83,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { isSucceed = true if err := updateSession(ctx, nil, map[interface{}]interface{}{ + // Set session IDs "uid": u.ID, "uname": u.Name, }); err != nil { @@ -245,10 +246,12 @@ func SignInPost(ctx *context.Context) { } updates := map[interface{}]interface{}{ + // User will need to use 2FA TOTP or WebAuthn, save data "twofaUid": u.ID, "twofaRemember": form.Remember, } if hasTOTPtwofa { + // User will need to use WebAuthn, save data updates["totpEnrolled"] = u.ID } if err := updateSession(ctx, nil, updates); err != nil { diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index e7cbc91a8fdde..9f24161cc766b 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -156,6 +156,7 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r } if err := updateSession(ctx, nil, map[interface{}]interface{}{ + // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, "twofaRemember": remember, "linkAccount": true, From c17a8e63245d71a248f336de853d17e248bde11c Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 9 Nov 2022 16:25:31 +0800 Subject: [PATCH 4/5] fix: use string as key --- routers/web/auth/auth.go | 12 ++++++------ routers/web/auth/linkaccount.go | 2 +- routers/web/auth/oauth.go | 6 +++--- routers/web/auth/openid.go | 2 +- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 405a382229a4f..cc096467c4b67 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -82,7 +82,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { isSucceed = true - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ // Set session IDs "uid": u.ID, "uname": u.Name, @@ -245,7 +245,7 @@ func SignInPost(ctx *context.Context) { return } - updates := map[interface{}]interface{}{ + updates := map[string]interface{}{ // User will need to use 2FA TOTP or WebAuthn, save data "twofaUid": u.ID, "twofaRemember": form.Remember, @@ -286,7 +286,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe setting.CookieRememberName, u.Name, days) } - if err := updateSession(ctx, []interface{}{ + if err := updateSession(ctx, []string{ // Delete the openid, 2fa and linkaccount data "openid_verified_uri", "openid_signin_remember", @@ -295,7 +295,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe "twofaUid", "twofaRemember", "linkAccount", - }, map[interface{}]interface{}{ + }, map[string]interface{}{ "uid": u.ID, "uname": u.Name, }); err != nil { @@ -734,7 +734,7 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) { log.Trace("User activated: %s", user.Name) - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ "uid": user.ID, "uname": user.Name, }); err != nil { @@ -780,7 +780,7 @@ func ActivateEmail(ctx *context.Context) { ctx.Redirect(setting.AppSubURL + "/user/settings/account") } -func updateSession(ctx *context.Context, deletes []interface{}, updates map[interface{}]interface{}) error { +func updateSession(ctx *context.Context, deletes []string, updates map[string]interface{}) error { if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil { return fmt.Errorf("regenerate session: %w", err) } diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index 9f24161cc766b..d3211eaa5c70a 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -155,7 +155,7 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r return } - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, "twofaRemember": remember, diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 30272d27cab38..af3a30d1a10f6 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1026,7 +1026,7 @@ func setUserGroupClaims(loginSource *auth.Source, u *user_model.User, gothUser * } func showLinkingLogin(ctx *context.Context, gothUser goth.User) { - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ "linkAccountGothUser": gothUser, }); err != nil { ctx.ServerError("RegenerateSession", err) @@ -1069,7 +1069,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model // 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 := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ "uid": u.ID, "uname": u.Name, }); err != nil { @@ -1125,7 +1125,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model } } - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ // User needs to use 2FA, save data and redirect to 2FA page. "twofaUid": u.ID, "twofaRemember": false, diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index 24e09f2f7da8a..daefca3bbe2f9 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -234,7 +234,7 @@ func signInOpenIDVerify(ctx *context.Context) { if u != nil { nickname = u.LowerName } - if err := updateSession(ctx, nil, map[interface{}]interface{}{ + if err := updateSession(ctx, nil, map[string]interface{}{ "openid_verified_uri": id, "openid_determined_email": email, "openid_determined_username": nickname, From 0fe6fd59223c252cc471410ebe5730e15a2d2fcb Mon Sep 17 00:00:00 2001 From: Jason Song Date: Thu, 10 Nov 2022 18:25:49 +0800 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: delvh --- routers/web/auth/auth.go | 8 ++++---- routers/web/auth/oauth.go | 6 +++--- routers/web/auth/openid.go | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index c48895e683e8b..2919fd351366d 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -87,7 +87,7 @@ func AutoSignIn(ctx *context.Context) (bool, error) { "uid": u.ID, "uname": u.Name, }); err != nil { - return false, fmt.Errorf("unable to RegenerateSession: Error: %w", err) + return false, fmt.Errorf("unable to updateSession: %w", err) } if err := resetLocale(ctx, u); err != nil { @@ -255,7 +255,7 @@ func SignInPost(ctx *context.Context) { updates["totpEnrolled"] = u.ID } if err := updateSession(ctx, nil, updates); err != nil { - ctx.ServerError("UserSignIn: Unable to set regenerate session", err) + ctx.ServerError("UserSignIn: Unable to update session", err) return } @@ -795,12 +795,12 @@ func updateSession(ctx *context.Context, deletes []string, updates map[string]in sessID := sess.ID() for _, k := range deletes { if err := sess.Delete(k); err != nil { - return fmt.Errorf("delete %v in session[%s]: %v", k, sessID, err) + return fmt.Errorf("delete %v in session[%s]: %w", k, sessID, err) } } for k, v := range updates { if err := sess.Set(k, v); err != nil { - return fmt.Errorf("set %v in session[%s]: %v", k, sessID, err) + return fmt.Errorf("set %v in session[%s]: %w", k, sessID, err) } } if err := sess.Release(); err != nil { diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index af3a30d1a10f6..d845f21a33012 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -1029,7 +1029,7 @@ func showLinkingLogin(ctx *context.Context, gothUser goth.User) { if err := updateSession(ctx, nil, map[string]interface{}{ "linkAccountGothUser": gothUser, }); err != nil { - ctx.ServerError("RegenerateSession", err) + ctx.ServerError("updateSession", err) return } ctx.Redirect(setting.AppSubURL + "/user/link_account") @@ -1073,7 +1073,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model "uid": u.ID, "uname": u.Name, }); err != nil { - ctx.ServerError("RegenerateSession", err) + ctx.ServerError("updateSession", err) return } @@ -1130,7 +1130,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model "twofaUid": u.ID, "twofaRemember": false, }); err != nil { - ctx.ServerError("RegenerateSession", err) + ctx.ServerError("updateSession", err) return } diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index daefca3bbe2f9..d34f4db7c0144 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -239,7 +239,7 @@ func signInOpenIDVerify(ctx *context.Context) { "openid_determined_email": email, "openid_determined_username": nickname, }); err != nil { - ctx.ServerError("RegenerateSession", err) + ctx.ServerError("updateSession", err) return }