Skip to content

Add PKCE method to OIDC auth providers#16392

Merged
rak-phillip merged 10 commits intorancher:masterfrom
rak-phillip:feature/15637-pkce
Jan 28, 2026
Merged

Add PKCE method to OIDC auth providers#16392
rak-phillip merged 10 commits intorancher:masterfrom
rak-phillip:feature/15637-pkce

Conversation

@rak-phillip
Copy link
Copy Markdown
Member

@rak-phillip rak-phillip commented Jan 16, 2026

Summary

This adds a new PKCE method input to the OIDC auth provider configuration.

Fixes #15637

Occurred changes and/or fixed issues

  • Add a new input for PKCE method
  • Reorders the OIDC form so that Endpoints and PKCE are rendered before scopes

Technical notes summary

The PKCE method is being added to OIDC providers in rancher/rancher#53285. This PR addresses the UI portion for the new field.

During reviews of the form, we decided that the sections should be reordered to better emphasize importance for configuration.

Areas or cases that should be tested

OIDC configuration:

  • Amazon Cognito
  • Generic
  • Keycloak

Areas which could experience regressions

This change is low risk. Configuration should still work in scenarios where the new option is not supported.

Screenshot/Video

Before

image

After

image

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes
  • The PR has been reviewed in terms of Accessibility
  • The PR has considered, and if applicable tested with, the three Global Roles Admin, Standard User and User Base

Copy link
Copy Markdown
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, spent quite a bit of time trying this and provisioning a new backend until I realised the backend PR is still a draft 🤦

Overall, LGTM. No red flags codewise. Would be nice to test it with the backend though... Let me know how you want to proceed.

FYI, I can't test keycloak OIDC anymore. My setup needs me to have an ngrok domain and we've hit the limit o max domains created for the whole ngrok account, so 🤷

@bigkevmcd
Copy link
Copy Markdown

@rak-phillip After security review, and some discussion we're going to drop support the plain option.

Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
The `plain` option will not be supported in Rancher, meaning that `S256` is the only supported option at this time. We can treat this as a toggle.

Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Copy link
Copy Markdown
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the image you told me to use bigkevmcd/rancher:v2.14-29d9b6212-head, the feature doesn't work:

Screen.Recording.2026-01-27.at.11.24.22.mov

@bigkevmcd
Copy link
Copy Markdown

bigkevmcd commented Jan 27, 2026

With the image you told me to use bigkevmcd/rancher:v2.14-29d9b6212-head, the feature doesn't work:
Screen.Recording.2026-01-27.at.11.24.22.mov

We dropped support for plain, but the field is still a string and not a boolean.

Can we keep it as a string so that it's future-proofed against different PKCE verification algorithms (or the possible return of plain in the event that a customer needs it).

@rak-phillip
Copy link
Copy Markdown
Member Author

We dropped support for plain, but the field is still a string and not a boolean.

Can we keep it as a string so that it's future-proofed against different PKCE verification algorithms (or the possible return of plain in the event that a customer needs it).

@bigkevmcd the value is a string, the checkbox is there to act as a toggle for the feature. See

@aalves08 I'll try to gather some more info to see why it might be failing

Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
@rak-phillip
Copy link
Copy Markdown
Member Author

@aalves08 the PKCE_S256 wasn't exposed to the template, fixing this should resolve the checkbox treating the value as a boolean. Can you try again?

@rak-phillip rak-phillip requested a review from aalves08 January 27, 2026 15:52
@bigkevmcd
Copy link
Copy Markdown

I can also build a newer image...that is from a few weeks ago?

@rak-phillip
Copy link
Copy Markdown
Member Author

I can also build a newer image...that is from a few weeks ago?

@bigkevmcd yeah, that's the original image from when we first synced.

Copy link
Copy Markdown
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing with bigkevmcd/rancher:v2.14-29d9b6212-head

Still not working @rak-phillip ... When we do the action=configureTest the payload is sent correctly. Same for action=testAndApply but the returning cognito network requests from the server return with null.

Screenshot 2026-01-27 at 17 24 43 Screenshot 2026-01-27 at 17 24 51 Screenshot 2026-01-27 at 17 24 58 Screenshot 2026-01-27 at 17 25 01 Screenshot 2026-01-27 at 17 25 03

Similar output with Keycloak.

Other topic, should we surface the PCKE information here, if it's active?

Screenshot 2026-01-27 at 17 17 37

@rak-phillip
Copy link
Copy Markdown
Member Author

Testing with bigkevmcd/rancher:v2.14-29d9b6212-head

Still not working @rak-phillip ... When we do the action=configureTest the payload is sent correctly. Same for action=testAndApply but the returning cognito network requests from the server return with null.

@aalves08 thanks for testing again - we have a new image that we can use to verify bigkevmcd/rancher:v2.14-776aa72c4-head. I'll see if I can repro the same behavior with this.

Other topic, should we surface the PCKE information here, if it's active?

That's a good point. I think we can add it.

Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
Copy link
Copy Markdown
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new image provided I could verify that both on Amazon Cognito and Keycloak everything works fine. User can enable and disable PCKE and the value persists. SLO feature also works fine.

Small change needed (missing translation key):

Image

Otherwise we are good to go

The original path was missing `oidc`, causing the lookup to fail

Signed-off-by: Phillip Rak <rak.phillip@gmail.com>
@rak-phillip rak-phillip requested a review from aalves08 January 28, 2026 15:29
Copy link
Copy Markdown
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All fixed! LGTM ✅

Image

@rak-phillip rak-phillip merged commit 20127be into rancher:master Jan 28, 2026
63 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow enabling PKCE for OIDC providers

3 participants