From 1d950cfc360adedede9a85c2fdc6f6d54789299d Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Tue, 19 Jul 2022 22:30:29 +0530 Subject: [PATCH 1/9] feat(api): delete the authenticated user Endpoint: DELETE /user --- routers/api/v1/api.go | 1 + routers/api/v1/user/user.go | 42 ++++++++++++++++++++++++++++++++++ templates/swagger/v1_json.tmpl | 21 +++++++++++++++++ 3 files changed, 64 insertions(+) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 6a98121b73a51..bd92cd1827f28 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -712,6 +712,7 @@ func Routes(ctx gocontext.Context) *web.Route { m.Group("/user", func() { m.Get("", user.GetAuthenticatedUser) + m.Delete("", user.DeleteAuthenticatedUser) m.Group("/settings", func() { m.Get("", user.GetUserSettings) m.Patch("", bind(api.UserSettingsOptions{}), user.UpdateUserSettings) diff --git a/routers/api/v1/user/user.go b/routers/api/v1/user/user.go index 69197aef23eb2..a554542b74004 100644 --- a/routers/api/v1/user/user.go +++ b/routers/api/v1/user/user.go @@ -6,13 +6,16 @@ package user import ( + "fmt" "net/http" activities_model "code.gitea.io/gitea/models/activities" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/routers/api/v1/utils" + user_service "code.gitea.io/gitea/services/user" ) // Search search users @@ -146,3 +149,42 @@ func GetUserHeatmapData(ctx *context.APIContext) { } ctx.JSON(http.StatusOK, heatmap) } + +// DeleteAuthenticatedUser deletes the current user +func DeleteAuthenticatedUser(ctx *context.APIContext) { + // swagger:operation DELETE /user user userDeleteCurrent + // --- + // summary: Delete the authenticated user + // produces: + // - application/json + // responses: + // responses: + // "204": + // "$ref": "#/responses/empty" + // "403": + // "$ref": "#/responses/forbidden" + // "422": + // "$ref": "#/responses/validationError" + + // Extract out the username as it's unavailable after deleting the user. + username := ctx.Doer.Name + + if ctx.Doer.IsOrganization() { + ctx.Error(http.StatusUnprocessableEntity, "", fmt.Errorf("%s is an organization not a user", username)) + return + } + + if err := user_service.DeleteUser(ctx, ctx.Doer, ctx.FormBool("purge")); err != nil { + if models.IsErrUserOwnRepos(err) || + models.IsErrUserHasOrgs(err) || + models.IsErrUserOwnPackages(err) { + ctx.Error(http.StatusUnprocessableEntity, "", err) + } else { + ctx.Error(http.StatusInternalServerError, "DeleteAuthenticatedUser", err) + } + return + } + log.Trace("Account deleted: %s", username) + + ctx.Status(http.StatusNoContent) +} diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 47100450f945a..f8e0fa3ecbc6c 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -11680,6 +11680,27 @@ "$ref": "#/responses/User" } } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "user" + ], + "summary": "Delete the authenticated user", + "operationId": "userDeleteCurrent", + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "422": { + "$ref": "#/responses/validationError" + } + } } }, "/user/applications/oauth2": { From 59304aa7636b874e644329385cc2e3d224233c78 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 20 Jul 2022 21:49:48 +0530 Subject: [PATCH 2/9] test: add test cases for delete user API --- tests/api_user_delete_test.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 tests/api_user_delete_test.go diff --git a/tests/api_user_delete_test.go b/tests/api_user_delete_test.go new file mode 100644 index 0000000000000..04aa1ed269d5c --- /dev/null +++ b/tests/api_user_delete_test.go @@ -0,0 +1,34 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "net/http" + "testing" + + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" +) + +func TestAPIDeleteUser(t *testing.T) { + defer prepareTestEnv(t)() + + // 1 -> Admin + // 8 -> Normal user + for _, userId := range []int{1, 8} { + username := fmt.Sprintf("user%d", userId) + t.Logf("Testing username %s", username) + + session := loginUser(t, username) + token := getTokenForLoggedInUser(t, session) + + req := NewRequest(t, "DELETE", "/api/v1/user?token="+token) + session.MakeRequest(t, req, http.StatusNoContent) + + assertUserDeleted(t, int64(userId)) + unittest.CheckConsistencyFor(t, &user_model.User{}) + } +} From 418b2bc1bd289fbe7c00154fba83d16d89e76857 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Jul 2022 08:17:48 +0530 Subject: [PATCH 3/9] fix: lint errors --- tests/api_user_delete_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/api_user_delete_test.go b/tests/api_user_delete_test.go index 04aa1ed269d5c..a5d535b0d900e 100644 --- a/tests/api_user_delete_test.go +++ b/tests/api_user_delete_test.go @@ -18,8 +18,8 @@ func TestAPIDeleteUser(t *testing.T) { // 1 -> Admin // 8 -> Normal user - for _, userId := range []int{1, 8} { - username := fmt.Sprintf("user%d", userId) + for _, userID := range []int{1, 8} { + username := fmt.Sprintf("user%d", userID) t.Logf("Testing username %s", username) session := loginUser(t, username) @@ -28,7 +28,7 @@ func TestAPIDeleteUser(t *testing.T) { req := NewRequest(t, "DELETE", "/api/v1/user?token="+token) session.MakeRequest(t, req, http.StatusNoContent) - assertUserDeleted(t, int64(userId)) + assertUserDeleted(t, int64(userID)) unittest.CheckConsistencyFor(t, &user_model.User{}) } } From 0c51dc98863d5fdcc82580a5e1881004ed6ea8ee Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 21 Jul 2022 23:28:02 +0530 Subject: [PATCH 4/9] docs(api): add query parameter info --- routers/api/v1/user/user.go | 6 +++++- templates/swagger/v1_json.tmpl | 8 ++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/user/user.go b/routers/api/v1/user/user.go index a554542b74004..375a252ed0ee0 100644 --- a/routers/api/v1/user/user.go +++ b/routers/api/v1/user/user.go @@ -157,7 +157,11 @@ func DeleteAuthenticatedUser(ctx *context.APIContext) { // summary: Delete the authenticated user // produces: // - application/json - // responses: + // parameters: + // - name: purge + // in: query + // description: Purge user, all their repositories, organizations and comments + // type: boolean // responses: // "204": // "$ref": "#/responses/empty" diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index f8e0fa3ecbc6c..4c5d16b2d32b9 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -11690,6 +11690,14 @@ ], "summary": "Delete the authenticated user", "operationId": "userDeleteCurrent", + "parameters": [ + { + "type": "boolean", + "description": "Purge user, all their repositories, organizations and comments", + "name": "purge", + "in": "query" + } + ], "responses": { "204": { "$ref": "#/responses/empty" From 42eb77c7f5e90990dd5dfa86ee926b492f5c29e7 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 22 Jul 2022 09:37:53 +0530 Subject: [PATCH 5/9] test: API purge user testing Check if it fails when user still has ownership of repositories and then verify the use of the purge parameter. --- tests/api_user_delete_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/api_user_delete_test.go b/tests/api_user_delete_test.go index a5d535b0d900e..9eca6c52e4eca 100644 --- a/tests/api_user_delete_test.go +++ b/tests/api_user_delete_test.go @@ -9,6 +9,7 @@ import ( "net/http" "testing" + repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" ) @@ -32,3 +33,22 @@ func TestAPIDeleteUser(t *testing.T) { unittest.CheckConsistencyFor(t, &user_model.User{}) } } + +func TestAPIPurgeUser(t *testing.T) { + defer prepareTestEnv(t)() + + session := loginUser(t, "user5") + token := getTokenForLoggedInUser(t, session) + + // Cannot delete the user as it still has ownership of repositories + req := NewRequest(t, "DELETE", "/api/v1/user?token="+token) + session.MakeRequest(t, req, http.StatusUnprocessableEntity) + + unittest.CheckConsistencyFor(t, &user_model.User{ID: 5}) + + req = NewRequest(t, "DELETE", "/api/v1/user?purge=true&token="+token) + session.MakeRequest(t, req, http.StatusNoContent) + + assertUserDeleted(t, 5) + unittest.CheckConsistencyFor(t, &user_model.User{}, &repo_model.Repository{}) +} From 8dabe1242a4f1e4f4d4a0a85675c3c0fa6756dfe Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 28 Sep 2022 20:40:53 +0530 Subject: [PATCH 6/9] refactor: use auth middleware for delete user api --- routers/api/v1/api.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index bd92cd1827f28..400c212c56562 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -712,7 +712,9 @@ func Routes(ctx gocontext.Context) *web.Route { m.Group("/user", func() { m.Get("", user.GetAuthenticatedUser) - m.Delete("", user.DeleteAuthenticatedUser) + m.Group("", func() { + m.Delete("", user.DeleteAuthenticatedUser) + }, reqBasicOrRevProxyAuth()) m.Group("/settings", func() { m.Get("", user.GetUserSettings) m.Patch("", bind(api.UserSettingsOptions{}), user.UpdateUserSettings) From 24ecbd0681455f01e1cc6768f7c25e3de705bf2b Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 28 Sep 2022 20:41:44 +0530 Subject: [PATCH 7/9] refactor: fix test for the latest changes --- routers/api/v1/user/user.go | 1 + tests/integration/api_user_delete_test.go | 52 +++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 tests/integration/api_user_delete_test.go diff --git a/routers/api/v1/user/user.go b/routers/api/v1/user/user.go index 375a252ed0ee0..6eddec2f8aeaf 100644 --- a/routers/api/v1/user/user.go +++ b/routers/api/v1/user/user.go @@ -9,6 +9,7 @@ import ( "fmt" "net/http" + "code.gitea.io/gitea/models" activities_model "code.gitea.io/gitea/models/activities" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" diff --git a/tests/integration/api_user_delete_test.go b/tests/integration/api_user_delete_test.go new file mode 100644 index 0000000000000..3df1e8027e8e0 --- /dev/null +++ b/tests/integration/api_user_delete_test.go @@ -0,0 +1,52 @@ +// Copyright 2022 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integration + +import ( + "net/http" + "testing" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/tests" +) + +func TestAPIDeleteUser(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + // 1 -> Admin + // 8 -> Normal user + for _, userID := range []int64{1, 8} { + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID}) + t.Logf("Testing username %s", user.Name) + + req := NewRequest(t, "DELETE", "/api/v1/user") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) + + assertUserDeleted(t, userID) + unittest.CheckConsistencyFor(t, &user_model.User{}) + } +} + +func TestAPIPurgeUser(t *testing.T) { + defer tests.PrepareTestEnv(t)() + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + + // Cannot delete the user as it still has ownership of repositories + req := NewRequest(t, "DELETE", "/api/v1/user") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusUnprocessableEntity) + + unittest.CheckConsistencyFor(t, &user_model.User{ID: 5}) + + req = NewRequest(t, "DELETE", "/api/v1/user?purge=true") + req = AddBasicAuthHeader(req, user.Name) + MakeRequest(t, req, http.StatusNoContent) + + assertUserDeleted(t, 5) + unittest.CheckConsistencyFor(t, &user_model.User{}, &repo_model.Repository{}) +} From 9c141146f592366e0a1f8d9cf9fb47845a93fd63 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 28 Sep 2022 20:58:44 +0530 Subject: [PATCH 8/9] chore: remove unnecessary test file --- tests/api_user_delete_test.go | 54 ----------------------------------- 1 file changed, 54 deletions(-) delete mode 100644 tests/api_user_delete_test.go diff --git a/tests/api_user_delete_test.go b/tests/api_user_delete_test.go deleted file mode 100644 index 9eca6c52e4eca..0000000000000 --- a/tests/api_user_delete_test.go +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package integrations - -import ( - "fmt" - "net/http" - "testing" - - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unittest" - user_model "code.gitea.io/gitea/models/user" -) - -func TestAPIDeleteUser(t *testing.T) { - defer prepareTestEnv(t)() - - // 1 -> Admin - // 8 -> Normal user - for _, userID := range []int{1, 8} { - username := fmt.Sprintf("user%d", userID) - t.Logf("Testing username %s", username) - - session := loginUser(t, username) - token := getTokenForLoggedInUser(t, session) - - req := NewRequest(t, "DELETE", "/api/v1/user?token="+token) - session.MakeRequest(t, req, http.StatusNoContent) - - assertUserDeleted(t, int64(userID)) - unittest.CheckConsistencyFor(t, &user_model.User{}) - } -} - -func TestAPIPurgeUser(t *testing.T) { - defer prepareTestEnv(t)() - - session := loginUser(t, "user5") - token := getTokenForLoggedInUser(t, session) - - // Cannot delete the user as it still has ownership of repositories - req := NewRequest(t, "DELETE", "/api/v1/user?token="+token) - session.MakeRequest(t, req, http.StatusUnprocessableEntity) - - unittest.CheckConsistencyFor(t, &user_model.User{ID: 5}) - - req = NewRequest(t, "DELETE", "/api/v1/user?purge=true&token="+token) - session.MakeRequest(t, req, http.StatusNoContent) - - assertUserDeleted(t, 5) - unittest.CheckConsistencyFor(t, &user_model.User{}, &repo_model.Repository{}) -} From fce394f53f316cb1877d94f6f5a165502611f477 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Thu, 29 Sep 2022 04:42:38 +0200 Subject: [PATCH 9/9] just use Combo --- routers/api/v1/api.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 7d1d0573db579..7621b2f54f149 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -711,10 +711,9 @@ func Routes(ctx gocontext.Context) *web.Route { }, reqToken()) m.Group("/user", func() { - m.Get("", user.GetAuthenticatedUser) - m.Group("", func() { - m.Delete("", user.DeleteAuthenticatedUser) - }, reqBasicOrRevProxyAuth()) + m.Combo(""). + Get(user.GetAuthenticatedUser). + Delete(reqBasicOrRevProxyAuth(), user.DeleteAuthenticatedUser) m.Group("/settings", func() { m.Get("", user.GetUserSettings) m.Patch("", bind(api.UserSettingsOptions{}), user.UpdateUserSettings)