Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add DefaultResourcesFilter.webauthn() #15970

Merged

Conversation

Kehrlann
Copy link
Contributor

@Kehrlann Kehrlann commented Oct 22, 2024

reviewer: @rwinch

Description

Use DefaultResourcesFilter with a proper static method for webauthn pages.

The filter is registered when:

  • the default login page is used
  • the default registration page is used

This means the filter may end up registered twice, which may cause a slight performance hit. We accept this tradeoff for code simplicity, since this is not intended for production workloads anyways.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 22, 2024
@sjohnr sjohnr requested a review from rwinch October 24, 2024 16:57
@sjohnr sjohnr added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement status: waiting-for-triage An issue we've not yet triaged and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 24, 2024
Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

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

I've placed comments inline

@Kehrlann Kehrlann force-pushed the webauthn-default-resource-filter branch from 9d708be to 05955a9 Compare November 13, 2024 17:36
@Kehrlann Kehrlann changed the title webauthn: use DefaultResourcesFilter without reflection webauthn: introduce DefaultResourcesFilter#webauthn, conditionally register default UI Nov 13, 2024
@Kehrlann Kehrlann requested a review from rwinch November 14, 2024 08:34
- Unconditionally use the DefaultResourcesFilter, because the javascript file is required by the
  DefaultWebAythnPageGeneratingFilter, which is always registered.
@Kehrlann Kehrlann force-pushed the webauthn-default-resource-filter branch from 05955a9 to 708e052 Compare November 14, 2024 12:52
Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

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

I've left feedback inline

@Kehrlann Kehrlann force-pushed the webauthn-default-resource-filter branch from 708e052 to b276958 Compare November 14, 2024 17:11
@rwinch rwinch changed the title webauthn: introduce DefaultResourcesFilter#webauthn, conditionally register default UI Add DefaultResourcesFilter.webauthn() Nov 14, 2024
@rwinch rwinch removed the status: waiting-for-triage An issue we've not yet triaged label Nov 14, 2024
@rwinch rwinch added this to the 6.4.0 milestone Nov 14, 2024
@rwinch rwinch merged commit 2639ac6 into spring-projects:main Nov 14, 2024
6 checks passed
@rwinch
Copy link
Member

rwinch commented Nov 14, 2024

Thanks for the Pull Request! This is now merged into main 😄

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants