-
Notifications
You must be signed in to change notification settings - Fork 427
backend: Implement support for Azure AKS managed OIDC auth provider #3177
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
backend: Implement support for Azure AKS managed OIDC auth provider #3177
Conversation
This is one option that is more flexible for other auth providers and allows overriding the client id and issuer for validation as well as allows for grabbing the access_token instead of id_token which is required for AKS's implementation of OIDC managed auth. Alternatively this can be implemented with a single flag specifically for Azure AKS managed OIDC (which I will take a stab at implementing before submitting a PR) to have both as options. Signed-off-by: David Dobmeier <david.dobmeier@live.com>
Thanks for all this. I'm starting to understand access_token is for authorization, and id_token is for authentication. It seems other providers use access_token from a bit of searching about. So I think this way is probably better, than the AKS specific flag. It would be helpful to have some guidance of how to set this up for testing? Setting up AKS (following the link you posted I guess?) Then where to get this? |
I think |
Yupp that's exactly right, I'll try to get the documentation and tests implemented shortly. Will include links to the MSFT docs for setting up entra OIDC with AKS and then steps for setting up the enterprise app + headlamp with the new parameters |
Updates the chart with the new parameters + also updates the tests. Fully backwards compatible with existing installations. For users using `external-secret` for oidc config will need to specify the useAccessToken in the values yaml or set `- "-oidc-use-access-token=$(OIDC_USE_ACCESS_TOKEN)"` in the extraArgs. This is done to maintain backwards compatibility. Signed-off-by: David Dobmeier <david.dobmeier@dcsg.com>
Planning on writing up the documentation tonight - I updated the helm chart accordingly for the 3 methods of setting the config properties (env, inline, or external secret) as well. Let me know if anything should be adjusted there, technically these are all parameters that could be set via the - "-oidc-validator-client-id=$(OIDC_VALIDATOR_CLIENT_ID)"
- "-oidc-validator-idp-issuer-url=$(OIDC_VALIDATOR_ISSUER_URL)"
- "-oidc-use-access-token=$(OIDC_USE_ACCESS_TOKEN)" Absolutely down to drop the helm chart changes it it introduces too much clutter in favor of just these parameters called out in the documentation (whichever you believe is the best approach) |
Signed-off-by: David Dobmeier <david.dobmeier@dcsg.com>
a0f728f
to
73d7013
Compare
Adds documentation for the new parameters for token validation, using `access_token` instead of `id_token`, and includes a full walkthrough of setting up an AKS cluster with OIDC + deploying headlamp using it. Credit to `magohl` for some of the photos and steps used in documentation, linked to in `Issue 2480`. Signed-off-by: David Dobmeier <david.dobmeier@dcsg.com>
73d7013
to
d614b50
Compare
Documentation and tutorials have been added - would be happy to add anything else for clarity or additional testing for validation. Let me know if there's anything else I should do for next steps. Thanks! |
Thanks @daviddob for adding this, I am in the process of reviewing and testing this, will get back to you with more updates |
I just want to second how awesome this is! Thank you @daviddob ! |
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.
🎉 thanks!
I didn’t test the chart changes. They look ok to me, but I’ll ask our local chart expert @knrt10 to have a quick look before we merge.
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.
Tested this with AKS and EntraID, and this works like charm, and the docs as well are on point, thanks, lets wait for @knrt10 final approval and then we are good to go
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.
Awesome. Thanks for the change @daviddob
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashu8912, daviddob, illume, knrt10 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 |
it seems this has broken oidc auth: #2643 (comment) We're getting this error now:
|
Thanks @viceice . We are looking into it. Thanks! |
Replied in issue #2643 (comment), can write up a quick patch for this approach as well as the doc version confusion here #3380 if desired and no one has triaged this yet. |
It seems that this doesn't handle the edge case where an access token for an Entra ID user has already been exchanged, and redeploying Headlamp loses the refresh token.
I've tried the following, they all result in the same
|
This allows for enabling configuration for AKS managed OIDC and is also more flexible for other auth providers in the following ways:
clientID
andissuerURL
for validation phasesaccess_token
instead ofid_token
.All three of these changes are required in the case of AKS due to its nonstandard OIDC flow where it returns an access token from a single MSFT controlled enterprise application with the ID
6dae42f8-4368-4678-94ff-3960e28e3630
msft docs source. This is also apparent with other auth providers having nonstandard flows and token issuance this could potentially resolve as well.The
access_token
returned is also using the older v1 token as opposed to the newer v2 tokens. This is not possible to override even when setting up your enterprise app to only issue v2 tokens, if the scope includes6dae42f8-4368-4678-94ff-3960e28e3630/user.read
, then it will be issues from a v1 endpoint. This means that the issuer returned in the token does not match the one obtained in discovery as it transitions fromhttps://login.microsoftonline.com/<tenant-uuid>/v2.0
tohttps://sts.windows.net/<tenant-uuid>/
msft sourceThe above can be confirmed with any AKS managed OIDC cluster and
kubelogin
flow followed by decoding the token returned to you.This implements three new flags
oidc-validator-idp-issuer-url
,oidc-validator-client-id
, andoidc-use-access-token
to permit the above required alterations to token validationCloses: #2480, #1498 (will provide tests and docs for the above)
Related to:
id_token
Testing & Docs
This can be tested by building the image from source, and deploying it to a K8s cluster with the following additional args defined in the helm chart (in addition to the standard OIDC config pointing at your azure ad enterprise app)
Before writing out all of the tests and docs (I have this working and tested against both this approach and the one discussed below), I wanted to get some initial feedback on the changes and which method is preferred. From there id be happy to button up the tests, helm chart, and docs accordingly to get users able to auth with AKS managed OIDC implementations.
Alternate Options
Alternatively this can be implemented with a single flag specifically for Azure AKS managed OIDC
oidc-use-aks-managed
. Implementation is here and adds only a single configuration value but is also then specific for AKS only which may not be desired. https://github.com/kubernetes-sigs/headlamp/compare/main...daviddob:headlamp:azure-aks-managed-oidc-support-simplified?expand=1