Skip to content

Add OIDC PKCE configuration through policy #7765

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

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

javorszky
Copy link
Contributor

@javorszky javorszky commented May 8, 2025

Proposed changes

Closes #6713

  • Adds a PKCEEnable property to the OIDC Policy Spec
  • fix whitespace issues in tests, templates, and snapshots
  • adds template test that covers PKCE enabled
  • adds a PKCE enabled to a failing virtual server config test
  • adds a pytest for PKCE flow

PKCE flow oauth client requirements

In the test we use keycloak, so the info is specific to that, however please consult your oauth provider on how to set up a client that will use PKCE. For keycloak you need a client that is

  • using standard flow
  • is NOT using client authentication
  • direct access is disabled
  • public client option is enabled (public client: client application can not keep a secret: mobile apps, frontend clients, etc; the other one is confidential client, they can keep a secret, see https://oauth.net/2/client-types/ for more info)
  • and the PKCE challenge method is S256

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added enhancement Pull requests for new features/feature enhancements go Pull requests that update Go code labels May 8, 2025
@javorszky
Copy link
Contributor Author

javorszky commented May 8, 2025

This is a draft while I do manual testing and write Pytests

done, updated PR text above

Copy link

codecov bot commented May 8, 2025

Codecov Report

Attention: Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.

Project coverage is 52.50%. Comparing base (b422a76) to head (9cbb395).

Files with missing lines Patch % Lines
internal/configs/virtualserver.go 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7765      +/-   ##
==========================================
+ Coverage   52.47%   52.50%   +0.03%     
==========================================
  Files          90       90              
  Lines       21550    21570      +20     
==========================================
+ Hits        11308    11325      +17     
- Misses       9773     9775       +2     
- Partials      469      470       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch from 07acf0b to b5c701a Compare May 13, 2025 11:56
@github-actions github-actions bot added python Pull requests that update Python code tests Pull requests that update tests labels May 13, 2025
@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch 3 times, most recently from 9127a18 to cb93a06 Compare May 16, 2025 14:51
@javorszky javorszky marked this pull request as ready for review May 19, 2025 08:15
@javorszky javorszky requested a review from a team as a code owner May 19, 2025 08:15
@vepatel
Copy link
Contributor

vepatel commented May 19, 2025

Also need to update the existing OIDC example and policy key in docs

@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch from d86ff10 to bafb76b Compare May 20, 2025 08:29
pdabelf5
pdabelf5 previously approved these changes May 20, 2025
@pdabelf5
Copy link
Collaborator

Closes #1782

jjngx
jjngx previously approved these changes May 20, 2025
AlexFenlon
AlexFenlon previously approved these changes May 21, 2025
@javorszky
Copy link
Contributor Author

Documentation for keycloak in the main doucmentation repo: nginx/documentation#585

@javorszky javorszky dismissed stale reviews from AlexFenlon, jjngx, and pdabelf5 via b8a8c8b May 27, 2025 09:35
@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch from c4bfa3f to b8a8c8b Compare May 27, 2025 09:35
@github-actions github-actions bot added documentation Pull requests/issues for documentation chore Pull requests for routine tasks labels May 27, 2025
AlexFenlon
AlexFenlon previously approved these changes May 27, 2025
@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch from fff9b47 to 915e981 Compare May 27, 2025 14:20
@javorszky javorszky force-pushed the feat/issue-6713-oidc-pkce-enable branch from a919e39 to a87153c Compare May 30, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation enhancement Pull requests for new features/feature enhancements go Pull requests that update Go code python Pull requests that update Python code tests Pull requests that update tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for $oidc_pkce_enable directive through OIDC Policy
6 participants