Skip to content

Commit 76053ac

Browse files
zeripathlafriks
zeripath
and
lafriks
authored
Whenever the ctx.Session is updated, release it to save it before sending the redirect (#11456)
Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: Lauris BH <[email protected]>
1 parent 50932ce commit 76053ac

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
@@ -387,6 +387,11 @@ func InstallPost(ctx *context.Context, form auth.InstallForm) {
387387
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
388388
return
389389
}
390+
391+
if err = ctx.Session.Release(); err != nil {
392+
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
393+
return
394+
}
390395
}
391396

392397
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
@@ -82,14 +82,18 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
8282
}
8383

8484
isSucceed = true
85-
err = ctx.Session.Set("uid", u.ID)
86-
if err != nil {
85+
86+
// Set session IDs
87+
if err := ctx.Session.Set("uid", u.ID); err != nil {
8788
return false, err
8889
}
89-
err = ctx.Session.Set("uname", u.Name)
90-
if err != nil {
90+
if err := ctx.Session.Set("uname", u.Name); err != nil {
91+
return false, err
92+
}
93+
if err := ctx.Session.Release(); err != nil {
9194
return false, err
9295
}
96+
9397
ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, setting.SessionConfig.Domain, setting.SessionConfig.Secure, true)
9498
return true, nil
9599
}
@@ -204,14 +208,16 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) {
204208
}
205209

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

@@ -408,10 +414,14 @@ func U2FChallenge(ctx *context.Context) {
408414
ctx.ServerError("u2f.NewChallenge", err)
409415
return
410416
}
411-
if err = ctx.Session.Set("u2fChallenge", challenge); err != nil {
412-
ctx.ServerError("UserSignIn", err)
417+
if err := ctx.Session.Set("u2fChallenge", challenge); err != nil {
418+
ctx.ServerError("UserSignIn: unable to set u2fChallenge in session", err)
413419
return
414420
}
421+
if err := ctx.Session.Release(); err != nil {
422+
ctx.ServerError("UserSignIn: unable to store session", err)
423+
}
424+
415425
ctx.JSON(200, challenge.SignRequest(regs.ToRegistrations()))
416426
}
417427

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

507518
// Language setting of the user overwrites the one previously set
@@ -594,9 +605,11 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
594605

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

614-
err = ctx.Session.Set("uid", u.ID)
615-
if err != nil {
616-
log.Error(fmt.Sprintf("Error setting session: %v", err))
627+
if err := ctx.Session.Set("uid", u.ID); err != nil {
628+
log.Error("Error setting uid in session: %v", err)
617629
}
618-
err = ctx.Session.Set("uname", u.Name)
619-
if err != nil {
620-
log.Error(fmt.Sprintf("Error setting session: %v", err))
630+
if err := ctx.Session.Set("uname", u.Name); err != nil {
631+
log.Error("Error setting uname in session: %v", err)
632+
}
633+
if err := ctx.Session.Release(); err != nil {
634+
log.Error("Error storing session: %v", err)
621635
}
622636

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

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

658673
// If U2F is enrolled -> Redirect to U2F instead
@@ -821,17 +836,17 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
821836
}
822837

823838
// User needs to use 2FA, save data and redirect to 2FA page.
824-
err = ctx.Session.Set("twofaUid", u.ID)
825-
if err != nil {
826-
log.Error(fmt.Sprintf("Error setting session: %v", err))
839+
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
840+
log.Error("Error setting twofaUid in session: %v", err)
827841
}
828-
err = ctx.Session.Set("twofaRemember", signInForm.Remember)
829-
if err != nil {
830-
log.Error(fmt.Sprintf("Error setting session: %v", err))
842+
if err := ctx.Session.Set("twofaRemember", signInForm.Remember); err != nil {
843+
log.Error("Error setting twofaRemember in session: %v", err)
831844
}
832-
err = ctx.Session.Set("linkAccount", true)
833-
if err != nil {
834-
log.Error(fmt.Sprintf("Error setting session: %v", err))
845+
if err := ctx.Session.Set("linkAccount", true); err != nil {
846+
log.Error("Error setting linkAccount in session: %v", err)
847+
}
848+
if err := ctx.Session.Release(); err != nil {
849+
log.Error("Error storing session: %v", err)
835850
}
836851

837852
// If U2F is enrolled -> Redirect to U2F instead
@@ -1200,14 +1215,16 @@ func Activate(ctx *context.Context) {
12001215

12011216
log.Trace("User activated: %s", user.Name)
12021217

1203-
err = ctx.Session.Set("uid", user.ID)
1204-
if err != nil {
1205-
log.Error(fmt.Sprintf("Error setting session: %v", err))
1218+
if err := ctx.Session.Set("uid", user.ID); err != nil {
1219+
log.Error(fmt.Sprintf("Error setting uid in session: %v", err))
12061220
}
1207-
err = ctx.Session.Set("uname", user.Name)
1208-
if err != nil {
1209-
log.Error(fmt.Sprintf("Error setting session: %v", err))
1221+
if err := ctx.Session.Set("uname", user.Name); err != nil {
1222+
log.Error(fmt.Sprintf("Error setting uname in session: %v", err))
12101223
}
1224+
if err := ctx.Session.Release(); err != nil {
1225+
log.Error("Error storing session: %v", err)
1226+
}
1227+
12111228
ctx.Flash.Success(ctx.Tr("auth.account_activated"))
12121229
ctx.Redirect(setting.AppSubURL + "/")
12131230
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)