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.

Sign up for GitHub

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

Jump to bottom

KNOX-2207 - TokenStateService revocation should remove persisted token state #252

Merged
merged 4 commits into from Feb 5, 2020
Merged

KNOX-2207 - TokenStateService revocation should remove persisted token state #252

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
949b1c9
KNOX-2207 - TokenStateService revocation should remove persisted toke…
moresandeep Feb 3, 2020
3200225
KNOX-2207 - Review changes
moresandeep Feb 4, 2020
e0bec60
KNOX-2207 - Review changes
moresandeep Feb 5, 2020
583b746
KNOX-2207 - Review changes
moresandeep Feb 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 15 additions & 7 deletions ...rc/main/java/org/apache/knox/gateway/services/token/impl/AliasBasedTokenStateService.java
View file
Open in desktop
Expand Up @@ -104,16 +104,11 @@ public long getTokenExpiration(final String token) {

@Override
public void revokeToken(final String token) {
// Record the revocation by setting the expiration to -1
updateExpiration(token, -1L);
/* no reason to keep revoked tokens around */
removeToken(token);
log.revokedToken(getTokenDisplayText(token));
}

@Override
protected boolean isRevoked(final String token) {
return (getTokenExpiration(token) < 0);
}

@Override
protected boolean isUnknown(final String token) {
boolean isUnknown = false;
Expand All @@ -125,6 +120,19 @@ protected boolean isUnknown(final String token) {
return isUnknown;
}

@Override
protected void removeToken(final String token) {
validateToken(token);

try {
aliasService.removeAliasForCluster(AliasService.NO_CLUSTER_NAME, token);
aliasService.removeAliasForCluster(AliasService.NO_CLUSTER_NAME,token + "--max");
} catch (AliasServiceException e) {
log.failedToUpdateTokenExpiration(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a misleading log message

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I'll get it fixed, thanks for letting me know @smolnar82

}

}

@Override
protected void updateExpiration(final String token, long expiration) {
if (isUnknown(token)) {
Expand Down
39 changes: 18 additions & 21 deletions ...r/src/main/java/org/apache/knox/gateway/services/token/impl/DefaultTokenStateService.java
View file
Open in desktop
Expand Up @@ -23,10 +23,8 @@
import org.apache.knox.gateway.services.security.token.impl.JWTToken;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

/**
* In-Memory authentication token state management implementation.
Expand All @@ -43,8 +41,6 @@ public class DefaultTokenStateService implements TokenStateService {

private final Map<String, Long> tokenExpirations = new HashMap<>();

private final Set<String> revokedTokens = new HashSet<>();

private final Map<String, Long> maxTokenLifetimes = new HashMap<>();


Expand Down Expand Up @@ -159,8 +155,8 @@ public void revokeToken(final JWTToken token) {

@Override
public void revokeToken(final String token) {
validateToken(token);
revokedTokens.add(token);
/* no reason to keep revoked tokens around */
removeToken(token);
log.revokedToken(getTokenDisplayText(token));
}

Expand All @@ -171,15 +167,15 @@ public boolean isExpired(final JWTToken token) {

@Override
public boolean isExpired(final String token) {
boolean isExpired;
boolean isUnknownToken;
moresandeep marked this conversation as resolved.
Show resolved Hide resolved

isExpired = isRevoked(token); // Check if it has been revoked first
if (!isExpired) {
// If it has not been revoked, check its expiration
isExpired = (getTokenExpiration(token) <= System.currentTimeMillis());
isUnknownToken = isUnknown(token); // Check if the token exist
moresandeep marked this conversation as resolved.
Show resolved Hide resolved
if (!isUnknownToken) {
// If it not unknown, check its expiration
isUnknownToken = (getTokenExpiration(token) <= System.currentTimeMillis());
}

return isExpired;
return isUnknownToken;
}

protected void setMaxLifetime(final String token, long issueTime, long maxLifetimeDuration) {
Expand Down Expand Up @@ -208,6 +204,16 @@ protected void updateExpiration(final String token, long expiration) {
}
}

protected void removeToken(final String token) {
validateToken(token);
synchronized (tokenExpirations) {
tokenExpirations.remove(token);
}
synchronized (maxTokenLifetimes) {
maxTokenLifetimes.remove(token);
}
}

protected boolean hasRemainingRenewals(final String token, long renewInterval) {
// Is the current time + 30-second buffer + the renewal interval is less than the max lifetime for the token?
return ((System.currentTimeMillis() + 30000 + renewInterval) < getMaxLifetime(token));
Expand All @@ -221,10 +227,6 @@ protected long getMaxLifetime(final String token) {
return result;
}

protected boolean isRevoked(final String token) {
return revokedTokens.contains(token);
}

protected boolean isValidIdentifier(final String token) {
return token != null && !token.isEmpty();
}
Expand Down Expand Up @@ -258,11 +260,6 @@ protected void validateToken(final String token, boolean includeRevocation) thro
log.unknownToken(getTokenDisplayText(token));
throw new IllegalArgumentException("Unknown token");
}

// Then, make sure it has not been revoked
if (includeRevocation && isRevoked(token)) {
throw new IllegalArgumentException("The specified token has been revoked");
}
}

protected String getTokenDisplayText(final String token) {
Expand Down