From d07c8c821c4b1e14eec1f55428201c2f33ad0949 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 17 Apr 2024 01:25:20 +0200 Subject: [PATCH] Do not require login_name & source_id for /admin/user/{username} When editing a user via the API, do not require setting `login_name` or `source_id`: for local accounts, these do not matter. However, when editing a non-local account, require *both*, as before. Fixes #1861. Signed-off-by: Gergely Nagy --- modules/structs/admin_user.go | 6 +-- routers/api/v1/admin/user.go | 12 +++++- templates/swagger/v1_json.tmpl | 4 -- tests/integration/api_admin_test.go | 59 ++++++++++++++++++++--------- tests/integration/user_test.go | 8 +--- 5 files changed, 56 insertions(+), 33 deletions(-) diff --git a/modules/structs/admin_user.go b/modules/structs/admin_user.go index ad86f4ca03..5b7df127b4 100644 --- a/modules/structs/admin_user.go +++ b/modules/structs/admin_user.go @@ -30,10 +30,8 @@ type CreateUserOption struct { // EditUserOption edit user options type EditUserOption struct { - // required: true - SourceID int64 `json:"source_id"` - // required: true - LoginName string `json:"login_name" binding:"Required"` + SourceID *int64 `json:"source_id"` + LoginName *string `json:"login_name"` // swagger:strfmt email Email *string `json:"email" binding:"MaxSize(254)"` FullName *string `json:"full_name" binding:"MaxSize(100)"` diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 12da8a9597..cef2a986e0 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -192,9 +192,17 @@ func EditUser(ctx *context.APIContext) { form := web.GetForm(ctx).(*api.EditUserOption) + // If either LoginSource or LoginName is given, the other must be present too. + if form.SourceID != nil || form.LoginName != nil { + if form.SourceID == nil || form.LoginName == nil { + ctx.Error(http.StatusUnprocessableEntity, "LoginSourceAndLoginName", fmt.Errorf("source_id and login_name must be specified together")) + return + } + } + authOpts := &user_service.UpdateAuthOptions{ - LoginSource: optional.FromNonDefault(form.SourceID), - LoginName: optional.Some(form.LoginName), + LoginSource: optional.FromPtr(form.SourceID), + LoginName: optional.FromPtr(form.LoginName), Password: optional.FromNonDefault(form.Password), MustChangePassword: optional.FromPtr(form.MustChangePassword), ProhibitLogin: optional.FromPtr(form.ProhibitLogin), diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 4e4971dfe9..3e61d5b165 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -20984,10 +20984,6 @@ "EditUserOption": { "description": "EditUserOption edit user options", "type": "object", - "required": [ - "source_id", - "login_name" - ], "properties": { "active": { "type": "boolean", diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index e8954f5b20..0e97e606f9 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -196,19 +196,13 @@ func TestAPIEditUser(t *testing.T) { urlStr := fmt.Sprintf("/api/v1/admin/users/%s", "user2") req := NewRequestWithValues(t, "PATCH", urlStr, map[string]string{ - // required - "login_name": "user2", - "source_id": "0", - // to change "full_name": "Full Name User 2", }).AddTokenAuth(token) MakeRequest(t, req, http.StatusOK) empty := "" req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ - LoginName: "user2", - SourceID: 0, - Email: &empty, + Email: &empty, }).AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusBadRequest) @@ -220,10 +214,6 @@ func TestAPIEditUser(t *testing.T) { assert.False(t, user2.IsRestricted) bTrue := true req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ - // required - LoginName: "user2", - SourceID: 0, - // to change Restricted: &bTrue, }).AddTokenAuth(token) MakeRequest(t, req, http.StatusOK) @@ -231,6 +221,45 @@ func TestAPIEditUser(t *testing.T) { assert.True(t, user2.IsRestricted) } +func TestAPIEditUserWithLoginName(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + adminUsername := "user1" + token := getUserToken(t, adminUsername, auth_model.AccessTokenScopeWriteAdmin) + urlStr := fmt.Sprintf("/api/v1/admin/users/%s", "user2") + + loginName := "user2" + loginSource := int64(0) + + t.Run("login_name only", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ + LoginName: &loginName, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("source_id only", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ + SourceID: &loginSource, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnprocessableEntity) + }) + + t.Run("login_name & source_id", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ + LoginName: &loginName, + SourceID: &loginSource, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + }) +} + func TestAPICreateRepoForUser(t *testing.T) { defer tests.PrepareTestEnv(t)() adminUsername := "user1" @@ -375,18 +404,14 @@ func TestAPIEditUser_NotAllowedEmailDomain(t *testing.T) { newEmail := "user2@example1.com" req := NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ - LoginName: "user2", - SourceID: 0, - Email: &newEmail, + Email: &newEmail, }).AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) assert.Equal(t, "the domain of user email user2@example1.com conflicts with EMAIL_DOMAIN_ALLOWLIST or EMAIL_DOMAIN_BLOCKLIST", resp.Header().Get("X-Gitea-Warning")) originalEmail := "user2@example.com" req = NewRequestWithJSON(t, "PATCH", urlStr, api.EditUserOption{ - LoginName: "user2", - SourceID: 0, - Email: &originalEmail, + Email: &originalEmail, }).AddTokenAuth(token) MakeRequest(t, req, http.StatusOK) } diff --git a/tests/integration/user_test.go b/tests/integration/user_test.go index 3a3932bdc2..02cc9b51cc 100644 --- a/tests/integration/user_test.go +++ b/tests/integration/user_test.go @@ -495,9 +495,7 @@ func TestUserPronouns(t *testing.T) { // Set the pronouns for user2 pronouns := "she/her" req := NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user2", &api.EditUserOption{ - LoginName: "user2", - SourceID: 0, - Pronouns: &pronouns, + Pronouns: &pronouns, }).AddTokenAuth(adminToken) resp := MakeRequest(t, req, http.StatusOK) @@ -596,9 +594,7 @@ func TestUserPronouns(t *testing.T) { // Set the pronouns to Unspecified (an empty string) via the API pronouns := "" req := NewRequestWithJSON(t, "PATCH", "/api/v1/admin/users/user2", &api.EditUserOption{ - LoginName: "user2", - SourceID: 0, - Pronouns: &pronouns, + Pronouns: &pronouns, }).AddTokenAuth(adminToken) MakeRequest(t, req, http.StatusOK)