AbstractAuthenticationFailureEvent published twice when parent ProviderManager throws ProviderNotFoundException

[theborakompanioni]

I am currently running into an issue with ProviderManager hierarchy and its error handling with regards to a custom AuthenticationProvider.

1. A custom AuthenticationProvider throws BadCredentialsException
2. Then the parent AuthenticationManager is called and throws ProviderNotFoundException
3. The ProviderNotFoundException is published via AuthenticationEventPublisher#publishAuthenticationFailure
4. The BadCredentialsException is also published as the exception of the parent is not saved in parentException. Hence the event is published additionally

Expected would be, that if the parent does not have any providers supporting the Authentication, the child exception takes precedence.

This is behaviour is partly guarded by tests

• ProviderManagerTests#providerNotFoundFromParentIsIgnored
• ProviderManagerTests#authenticateWhenFailsInParentAndPublishesThenChildDoesNotPublish

.. but it seems it does not take into account a parent provider that throw ProviderNotFoundException (the mocks in the test hides the fact that the parent itself will publish (if it is of class ProviderManager)).
Can this be prevented by probing if the parent has any supporting providers before calling its authenticate method?

Maybe my setup is somehow invalid.. but it is quite a small example application and I'd say mostly defaults. Maybe it is not supported for a ProviderManager to have another ProviderManager instance as parent.

Related #6281

[jgrandja]

@theborakompanioni I'm finding it difficult to follow the scenario. Can you please provide a minimal sample or a test?

[theborakompanioni]

@theborakompanioni I'm finding it difficult to follow the scenario. Can you please provide a minimal sample or a test?

Hey @jgrandja, thanks for the follow-up.

Hm, if you'd be willing to see this test in bitcoin-spring-boot-starter#spring-security/issue-10206: LnurlSessionAuthenticationFilterTest#springSecurityIssue10206.

In this test I tried to show that the issue is reproducible.

If you place a breakpoint in ProviderManager#prepareException you'll notice it will execute twice. And also the test should prove that both events will be triggered.

I tried to explain it more clearly but always end up with the steps I already provided in the issue description.
Please forgive me if this is still not comprehensible enough. (It was also rather difficult for me to grasp the behavior at first with the "ignored"/saved exceptions in ProviderManager.)

I hope I'll soon have the time to download the spring-security source and try to come up with a solution/test/example if it cannot be reproduced on your end.

Thank your for your time.
Have a nice day.

[jgrandja]

@theborakompanioni I'll wait until you provide a minimal sample or test. There are a lot of dependencies in bitcoin-spring-boot-starter and I'd rather keep the issue very focused and minimal.

[theborakompanioni]

I'll wait until you provide a minimal sample or test.

Hope this is a useful minimal sample for you to determine if this is a bug or intended behavior: theborakompanioni@ba98c94

If you need any more information, please feel free to reach out. Have a nice day!

edit: all verify(publisher, atMostOnce()) parts should of course be verify(publisher, times(1)).

[jgrandja]

Thanks for the sample test @theborakompanioni. This really helped me clarify the scenario.

In short, this is expected behaviour.

If an AuthenticationManager is not composed of any AuthenticationProvider's that can handle an Authentication attempt, then it will return (and publish) ProviderNotFoundException. This is what happens in your setup with the Parent AuthenticationManager.

However, the AuthenticationException thrown by the Child AuthenticationManager is honoured as this is the exception that is returned to the caller and is also published, resulting in a 2nd publishAuthenticationFailure().

The end result is 2 publishAuthenticationFailure() events and the expected AuthenticationException returned to the caller. This behaviour is expected.

Now, if you were to modify MockProvider.supports() to ultimately return true in that test and it ended up throwing an AuthenticationException then this would be the exception returned to the caller NOT the child AuthenticationException and publishAuthenticationFailure() event would be called once only.

What I would recommend to avoid the ProviderNotFoundException published event is to configure the caller to only use an AuthenticationManager that has a supported AuthenticationProvider, including the parent AuthenticationManager if one is configured.

For example, let's assume a formLogin() authentication attempt. If there is only one source for the user database then the AuthenticationManager should have one AuthenticationProvider that supports the Authentication request. Also, it should not be configured with a parent AuthenticationManager, since it wouldn't have any supported AuthenticationProvider and therefore will throw ProviderNotFoundException.

However, if there are 2x sources for the user database, then it would make sense to have the secondary AuthenticationProvider in the parent AuthenticationManager. Alternatively, the secondary AuthenticationProvider could be composed in the same AuthenticationManager as the primary AuthenticationProvider, without the need of a parent.

I hope this makes sense? I'm going to close this as the behaviour is expected. I believe adjusting your AuthenticationManager configuration will resolve the extra published event.

[theborakompanioni]

I hope this makes sense? I'm going to close this as the behaviour is expected. I believe adjusting your AuthenticationManager configuration will resolve the extra published event.

Thank you for your feedback and your time @jgrandja.
I guess it kinda makes sense, and the behaviour might very well be expected if you dive into the code. I do not necessarily think it is the intended behaviour, but this does not need to be discussed here.

This behaviour was not triggered by 2 sources for the user database. This setup originates from this configuration: https://github.com/theborakompanioni/bitcoin-spring-boot-starter/blob/378838a3c8bea46fc84bf520318ced627f0dbc13/incubator/spring-lnurl/spring-lnurl-auth-example-application/src/main/java/org/tbk/lightning/lnurl/example/LnurlAuthExampleApplicationSecurityConfig.java#L69-L121 (no formLogin(), no httpBasic(), no oauth2Login()).

I do not know how a configuration you describted would be setup.. guess I have to deep dive into the inner workings of the code more thoroughly.

Thank you.