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

Provide AccessToken object to storeToken and removeToken methods #437

Closed
longwa opened this issue Apr 8, 2020 · 5 comments
Closed

Provide AccessToken object to storeToken and removeToken methods #437

longwa opened this issue Apr 8, 2020 · 5 comments

Comments

@longwa
Copy link
Contributor

longwa commented Apr 8, 2020

The storeToken and removeToken methods are not useful in many cases because you need access to the refreshToken (in both cases) and the principal (in the removeToken case).

The most common use case for these methods would be to implement some kind of stateful handling of the token (either a whitelist for active tokens or a blacklist of blocked tokens).

In both cases, you need to be able to handle both the accessToken and the refreshToken including access to other JWT attribute such as expiration, subject, etc. For instance, if you are trying to blacklist a token when the user explicitly logs out, you need to also blacklist the refreshToken since someone with access to the token could just use it to create a new accessToken.

My suggestion is to add additional method signatures:

void storeToken(AccessToken accessToken)

void removeToken(AccessToken accessToken) throws TokenNotFoundException

To keep backwards compatibility, we can just have these call the other methods by default in the JwtTokenStorageService.

The places that call store and remove will be changed to pass the AccessToken, allowing for more flexibility.

@longwa
Copy link
Contributor Author

longwa commented Apr 8, 2020

This is related to #298 as it would provide a better mechanism to implement it.

longwa added a commit to longwa/grails-spring-security-rest that referenced this issue Apr 8, 2020
longwa added a commit to longwa/grails-spring-security-rest that referenced this issue Apr 8, 2020
@alvarosanchez
Copy link
Contributor

Seems legit to me. I was going to suggest that you propose a PR, but I see you're already in progress with that. Submit it when done and I'll review it.

@longwa
Copy link
Contributor Author

longwa commented Apr 8, 2020

One issue is that for removeToken it isn't really useful to have the full token as only the accessToken field is populated.

It's only when the AccessToken is initially created is all of the data populated (refresh token, principal, etc.). I still think it's useful to have the entire object on the call to storeToken so that it has feature parity with the event listener (which already gets the entire object).

I'll submit a PR once I have it all working in my real application to make sure there aren't any other surprises.

longwa added a commit to longwa/grails-spring-security-rest that referenced this issue Apr 8, 2020
jdaugherty added a commit to jdaugherty/grails-spring-security-rest that referenced this issue Jan 14, 2024
jdaugherty added a commit to jdaugherty/grails-spring-security-rest that referenced this issue Jan 14, 2024
@jdaugherty
Copy link
Contributor

#521 will resolve this.

@jdaugherty
Copy link
Contributor

This is complete in the 5.0.x branch.

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

3 participants