Skip to content
This repository was archived by the owner on Dec 20, 2025. It is now read-only.

Comments

feat(fiat/google-groups): expand indirect google groups for emails#1213

Merged
jasonmcintosh merged 1 commit intospinnaker:masterfrom
himanhsugusain:master
May 8, 2025
Merged

feat(fiat/google-groups): expand indirect google groups for emails#1213
jasonmcintosh merged 1 commit intospinnaker:masterfrom
himanhsugusain:master

Conversation

@himanhsugusain
Copy link
Contributor

@himanhsugusain himanhsugusain commented May 4, 2025

Google groups list api does not return the groups that the user is indirectly part of.
This change recursively traverse each group and get its parent group, we also keep track of already visited groups and not query it again.
Google groups does not allow one group to be in subgroup of itself, so that case check is not needed.

Eg.
User A if member of Group G1 and G1 is member of G2, so A is member of G2 indireclty.
Before:
Access provided to group G2 will not be inherited by A.
After
Access provided to G2 will be inherited by A.

PS: This is how Google own permission model work. Too bad they don't provide a direct api for it.

This also updates com.google.api.client.googleapis.auth.oauth2.GoogleCredential(deprecated) to com.google.auth.oauth2.GoogleCredentials

@himanhsugusain himanhsugusain requested a review from ttomsu as a code owner May 4, 2025 10:33
@spinnakerbot
Copy link
Contributor

The following commits need their title changed:

Please format your commit title into the form:

<type>(<scope>): <subject>, e.g. fix(kubernetes): address NPE in status check

This allows us to easily generate changelogs & determine semantic version numbers when cutting releases. You can read more about commit conventions here.

@dbyron-sf
Copy link
Contributor

Thanks for the PR @himanhsugusain! Can you add some info about the impact of this change? Like, before this PR a user who is a member of ABC groups isn't authorized for XYZ, but is afterwards (or whatever the impact actually is)?

@dbyron-sf
Copy link
Contributor

Also, since this is a feature, it'd be great to have the PR against the monorepo.

@himanhsugusain
Copy link
Contributor Author

@dbyron-sf Made the recommended fix to test. You can review once more.
Added more info on the change.
I did start with monorepo, but could not get my IDE setup to work with it properly ( too many things to build ).
Can you please cherry-pick this to monorepo after merge.
I'll try to raise next PRs to monorepo.

@jasonmcintosh jasonmcintosh merged commit 2540491 into spinnaker:master May 8, 2025
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants