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

Add support for token revocation #126

Merged
merged 9 commits into from
Feb 3, 2024
Merged

Conversation

adambom
Copy link
Contributor

@adambom adambom commented Oct 25, 2023

This PR adds support for token revocation. It's implemented similarly to how token introspection is implemented. Let me know what you think. Happy to write tests too just let me know how you'd like them structured and where. I didn't see an obvious place for the tests to live.

Usage:

const client = new OAuth2Client({
  // ...
  
  // Can also be auto-discovered if discoveryEndpoint is supplied
  revocationEndpoint: '/revoke'
});

const token = await client.clientCredentials();

await client.revoke(token);

// can also supply tokenTypeHint (default is 'access_token')
await client.revoke(token, 'access_token');

// To revoke a refresh token
await client.revoke(token, 'refresh_token');

Fixes #125

Copy link
Collaborator

@evert evert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! High quality PR, awesome!

There are currently unittests in the test/ directory. Running 'npm test' should execute them. If you're not comfortable adding these I'll find some time writing them

Thank you!

src/client.ts Outdated Show resolved Hide resolved
src/client.ts Show resolved Hide resolved
@adambom
Copy link
Contributor Author

adambom commented Oct 27, 2023

There are currently unittests in the test/ directory. Running 'npm test' should execute them. If you're not comfortable adding these I'll find some time writing them

More specifically, I was looking for tests that invoke client.introspect so I could copy them, but I couldn't find any. Am I missing something?

@evert
Copy link
Collaborator

evert commented Oct 27, 2023

There are currently unittests in the test/ directory. Running 'npm test' should execute them. If you're not comfortable adding these I'll find some time writing them

More specifically, I was looking for tests that invoke client.introspect so I could copy them, but I couldn't find any. Am I missing something?

No, this library doesn't have a 100% coverage , but I've been trying to add them with every change or new feature

@adambom
Copy link
Contributor Author

adambom commented Oct 30, 2023

There are currently unittests in the test/ directory. Running 'npm test' should execute them. If you're not comfortable adding these I'll find some time writing them

More specifically, I was looking for tests that invoke client.introspect so I could copy them, but I couldn't find any. Am I missing something?

No, this library doesn't have a 100% coverage , but I've been trying to add them with every change or new feature

I'll see what I can do

@adambom
Copy link
Contributor Author

adambom commented Oct 30, 2023

Ok, have a look, let me know what you think

test/client.ts Outdated Show resolved Hide resolved
src/messages.ts Outdated
token_type_hint?: OAuth2TokenTypeHint
}

export type RevocationResponse = {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just took a look at the spec and it looks like there's no requirement for servers to return JSON. So if a revoke endpoint returns something else, parsing the response body will likely result in an exception

src/client.ts Show resolved Hide resolved
@evert
Copy link
Collaborator

evert commented Dec 12, 2023

Geez my last 2 comments were in 'draft' for a month and a half , sorry . Forgot to hit the green button

@evert evert merged commit f53cfd0 into badgateway:main Feb 3, 2024
5 checks passed
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 this pull request may close these issues.

Revoke token
3 participants