✨ Add original user/group as extra to the impersonating client used by virtual workspace #3155
Conversation
kcp-ci-bot
added
release-note
|
Hi @turkenh. Thanks for your PR. I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
turkenh
changed the title
| // the impersonation config so that the receiver can make | ||
| // decisions based on the original user/groups. | ||
| impersonationConfig.Impersonate.Extra[OriginalUserAnnotationKey] = []string{user.GetName()} | ||
| impersonationConfig.Impersonate.Extra[OriginalGroupsAnnotationKey] = user.GetGroups() |
There was a problem hiding this comment.
trying to wrap my head around whether we actually want to preserve the original groups as part of the impersonated identity.
What we really want is this:
- inside of the virtualized workspace this user should have the admin CRUD permissions because the authorization is done by the virtual workspace.
- outside of the virtualized workspace (e.g. in admission webhooks) we want the actually export owner identity to be effective.
I think we have that partially somewhere called "soft impersonation". Have to double check our thinking there.
🤔
There was a problem hiding this comment.
AFAIR I also believe we should leave groups out. we are really only interested in the person's identity. Groups are per-workspace and not global.
Take this with a grain of salt: it's been a while.
There was a problem hiding this comment.
outside of the virtualized workspace (e.g. in admission webhooks) we want the actually export owner identity to be effective.
Owner identity is both its username and groups. Even we add its groups to the list of groups, it would be partial, permissions might have been granted to its user only. So, on the admission side, I would rather rely on extra where I can find both username/group.
|
With #3156 the problem here goes away as the external virtual workspace user is preserved. |
Signed-off-by: Hasan Turken <[email protected]>
turkenh
force-pushed
the
impersonate-with-original
branch
from
dd572dd to
a6b9309
Compare
|
/lgtm |
|
LGTM label has been added. Git tree hash: ff7c38f70b4fa926336c4744c77bfa3758356b8b
|
kcp-ci-bot
added
ok-to-test
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kcp-ci-bot
added
the
approved
Summary
We have some validations on our CRDs where we would like to act based on the user information, i.e., we want to allow a specific field to only be initialized by a controller but no one else. The current implementation of build virtual workspaces loses the original user info while doing impersonation.
This PR adds original user and groups information to impersonating client as
extra, so that our validator can extract the user/group information and make decisions accordingly.Related issue(s)
N/A
Release Notes