directory-connector icon indicating copy to clipboard operation
directory-connector copied to clipboard

[PS-1213] GSuite: Preflat groups using includeDerivedMembership flag.

Open tm-drtina opened this issue 3 years ago • 1 comments
trafficstars

Type of change

  • [x] Bug fix
  • [ ] New feature development
  • [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • [ ] Build/deploy pipeline (DevOps)
  • [ ] Other

Objective

Since Bitwarden doesn't have nested groups the function flattenUsersToGroups is used to flatten nested groups into one list of users. GSuite SDK supports includeDerivedMembership which already pre-flattens the user list while fetching if from the GSuite API. (It also returns all intermediate groups, but at that point we can safely ignore them.)

The motivation is that if we ignore some intermediate groups (because we don't want them in BW) the flattening function cannot fetch members of that group. So either we have to include all intermediate groups or we won't get all nested users.

Example:

GSuite group "BW Admins" has one member group "GS Admins". GSuite group "GS Admins" has one member account "[email protected]".

I want to import group "BW Admins" with one member "[email protected]". Currently I need to import also the group "GS Admins", otherwise this tool doesn't resolve members of the intermediate group (because it is excluded).

Potential breaking change?

This might be breaking, because someone might be relying on the exclusion of the intermediate groups, but I think this is more a bug than a feature. If it is breaking I can also introduce new sync config option to have it as opt-in.

Code changes

I've added includeDerivedMembership: true parameter to member.list GSuite SDK call, which does flattening of nested groups in GSuite rather than in the code. Also we can safely ignore all groups return by this call, since it is only used for nested group flattening. https://developers.google.com/admin-sdk/directory/reference/rest/v1/members/list#query-parameters

Testing requirements

Before you submit

  • [x] I have checked for linting errors (npm run lint) (required)
  • [ ] I have added unit tests where it makes sense to do so (encouraged but not required)
  • [ ] This change requires a documentation update (notify the documentation team)
  • [ ] This change has particular deployment requirements (notify the DevOps team)

tm-drtina avatar Jul 28 '22 12:07 tm-drtina

Thank you for your contribution! We've added this to our internal Community PR board for review. ID: PS-1213

bitwarden-bot avatar Jul 28 '22 12:07 bitwarden-bot