Skip to content

Commit 04e480d

Browse files
zeripathlafriks
zeripath
and
lafriks
authored
Whenever the ctx.Session is updated, release it to save it before sending the redirect (#11456) (#11457)
Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: Lauris BH <lauris@nix.lv> Co-authored-by: Lauris BH <lauris@nix.lv>
1 parent de9a96c commit 04e480d

File tree

6 files changed

+168
-100
lines changed

6 files changed

+168
-100
lines changed

‎routers/install.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ func InstallPost(ctx *context.Context, form auth.InstallForm) {
384384
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
385385
return
386386
}
387+
388+
if err = ctx.Session.Release(); err != nil {
389+
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
390+
return
391+
}
387392
}
388393

389394
log.Info("First-time run install finished!")

‎routers/user/auth.go‎

Lines changed: 65 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,18 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
8181
}
8282

8383
isSucceed = true
84-
err = ctx.Session.Set("uid", u.ID)
85-
if err != nil {
84+
85+
// Set session IDs
86+
if err := ctx.Session.Set("uid", u.ID); err != nil {
8687
return false, err
8788
}
88-
err = ctx.Session.Set("uname", u.Name)
89-
if err != nil {
89+
if err := ctx.Session.Set("uname", u.Name); err != nil {
90+
return false, err
91+
}
92+
if err := ctx.Session.Release(); err != nil {
9093
return false, err
9194
}
95+
9296
ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, setting.SessionConfig.Domain, setting.SessionConfig.Secure, true)
9397
return true, nil
9498
}
@@ -203,14 +207,16 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) {
203207
}
204208

205209
// User needs to use 2FA, save data and redirect to 2FA page.
206-
err = ctx.Session.Set("twofaUid", u.ID)
207-
if err != nil {
208-
ctx.ServerError("UserSignIn", err)
210+
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
211+
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
209212
return
210213
}
211-
err = ctx.Session.Set("twofaRemember", form.Remember)
212-
if err != nil {
213-
ctx.ServerError("UserSignIn", err)
214+
if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil {
215+
ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err)
216+
return
217+
}
218+
if err := ctx.Session.Release(); err != nil {
219+
ctx.ServerError("UserSignIn: Unable to save session", err)
214220
return
215221
}
216222

@@ -407,10 +413,14 @@ func U2FChallenge(ctx *context.Context) {
407413
ctx.ServerError("u2f.NewChallenge", err)
408414
return
409415
}
410-
if err = ctx.Session.Set("u2fChallenge", challenge); err != nil {
411-
ctx.ServerError("UserSignIn", err)
416+
if err := ctx.Session.Set("u2fChallenge", challenge); err != nil {
417+
ctx.ServerError("UserSignIn: unable to set u2fChallenge in session", err)
412418
return
413419
}
420+
if err := ctx.Session.Release(); err != nil {
421+
ctx.ServerError("UserSignIn: unable to store session", err)
422+
}
423+
414424
ctx.JSON(200, challenge.SignRequest(regs.ToRegistrations()))
415425
}
416426

@@ -494,13 +504,14 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR
494504
_ = ctx.Session.Delete("twofaRemember")
495505
_ = ctx.Session.Delete("u2fChallenge")
496506
_ = ctx.Session.Delete("linkAccount")
497-
err := ctx.Session.Set("uid", u.ID)
498-
if err != nil {
499-
log.Error(fmt.Sprintf("Error setting session: %v", err))
507+
if err := ctx.Session.Set("uid", u.ID); err != nil {
508+
log.Error("Error setting uid %d in session: %v", u.ID, err)
500509
}
501-
err = ctx.Session.Set("uname", u.Name)
502-
if err != nil {
503-
log.Error(fmt.Sprintf("Error setting session: %v", err))
510+
if err := ctx.Session.Set("uname", u.Name); err != nil {
511+
log.Error("Error setting uname %s session: %v", u.Name, err)
512+
}
513+
if err := ctx.Session.Release(); err != nil {
514+
log.Error("Unable to store session: %v", err)
504515
}
505516

506517
// Language setting of the user overwrites the one previously set
@@ -593,9 +604,11 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
593604

594605
if u == nil {
595606
// no existing user is found, request attach or new account
596-
err = ctx.Session.Set("linkAccountGothUser", gothUser)
597-
if err != nil {
598-
log.Error(fmt.Sprintf("Error setting session: %v", err))
607+
if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil {
608+
log.Error("Error setting linkAccountGothUser in session: %v", err)
609+
}
610+
if err := ctx.Session.Release(); err != nil {
611+
log.Error("Error storing session: %v", err)
599612
}
600613
ctx.Redirect(setting.AppSubURL + "/user/link_account")
601614
return
@@ -610,13 +623,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
610623
return
611624
}
612625

613-
err = ctx.Session.Set("uid", u.ID)
614-
if err != nil {
615-
log.Error(fmt.Sprintf("Error setting session: %v", err))
626+
if err := ctx.Session.Set("uid", u.ID); err != nil {
627+
log.Error("Error setting uid in session: %v", err)
616628
}
617-
err = ctx.Session.Set("uname", u.Name)
618-
if err != nil {
619-
log.Error(fmt.Sprintf("Error setting session: %v", err))
629+
if err := ctx.Session.Set("uname", u.Name); err != nil {
630+
log.Error("Error setting uname in session: %v", err)
631+
}
632+
if err := ctx.Session.Release(); err != nil {
633+
log.Error("Error storing session: %v", err)
620634
}
621635

622636
// Clear whatever CSRF has right now, force to generate a new one
@@ -645,13 +659,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
645659
}
646660

647661
// User needs to use 2FA, save data and redirect to 2FA page.
648-
err = ctx.Session.Set("twofaUid", u.ID)
649-
if err != nil {
650-
log.Error(fmt.Sprintf("Error setting session: %v", err))
662+
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
663+
log.Error("Error setting twofaUid in session: %v", err)
651664
}
652-
err = ctx.Session.Set("twofaRemember", false)
653-
if err != nil {
654-
log.Error(fmt.Sprintf("Error setting session: %v", err))
665+
if err := ctx.Session.Set("twofaRemember", false); err != nil {
666+
log.Error("Error setting twofaRemember in session: %v", err)
667+
}
668+
if err := ctx.Session.Release(); err != nil {
669+
log.Error("Error storing session: %v", err)
655670
}
656671

657672
// If U2F is enrolled -> Redirect to U2F instead
@@ -816,17 +831,17 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
816831
}
817832

818833
// User needs to use 2FA, save data and redirect to 2FA page.
819-
err = ctx.Session.Set("twofaUid", u.ID)
820-
if err != nil {
821-
log.Error(fmt.Sprintf("Error setting session: %v", err))
834+
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
835+
log.Error("Error setting twofaUid in session: %v", err)
822836
}
823-
err = ctx.Session.Set("twofaRemember", signInForm.Remember)
824-
if err != nil {
825-
log.Error(fmt.Sprintf("Error setting session: %v", err))
837+
if err := ctx.Session.Set("twofaRemember", signInForm.Remember); err != nil {
838+
log.Error("Error setting twofaRemember in session: %v", err)
826839
}
827-
err = ctx.Session.Set("linkAccount", true)
828-
if err != nil {
829-
log.Error(fmt.Sprintf("Error setting session: %v", err))
840+
if err := ctx.Session.Set("linkAccount", true); err != nil {
841+
log.Error("Error setting linkAccount in session: %v", err)
842+
}
843+
if err := ctx.Session.Release(); err != nil {
844+
log.Error("Error storing session: %v", err)
830845
}
831846

832847
// If U2F is enrolled -> Redirect to U2F instead
@@ -1184,14 +1199,16 @@ func Activate(ctx *context.Context) {
11841199

11851200
log.Trace("User activated: %s", user.Name)
11861201

1187-
err = ctx.Session.Set("uid", user.ID)
1188-
if err != nil {
1189-
log.Error(fmt.Sprintf("Error setting session: %v", err))
1202+
if err := ctx.Session.Set("uid", user.ID); err != nil {
1203+
log.Error(fmt.Sprintf("Error setting uid in session: %v", err))
11901204
}
1191-
err = ctx.Session.Set("uname", user.Name)
1192-
if err != nil {
1193-
log.Error(fmt.Sprintf("Error setting session: %v", err))
1205+
if err := ctx.Session.Set("uname", user.Name); err != nil {
1206+
log.Error(fmt.Sprintf("Error setting uname in session: %v", err))
11941207
}
1208+
if err := ctx.Session.Release(); err != nil {
1209+
log.Error("Error storing session: %v", err)
1210+
}
1211+
11951212
ctx.Flash.Success(ctx.Tr("auth.account_activated"))
11961213
ctx.Redirect(setting.AppSubURL + "/")
11971214
return

‎routers/user/auth_openid.go‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,12 @@ func SignInOpenIDPost(ctx *context.Context, form auth.SignInOpenIDForm) {
128128
url += "&openid.sreg.optional=nickname%2Cemail"
129129

130130
log.Trace("Form-passed openid-remember: %t", form.Remember)
131-
err = ctx.Session.Set("openid_signin_remember", form.Remember)
132-
if err != nil {
133-
log.Error("SignInOpenIDPost: Could not set session: %v", err.Error())
131+
132+
if err := ctx.Session.Set("openid_signin_remember", form.Remember); err != nil {
133+
log.Error("SignInOpenIDPost: Could not set openid_signin_remember in session: %v", err)
134+
}
135+
if err := ctx.Session.Release(); err != nil {
136+
log.Error("SignInOpenIDPost: Unable to save changes to the session: %v", err)
134137
}
135138

136139
ctx.Redirect(url)
@@ -227,23 +230,22 @@ func signInOpenIDVerify(ctx *context.Context) {
227230
}
228231
}
229232

230-
err = ctx.Session.Set("openid_verified_uri", id)
231-
if err != nil {
232-
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
233+
if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
234+
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
233235
}
234-
235-
err = ctx.Session.Set("openid_determined_email", email)
236-
if err != nil {
237-
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
236+
if err := ctx.Session.Set("openid_determined_email", email); err != nil {
237+
log.Error("signInOpenIDVerify: Could not set openid_determined_email in session: %v", err)
238238
}
239239

240240
if u != nil {
241241
nickname = u.LowerName
242242
}
243243

244-
err = ctx.Session.Set("openid_determined_username", nickname)
245-
if err != nil {
246-
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
244+
if err := ctx.Session.Set("openid_determined_username", nickname); err != nil {
245+
log.Error("signInOpenIDVerify: Could not set openid_determined_username in session: %v", err)
246+
}
247+
if err := ctx.Session.Release(); err != nil {
248+
log.Error("signInOpenIDVerify: Unable to save changes to the session: %v", err)
247249
}
248250

249251
if u != nil || !setting.Service.EnableOpenIDSignUp {

‎routers/user/oauth.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,11 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) {
229229
}, form.RedirectURI)
230230
return
231231
}
232+
// Here we're just going to try to release the session early
233+
if err := ctx.Session.Release(); err != nil {
234+
// we'll tolerate errors here as they *should* get saved elsewhere
235+
log.Error("Unable to save changes to the session: %v", err)
236+
}
232237
case "":
233238
break
234239
default:
@@ -287,6 +292,11 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) {
287292
log.Error(err.Error())
288293
return
289294
}
295+
// Here we're just going to try to release the session early
296+
if err := ctx.Session.Release(); err != nil {
297+
// we'll tolerate errors here as they *should* get saved elsewhere
298+
log.Error("Unable to save changes to the session: %v", err)
299+
}
290300
ctx.HTML(200, tplGrantAccess)
291301
}
292302

0 commit comments

Comments
 (0)