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

AuthenticationSuccessEvent event is not created on successful authorization using FormAuthenticationMechanism #39391

Closed
resfakchion opened this issue Mar 13, 2024 · 8 comments · Fixed by #39493
Labels
area/security kind/bug Something isn't working
Milestone

Comments

@resfakchion
Copy link

Describe the bug

When authenticate using Form-based authentication, I try to want to get an event of successful authentication , but it doesn't come. In case of unsuccessful authentication, I get an event about it.

Expected behavior

I expect to receive AuthenticationSuccessEvent in case of successful authentication using Form-based authentication and AuthenticationFailureEvent in case of unsuccessful authentication .

Actual behavior

Currently I only get AuthenticationFailureEvent in case of unsuccessful authorization

How to Reproduce?

event_test.zip

  1. Run SecurityEventObserverTest
  2. See that there are no log entries in the console upon successful authentication
  3. If the authentication is not successful, there is a log entry in the console

The class responsible for capturing events com/resfa/auth/SecurityEventObserver.java

Output of uname -a or ver

MINGW64_NT-10.0-19045 T1-024-001182 3.3.6-341.x86_64 2022-09-05 20:28 UTC x86_64 Msys

Output of java -version

java version "21.0.1" 2023-10-17 LTS Java(TM) SE Runtime Environment (build 21.0.1+12-LTS-29) Java HotSpot(TM) 64-Bit Server VM (build 21.0.1+12-LTS-29, mixed mode, sharing)

Quarkus version or git rev

3.8.2

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.9.5 (57804ffe001d7215b5e7bcb531cf83df38f93546)

Additional information

No response

@resfakchion resfakchion added the kind/bug Something isn't working label Mar 13, 2024
@michalvavrik
Copy link
Member

I'll have a look later.

@michalvavrik michalvavrik self-assigned this Mar 13, 2024
@michalvavrik
Copy link
Member

Alright, so what happens is that Form auth mechanism redirects before the event is fired. Which would mean we need to know inside HTTP auth specific mechanism if we count in redirects. I'll think about that. Thanks for reporting the issue.

@michalvavrik
Copy link
Member

michalvavrik commented Mar 13, 2024

@resfakchion fun fact - the first time that session cookie is going to be actually used, the event is going to be reported. So, yes, technically, you didn't get event, but also it is very specific situation when authentication completes next time. I will still try to find a proper way to handle this, but just so that you know the event is going to be reported if you ever try to use it.

@resfakchion
Copy link
Author

@michalvavrik Sounds really interesting, thanks for such a quick response. I'll look forward to your solution.

@michalvavrik
Copy link
Member

michalvavrik commented Mar 13, 2024

Thanks, I'm not actually sure whether we should fix this:

//we have authenticated, but we want to just redirect back to the original page

Citing from the code comment we have just set a cookie so the redirected request will be authenticated. The login endpoint is not application endpoint and it will mean that authentication event will be reported twice (once on credentials verification, once on session cookie verification after redirect).

I suggest to collect opposite opinions as I understand it is not intuitive. Let's keep it open for a little while.

Copy link

quarkus-bot bot commented Mar 13, 2024

/cc @pedroigor (bearer-token), @sberyozkin (bearer-token,jwt,security)

@michalvavrik michalvavrik added the triage/consider-closing Bugs that are considered to be closed because too old. Using the label to do a mark and sweep proces label Mar 13, 2024
@sberyozkin
Copy link
Member

sberyozkin commented Mar 13, 2024

We can have a Form authentication specific event, FORM_LOGIN , for this case, as in case of OIDC, which fires OIDC_LOGIN, but also fires AuthenticationSuccessEvent whenever the session cookie is verified.

With Form, we have both the endpoint and the login management combined, for the latter, FORM_LOGIN can help to differentiate

@michalvavrik
Copy link
Member

We can have a Form authentication specific event, FORM_LOGIN , for this case, as in case of OIDC, which fires OIDC_LOGIN, but also fires AuthenticationSuccessEvent whenever the session cookie is verified.

With Form, we have both the endpoint and the login management combined, for the latter, FORM_LOGIN can help to differentiate

Sounds good, let's do it.

@michalvavrik michalvavrik removed the triage/consider-closing Bugs that are considered to be closed because too old. Using the label to do a mark and sweep proces label Mar 13, 2024
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants