feat(brokers/cache): Change user info validations to require at least one remote group

[denisonbarbosa]

We used to allow the brokers to provide user information without any group. This creates problems when managing local groups. To prevent this, we now force the brokers to return at least one group for the user.

The user group list must contain at least one remote group (with UGID) to act as the default group for the user. This means that it must be the first group in the list.

UDENG-1865

[denisonbarbosa]

Ok, I must be missing something. It can't be this simple, can it?

[GabrielNagy]

Looks good!

[didrocks]

Ok, I must be missing something. It can't be this simple, can it?

To me yes, you are missing something out :)

I mentioned in the HO that our cache also had this logic and needs to be changed. Basically, it should fail too if no groups is provided and not handle special case (and so, tests should be changed too):

		// No groups were specified for this request.
		if userDB.GID == -1 {

Also, as part of the Jira card, it’s written: - always force the broker to cache and return everything, from group name to users. Here it seems you handle the group and link it to the card, what about the mandatory user information? It may already be the case that we enforce them to be here, but I think part of this card is to double check it, that we have tests for them and so on. Some of the fields like Gecos and avatars are optional, some other are mandatory. I think we should just mirror the mandatory info in adduser (+uid ofc).

[denisonbarbosa]

It may already be the case that we enforce them to be here, but I think part of this card is to double check it, that we have tests for them and so on.

Our validation of the user info provided by the broker was already robust, only lacking the new group checking blocks that are being added in this PR.

cc @didrocks

[didrocks]

Ok, one finale thing with a suggestion and some double checking, and we should be good to go!

Do we have a test showing up the non mandatory values? Like no gecos and no avatar?

I think we should write the optional fields as comments in the struct next to them.

[didrocks]

Thanks for the christmas' gift :p

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ca985a4) 88.28% compared to head (a220678) 88.25%.
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
- Coverage   88.28%   88.25%   -0.04%     
==========================================
  Files          32       32              
  Lines        2433     2426       -7     
==========================================
- Hits         2148     2141       -7     
  Misses        219      219              
  Partials       66       66              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.