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

Move validateScope outside promise.all #84

Closed
jorenvandeweyer opened this issue Nov 23, 2021 · 2 comments
Closed

Move validateScope outside promise.all #84

jorenvandeweyer opened this issue Nov 23, 2021 · 2 comments
Milestone

Comments

@jorenvandeweyer
Copy link
Member

When there is an invalid scope passed, there is still an accessToken and refreshToken generated.

Code

PasswordGrantType.prototype.saveToken = function(user, client, scope) {
var fns = [
this.validateScope(user, client, scope),
this.generateAccessToken(client, user, scope),
this.generateRefreshToken(client, user, scope),
this.getAccessTokenExpiresAt(),
this.getRefreshTokenExpiresAt()
];
return Promise.all(fns)
.bind(this)
.spread(function(scope, accessToken, refreshToken, accessTokenExpiresAt, refreshTokenExpiresAt) {
var token = {
accessToken: accessToken,
accessTokenExpiresAt: accessTokenExpiresAt,
refreshToken: refreshToken,
refreshTokenExpiresAt: refreshTokenExpiresAt,
scope: scope
};
return promisify(this.model.saveToken, 3).call(this.model, token, client, user);
});
};

Suggestion

Move this.validateScope(user, client, scope) out of the array and check this before generating the tokens.

Use case

We use JWT's and only an internal token id is saved instead of the full JWT string. This means generating the token automatically means saving the token. So we are not actually using the saveToken function.

@jorenvandeweyer
Copy link
Member Author

It seems like some suggestions in #64 might fix this indirectly.

@jankapunkt jankapunkt linked a pull request Dec 10, 2021 that will close this issue
@jankapunkt jankapunkt added this to the v4.3 milestone Aug 25, 2022
@jankapunkt jankapunkt modified the milestones: v4.3, v4.4 Mar 21, 2023
@jorenvandeweyer
Copy link
Member Author

fixed in #229 & #228

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

Successfully merging a pull request may close this issue.

2 participants