Make LoginName optional for admin edit user API

[me-heer]

Fixes #18935
Related #28733

• Remove required doc comment for EditUserOption.LoginName
• Add check in EditUser to only update LoginName when provided
• Modify TestAPIEditUser to load bean using LowerName instead of LoginName
• Add case in TestAPIEditUser to check LoginName is updated when passed

[KN4CK3R]

LoginName should be required if LoginSource is set. So maybe make LoginSource optional too. (related: #28733)

[me-heer]

@KN4CK3R I didn't get you, LoginSource is not used in this API

[KN4CK3R]

type EditUserOption struct {
	// required: true
	SourceID  int64  `json:"source_id"`

There it is.

[KN4CK3R]

Now you made both optional but I meant that if one was set, the other must be set too.

[KN4CK3R]

LoginName gets set from parseAuthSource too.

[me-heer]

So, should there be a validation for it? or should we simply return from parseAuthSource if (sourceID == 0 || len(form.LoginName) == 0)

[KN4CK3R]

I would remove the lines 232-234 and add the check to the (faulty) if sourceID == 0 in parseAuthSource. Don't make it too beautiful because that part will be replaced with #28733 anyway.

[me-heer]

I did add the check in parseAuthSource, but we can't remove 232-234 otherwise users will only be able to update LoginName when sourceId != 0
(See failing test when 232-234 is removed)

[KN4CK3R]

As I said, the sourceId != 0 is just wrong. 0 is the valid id of the default database login. With the admin api it's currently not possible to set a user login source to "use database".

gitea/models/auth/source.go

Lines 276 to 284 in 5574968

	func GetSourceByID(ctx context.Context, id int64) (*Source, error) {
	source := new(Source)
	if id == 0 {
	source.Cfg = registeredConfigs[NoType]()
	// Set this source to active
	// FIXME: allow disabling of db based password authentication in future
	source.IsActive = true
	return source, nil
	}

I think that check is there because if you don't provide a value in the request, it defaults to 0. Therefore you can't distinguish between "not set" and "set to 0". Other fields in the EditUserOption form use pointers which can be tested for nil to have a "is set" check. As LoginName is optional too now, both fields should be pointers too. You may need to change other tests then too and util.ToPointer will be your friend. 👍

[me-heer]

Thanks, this helped.
The fix for this case became messy, so I'll convert this to a draft for now, I'll pick it up once #28733 is merged.

[me-heer]

Can you take a look at it now, please?