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

Problem with sessionStore in v5.0.0 #563

Closed
rodolfosaraiva opened this issue Dec 13, 2024 · 9 comments
Closed

Problem with sessionStore in v5.0.0 #563

rodolfosaraiva opened this issue Dec 13, 2024 · 9 comments

Comments

@rodolfosaraiva
Copy link

Issue description

I'm using grails-spring-security-rest version 5.0.0 with grails 6.2.2

When trying to log in with Google2Client, I'm getting the following error:

Cannot invoke "org.pac4j.core.context.session.SessionStore.set(org.pac4j.core.context.WebContext, String, Object)" because "sessionStore" is null

Trying to debug, I saw that in RestOauthService it is passing the sessionStore as null:
image

And it's used in final Optional<Credentials> getCredentials:

image

What could I be doing wrong?

If I customize the client, overriding retrieveCredentials to use JEESessionStore.INSTANCE.

The exception is thrown in cleanAttemptedAuthentication. which I was unable to override.

@jdaugherty
Copy link
Contributor

Thanks for reporting the issue. When we back ported these changes, it looks like this got missed (the session doesn't exist in the later versions).

Do you have an example project that reproduces this scenario?

jdaugherty added a commit to jdaugherty/grails-spring-security-rest that referenced this issue Dec 13, 2024
@rodolfosaraiva
Copy link
Author

Thanks for reporting the issue. When we back ported these changes, it looks like this got missed (the session doesn't exist in the later versions).

Do you have an example project that reproduces this scenario?

I created a project to reproduce this error:
https://github.com/rodolfosaraiva/spring-security-rest-test-impl

This is the basic authentication flow through the Google client. In the project, I included a readme that simulates the Google callback (so you don't have to configure an app on Google). The error occurs on the first call

@rodolfosaraiva
Copy link
Author

If you can release the release with these changes I would really appreciate it.

@jdaugherty
Copy link
Contributor

Thank you for doing the leg work on the project. This will make sure we get a test added so it doesn't break in the future. I'll get the snapshot done today. Can you test with it first then we'll release?

@rodolfosaraiva
Copy link
Author

Thank you for doing the leg work on the project. This will make sure we get a test added so it doesn't break in the future. I'll get the snapshot done today. Can you test with it first then we'll release?

Sure, I can test it

jdaugherty added a commit that referenced this issue Dec 16, 2024
fix #563 - set the session store to JEESessionStore.INSTANCE
@jdaugherty
Copy link
Contributor

I've merged the pull request after testing it locally. It's now published (https://github.com/grails/grails-spring-security-rest/actions/runs/12355344062/job/34478858931)

Assuming you have a repository in your build.gradle to include the grails repo:

repositories {
    maven { url "https://repo.grails.org/grails/core" }
}

Then you should be able to use implementation 'org.grails.plugins:spring-security-rest:5.0.1-SNAPSHOT' to test this issue with the snapshot build. The only change in the snapshot is this fix.

@rodolfosaraiva
Copy link
Author

I've merged the pull request after testing it locally. It's now published (https://github.com/grails/grails-spring-security-rest/actions/runs/12355344062/job/34478858931)

Assuming you have a repository in your build.gradle to include the grails repo:

repositories {
    maven { url "https://repo.grails.org/grails/core" }
}

Then you should be able to use implementation 'org.grails.plugins:spring-security-rest:5.0.1-SNAPSHOT' to test this issue with the snapshot build. The only change in the snapshot is this fix.

It worked! Thanks for solving the problem.

@jdaugherty
Copy link
Contributor

You are welcome. I'll see if I can get this released quickly.

@jdaugherty
Copy link
Contributor

5.0.1 is released with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants