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

[feat] allow revocation of all users or activations #114

Merged
merged 4 commits into from
Oct 30, 2020

Conversation

aricart
Copy link
Member

@aricart aricart commented Oct 29, 2020

added the ability to revoke users/activations by timestamp. If the value for the public key is "*", any JWT issued before the timestamp specified will be considered revoked.

…lue for the public key is "*", any JWT issued before the timestamp specified will be considered revoked.
@aricart aricart changed the title [feat] allow revocation for all users or activation [feat] allow revocation of all users or activations Oct 29, 2020
@aricart aricart requested a review from matthiashanel October 29, 2020 21:53
go.mod Outdated
@@ -1,6 +1,7 @@
module github.com/nats-io/jwt

require (
github.com/nats-io/jwt/v2 v2.0.0-20201015190852-e11ce317263c
Copy link
Contributor

Choose a reason for hiding this comment

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

really?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch the scripts were not fmt or running tests for both versions on local env.

@@ -238,11 +238,11 @@ func (a *AccountClaims) Revoke(pubKey string) {
// RevokeAt enters a revocation by public key and timestamp into this account
// This will revoke all jwt issued for pubKey, prior to timestamp
// If there is already a revocation for this public key that is newer, it is kept.
// The value is expected to be a public key or "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe add * means all

v2/go.sum Outdated
@@ -1,3 +1,4 @@
github.com/nats-io/jwt/v2 v2.0.0-20201015190852-e11ce317263c/go.mod h1:vs+ZEjP+XKy8szkBmQwCB7RjYdIlMaPsFPs4VdS4bTQ=
Copy link
Contributor

Choose a reason for hiding this comment

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

self import?

Copy link
Member Author

Choose a reason for hiding this comment

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

go mod tidy fixed all that

Copy link
Contributor

@matthiashanel matthiashanel left a comment

Choose a reason for hiding this comment

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

Except for comment and go.mod stuff LGTM

Copy link
Contributor

@matthiashanel matthiashanel left a comment

Choose a reason for hiding this comment

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

LGTM

@aricart aricart requested a review from kozlovic October 30, 2020 15:43
@@ -39,9 +41,19 @@ func (r RevocationList) ClearRevocation(pubKey string) {
}

// IsRevoked checks if the public key is in the revoked list with a timestamp later than
// the one passed in. Generally this method is called with time.Now() but other time's can
// the one passed in. Generally this method is called with an issue time but other time's can
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I just merged #113 that was a port of some changes that were made in v2/ but not v1/ but were done in the branch_v1 :-). So you should rebase and see if there are some changes that are no longer needed.

@aricart aricart merged commit 7d55e83 into master Oct 30, 2020
@aricart aricart deleted the allow-all-revocation branch October 30, 2020 15:57
@aricart aricart restored the allow-all-revocation branch October 30, 2020 20:31
@aricart aricart deleted the allow-all-revocation branch September 18, 2023 16:09
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.

3 participants