Skip to content

Whenever the ctx.Session is updated, release to save before sending the redirect #11456

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 17, 2020
Merged

Whenever the ctx.Session is updated, release to save before sending the redirect #11456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bf2072
Whenever the ctx.Session is updated, release it to save it before sen…
zeripath May 17, 2020
62c9973
Merge branch 'master' into fix-intermittent-500-due-to-session-out-of…
lafriks May 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions routers/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,11 @@ func InstallPost(ctx *context.Context, form auth.InstallForm) {
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
return
}

if err = ctx.Session.Release(); err != nil {
ctx.RenderWithErr(ctx.Tr("install.save_config_failed", err), tplInstall, &form)
return
}
}

log.Info("First-time run install finished!")
Expand Down
113 changes: 65 additions & 48 deletions routers/user/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,18 @@ func AutoSignIn(ctx *context.Context) (bool, error) {
}

isSucceed = true
err = ctx.Session.Set("uid", u.ID)
if err != nil {

// Set session IDs
if err := ctx.Session.Set("uid", u.ID); err != nil {
return false, err
}
err = ctx.Session.Set("uname", u.Name)
if err != nil {
if err := ctx.Session.Set("uname", u.Name); err != nil {
return false, err
}
if err := ctx.Session.Release(); err != nil {
return false, err
}

ctx.SetCookie(setting.CSRFCookieName, "", -1, setting.AppSubURL, setting.SessionConfig.Domain, setting.SessionConfig.Secure, true)
return true, nil
}
Expand Down Expand Up @@ -204,14 +208,16 @@ func SignInPost(ctx *context.Context, form auth.SignInForm) {
}

// User needs to use 2FA, save data and redirect to 2FA page.
err = ctx.Session.Set("twofaUid", u.ID)
if err != nil {
ctx.ServerError("UserSignIn", err)
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
ctx.ServerError("UserSignIn: Unable to set twofaUid in session", err)
return
}
err = ctx.Session.Set("twofaRemember", form.Remember)
if err != nil {
ctx.ServerError("UserSignIn", err)
if err := ctx.Session.Set("twofaRemember", form.Remember); err != nil {
ctx.ServerError("UserSignIn: Unable to set twofaRemember in session", err)
return
}
if err := ctx.Session.Release(); err != nil {
ctx.ServerError("UserSignIn: Unable to save session", err)
return
}

Expand Down Expand Up @@ -408,10 +414,14 @@ func U2FChallenge(ctx *context.Context) {
ctx.ServerError("u2f.NewChallenge", err)
return
}
if err = ctx.Session.Set("u2fChallenge", challenge); err != nil {
ctx.ServerError("UserSignIn", err)
if err := ctx.Session.Set("u2fChallenge", challenge); err != nil {
ctx.ServerError("UserSignIn: unable to set u2fChallenge in session", err)
return
}
if err := ctx.Session.Release(); err != nil {
ctx.ServerError("UserSignIn: unable to store session", err)
}

ctx.JSON(200, challenge.SignRequest(regs.ToRegistrations()))
}

Expand Down Expand Up @@ -495,13 +505,14 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR
_ = ctx.Session.Delete("twofaRemember")
_ = ctx.Session.Delete("u2fChallenge")
_ = ctx.Session.Delete("linkAccount")
err := ctx.Session.Set("uid", u.ID)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("uid", u.ID); err != nil {
log.Error("Error setting uid %d in session: %v", u.ID, err)
}
err = ctx.Session.Set("uname", u.Name)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", 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
Expand Down Expand Up @@ -594,9 +605,11 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context

if u == nil {
// no existing user is found, request attach or new account
err = ctx.Session.Set("linkAccountGothUser", gothUser)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
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")
return
Expand All @@ -611,13 +624,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
return
}

err = ctx.Session.Set("uid", u.ID)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("uid", u.ID); err != nil {
log.Error("Error setting uid in session: %v", err)
}
err = ctx.Session.Set("uname", u.Name)
if err != nil {
log.Error(fmt.Sprintf("Error setting 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 has right now, force to generate a new one
Expand Down Expand Up @@ -646,13 +660,14 @@ func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context
}

// User needs to use 2FA, save data and redirect to 2FA page.
err = ctx.Session.Set("twofaUid", u.ID)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
log.Error("Error setting twofaUid in session: %v", err)
}
err = ctx.Session.Set("twofaRemember", false)
if err != nil {
log.Error(fmt.Sprintf("Error setting 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 U2F is enrolled -> Redirect to U2F instead
Expand Down Expand Up @@ -821,17 +836,17 @@ func LinkAccountPostSignIn(ctx *context.Context, signInForm auth.SignInForm) {
}

// User needs to use 2FA, save data and redirect to 2FA page.
err = ctx.Session.Set("twofaUid", u.ID)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("twofaUid", u.ID); err != nil {
log.Error("Error setting twofaUid in session: %v", err)
}
err = ctx.Session.Set("twofaRemember", signInForm.Remember)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("twofaRemember", signInForm.Remember); err != nil {
log.Error("Error setting twofaRemember in session: %v", err)
}
err = ctx.Session.Set("linkAccount", true)
if err != nil {
log.Error(fmt.Sprintf("Error setting 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 U2F is enrolled -> Redirect to U2F instead
Expand Down Expand Up @@ -1200,14 +1215,16 @@ func Activate(ctx *context.Context) {

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

err = ctx.Session.Set("uid", user.ID)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("uid", user.ID); err != nil {
log.Error(fmt.Sprintf("Error setting uid in session: %v", err))
}
err = ctx.Session.Set("uname", user.Name)
if err != nil {
log.Error(fmt.Sprintf("Error setting session: %v", err))
if err := ctx.Session.Set("uname", user.Name); err != nil {
log.Error(fmt.Sprintf("Error setting uname in session: %v", err))
}
if err := ctx.Session.Release(); err != nil {
log.Error("Error storing session: %v", err)
}

ctx.Flash.Success(ctx.Tr("auth.account_activated"))
ctx.Redirect(setting.AppSubURL + "/")
return
Expand Down
28 changes: 15 additions & 13 deletions routers/user/auth_openid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,12 @@ func SignInOpenIDPost(ctx *context.Context, form auth.SignInOpenIDForm) {
url += "&openid.sreg.optional=nickname%2Cemail"

log.Trace("Form-passed openid-remember: %t", form.Remember)
err = ctx.Session.Set("openid_signin_remember", form.Remember)
if err != nil {
log.Error("SignInOpenIDPost: Could not set session: %v", err.Error())

if err := ctx.Session.Set("openid_signin_remember", form.Remember); err != nil {
log.Error("SignInOpenIDPost: Could not set openid_signin_remember in session: %v", err)
}
if err := ctx.Session.Release(); err != nil {
log.Error("SignInOpenIDPost: Unable to save changes to the session: %v", err)
}

ctx.Redirect(url)
Expand Down Expand Up @@ -227,23 +230,22 @@ func signInOpenIDVerify(ctx *context.Context) {
}
}

err = ctx.Session.Set("openid_verified_uri", id)
if err != nil {
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
if err := ctx.Session.Set("openid_verified_uri", id); err != nil {
log.Error("signInOpenIDVerify: Could not set openid_verified_uri in session: %v", err)
}

err = ctx.Session.Set("openid_determined_email", email)
if err != nil {
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
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
}

err = ctx.Session.Set("openid_determined_username", nickname)
if err != nil {
log.Error("signInOpenIDVerify: Could not set session: %v", err.Error())
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 u != nil || !setting.Service.EnableOpenIDSignUp {
Expand Down
10 changes: 10 additions & 0 deletions routers/user/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) {
}, form.RedirectURI)
return
}
// Here we're just going to try to release the session early
if err := ctx.Session.Release(); err != nil {
// we'll tolerate errors here as they *should* get saved elsewhere
log.Error("Unable to save changes to the session: %v", err)
}
case "":
break
default:
Expand Down Expand Up @@ -287,6 +292,11 @@ func AuthorizeOAuth(ctx *context.Context, form auth.AuthorizationForm) {
log.Error(err.Error())
return
}
// Here we're just going to try to release the session early
if err := ctx.Session.Release(); err != nil {
// we'll tolerate errors here as they *should* get saved elsewhere
log.Error("Unable to save changes to the session: %v", err)
}
ctx.HTML(200, tplGrantAccess)
}

Expand Down
Loading