feat(api): add groups field to password requests and responses - #4989
Open
CasLubbers wants to merge 1 commit into
Open
feat(api): add groups field to password requests and responses#4989CasLubbers wants to merge 1 commit into
CasLubbers wants to merge 1 commit into
Conversation
Signed-off-by: Cas Lubbers <clubbers@akamai.com>
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Adds
groupsto the gRPC API'sPasswordmessage andnew_groupstoUpdatePasswordReq, so local users managed over the API can carry groups.storage.Password.Groupsalready existed; only the API surface was missing.What this PR does / why we need it
Groups for local passwords were added in #4456 and work through the config file, but the gRPC API could not set them. A
user created over the API authenticates with no groups, so any relying party that authorizes on the
groupsclaim treatsthem as unprivileged. That left the config file as the only way to manage users with groups, which requires a restart to dex to take effect.
CreatePasswordcopiesgroupsinto storageUpdatePasswordacceptsnew_groupsand counts it as a change on its ownListPasswordsreturnsgroups, so the field is not write-onlyNo storage changes and no migration, the column has existed since #4456.
Closes #4972
Special notes for your reviewer
Groups can be set and replaced, but not cleared. Proto3 encodes an empty repeated field to the same zero bytes as an absent one, so
new_groups: []arrives as nil and reads as "leave alone". This matches every repeated field already in the API,UpdateClientReqtreatsredirect_uris,trusted_peers,allowed_connectors,sso_shared_withandpost_logout_redirect_uristhe same way, and none of them can be cleared either. I followed the existing convention rather than making this one field special. If clearing is wanted, it seems better solved once for all six fields, aFieldMaskon the request, or a wrapper message per field, and would need a new tag number, since changing tag 4 fromrepeated stringto a message is wire-breaking.Other notes:
api.pb.gois regenerated with the pinned toolchain (protoc 29.3, protoc-gen-go 1.36.5); most of the diff is thedescriptor blob re-wrapping.
TestPasswordnow covers groups end to end: set on create, read back via storage andListPasswords, changed viaupdate, and left untouched by an update that does not mention them.
storage.type: kubernetes— groups set over gRPC reach thegroupsclaim in the IDtoken, and a password created without groups omits the key in the CR.