Skip to content

Conversation

britter
Copy link
Contributor

@britter britter commented Aug 21, 2017

In by simple Spring Boot application with Basic Authentication, I can add localization and custom messages if somebody tries to access a resource with wrong credentials. This is implemented in AbstractUserDetailsAuthenticationProvider.

However when somebody forgets to add authentication information the ExceptionTranslationFilter will throw an InsufficientAuthenticationException. Since the message is hard coded, there is no easy way to customize or localize the error message.

This PR adds localization using message bundles and a MessageSourceAccessor the same way as it is implemented in AbstractUserDetailsAuthenticationProvider.

@britter
Copy link
Contributor Author

britter commented Oct 23, 2017

@jgrandja any feedback?

@jgrandja jgrandja self-assigned this Oct 23, 2017
@jgrandja jgrandja added type: enhancement A general enhancement in: web An issue in web modules (web, webmvc) labels Oct 23, 2017
Copy link
Contributor

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @britter !

There has been very little updates to the localization of error messages in the past few years as it's not recommended to give any information to potential attackers on what the resulting error was. However, the updates you have provided are fine so I'll merge this PR after you apply the requested changes from my comments and add a test.

@@ -83,6 +86,8 @@

private RequestCache requestCache = new HttpSessionRequestCache();

private MessageSourceAccessor messages = SpringSecurityMessageSource.getAccessor();
Copy link
Contributor

Choose a reason for hiding this comment

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

Add final modifier

@@ -0,0 +1 @@
ExceptionTranslationFilter.insufficientAuthentication=Full authentication is required to access this resource
Copy link
Contributor

Choose a reason for hiding this comment

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

Add this key to messages.properties (default bundle) as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@britter
Copy link
Contributor Author

britter commented Oct 23, 2017

I need some time to work on a test, since I have to understand how the current tests work and how to make the test reliable on different system locales. Thank you!

@britter britter force-pushed the insufficient-authentication-messages branch from af837a8 to ee54038 Compare November 3, 2017 15:28
@britter britter force-pushed the insufficient-authentication-messages branch from ee54038 to c6e9376 Compare November 3, 2017 15:39
@britter
Copy link
Contributor Author

britter commented Nov 3, 2017

@jgrandja all done. Let's see what Travis thinks.

@rwinch rwinch dismissed jgrandja’s stale review November 16, 2017 17:20

The changes have been made

@rwinch rwinch assigned rwinch and unassigned jgrandja Nov 16, 2017
@rwinch rwinch added this to the 5.0.0 milestone Nov 16, 2017
@rwinch rwinch closed this in fffd781 Nov 16, 2017
@rwinch
Copy link
Member

rwinch commented Nov 16, 2017

Thanks for the PR @britter! This is now merged into master via fffd781

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

3 participants