node-oauth2-server
node-oauth2-server copied to clipboard
Question on `alwaysIssueNewRefreshToken`, how is my saveToken() suppose to know it was from a refresh_token grant type?
When alwaysIssueNewRefreshToken
is set to false, the revokeToken()
method is never ran, and as such the saveToken()
method is called directly without removing the older token (makes sense).
So in my example, saveToken()
is called, and in it I have a database query that executes an INSERT to my database. My query fails since in the schema I made, it is expecting both refreshToken and access token, but when alwaysIssueNewRefreshToken
is false, I don't get a new refresh_token and only a new access_token, so I have to now run an UPDATE on my db query....Now yes, I can update my saveToken()
method to be flexible for this, but the problem is that there is really no way (expect seeing that I have no refresh token) for me to know if saveToken()
is being called to save a brand new token or to simply update a new token. I guess I can do
if(!token.refreshToken) run UPDATE query instead
But shouldn't there be some sort of flag or maybe a updateToken()
method that runs instead? I guess I could run an upsert query instead, but now saveToken()
isn't really "saving a token" now, it's doing an upsert or conditional insert if it doesn't have a refresh_token.
TL;DR -- Maybe incorporate an optional 'updateToken()' method that runs when alwaysIssueNewRefreshToken
is false?
In addition, since I only retrieve the access_token from 'saveToken()', I won't be able to fully return the entire token object since I am missing the refresh_token, https://oauth2-server.readthedocs.io/en/latest/model/spec.html?highlight=getRefreshToken#savetoken-token-client-user-callback
The return values expect to contain the refresh_token when I can't provide it because it's not being returned.
What I think needs to happen is instead of only returning access_token in saveToken() during a refresh_token grant, to also include the existing refresh_token that wasn't deleted. This way we can return the refresh_token (which is just the same) and also so we don't lose the reference of the tokens we need to update.
https://github.com/oauthjs/node-oauth2-server/blob/master/lib/grant-types/refresh-token-grant-type.js#L161 here instead of not including the refresh_token if alwaysIssueNewRefreshToken
is false, we should include the refresh_token that was not revoked/removed. I can create an MR if needed, just let me know if this is something you guys would like to see.
I understand that I can add certain checks and change a few things on my side, but then I am not leveragling this API at all and instead finding patches/fixes for things that should be supported.
Let me know if you guys would like me to make an MR for this if you deem it necessary/needed.
but the problem is that there is really no way (expect seeing that I have no refresh token) for me to know if
saveToken()
is being called to save a brand new token or to simply update a new token. I guess I can do
It is exactly how you know if it's an update or a creation, the lack of a refreshToken... The library is doing it's data validation in the background, so you can safely assume that it's an update if no refreshToken is present.
Having a seperate updateToken
function would just split the logic into two functions, besides that, it would work exactly the same way. You can achieve this yourself by creating the logic.
saveToken: async (token, client, user) => {
if (refreshToken) this.updateToken(token, client, user)
else this.createToken(token, client, user)
}
Just a FYI: It sounds like you are storing the accessToken and refreshToken in the same table, you could ask yourself, why not combine getAccessToken
and getRefreshToken
? Your setup is "abnormal", as the common practice is to store accessTokens and refreshTokens in two seperate tables, apart from each other.