diff --git a/models/user/openid.go b/models/user/openid.go index 420c67ca18..5baa48c824 100644 --- a/models/user/openid.go +++ b/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) { - _, err = db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `id` = ?", id) - return err +func ToggleUserOpenIDVisibility(ctx context.Context, id int64, user *User) error { + affected, err := db.GetEngine(ctx).Exec("update `user_open_id` set `show` = not `show` where `id` = ? AND uid = ?", id, user.ID) + if err != nil { + return err + } + if n, _ := affected.RowsAffected(); n != 1 { + return util.NewNotExistErrorf("OpenID is unknown") + } + return nil } diff --git a/models/user/openid_test.go b/models/user/openid_test.go index fa260e7a9e..6d2260324f 100644 --- a/models/user/openid_test.go +++ b/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) +} diff --git a/routers/web/user/setting/security/main_test.go b/routers/web/user/setting/security/main_test.go new file mode 100644 index 0000000000..2a27cd6dbf --- /dev/null +++ b/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) +} diff --git a/routers/web/user/setting/security/openid.go b/routers/web/user/setting/security/openid.go index 9a57657912..20eb4c015b 100644 --- a/routers/web/user/setting/security/openid.go +++ b/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 { - ctx.ServerError("ToggleUserOpenIDVisibility", err) + if err := user_model.ToggleUserOpenIDVisibility(ctx, ctx.FormInt64("id"), ctx.Doer); err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.HTTPError(http.StatusNotFound) + } else { + ctx.ServerError("ToggleUserOpenIDVisibility", err) + } return } diff --git a/routers/web/user/setting/security/openid_test.go b/routers/web/user/setting/security/openid_test.go new file mode 100644 index 0000000000..860639ea1c --- /dev/null +++ b/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()) +}