Add optional filter on AzureAD auth group memberships

[gaktive]

Internal reference: SURE-5641

There is a request to use a filter so that only groups that begins with a specific name are being returned instead of all the groups a user has attached to that account. This needs UI to allow the filter to be seen.

In the case where there are many users that have a lot of groups, it was discovered that a lot of logs are generated because all of the associated groups are displayed in the logs.

It was spotted in the function below that there is a filter option available to could help reduce the number of groups being fetched:

// ListGroups fetches all group principals in a directory from the Microsoft Graph API. func (c azureMSGraphClient) ListGroups(filter string) ([]v3.Principal, error) { groups, _, err := c.groupClient.List(context.Background(), odata.Query{Filter: filter}) var principals []v3.Principal for _, u := range *groups { principal, err := c.groupToPrincipal(u) if err != nil { return nil, err } principals = append(principals, principal) } return principals, err }

Backend has a solution being worked on in https://github.com/rancher/rancher/issues/42940; from @andreas-kupries:

The current code is in https://github.com/rancher/rancher/pull/44868 It adds a new string field GroupMembershipFilter to the AzureADConfig structure. The UI for the AzureAD AuthProvider has to be extended with an equivalent field to allow entry of the filter string. Default filter is the empty string. The string is passed into the system like all the other AzureAD information, json field groupMembershipFilter.

[samjustus]

"While the backend might form the data in a given way, the UI should only offer the specific use-case(s) we decide to support."

not sure if you guys agree but just FYI on this from support via sure-5641

[andreas-kupries]

I believe from that text that @kwwii is ok with the backend taking a general filter clause which can do more than just filtering by groups. He simply does not wish to expose the full general capabilities in the UI yet, only a limited form to filter just by groups. And I am ok with the UI restricting this if can make use of the general backend API, i.e. does not ask me to restrict the API for this.

[aalves08]

For e2e, just check if correct payload is being sent to the backend. As per indicated by @nwmac on sprint planning that would suffice in terms of testing from the UI side.

[momesgin]

@andreas-kupries Trying to confirm my understanding of how the payload should look like. The UI exposes a text input to the users that they can add a group name. They'll be able to add multiple group names by separating each one by a comma, and the payload will look like this:

Single case

{
  ...

  groupMembershipFilter: ['group1']
}

Multiple case

{
  ...

  groupMembershipFilter: ['group1', 'group2', 'group3']
}

[andreas-kupries]

Sam, the parts of interest to you are at the bottom of this comment, see This would require..., or maybe the paragraph before that.

@andreas-kupries Trying to confirm my understanding of how the payload should look like. The UI exposes a text input to the users that they can add a group name. They'll be able to add multiple group names by separating each one by a comma, and the payload will look like this:

Both examples are wrong. The groupMembershipFilter field is of type string. This means

Single case

  groupMembershipFilter: ['group1']

has to be

groupMembershipFilter: "group1"

and Multiple case

  groupMembershipFilter: ['group1', 'group2', 'group3']

would be

groupMembershipFilter: "group1, group2,  ..."

For your examples to work the field groupMembershipFilter would have to be a json array. Or, using go terminology and syntax, a slice of strings, i.e. type []string.

Of course, the backend currently expects a string which is a proper OData filter expression, which would look like

groupMembershipFilter: "displayName eq 'group1'"

and

groupMembershipFilter: "(displayName eq 'group1') or (displayName eq 'group2')  or ..."

This would require conversion in the UI from the list of groups to this syntax. Regarding your note about this, i.e.

Hi Andreas, I think FE should never do that kind of conversion as it'll expose the logic, and it's inconsistent with the rest of the app, for the sake of separation of concerns and maintainability I believe the conversion should happen in the BE

I read the original conversation about this topic, i.e.

• https://github.com/rancher/dashboard/issues/10881#issuecomment-2085759223 (Quote origin https://jira.suse.com/browse/SURE-5641?focusedId=1346317&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-1346317)
• https://github.com/rancher/dashboard/issues/10881#issuecomment-2089922578
• https://jira.suse.com/browse/SURE-5641?focusedId=1346306&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-1346306

to mean that this kind of conversion in the UI is ok, with a general API provided by the backend and the UI exposing this in restricted form.

@richard-cox @kwwii I believe we will need some clarification here.

Note, should we come to decide that the backend API should be as restricted as the UI, to a list of groups, then I believe that the field groupMembershipFilter should move from type string to []string, i.e. an array of strings. That would avoid an additional parsing step in the changed backend.

@samjustus FYI

[richard-cox]

If the plan is to...

• support simple filtering by one or more specific group names now
• support more advanced filtering in the future
• do both with the same groupMembershipFilter value

We should probably keep groupMembershipFilter as an OData filter to not break older installs when the switch is made.

From there form wise we could have some kind of toggle, Group Names / OData Filter

• Group Names
  • On create
  • The UI can show an ArrayList
  • The UI will parse this to OData filter eq format before saving
  • On Edit
  • The UI parses OData eq format to array
    • If this parses ok show the ArrayList
    • If this fails just show OData selected instead
• OData Filter
  • just show a LabeledInput

@kwwii does that sound alright?

[nwmac]

Hang on @richard-cox @andreas-kupries @samjustus - my understanding was that UI would present a simple labeled input where the user could enter an arbitrary string (filter) - we'd send this to the backend and for editing, the backend would return that back.

This is an advanced feature, so this felt okay.

We're now talking in the other comments about more than this - maybe we mis-understood, but if this is any more than a string, this feature will need to be pushed out of 2.9.0.

^^ FYI @gaktive @kwwii - we need an answer on this asap, because @momesgin is blocked from moving forward

[richard-cox]

Apologies, I introduced scope creep.

Options for groupMembershipFilter

1. a direct OData string which supports all types of filtering
   • example: (displayName eq 'group1') or (displayName eq 'group2') or ...
   • Great for functionality, not great for users to construct
2. a comma separated string of groups
   • example: group1,group2, ...
   • Fulfils specific request(?) but inflexible. Makes is difficult to switch to option 3 in future
3. UI shows input controls to allow easy construction of more comples OData filters (like option 1)
   • Does it all, but much more work

If this needs to be in 2.9.0 then it's either option 1 or 2?

[gaktive]

@nwmac since Collie is eager to get this feature wrapped up, do we need to discuss with them? or should we pick Option 1 for now since that gives us flexibility to get into Option 3?

[richard-cox]

Discussion moved to new slack channel, but briefly we're favouring option 1

[gaktive]

For UI QA, the related backend ticket will be tested by @anupama2501 so sync with her in case there's anything to coordinate or if it'll be fine for one team to test.

[gaktive]

Per @yonasberhe23, this will be a mixture of automated & manual testing.

[martyav]

We recently received an open issue on this in the docs repo. Is this in a complete enough state that we can document the UI steps? Are there also CLI steps we'd need to describe? https://github.com/rancher/rancher-docs/issues/1370

[gaktive]

@martyav the UI portion is complete as it's now over to QA. I'll defer to the backend folks on the CLI side though I'd use @andreas-kupries' example syntax as guidance.

[gaktive]

For UI QA, Collie QA did their own testing around here per https://github.com/rancher/rancher/issues/42940#issuecomment-2232337738

[izaac]

Rancher version: v2.9.0-rc2 Tested using the field Limit users by group membership during the Auth provider activation on Rancher. I mainly used a few filter expressions to filter or exclude groups by startWith or contains functions.

After that it is possible to validate the groups filters by using the Restrict access to only the authorized users & groups after the Auth provider activation.

I tested Logged in and out using different users from the filtered groups with authorization.

The validation is based on the QA done on the backend in this issue

Negative testing done using invalid filter properties to receive API errors. Attempt to login using a user from a group not allowed/filtered.