1
1
mirror of https://github.com/go-gitea/gitea synced 2025-07-22 18:28:37 +00:00

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

Signed-off-by: Andrew Thornton <art27@cantab.net>

Co-authored-by: Lauris BH <lauris@nix.lv>
This commit is contained in:
zeripath
2020-05-17 13:43:29 +01:00
committed by GitHub
parent 50932ce8af
commit 76053ac31a
6 changed files with 168 additions and 100 deletions

View File

@@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/auth"
"code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"github.com/pquerna/otp"
@@ -28,18 +29,22 @@ func RegenerateScratchTwoFactor(ctx *context.Context) {
t, err := models.GetTwoFactorByUID(ctx.User.ID)
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
if models.IsErrTwoFactorNotEnrolled(err) {
ctx.Flash.Error(ctx.Tr("setting.twofa_not_enrolled"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
}
ctx.ServerError("SettingsTwoFactor: Failed to GetTwoFactorByUID", err)
return
}
token, err := t.GenerateScratchToken()
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: Failed to GenerateScratchToken", err)
return
}
if err = models.UpdateTwoFactor(t); err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: Failed to UpdateTwoFactor", err)
return
}
@@ -54,12 +59,21 @@ func DisableTwoFactor(ctx *context.Context) {
t, err := models.GetTwoFactorByUID(ctx.User.ID)
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
if models.IsErrTwoFactorNotEnrolled(err) {
ctx.Flash.Error(ctx.Tr("setting.twofa_not_enrolled"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
}
ctx.ServerError("SettingsTwoFactor: Failed to GetTwoFactorByUID", err)
return
}
if err = models.DeleteTwoFactorByID(t.ID, ctx.User.ID); err != nil {
ctx.ServerError("SettingsTwoFactor", err)
if models.IsErrTwoFactorNotEnrolled(err) {
// There is a potential DB race here - we must have been disabled by another request in the intervening period
ctx.Flash.Success(ctx.Tr("settings.twofa_disabled"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
}
ctx.ServerError("SettingsTwoFactor: Failed to DeleteTwoFactorByID", err)
return
}
@@ -74,7 +88,7 @@ func twofaGenerateSecretAndQr(ctx *context.Context) bool {
if uri != nil {
otpKey, err = otp.NewKeyFromURL(uri.(string))
if err != nil {
ctx.ServerError("SettingsTwoFactor: NewKeyFromURL: ", err)
ctx.ServerError("SettingsTwoFactor: Failed NewKeyFromURL: ", err)
return false
}
}
@@ -87,7 +101,7 @@ func twofaGenerateSecretAndQr(ctx *context.Context) bool {
AccountName: ctx.User.Name,
})
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: totpGenerate Failed", err)
return false
}
}
@@ -95,27 +109,33 @@ func twofaGenerateSecretAndQr(ctx *context.Context) bool {
ctx.Data["TwofaSecret"] = otpKey.Secret()
img, err := otpKey.Image(320, 240)
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: otpKey image generation failed", err)
return false
}
var imgBytes bytes.Buffer
if err = png.Encode(&imgBytes, img); err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: otpKey png encoding failed", err)
return false
}
ctx.Data["QrUri"] = template.URL("data:image/png;base64," + base64.StdEncoding.EncodeToString(imgBytes.Bytes()))
err = ctx.Session.Set("twofaSecret", otpKey.Secret())
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
if err := ctx.Session.Set("twofaSecret", otpKey.Secret()); err != nil {
ctx.ServerError("SettingsTwoFactor: Failed to set session for twofaSecret", err)
return false
}
err = ctx.Session.Set("twofaUri", otpKey.String())
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
if err := ctx.Session.Set("twofaUri", otpKey.String()); err != nil {
ctx.ServerError("SettingsTwoFactor: Failed to set session for twofaUri", err)
return false
}
// 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)
}
return true
}
@@ -126,12 +146,14 @@ func EnrollTwoFactor(ctx *context.Context) {
t, err := models.GetTwoFactorByUID(ctx.User.ID)
if t != nil {
// already enrolled
ctx.ServerError("SettingsTwoFactor", err)
// already enrolled - we should redirect back!
log.Warn("Trying to re-enroll %-v in twofa when already enrolled", ctx.User)
ctx.Flash.Error(ctx.Tr("setting.twofa_is_enrolled"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
return
}
if err != nil && !models.IsErrTwoFactorNotEnrolled(err) {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: GetTwoFactorByUID", err)
return
}
@@ -150,11 +172,12 @@ func EnrollTwoFactorPost(ctx *context.Context, form auth.TwoFactorAuthForm) {
t, err := models.GetTwoFactorByUID(ctx.User.ID)
if t != nil {
// already enrolled
ctx.ServerError("SettingsTwoFactor", err)
ctx.Flash.Error(ctx.Tr("setting.twofa_is_enrolled"))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
return
}
if err != nil && !models.IsErrTwoFactorNotEnrolled(err) {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: Failed to check if already enrolled with GetTwoFactorByUID", err)
return
}
@@ -181,30 +204,37 @@ func EnrollTwoFactorPost(ctx *context.Context, form auth.TwoFactorAuthForm) {
}
err = t.SetSecret(secret)
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: Failed to set secret", err)
return
}
token, err := t.GenerateScratchToken()
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
ctx.ServerError("SettingsTwoFactor: Failed to generate scratch token", err)
return
}
// Now we have to delete the secrets - because if we fail to insert then it's highly likely that they have already been used
// If we can detect the unique constraint failure below we can move this to after the NewTwoFactor
if err := ctx.Session.Delete("twofaSecret"); err != nil {
// tolerate this failure - it's more important to continue
log.Error("Unable to delete twofaSecret from the session: Error: %v", err)
}
if err := ctx.Session.Delete("twofaUri"); err != nil {
// tolerate this failure - it's more important to continue
log.Error("Unable to delete twofaUri from the session: Error: %v", err)
}
if err := ctx.Session.Release(); err != nil {
// tolerate this failure - it's more important to continue
log.Error("Unable to save changes to the session: %v", err)
}
if err = models.NewTwoFactor(t); err != nil {
ctx.ServerError("SettingsTwoFactor", err)
// FIXME: We need to handle a unique constraint fail here it's entirely possible that another request has beaten us.
// If there is a unique constraint fail we should just tolerate the error
ctx.ServerError("SettingsTwoFactor: Failed to save two factor", err)
return
}
err = ctx.Session.Delete("twofaSecret")
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
return
}
err = ctx.Session.Delete("twofaUri")
if err != nil {
ctx.ServerError("SettingsTwoFactor", err)
return
}
ctx.Flash.Success(ctx.Tr("settings.twofa_enrolled", token))
ctx.Redirect(setting.AppSubURL + "/user/settings/security")
}