Default Log In Page should always be generic message

The error message on the default log in pages should always be a generic message so that it does not have any information leakage when AuthenticationException.message includes details about the failure. To help developers, we should also ensure that the failure is logged at the debug level (likely in the AuthenticationManager so that it happens for all failures).

@rwinch

Hi @rwinch
Can you assign this issue to me? I will work on this

@Tejas-Teju

Thanks, @Tejas-Teju! Looking forward to another PR from you.

@jzheaux

@jzheaux

Need your inputs

Can DefaultLoginPageGeneratingFilter.getLoginErrorMessage() be updated for generic messages in login pages? This method returns a Local message for AuthenticationException types but "Invalid credentials" for others. If we can localize the generic message say Bad credentials will that be useful?
The DEBUG messages can be logged at ProviderManager?

We can even log the error at OneTimeTokenAuthenticationProvider and re-throw the exception as BadCredentialsException (Refer PR)

The change is to not check for an exception message. Because of that, the answer is always "Invalid Credentials". That means this:

String errorMsg = loginError ? getLoginErrorMessage(request) : "Invalid credentials";

should be able to change to:

String errorMsg = "Invalid Credentials";

and getLoginErrorMessage should not be necessary anymore.

The format for failure logs that we usually follow is:

{what the code failed to do} since {reason for the failure}

For example,

Failed to authenticate token since it was either expired or missing

As for Provider Manager, I'd probably have a generic message like:

Denying authentication since all attempted providers failed

for the end and a more contextual message for the AccountStatusException and InternalAuthenticationServiceException catch block.

@Tejas-Teju

We can even log the error at OneTimeTokenAuthenticationProvider and re-throw the exception as BadCredentialsException

Good idea, @Tejas-Teju, please go ahead an just focus on ProviderManager and DefaultLoginPageGeneratingFilter, though, for this ticket. And would you mind creating a separate ticket to improve logging in One-Time Token? That way we can review that entire feature holistically.

@jzheaux

Thanks @jzheaux

There was a test case for getting the error message in KOREA; hence, I thought we'll have to render the message in Local language.

Sure, I'll raise a separate ticket for logging.

Add debug messages for auth exceptions in ProviderManager

Return Invalid Credentials message on login error

Add debug messages for auth exceptions in ProviderManager

Polish Debug Messages

Return Invalid Credentials message on login error

Superceded by #16575

Update Asserted Login Page Error Message

Navigation Menu

Repository navigation

Default Log In Page should always be generic message #16484

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Default Log In Page should always be generic message #16484

Description

Activity

Tejas-Teju commented on Jan 31, 2025

jzheaux commented on Feb 3, 2025

Tejas-Teju commented on Feb 6, 2025

jzheaux commented on Feb 7, 2025

jzheaux commented on Feb 7, 2025

Tejas-Teju commented on Feb 10, 2025

jzheaux commented on Feb 18, 2025

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions