Skip to content
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

💥 Disable legacy OIDC endpoints by default #4912

Merged

Conversation

sergei-maertens
Copy link
Member

Part of #3283 - stumbled on this while working on #4911

Changes

While the removal of the legacy endpoints is postponed to Open Forms 4.0, I thought we should grab the chance now to switch the default value for the configuration options from legacy-enabled to legacy-disabled. This makes it more likely that fresh instances are set up with the best situation possible and won't have to deal with breaking changes later for 4.0.

Checklist

Check off the items that are completed or not relevant.

  • Impact on features

    • Checked copying a form
    • Checked import/export of a form
    • Config checks in the configuration overview admin page
    • Problem detection in the admin email digest is handled
  • Release management

    • I have labelled the PR as "needs-backport" accordingly
  • I have updated the translations assets (you do NOT need to provide translations)

    • Ran ./bin/makemessages_js.sh
    • Ran ./bin/compilemessages_js.sh
  • Dockerfile/scripts

    • Updated the Dockerfile with the necessary scripts from the ./bin folder
  • Commit hygiene

    • Commit messages refer to the relevant Github issue
    • Commit messages explain the "why" of change, not the how

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.61%. Comparing base (f8095b4) to head (64f6152).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4912   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files         760      760           
  Lines       25820    25820           
  Branches     3383     3383           
=======================================
  Hits        24947    24947           
  Misses        608      608           
  Partials      265      265           

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

@sergei-maertens sergei-maertens changed the title Disable legacy OIDC endpoints by default 💥 Disable legacy OIDC endpoints by default Dec 13, 2024
@sergei-maertens sergei-maertens force-pushed the cleanup/3283-enable-new-oidc-endpoint-default branch from 67ae388 to 990fd6e Compare December 17, 2024 14:03
@sergei-maertens sergei-maertens marked this pull request as ready for review December 17, 2024 14:03
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

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

Regarding the VCR tests I made sure that I see in the diff all the directories related to the OIDC.

With Open Forms 3.0, we can make the breaking change of updating the
default.

The new OIDC API endpoints all point to a single callback URL, so it
means less configuration overhead on the identity provider side. The
breaking change means that the new URI's must be added to the
allowlist of the identity provider.

The new endpoint is '/auth/oidc/callback/', and it applies to all
OIDC configuration flavours.
@sergei-maertens sergei-maertens force-pushed the cleanup/3283-enable-new-oidc-endpoint-default branch from 990fd6e to 64f6152 Compare December 19, 2024 08:55
@sergei-maertens sergei-maertens merged commit eedffb3 into master Dec 19, 2024
32 of 34 checks passed
@sergei-maertens sergei-maertens deleted the cleanup/3283-enable-new-oidc-endpoint-default branch December 19, 2024 09:24
@vaszig vaszig mentioned this pull request Dec 19, 2024
10 tasks
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.

2 participants