Ensure source_id parameter is not skipped when set to 0 and correctly filter users in /api/v1/admin/users endpoint (#6240)

Signed-off-by: Awiteb <a@4rs.nl>
Fixes: #6239

## Checklist

### Tests

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [X] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [X] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] I do not want this change to show in the release notes.
- [X] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6240
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Awiteb <a@4rs.nl>
Co-committed-by: Awiteb <a@4rs.nl>

models/user/search.go

@@ -25,7 +25,7 @@ type SearchUserOptions struct {
 	Type          UserType
 	UID           int64
 	LoginName     string                 // this option should be used only for admin user
-	SourceID      int64  // this option should be used only for admin user
+	SourceID      optional.Option[int64] // this option should be used only for admin user
 	OrderBy       db.SearchOrderBy
 	Visible       []structs.VisibleType
 	Actor         *User // The user doing the search
@@ -98,8 +98,8 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess
 		cond = cond.And(builder.Eq{"id": opts.UID})
 	}
 
-	if opts.SourceID > 0 {
-		cond = cond.And(builder.Eq{"login_source": opts.SourceID})
+	if opts.SourceID.Has() {
+		cond = cond.And(builder.Eq{"login_source": opts.SourceID.Value()})
 	}
 	if opts.LoginName != "" {
 		cond = cond.And(builder.Eq{"login_name": opts.LoginName})

routers/api/v1/admin/user.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"strconv"
 
 	"code.gitea.io/gitea/models"
 	asymkey_model "code.gitea.io/gitea/models/asymkey"
@@ -430,12 +431,19 @@ func SearchUsers(ctx *context.APIContext) {
 	//     "$ref": "#/responses/forbidden"
 
 	listOptions := utils.GetListOptions(ctx)
+	intSource, err := strconv.ParseInt(ctx.FormString("source_id"), 10, 64)
+	var sourceID optional.Option[int64]
+	if ctx.FormString("source_id") == "" || err != nil {
+		sourceID = optional.None[int64]()
+	} else {
+		sourceID = optional.Some(intSource)
+	}
 
 	users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
 		Actor:       ctx.Doer,
 		Type:        user_model.UserTypeIndividual,
 		LoginName:   ctx.FormTrim("login_name"),
-		SourceID:    ctx.FormInt64("source_id"),
+		SourceID:    sourceID,
 		OrderBy:     db.SearchOrderByAlphabetically,
 		ListOptions: listOptions,
 	})

tests/integration/admin_user_test.go

@@ -4,14 +4,17 @@
 package integration
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"strconv"
 	"testing"
 
+	auth_model "code.gitea.io/gitea/models/auth"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -89,3 +92,43 @@ func TestAdminDeleteUser(t *testing.T) {
 	assertUserDeleted(t, userID, true)
 	unittest.CheckConsistencyFor(t, &user_model.User{})
 }
+
+func TestSourceId(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	testUser23 := &user_model.User{
+		Name:        "ausersourceid23",
+		LoginName:   "ausersourceid23",
+		Email:       "ausersourceid23@example.com",
+		Passwd:      "ausersourceid23password",
+		Type:        user_model.UserTypeIndividual,
+		LoginType:   auth_model.Plain,
+		LoginSource: 23,
+	}
+	defer createUser(context.Background(), t, testUser23)()
+
+	session := loginUser(t, "user1")
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadAdmin)
+
+	// Our new user start with 'a' so it should be the first one
+	req := NewRequest(t, "GET", "/api/v1/admin/users?limit=1").AddTokenAuth(token)
+	resp := session.MakeRequest(t, req, http.StatusOK)
+	var users []api.User
+	DecodeJSON(t, resp, &users)
+	assert.Len(t, users, 1)
+	assert.Equal(t, "ausersourceid23", users[0].UserName)
+
+	// Now our new user should not be in the list, because we filter by source_id 0
+	req = NewRequest(t, "GET", "/api/v1/admin/users?limit=1&source_id=0").AddTokenAuth(token)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &users)
+	assert.Len(t, users, 1)
+	assert.Equal(t, "the_34-user.with.all.allowedChars", users[0].UserName)
+
+	// Now our new user should be in the list, because we filter by source_id 23
+	req = NewRequest(t, "GET", "/api/v1/admin/users?limit=1&source_id=23").AddTokenAuth(token)
+	resp = session.MakeRequest(t, req, http.StatusOK)
+	DecodeJSON(t, resp, &users)
+	assert.Len(t, users, 1)
+	assert.Equal(t, "ausersourceid23", users[0].UserName)
+}