Refactor UpdateOAuth2Application (#11034)

Following on from #11008 refactor UpdateOAuth2Application

3 files changed, 30 insertions, 24 deletions

diff --git a/models/oauth2_application.go b/models/oauth2_application.go
index f888cf61b8..eba03d9480 100644
--- a/models/oauth2_application.go
+++ b/models/oauth2_application.go
@@ -196,18 +196,34 @@ type UpdateOAuth2ApplicationOptions struct {
 }
 
 // UpdateOAuth2Application updates an oauth2 application
-func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) error {
-	return updateOAuth2Application(x, opts)
-}
+func UpdateOAuth2Application(opts UpdateOAuth2ApplicationOptions) (*OAuth2Application, error) {
+	sess := x.NewSession()
+	if err := sess.Begin(); err != nil {
+		return nil, err
+	}
+	defer sess.Close()
 
-func updateOAuth2Application(e Engine, opts UpdateOAuth2ApplicationOptions) error {
-	app := &OAuth2Application{
-		ID:           opts.ID,
-		UID:          opts.UserID,
-		Name:         opts.Name,
-		RedirectURIs: opts.RedirectURIs,
+	app, err := getOAuth2ApplicationByID(sess, opts.ID)
+	if err != nil {
+		return nil, err
+	}
+	if app.UID != opts.UserID {
+		return nil, fmt.Errorf("UID missmatch")
 	}
-	if _, err := e.ID(opts.ID).Update(app); err != nil {
+
+	app.Name = opts.Name
+	app.RedirectURIs = opts.RedirectURIs
+
+	if err = updateOAuth2Application(sess, app); err != nil {
+		return nil, err
+	}
+	app.ClientSecret = ""
+
+	return app, sess.Commit()
+}
+
+func updateOAuth2Application(e Engine, app *OAuth2Application) error {
+	if _, err := e.ID(app.ID).Update(app); err != nil {
 		return err
 	}
 	return nil
diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go
index f29572ef62..f426721c82 100644
--- a/routers/api/v1/user/app.go
+++ b/routers/api/v1/user/app.go
@@ -301,18 +301,13 @@ func UpdateOauth2Application(ctx *context.APIContext, data api.CreateOAuth2Appli
 	//     "$ref": "#/responses/OAuth2Application"
 	appID := ctx.ParamsInt64(":id")
 
-	err := models.UpdateOAuth2Application(models.UpdateOAuth2ApplicationOptions{
+	app, err := models.UpdateOAuth2Application(models.UpdateOAuth2ApplicationOptions{
 		Name:         data.Name,
 		UserID:       ctx.User.ID,
 		ID:           appID,
 		RedirectURIs: data.RedirectURIs,
 	})
 	if err != nil {
-		ctx.Error(http.StatusBadRequest, "", "error updating oauth2 application")
-		return
-	}
-	app, err := models.GetOAuth2ApplicationByID(appID)
-	if err != nil {
 		if models.IsErrOauthClientIDInvalid(err) || models.IsErrOAuthApplicationNotFound(err) {
 			ctx.NotFound()
 		} else {
@@ -320,12 +315,11 @@ func UpdateOauth2Application(ctx *context.APIContext, data api.CreateOAuth2Appli
 		}
 		return
 	}
-	secret, err := app.GenerateClientSecret()
+	app.ClientSecret, err = app.GenerateClientSecret()
 	if err != nil {
 		ctx.Error(http.StatusBadRequest, "", "error updating application secret")
 		return
 	}
-	app.ClientSecret = secret
 
 	ctx.JSON(http.StatusOK, convert.ToOAuth2Application(app))
 }
diff --git a/routers/user/setting/oauth2.go b/routers/user/setting/oauth2.go
index 265e32642b..f42c1123e1 100644
--- a/routers/user/setting/oauth2.go
+++ b/routers/user/setting/oauth2.go
@@ -62,7 +62,8 @@ func OAuthApplicationsEdit(ctx *context.Context, form auth.EditOAuth2Application
 		return
 	}
 	// TODO validate redirect URI
-	if err := models.UpdateOAuth2Application(models.UpdateOAuth2ApplicationOptions{
+	var err error
+	if ctx.Data["App"], err = models.UpdateOAuth2Application(models.UpdateOAuth2ApplicationOptions{
 		ID:           ctx.ParamsInt64("id"),
 		Name:         form.Name,
 		RedirectURIs: []string{form.RedirectURI},
@@ -71,11 +72,6 @@ func OAuthApplicationsEdit(ctx *context.Context, form auth.EditOAuth2Application
 		ctx.ServerError("UpdateOAuth2Application", err)
 		return
 	}
-	var err error
-	if ctx.Data["App"], err = models.GetOAuth2ApplicationByID(ctx.ParamsInt64("id")); err != nil {
-		ctx.ServerError("GetOAuth2ApplicationByID", err)
-		return
-	}
 	ctx.Flash.Success(ctx.Tr("settings.update_oauth2_application_success"))
 	ctx.HTML(200, tplSettingsOAuthApplications)
 }