Skip to content

Commit

Permalink
Merge pull request #103 from nats-io/revoke
Browse files Browse the repository at this point in the history
Reworded comments and added IsClaimRevoked to check user claim revocations
  • Loading branch information
kozlovic committed Oct 6, 2020
2 parents c4fd08d + c3900a5 commit c4d8a4f
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 25 deletions.
15 changes: 8 additions & 7 deletions account_claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ func (a *AccountClaims) Revoke(pubKey string) {
a.RevokeAt(pubKey, time.Now())
}

// RevokeAt enters a revocation by publickey and timestamp into this export
// 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.
func (a *AccountClaims) RevokeAt(pubKey string, timestamp time.Time) {
if a.Revocations == nil {
Expand All @@ -209,14 +210,14 @@ func (a *AccountClaims) ClearRevocation(pubKey string) {
a.Revocations.ClearRevocation(pubKey)
}

// IsRevokedAt 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
// be used for testing.
// IsRevokedAt 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 the subject and issue time of the jwt to be tested.
// DO NOT pass time.Now(), it will not produce a stable/expected response.
func (a *AccountClaims) IsRevokedAt(pubKey string, timestamp time.Time) bool {
return a.Revocations.IsRevoked(pubKey, timestamp)
}

// IsRevoked checks if the public key is in the revoked list with time.Now()
func (a *AccountClaims) IsRevoked(pubKey string) bool {
return a.Revocations.IsRevoked(pubKey, time.Now())
// IsRevoked does not perform a valid check. Use IsRevokedAt instead.
func (a *AccountClaims) IsRevoked(_ string) bool {
return true
}
23 changes: 14 additions & 9 deletions v2/account_claims.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ func (a *AccountClaims) Revoke(pubKey string) {
a.RevokeAt(pubKey, time.Now())
}

// RevokeAt enters a revocation by public key and timestamp into this export
// 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.
func (a *AccountClaims) RevokeAt(pubKey string, timestamp time.Time) {
if a.Revocations == nil {
Expand All @@ -250,14 +251,18 @@ func (a *AccountClaims) ClearRevocation(pubKey string) {
a.Revocations.ClearRevocation(pubKey)
}

// IsRevokedAt 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
// be used for testing.
func (a *AccountClaims) IsRevokedAt(pubKey string, timestamp time.Time) bool {
return a.Revocations.IsRevoked(pubKey, timestamp)
// 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 the subject and issue time of the jwt to be tested.
// DO NOT pass time.Now(), it will not produce a stable/expected response.
func (a *AccountClaims) isRevoked(pubKey string, claimIssuedAt time.Time) bool {
return a.Revocations.IsRevoked(pubKey, claimIssuedAt)
}

// IsRevoked checks if the public key is in the revoked list with time.Now()
func (a *AccountClaims) IsRevoked(pubKey string) bool {
return a.Revocations.IsRevoked(pubKey, time.Now())
// IsClaimRevoked checks if the account revoked the claim passed in.
// Invalid claims (nil, no Subject or IssuedAt) will return true.
func (a *AccountClaims) IsClaimRevoked(claim *UserClaims) bool {
if claim == nil || claim.IssuedAt == 0 || claim.Subject == "" {
return true
}
return a.isRevoked(claim.Subject, time.Unix(claim.IssuedAt, 0))
}
23 changes: 15 additions & 8 deletions v2/account_claims_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,48 +452,55 @@ func TestUserRevocation(t *testing.T) {
apk := publicKey(akp, t)
account := NewAccountClaims(apk)

pubKey := "bar"
ukp := createUserNKey(t)
pubKey := publicKey(ukp, t)
uc := NewUserClaims(pubKey)
uJwt, _ := uc.Encode(akp)
uc, err := DecodeUserClaims(uJwt)
if err != nil {
t.Errorf("Failed to decode user claim: %v", err)
}
now := time.Now()

// test that clear is safe before we add any
account.ClearRevocation(pubKey)

if account.IsRevokedAt(pubKey, now) {
if account.isRevoked(pubKey, now) {
t.Errorf("no revocation was added so is revoked should be false")
}

account.RevokeAt(pubKey, now.Add(time.Second*100))

if !account.IsRevokedAt(pubKey, now) {
if !account.isRevoked(pubKey, now) {
t.Errorf("revocation should hold when timestamp is in the future")
}

if account.IsRevokedAt(pubKey, now.Add(time.Second*150)) {
if account.isRevoked(pubKey, now.Add(time.Second*150)) {
t.Errorf("revocation should time out")
}

account.RevokeAt(pubKey, now.Add(time.Second*50)) // shouldn't change the revocation, you can't move it in

if !account.IsRevokedAt(pubKey, now.Add(time.Second*60)) {
if !account.isRevoked(pubKey, now.Add(time.Second*60)) {
t.Errorf("revocation should hold, 100 > 50")
}

encoded, _ := account.Encode(akp)
decoded, _ := DecodeAccountClaims(encoded)

if !decoded.IsRevokedAt(pubKey, now.Add(time.Second*60)) {
if !decoded.isRevoked(pubKey, now.Add(time.Second*60)) {
t.Errorf("revocation should last across encoding")
}

account.ClearRevocation(pubKey)

if account.IsRevokedAt(pubKey, now) {
if account.IsClaimRevoked(uc) {
t.Errorf("revocations should be cleared")
}

account.RevokeAt(pubKey, now.Add(time.Second*1000))

if !account.IsRevoked(pubKey) {
if !account.IsClaimRevoked(uc) {
t.Errorf("revocation be true we revoked in the future")
}
}
Expand Down
2 changes: 1 addition & 1 deletion v2/revocation_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ 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
// be used for testing.
func (r RevocationList) IsRevoked(pubKey string, timestamp time.Time) bool {
ts, ok := r[pubKey]
Expand Down

0 comments on commit c4d8a4f

Please sign in to comment.