Merge pull request 'Do not require login_name & source_id for /admin/user/{username}' (#3278) from algernon/forgejo:leave-your-name-at-the-door into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3278
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

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"`
-	SourceID int64 `json:"source_id"`
+	LoginName *string `json:"login_name"`
-	// required: true
-	LoginName string `json:"login_name" binding:"Required"`
 	// swagger:strfmt email
 	Email                   *string `json:"email" binding:"MaxSize(254)"`
 	FullName                *string `json:"full_name" binding:"MaxSize(100)"`

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),
+		LoginSource:        optional.FromPtr(form.SourceID),
-		LoginName:          optional.Some(form.LoginName),
+		LoginName:          optional.FromPtr(form.LoginName),
 		Password:           optional.FromNonDefault(form.Password),
 		MustChangePassword: optional.FromPtr(form.MustChangePassword),
 		ProhibitLogin:      optional.FromPtr(form.ProhibitLogin),

templates/swagger/v1_json.tmpl

@@ -20989,10 +20989,6 @@
     "EditUserOption": {
       "description": "EditUserOption edit user options",
       "type": "object",
-      "required": [
-        "source_id",
-        "login_name"
-      ],
       "properties": {
         "active": {
           "type": "boolean",

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",
+		Email: &empty,
-		SourceID:  0,
-		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",
+		Email: &newEmail,
-		SourceID:  0,
-		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",
+		Email: &originalEmail,
-		SourceID:  0,
-		Email:     &originalEmail,
 	}).AddTokenAuth(token)
 	MakeRequest(t, req, http.StatusOK)
 }

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",
+				Pronouns: &pronouns,
-				SourceID:  0,
-				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",
+			Pronouns: &pronouns,
-			SourceID:  0,
-			Pronouns:  &pronouns,
 		}).AddTokenAuth(adminToken)
 		MakeRequest(t, req, http.StatusOK)