-
Notifications
You must be signed in to change notification settings - Fork 38.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OIDC authentication against API server: allow multiple values in token for requiredClaims #101314
Conversation
@devkid: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The 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. |
Hi @devkid. Thanks for your PR. I'm waiting for a kubernetes 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: devkid The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c438052
to
151df66
Compare
…n for requiredClaims
151df66
to
43893bc
Compare
/remove-sig api-machinery |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I am unsure about this change as it changes the meaning of the existing check.
foo == val
and foo in [val1 val2 val3]
are not exactly the same thing.
The existing flag based config is really terrible, and I do not want to expand it further but this seems dangerous without opting in somehow. This also opens the door to wanting stuff like both foo and bar in [val1, val2, ...]
. I would like to see this stuff more holistically handled in a proper config format.
/hold
staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
Outdated
Show resolved
Hide resolved
staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go
Outdated
Show resolved
Hide resolved
/remove-sig api-machinery |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. In response to this:
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. |
@alfredkrohmer see if #121078 works for your use case. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
When using
requiredClaims
, currently only sting values in the OIDC token are accepted. We want to use this so that we can have an array of values in the token and the value inrequiredClaims
must be in that array.Example:
OIDC token contains claim
roles
with values["dev", "prod"]
. We want to be able to specifyrequiredClaims
asroles: dev
so that any user that has thedev
role can be authenticated.Which issue(s) this PR fixes:
Fixes #101291
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: