From 69bfd81b1cc4b95d6bc49e7b027d2e5ff03167a5 Mon Sep 17 00:00:00 2001
From: David Schneiderbauer <daviian@users.noreply.github.com>
Date: Mon, 18 Jun 2018 20:24:45 +0200
Subject: [PATCH] fix missing data on redirects (#3975)

---
 routers/user/setting/account.go         | 32 ++++++++-------
 routers/user/setting/applications.go    | 24 +++++------
 routers/user/setting/keys.go            | 54 ++++++++++++-------------
 routers/user/setting/profile.go         |  1 +
 routers/user/setting/security.go        | 44 +++++++++++---------
 routers/user/setting/security_openid.go | 21 ++++------
 6 files changed, 90 insertions(+), 86 deletions(-)

diff --git a/routers/user/setting/account.go b/routers/user/setting/account.go
index 966d96aeda..bcf602c5ed 100644
--- a/routers/user/setting/account.go
+++ b/routers/user/setting/account.go
@@ -24,12 +24,7 @@ func Account(ctx *context.Context) {
 	ctx.Data["PageIsSettingsAccount"] = true
 	ctx.Data["Email"] = ctx.User.Email
 
-	emails, err := models.GetEmailAddresses(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("GetEmailAddresses", err)
-		return
-	}
-	ctx.Data["Emails"] = emails
+	loadAccountData(ctx)
 
 	ctx.HTML(200, tplSettingsAccount)
 }
@@ -40,6 +35,8 @@ func AccountPost(ctx *context.Context, form auth.ChangePasswordForm) {
 	ctx.Data["PageIsSettingsAccount"] = true
 
 	if ctx.HasError() {
+		loadAccountData(ctx)
+
 		ctx.HTML(200, tplSettingsAccount)
 		return
 	}
@@ -85,15 +82,9 @@ func EmailPost(ctx *context.Context, form auth.AddEmailForm) {
 		return
 	}
 
-	// Add Email address.
-	emails, err := models.GetEmailAddresses(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("GetEmailAddresses", err)
-		return
-	}
-	ctx.Data["Emails"] = emails
-
 	if ctx.HasError() {
+		loadAccountData(ctx)
+
 		ctx.HTML(200, tplSettingsAccount)
 		return
 	}
@@ -105,6 +96,8 @@ func EmailPost(ctx *context.Context, form auth.AddEmailForm) {
 	}
 	if err := models.AddEmailAddress(email); err != nil {
 		if models.IsErrEmailAlreadyUsed(err) {
+			loadAccountData(ctx)
+
 			ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSettingsAccount, &form)
 			return
 		}
@@ -149,6 +142,8 @@ func DeleteAccount(ctx *context.Context) {
 
 	if _, err := models.UserSignIn(ctx.User.Name, ctx.Query("password")); err != nil {
 		if models.IsErrUserNotExist(err) {
+			loadAccountData(ctx)
+
 			ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_password"), tplSettingsAccount, nil)
 		} else {
 			ctx.ServerError("UserSignIn", err)
@@ -172,3 +167,12 @@ func DeleteAccount(ctx *context.Context) {
 		ctx.Redirect(setting.AppSubURL + "/")
 	}
 }
+
+func loadAccountData(ctx *context.Context) {
+	emails, err := models.GetEmailAddresses(ctx.User.ID)
+	if err != nil {
+		ctx.ServerError("GetEmailAddresses", err)
+		return
+	}
+	ctx.Data["Emails"] = emails
+}
diff --git a/routers/user/setting/applications.go b/routers/user/setting/applications.go
index f292b65d70..ac7252469a 100644
--- a/routers/user/setting/applications.go
+++ b/routers/user/setting/applications.go
@@ -22,12 +22,7 @@ func Applications(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("settings")
 	ctx.Data["PageIsSettingsApplications"] = true
 
-	tokens, err := models.ListAccessTokens(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("ListAccessTokens", err)
-		return
-	}
-	ctx.Data["Tokens"] = tokens
+	loadApplicationsData(ctx)
 
 	ctx.HTML(200, tplSettingsApplications)
 }
@@ -38,12 +33,8 @@ func ApplicationsPost(ctx *context.Context, form auth.NewAccessTokenForm) {
 	ctx.Data["PageIsSettingsApplications"] = true
 
 	if ctx.HasError() {
-		tokens, err := models.ListAccessTokens(ctx.User.ID)
-		if err != nil {
-			ctx.ServerError("ListAccessTokens", err)
-			return
-		}
-		ctx.Data["Tokens"] = tokens
+		loadApplicationsData(ctx)
+
 		ctx.HTML(200, tplSettingsApplications)
 		return
 	}
@@ -75,3 +66,12 @@ func DeleteApplication(ctx *context.Context) {
 		"redirect": setting.AppSubURL + "/user/settings/applications",
 	})
 }
+
+func loadApplicationsData(ctx *context.Context) {
+	tokens, err := models.ListAccessTokens(ctx.User.ID)
+	if err != nil {
+		ctx.ServerError("ListAccessTokens", err)
+		return
+	}
+	ctx.Data["Tokens"] = tokens
+}
diff --git a/routers/user/setting/keys.go b/routers/user/setting/keys.go
index ef986ef8c9..c62b117a76 100644
--- a/routers/user/setting/keys.go
+++ b/routers/user/setting/keys.go
@@ -23,19 +23,7 @@ func Keys(ctx *context.Context) {
 	ctx.Data["PageIsSettingsKeys"] = true
 	ctx.Data["DisableSSH"] = setting.SSH.Disabled
 
-	keys, err := models.ListPublicKeys(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("ListPublicKeys", err)
-		return
-	}
-	ctx.Data["Keys"] = keys
-
-	gpgkeys, err := models.ListGPGKeys(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("ListGPGKeys", err)
-		return
-	}
-	ctx.Data["GPGKeys"] = gpgkeys
+	loadKeysData(ctx)
 
 	ctx.HTML(200, tplSettingsKeys)
 }
@@ -45,21 +33,9 @@ func KeysPost(ctx *context.Context, form auth.AddKeyForm) {
 	ctx.Data["Title"] = ctx.Tr("settings")
 	ctx.Data["PageIsSettingsKeys"] = true
 
-	keys, err := models.ListPublicKeys(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("ListPublicKeys", err)
-		return
-	}
-	ctx.Data["Keys"] = keys
-
-	gpgkeys, err := models.ListGPGKeys(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("ListGPGKeys", err)
-		return
-	}
-	ctx.Data["GPGKeys"] = gpgkeys
-
 	if ctx.HasError() {
+		loadKeysData(ctx)
+
 		ctx.HTML(200, tplSettingsKeys)
 		return
 	}
@@ -73,9 +49,13 @@ func KeysPost(ctx *context.Context, form auth.AddKeyForm) {
 				ctx.Flash.Error(ctx.Tr("form.invalid_gpg_key", err.Error()))
 				ctx.Redirect(setting.AppSubURL + "/user/settings/keys")
 			case models.IsErrGPGKeyIDAlreadyUsed(err):
+				loadKeysData(ctx)
+
 				ctx.Data["Err_Content"] = true
 				ctx.RenderWithErr(ctx.Tr("settings.gpg_key_id_used"), tplSettingsKeys, &form)
 			case models.IsErrGPGNoEmailFound(err):
+				loadKeysData(ctx)
+
 				ctx.Data["Err_Content"] = true
 				ctx.RenderWithErr(ctx.Tr("settings.gpg_no_key_email_found"), tplSettingsKeys, &form)
 			default:
@@ -103,9 +83,13 @@ func KeysPost(ctx *context.Context, form auth.AddKeyForm) {
 			ctx.Data["HasSSHError"] = true
 			switch {
 			case models.IsErrKeyAlreadyExist(err):
+				loadKeysData(ctx)
+
 				ctx.Data["Err_Content"] = true
 				ctx.RenderWithErr(ctx.Tr("settings.ssh_key_been_used"), tplSettingsKeys, &form)
 			case models.IsErrKeyNameAlreadyUsed(err):
+				loadKeysData(ctx)
+
 				ctx.Data["Err_Title"] = true
 				ctx.RenderWithErr(ctx.Tr("settings.ssh_key_name_used"), tplSettingsKeys, &form)
 			default:
@@ -147,3 +131,19 @@ func DeleteKey(ctx *context.Context) {
 		"redirect": setting.AppSubURL + "/user/settings/keys",
 	})
 }
+
+func loadKeysData(ctx *context.Context) {
+	keys, err := models.ListPublicKeys(ctx.User.ID)
+	if err != nil {
+		ctx.ServerError("ListPublicKeys", err)
+		return
+	}
+	ctx.Data["Keys"] = keys
+
+	gpgkeys, err := models.ListGPGKeys(ctx.User.ID)
+	if err != nil {
+		ctx.ServerError("ListGPGKeys", err)
+		return
+	}
+	ctx.Data["GPGKeys"] = gpgkeys
+}
diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go
index cf222d0027..22511ab891 100644
--- a/routers/user/setting/profile.go
+++ b/routers/user/setting/profile.go
@@ -32,6 +32,7 @@ const (
 func Profile(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("settings")
 	ctx.Data["PageIsSettingsProfile"] = true
+
 	ctx.HTML(200, tplSettingsProfile)
 }
 
diff --git a/routers/user/setting/security.go b/routers/user/setting/security.go
index 860730303f..862e4413c7 100644
--- a/routers/user/setting/security.go
+++ b/routers/user/setting/security.go
@@ -22,6 +22,30 @@ func Security(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("settings")
 	ctx.Data["PageIsSettingsSecurity"] = true
 
+	if ctx.Query("openid.return_to") != "" {
+		settingsOpenIDVerify(ctx)
+		return
+	}
+
+	loadSecurityData(ctx)
+
+	ctx.HTML(200, tplSettingsSecurity)
+}
+
+// DeleteAccountLink delete a single account link
+func DeleteAccountLink(ctx *context.Context) {
+	if _, err := models.RemoveAccountLink(ctx.User, ctx.QueryInt64("loginSourceID")); err != nil {
+		ctx.Flash.Error("RemoveAccountLink: " + err.Error())
+	} else {
+		ctx.Flash.Success(ctx.Tr("settings.remove_account_link_success"))
+	}
+
+	ctx.JSON(200, map[string]interface{}{
+		"redirect": setting.AppSubURL + "/user/settings/security",
+	})
+}
+
+func loadSecurityData(ctx *context.Context) {
 	enrolled := true
 	_, err := models.GetTwoFactorByUID(ctx.User.ID)
 	if err != nil {
@@ -71,30 +95,10 @@ func Security(ctx *context.Context) {
 	}
 	ctx.Data["AccountLinks"] = sources
 
-	if ctx.Query("openid.return_to") != "" {
-		settingsOpenIDVerify(ctx)
-		return
-	}
-
 	openid, err := models.GetUserOpenIDs(ctx.User.ID)
 	if err != nil {
 		ctx.ServerError("GetUserOpenIDs", err)
 		return
 	}
 	ctx.Data["OpenIDs"] = openid
-
-	ctx.HTML(200, tplSettingsSecurity)
-}
-
-// DeleteAccountLink delete a single account link
-func DeleteAccountLink(ctx *context.Context) {
-	if _, err := models.RemoveAccountLink(ctx.User, ctx.QueryInt64("loginSourceID")); err != nil {
-		ctx.Flash.Error("RemoveAccountLink: " + err.Error())
-	} else {
-		ctx.Flash.Success(ctx.Tr("settings.remove_account_link_success"))
-	}
-
-	ctx.JSON(200, map[string]interface{}{
-		"redirect": setting.AppSubURL + "/user/settings/security",
-	})
 }
diff --git a/routers/user/setting/security_openid.go b/routers/user/setting/security_openid.go
index c98dc2cda9..6813765f6f 100644
--- a/routers/user/setting/security_openid.go
+++ b/routers/user/setting/security_openid.go
@@ -19,12 +19,8 @@ func OpenIDPost(ctx *context.Context, form auth.AddOpenIDForm) {
 	ctx.Data["PageIsSettingsSecurity"] = true
 
 	if ctx.HasError() {
-		openid, err := models.GetUserOpenIDs(ctx.User.ID)
-		if err != nil {
-			ctx.ServerError("GetUserOpenIDs", err)
-			return
-		}
-		ctx.Data["OpenIDs"] = openid
+		loadSecurityData(ctx)
+
 		ctx.HTML(200, tplSettingsSecurity)
 		return
 	}
@@ -37,6 +33,8 @@ func OpenIDPost(ctx *context.Context, form auth.AddOpenIDForm) {
 
 	id, err := openid.Normalize(form.Openid)
 	if err != nil {
+		loadSecurityData(ctx)
+
 		ctx.RenderWithErr(err.Error(), tplSettingsSecurity, &form)
 		return
 	}
@@ -53,6 +51,8 @@ func OpenIDPost(ctx *context.Context, form auth.AddOpenIDForm) {
 	// Check that the OpenID is not already used
 	for _, obj := range oids {
 		if obj.URI == id {
+			loadSecurityData(ctx)
+
 			ctx.RenderWithErr(ctx.Tr("form.openid_been_used", id), tplSettingsSecurity, &form)
 			return
 		}
@@ -61,6 +61,8 @@ func OpenIDPost(ctx *context.Context, form auth.AddOpenIDForm) {
 	redirectTo := setting.AppURL + "user/settings/security"
 	url, err := openid.RedirectURL(id, redirectTo, setting.AppURL)
 	if err != nil {
+		loadSecurityData(ctx)
+
 		ctx.RenderWithErr(err.Error(), tplSettingsSecurity, &form)
 		return
 	}
@@ -73,13 +75,6 @@ func settingsOpenIDVerify(ctx *context.Context) {
 	fullURL := setting.AppURL + ctx.Req.Request.URL.String()[1:]
 	log.Trace("Full URL: " + fullURL)
 
-	oids, err := models.GetUserOpenIDs(ctx.User.ID)
-	if err != nil {
-		ctx.ServerError("GetUserOpenIDs", err)
-		return
-	}
-	ctx.Data["OpenIDs"] = oids
-
 	id, err := openid.Verify(fullURL)
 	if err != nil {
 		ctx.RenderWithErr(err.Error(), tplSettingsSecurity, &auth.AddOpenIDForm{