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

Session based authentication doesn't work with Micronaut framework version 4.3.8 or later #1682

Closed
eismint opened this issue Apr 26, 2024 · 3 comments
Assignees

Comments

@eismint
Copy link

eismint commented Apr 26, 2024

Expected Behavior

Since update of Micronaut framework version from 4.3.7 to 4.3.8 the session based authorization doesn't work anymore. I can reproduce it with your example application (https://guides.micronaut.io/latest/micronaut-security-session-maven-java.html).

Actual Behaviour

After login with the correct credentials the user is not authorized.

Steps To Reproduce

  1. download and unzip the sample application (https://guides.micronaut.io/latest/micronaut-security-session-maven-java.zip)
  2. start the application (./mvnw mn:run)
  3. go to login page and login with the correct credential
  4. you will see that the login was not successful
  5. change the micronaut-parent and the property of micronaut.version to version 4.3.7 in the pom.xml
  6. go to login page and login with the correct credential
  7. you are logged in successfully - you see the user name on the page

Environment Information

Operation System: Windows
Java version: JDK 17

Example Application

https://guides.micronaut.io/latest/micronaut-security-session-maven-java.zip

Version

4.3.8 or later

@sdelamo sdelamo assigned sdelamo and jeremyg484 and unassigned sdelamo Apr 26, 2024
@eismint eismint changed the title session based authentication doesn't work with Micronaut framework version 4.3.8 or later Session based authentication doesn't work with Micronaut framework version 4.3.8 or later Apr 26, 2024
@jeremyg484
Copy link
Contributor

This looks to be cause by some changes we made in Micronaut Core to how cookies are encoded. The non-Netty implementation of CookieFactory is getting loaded, whilst the Netty implementation of CookieEncoder is being loaded. This ultimately results in the session cookie being written with Max-Age=-1 which tells the browser not to store the cookie at all.

We will need to correct the service loading so that the correct implementations of the cookie interfaces are loaded.

In the meantime, this can be worked around by explictly setting the MaxAge of the session cookie via either micronaut.session.http.remember-me=true or by setting micronaut.session.http.cookie-max-age to an explicit positive value.

@sdelamo
Copy link
Contributor

sdelamo commented Apr 28, 2024

@jeremyg484 I think it is a bug in Micronaut Session not to se the cookie's max-age correctly. don't you think?

@jeremyg484
Copy link
Contributor

jeremyg484 commented Apr 28, 2024

@jeremyg484 I think it is a bug in Micronaut Session not to se the cookie's max-age correctly. don't you think?

@sdelamo

I'm not certain that Micronaut Session is where it needs to be fixed.

I think the most sensible default is to not set Max-Age at all in the Set-Cookie header. If neither Max-Age nor Expires are set on the cookie, then it becomes a "session cookie", ie a cookie that persists until the given browser session is ended. (See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie).

HttpSessionConfiguration in Micronaut Session has no default value, and only a non-null value gets applied to the Cookie in CookieHttpSessionStrategy when encoding the cookie for the client. This would seem to match with the sensible default.

The problem though is that the default value of max age of the Cookie implementation instantiated by HttpCookieFactory is -1. There is logic only in DefaultServerCookieEncoder to treat -1 as the value not having been set. The Netty implementation on the other hand uses Long.MIN_VALUE to indicate an unset value.

We could change the default value in HttpSessionConfiguration to be Long.MIN_VALUE, as this would work across both existing encoder implementations - but IMO that ties HttpSessionConfiguration too much to a hidden implementation detail of the cookie encoder implementations.

I think perhaps we should rather update the contract of the io.micronaut.http.cookie.Cookie interface to consistently define Long.MIN_VALUE as the lone indicator that Max-Age should not be written. If we do that, then it shouldn't matter what combination of CookieFactory and ServerCookieEncoder implementations are being used, as they would all need to adhere to that contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants