Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fixed getUserFromClient not being awaited in client credentials grant. #218

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

Merged
jankapunkt merged 5 commits into node-oauth:release-5.0.0 from shrihari-prakash:release-5.0.0
Aug 15, 2023

Conversation

@shrihari-prakash
Copy link
Collaborator

@shrihari-prakash shrihari-prakash commented Aug 15, 2023

Summary

getUserFromClient was not awaited in client credentials grant, this results in a promise being passed to the validateScope function in the model instead of the actual user.

Linked issue(s)

#217 217

Involved parts of the project

Client Credentials grant

Added tests?

NA

OAuth2 standard

Reproduction

@shrihari-prakash shrihari-prakash changed the title (削除) Release 5.0.0 (削除ここまで) (追記) Fixed getUserFromClient not being awaited in client credentials grant. (追記ここまで) Aug 15, 2023
Copy link
Member

Thanks a lot @shrihari-prakash !!!

Can you please update the following test in /test/integration/grant-types/client-credentials-grant-type_test.js at line 93:

it('should return a token', function() {
 const token = {};
 const model = {
 getUserFromClient: async function(client) {
 client.foo.should.equal('bar');
 return { id: '123'};
 },
 saveToken: async function(_token, client, user) {
 client.foo.should.equal('bar');
 user.id.should.equal('123');
 return token;
 },
 validateScope: function() { return 'foo'; }
 };
 const grantType = new ClientCredentialsGrantType({ accessTokenLifetime: 120, model: model });
 const request = new Request({ body: {}, headers: {}, method: {}, query: {} });
 return grantType.handle(request, { foo: 'bar' })
 .then(function(data) {
 data.should.equal(token);
 })
 .catch(should.fail);
 });

Please use lint:fix to format the code afterwards.

@jankapunkt jankapunkt linked an issue Aug 15, 2023 that may be closed by this pull request
Copy link
Collaborator Author

@jankapunkt , lint:fix seems to be modifying every js file in the repository. Is this expected? I am not seeing any visible line changes, but possibly it is changing all crlf line endings to lf.

Copy link
Collaborator Author

For now, I just added the test. Let me know if lint:fix is still required. Will remember to add respective tests in future :)

Copy link
Member

@shrihari-prakash no worries, it was just a hint, because sometimes pasting crashes the formatting. Looks good to me.

Copy link
Collaborator Author

Integrations seem to be failing now...

@jankapunkt jankapunkt merged commit c299425 into node-oauth:release-5.0.0 Aug 15, 2023
Copy link
Member

@shrihari-prakash no worries this is a CI config issues, which is already fixed on the 5.0.0 branch, CI should complete there sucessdully

shrihari-prakash reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Client Credentials broken in 5.0.0-rc.1

AltStyle によって変換されたページ (->オリジナル) /