Skip to content

Commit 1d22911

Browse files
wolfogrelunnyzeripathdelvh
authored
Extract updateSession function to reduce repetition (#21735)
A simple refactor to reduce duplicate codes. Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: zeripath <[email protected]> Co-authored-by: delvh <[email protected]>
1 parent 385462d commit 1d22911

File tree

4 files changed

+80
-135
lines changed

4 files changed

+80
-135
lines changed

Diff for: ‎routers/web/auth/auth.go

+52-65
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,12 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
8282

8383
isSucceed = true
8484

85-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
86-
return false, fmt.Errorf("unable to RegenerateSession: Error: %w", err)
87-
}
88-
89-
// Set session IDs
90-
if err := ctx.Session.Set("uid", u.ID); err != nil {
91-
return false, err
92-
}
93-
if err := ctx.Session.Set("uname", u.Name); err != nil {
94-
return false, err
95-
}
96-
if err := ctx.Session.Release(); err != nil {
97-
return false, err
85+
if err := updateSession(ctx, nil, map[string]interface{}{
86+
// Set session IDs
87+
"uid": u.ID,
88+
"uname": u.Name,
89+
}); err != nil {
90+
return false, fmt.Errorf("unable to updateSession: %w", err)
9891
}
9992

10093
if err := resetLocale(ctx, u); err != nil {
@@ -252,32 +245,17 @@ func SignInPost(ctx *context.Context) {
252245
return
253246
}
254247

255-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
256-
ctx.ServerError("UserSignIn: Unable to set regenerate session", err)
257-
return
258-
}
259-
260-
// User will need to use 2FA TOTP or WebAuthn, save data
261-
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
262-
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
263-
return
264-
}
265-
266-
if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil {
267-
ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err)
268-
return
248+
updates := map[string]interface{}{
249+
// User will need to use 2FA TOTP or WebAuthn, save data
250+
"twofaUid": u.ID,
251+
"twofaRemember": form.Remember,
269252
}
270-
271253
if hasTOTPtwofa {
272254
// User will need to use WebAuthn, save data
273-
if err := ctx.Session.Set("totpEnrolled", u.ID); err != nil {
274-
ctx.ServerError("UserSignIn: Unable to set WebAuthn Enrolled in session", err)
275-
return
276-
}
255+
updates["totpEnrolled"] = u.ID
277256
}
278-
279-
if err := ctx.Session.Release(); err != nil {
280-
ctx.ServerError("UserSignIn: Unable to save session", err)
257+
if err := updateSession(ctx, nil, updates); err != nil {
258+
ctx.ServerError("UserSignIn: Unable to update session", err)
281259
return
282260
}
283261

@@ -308,29 +286,23 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
308286
setting.CookieRememberName, u.Name, days)
309287
}
310288

311-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
289+
if err := updateSession(ctx, []string{
290+
// Delete the openid, 2fa and linkaccount data
291+
"openid_verified_uri",
292+
"openid_signin_remember",
293+
"openid_determined_email",
294+
"openid_determined_username",
295+
"twofaUid",
296+
"twofaRemember",
297+
"linkAccount",
298+
}, map[string]interface{}{
299+
"uid": u.ID,
300+
"uname": u.Name,
301+
}); err != nil {
312302
ctx.ServerError("RegenerateSession", err)
313303
return setting.AppSubURL + "/"
314304
}
315305

316-
// Delete the openid, 2fa and linkaccount data
317-
_ = ctx.Session.Delete("openid_verified_uri")
318-
_ = ctx.Session.Delete("openid_signin_remember")
319-
_ = ctx.Session.Delete("openid_determined_email")
320-
_ = ctx.Session.Delete("openid_determined_username")
321-
_ = ctx.Session.Delete("twofaUid")
322-
_ = ctx.Session.Delete("twofaRemember")
323-
_ = ctx.Session.Delete("linkAccount")
324-
if err := ctx.Session.Set("uid", u.ID); err != nil {
325-
log.Error("Error setting uid %d in session: %v", u.ID, err)
326-
}
327-
if err := ctx.Session.Set("uname", u.Name); err != nil {
328-
log.Error("Error setting uname %s session: %v", u.Name, err)
329-
}
330-
if err := ctx.Session.Release(); err != nil {
331-
log.Error("Unable to store session: %v", err)
332-
}
333-
334306
// Language setting of the user overwrites the one previously set
335307
// If the user does not have a locale set, we save the current one.
336308
if len(u.Language) == 0 {
@@ -762,22 +734,15 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
762734

763735
log.Trace("User activated: %s", user.Name)
764736

765-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
737+
if err := updateSession(ctx, nil, map[string]interface{}{
738+
"uid": user.ID,
739+
"uname": user.Name,
740+
}); err != nil {
766741
log.Error("Unable to regenerate session for user: %-v with email: %s: %v", user, user.Email, err)
767742
ctx.ServerError("ActivateUserEmail", err)
768743
return
769744
}
770745

771-
if err := ctx.Session.Set("uid", user.ID); err != nil {
772-
log.Error("Error setting uid in session[%s]: %v", ctx.Session.ID(), err)
773-
}
774-
if err := ctx.Session.Set("uname", user.Name); err != nil {
775-
log.Error("Error setting uname in session[%s]: %v", ctx.Session.ID(), err)
776-
}
777-
if err := ctx.Session.Release(); err != nil {
778-
log.Error("Error storing session[%s]: %v", ctx.Session.ID(), err)
779-
}
780-
781746
if err := resetLocale(ctx, user); err != nil {
782747
ctx.ServerError("resetLocale", err)
783748
return
@@ -821,3 +786,25 @@ func ActivateEmail(ctx *context.Context) {
821786
// Should users be logged in automatically here? (consider 2FA requirements, etc.)
822787
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
823788
}
789+
790+
func updateSession(ctx *context.Context, deletes []string, updates map[string]interface{}) error {
791+
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
792+
return fmt.Errorf("regenerate session: %w", err)
793+
}
794+
sess := ctx.Session
795+
sessID := sess.ID()
796+
for _, k := range deletes {
797+
if err := sess.Delete(k); err != nil {
798+
return fmt.Errorf("delete %v in session[%s]: %w", k, sessID, err)
799+
}
800+
}
801+
for k, v := range updates {
802+
if err := sess.Set(k, v); err != nil {
803+
return fmt.Errorf("set %v in session[%s]: %w", k, sessID, err)
804+
}
805+
}
806+
if err := sess.Release(); err != nil {
807+
return fmt.Errorf("store session[%s]: %w", sessID, err)
808+
}
809+
return nil
810+
}

Diff for: ‎routers/web/auth/linkaccount.go

+6-16
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"code.gitea.io/gitea/modules/log"
1919
"code.gitea.io/gitea/modules/mcaptcha"
2020
"code.gitea.io/gitea/modules/recaptcha"
21-
"code.gitea.io/gitea/modules/session"
2221
"code.gitea.io/gitea/modules/setting"
2322
"code.gitea.io/gitea/modules/web"
2423
auth_service "code.gitea.io/gitea/services/auth"
@@ -156,25 +155,16 @@ func linkAccount(ctx *context.Context, u *user_model.User, gothUser goth.User, r
156155
return
157156
}
158157

159-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
158+
if err := updateSession(ctx, nil, map[string]interface{}{
159+
// User needs to use 2FA, save data and redirect to 2FA page.
160+
"twofaUid": u.ID,
161+
"twofaRemember": remember,
162+
"linkAccount": true,
163+
}); err != nil {
160164
ctx.ServerError("RegenerateSession", err)
161165
return
162166
}
163167

164-
// User needs to use 2FA, save data and redirect to 2FA page.
165-
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
166-
log.Error("Error setting twofaUid in session: %v", err)
167-
}
168-
if err := ctx.Session.Set("twofaRemember", remember); err != nil {
169-
log.Error("Error setting twofaRemember in session: %v", err)
170-
}
171-
if err := ctx.Session.Set("linkAccount", true); err != nil {
172-
log.Error("Error setting linkAccount in session: %v", err)
173-
}
174-
if err := ctx.Session.Release(); err != nil {
175-
log.Error("Error storing session: %v", err)
176-
}
177-
178168
// If WebAuthn is enrolled -> Redirect to WebAuthn instead
179169
regs, err := auth.GetWebAuthnCredentialsByUID(u.ID)
180170
if err == nil && len(regs) > 0 {

Diff for: ‎routers/web/auth/oauth.go

+15-35
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"code.gitea.io/gitea/modules/context"
2323
"code.gitea.io/gitea/modules/json"
2424
"code.gitea.io/gitea/modules/log"
25-
"code.gitea.io/gitea/modules/session"
2625
"code.gitea.io/gitea/modules/setting"
2726
"code.gitea.io/gitea/modules/timeutil"
2827
"code.gitea.io/gitea/modules/util"
@@ -1027,17 +1026,12 @@ func setUserGroupClaims(loginSource *auth.Source, u *user_model.User, gothUser *
10271026
}
10281027

10291028
func showLinkingLogin(ctx *context.Context, gothUser goth.User) {
1030-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1031-
ctx.ServerError("RegenerateSession", err)
1029+
if err := updateSession(ctx, nil, map[string]interface{}{
1030+
"linkAccountGothUser": gothUser,
1031+
}); err != nil {
1032+
ctx.ServerError("updateSession", err)
10321033
return
10331034
}
1034-
1035-
if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
1036-
log.Error("Error setting linkAccountGothUser in session: %v", err)
1037-
}
1038-
if err := ctx.Session.Release(); err != nil {
1039-
log.Error("Error storing session: %v", err)
1040-
}
10411035
ctx.Redirect(setting.AppSubURL + "/user/link_account")
10421036
}
10431037

@@ -1075,21 +1069,14 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
10751069
// If this user is enrolled in 2FA and this source doesn't override it,
10761070
// we can't sign the user in just yet. Instead, redirect them to the 2FA authentication page.
10771071
if !needs2FA {
1078-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1079-
ctx.ServerError("RegenerateSession", err)
1072+
if err := updateSession(ctx, nil, map[string]interface{}{
1073+
"uid": u.ID,
1074+
"uname": u.Name,
1075+
}); err != nil {
1076+
ctx.ServerError("updateSession", err)
10801077
return
10811078
}
10821079

1083-
if err := ctx.Session.Set("uid", u.ID); err != nil {
1084-
log.Error("Error setting uid in session: %v", err)
1085-
}
1086-
if err := ctx.Session.Set("uname", u.Name); err != nil {
1087-
log.Error("Error setting uname in session: %v", err)
1088-
}
1089-
if err := ctx.Session.Release(); err != nil {
1090-
log.Error("Error storing session: %v", err)
1091-
}
1092-
10931080
// Clear whatever CSRF cookie has right now, force to generate a new one
10941081
middleware.DeleteCSRFCookie(ctx.Resp)
10951082

@@ -1138,22 +1125,15 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
11381125
}
11391126
}
11401127

1141-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
1142-
ctx.ServerError("RegenerateSession", err)
1128+
if err := updateSession(ctx, nil, map[string]interface{}{
1129+
// User needs to use 2FA, save data and redirect to 2FA page.
1130+
"twofaUid": u.ID,
1131+
"twofaRemember": false,
1132+
}); err != nil {
1133+
ctx.ServerError("updateSession", err)
11431134
return
11441135
}
11451136

1146-
// User needs to use 2FA, save data and redirect to 2FA page.
1147-
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
1148-
log.Error("Error setting twofaUid in session: %v", err)
1149-
}
1150-
if err := ctx.Session.Set("twofaRemember", false); err != nil {
1151-
log.Error("Error setting twofaRemember in session: %v", err)
1152-
}
1153-
if err := ctx.Session.Release(); err != nil {
1154-
log.Error("Error storing session: %v", err)
1155-
}
1156-
11571137
// If WebAuthn is enrolled -> Redirect to WebAuthn instead
11581138
regs, err := auth.GetWebAuthnCredentialsByUID(u.ID)
11591139
if err == nil && len(regs) > 0 {

Diff for: ‎routers/web/auth/openid.go

+7-19
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"code.gitea.io/gitea/modules/log"
1818
"code.gitea.io/gitea/modules/mcaptcha"
1919
"code.gitea.io/gitea/modules/recaptcha"
20-
"code.gitea.io/gitea/modules/session"
2120
"code.gitea.io/gitea/modules/setting"
2221
"code.gitea.io/gitea/modules/util"
2322
"code.gitea.io/gitea/modules/web"
@@ -232,27 +231,16 @@ func signInOpenIDVerify(ctx *context.Context) {
232231
}
233232
}
234233

235-
if _, err := session.RegenerateSession(ctx.Resp, ctx.Req); err != nil {
236-
ctx.ServerError("RegenerateSession", err)
237-
return
238-
}
239-
240-
if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
241-
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
242-
}
243-
if err := ctx.Session.Set("openid_determined_email", email); err != nil {
244-
log.Error("signInOpenIDVerify: Could not set openid_determined_email in session: %v", err)
245-
}
246-
247234
if u != nil {
248235
nickname = u.LowerName
249236
}
250-
251-
if err := ctx.Session.Set("openid_determined_username", nickname); err != nil {
252-
log.Error("signInOpenIDVerify: Could not set openid_determined_username in session: %v", err)
253-
}
254-
if err := ctx.Session.Release(); err != nil {
255-
log.Error("signInOpenIDVerify: Unable to save changes to the session: %v", err)
237+
if err := updateSession(ctx, nil, map[string]interface{}{
238+
"openid_verified_uri": id,
239+
"openid_determined_email": email,
240+
"openid_determined_username": nickname,
241+
}); err != nil {
242+
ctx.ServerError("updateSession", err)
243+
return
256244
}
257245

258246
if u != nil || !setting.Service.EnableOpenIDSignUp || setting.Service.AllowOnlyInternalRegistration {

0 commit comments

Comments
 (0)