Fix openid setting check (#36346)

models/user/openid.go

@@ -102,7 +102,13 @@ func DeleteUserOpenID(ctx context.Context, openid *UserOpenID) (err error) {
 }
 
 // ToggleUserOpenIDVisibility toggles visibility of an openid address of given user.
-func ToggleUserOpenIDVisibility(ctx context.Context, id int64) (err error) {
+func ToggleUserOpenIDVisibility(ctx context.Context, id int64, user *User) error {
-	_, err = db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `id` = ?", id)
+	affected, err := db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `id` = ? AND uid = ?", id, user.ID)
-	return err
+	if err != nil {
+		return err
+	}
+	if n, _ := affected.RowsAffected(); n != 1 {
+		return util.NewNotExistErrorf("OpenID is unknown")
+	}
+	return nil
 }

models/user/openid_test.go

@@ -8,6 +8,7 @@ import (
 
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -33,12 +34,14 @@ func TestGetUserOpenIDs(t *testing.T) {
 
 func TestToggleUserOpenIDVisibility(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
+	user, err := user_model.GetUserByID(t.Context(), int64(2))
+	require.NoError(t, err)
 	oids, err := user_model.GetUserOpenIDs(t.Context(), int64(2))
 	require.NoError(t, err)
 	require.Len(t, oids, 1)
 	assert.True(t, oids[0].Show)
 
-	err = user_model.ToggleUserOpenIDVisibility(t.Context(), oids[0].ID)
+	err = user_model.ToggleUserOpenIDVisibility(t.Context(), oids[0].ID, user)
 	require.NoError(t, err)
 
 	oids, err = user_model.GetUserOpenIDs(t.Context(), int64(2))
@@ -46,7 +49,7 @@ func TestToggleUserOpenIDVisibility(t *testing.T) {
 	require.Len(t, oids, 1)
 
 	assert.False(t, oids[0].Show)
-	err = user_model.ToggleUserOpenIDVisibility(t.Context(), oids[0].ID)
+	err = user_model.ToggleUserOpenIDVisibility(t.Context(), oids[0].ID, user)
 	require.NoError(t, err)
 
 	oids, err = user_model.GetUserOpenIDs(t.Context(), int64(2))
@@ -55,3 +58,13 @@ func TestToggleUserOpenIDVisibility(t *testing.T) {
 		assert.True(t, oids[0].Show)
 	}
 }
+
+func TestToggleUserOpenIDVisibilityRequiresOwnership(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+	unauthorizedUser, err := user_model.GetUserByID(t.Context(), int64(2))
+	require.NoError(t, err)
+
+	err = user_model.ToggleUserOpenIDVisibility(t.Context(), int64(1), unauthorizedUser)
+	require.Error(t, err)
+	assert.ErrorIs(t, err, util.ErrNotExist)
+}

routers/web/user/setting/security/main_test.go

@@ -0,0 +1,14 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package security
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/unittest"
+)
+
+func TestMain(m *testing.M) {
+	unittest.MainTest(m)
+}

routers/web/user/setting/security/openid.go

@@ -4,12 +4,14 @@
 package security
 
 import (
+	"errors"
 	"net/http"
 
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/auth/openid"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
@@ -116,7 +118,11 @@ func DeleteOpenID(ctx *context.Context) {
 	}
 
 	if err := user_model.DeleteUserOpenID(ctx, &user_model.UserOpenID{ID: ctx.FormInt64("id"), UID: ctx.Doer.ID}); err != nil {
-		ctx.ServerError("DeleteUserOpenID", err)
+		if errors.Is(err, util.ErrNotExist) {
+			ctx.HTTPError(http.StatusNotFound)
+		} else {
+			ctx.ServerError("DeleteUserOpenID", err)
+		}
 		return
 	}
 	log.Trace("OpenID address deleted: %s", ctx.Doer.Name)
@@ -132,8 +138,12 @@ func ToggleOpenIDVisibility(ctx *context.Context) {
 		return
 	}
 
-	if err := user_model.ToggleUserOpenIDVisibility(ctx, ctx.FormInt64("id")); err != nil {
+	if err := user_model.ToggleUserOpenIDVisibility(ctx, ctx.FormInt64("id"), ctx.Doer); err != nil {
-		ctx.ServerError("ToggleUserOpenIDVisibility", err)
+		if errors.Is(err, util.ErrNotExist) {
+			ctx.HTTPError(http.StatusNotFound)
+		} else {
+			ctx.ServerError("ToggleUserOpenIDVisibility", err)
+		}
 		return
 	}
 

routers/web/user/setting/security/openid_test.go

@@ -0,0 +1,36 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package security
+
+import (
+	"net/http"
+	"testing"
+
+	"code.gitea.io/gitea/models/unittest"
+	"code.gitea.io/gitea/services/contexttest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestDeleteOpenIDReturnsNotFoundForOtherUsersAddress(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx, _ := contexttest.MockContext(t, "POST /user/settings/security")
+	contexttest.LoadUser(t, ctx, 2)
+	ctx.SetFormString("id", "1")
+
+	DeleteOpenID(ctx)
+
+	assert.Equal(t, http.StatusNotFound, ctx.Resp.WrittenStatus())
+}
+
+func TestToggleOpenIDVisibilityReturnsNotFoundForOtherUsersAddress(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+	ctx, _ := contexttest.MockContext(t, "POST /user/settings/security")
+	contexttest.LoadUser(t, ctx, 2)
+	ctx.SetFormString("id", "1")
+
+	ToggleOpenIDVisibility(ctx)
+
+	assert.Equal(t, http.StatusNotFound, ctx.Resp.WrittenStatus())
+}