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

Jump to bottom

Remove bwc logic for token invalidation #36893

Merged
merged 10 commits into from Dec 28, 2018
Merged

Remove bwc logic for token invalidation #36893

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d0e072d
Removes bwc for token invalidation
jkakavas Dec 19, 2018
494a5fb
Do not check for invalidated token documents anymore
jkakavas Dec 20, 2018
1236d9e
Fix tests to mock Get requests for invalidation
jkakavas Dec 20, 2018
5ae8f86
merge getAndValidateToken with decodeAndValidateToken
jkakavas Dec 20, 2018
8e87a27
define document id
jkakavas Dec 20, 2018
9801f22
fix test
jkakavas Dec 20, 2018
fed803f
Remove created field from rest tests
jkakavas Dec 20, 2018
ca9729c
Merge remote-tracking branch 'origin/master' into remove-bwc-invalida…
jkakavas Dec 20, 2018
07c11fe
Remove unnecessary bool query clause
jkakavas Dec 28, 2018
e6cc565
Merge remote-tracking branch 'origin/master' into remove-bwc-invalida…
jkakavas Dec 28, 2018
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
17 changes: 4 additions & 13 deletions ...t-high-level/src/main/java/org/elasticsearch/client/security/InvalidateTokenResponse.java
View file
Open in desktop
Expand Up @@ -42,13 +42,11 @@
*/
public final class InvalidateTokenResponse {

public static final ParseField CREATED = new ParseField("created");
public static final ParseField INVALIDATED_TOKENS = new ParseField("invalidated_tokens");
public static final ParseField PREVIOUSLY_INVALIDATED_TOKENS = new ParseField("previously_invalidated_tokens");
public static final ParseField ERROR_COUNT = new ParseField("error_count");
public static final ParseField ERRORS = new ParseField("error_details");

private final boolean created;
private final int invalidatedTokens;
private final int previouslyInvalidatedTokens;
private List<ElasticsearchException> errors;
Expand All @@ -57,19 +55,17 @@ public final class InvalidateTokenResponse {
private static final ConstructingObjectParser<InvalidateTokenResponse, Void> PARSER = new ConstructingObjectParser<>(
"tokens_invalidation_result", true,
// we parse but do not use the count of errors as we implicitly have this in the size of the Exceptions list
args -> new InvalidateTokenResponse((boolean) args[0], (int) args[1], (int) args[2], (List<ElasticsearchException>) args[4]));
args -> new InvalidateTokenResponse((int) args[0], (int) args[1], (List<ElasticsearchException>) args[3]));

static {
PARSER.declareBoolean(constructorArg(), CREATED);
PARSER.declareInt(constructorArg(), INVALIDATED_TOKENS);
PARSER.declareInt(constructorArg(), PREVIOUSLY_INVALIDATED_TOKENS);
PARSER.declareInt(constructorArg(), ERROR_COUNT);
PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> ElasticsearchException.fromXContent(p), ERRORS);
}

public InvalidateTokenResponse(boolean created, int invalidatedTokens, int previouslyInvalidatedTokens,
public InvalidateTokenResponse(int invalidatedTokens, int previouslyInvalidatedTokens,
@Nullable List<ElasticsearchException> errors) {
this.created = created;
this.invalidatedTokens = invalidatedTokens;
this.previouslyInvalidatedTokens = previouslyInvalidatedTokens;
if (null == errors) {
Expand All @@ -79,10 +75,6 @@ public InvalidateTokenResponse(boolean created, int invalidatedTokens, int previ
}
}

public boolean isCreated() {
return created;
}

public int getInvalidatedTokens() {
return invalidatedTokens;
}
Expand All @@ -104,15 +96,14 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
InvalidateTokenResponse that = (InvalidateTokenResponse) o;
return created == that.created &&
invalidatedTokens == that.invalidatedTokens &&
return invalidatedTokens == that.invalidatedTokens &&
previouslyInvalidatedTokens == that.previouslyInvalidatedTokens &&
Objects.equals(errors, that.errors);
}

@Override
public int hashCode() {
return Objects.hash(created, invalidatedTokens, previouslyInvalidatedTokens, errors);
return Objects.hash(invalidatedTokens, previouslyInvalidatedTokens, errors);
}

public static InvalidateTokenResponse fromXContent(XContentParser parser) throws IOException {
Expand Down
4 changes: 0 additions & 4 deletions ...h-level/src/test/java/org/elasticsearch/client/security/InvalidateTokenResponseTests.java
View file
Open in desktop
Expand Up @@ -41,7 +41,6 @@ public void testFromXContent() throws IOException {
final int invalidatedTokens = randomInt(32);
final int previouslyInvalidatedTokens = randomInt(32);
builder.startObject()
.field("created", false)
.field("invalidated_tokens", invalidatedTokens)
.field("previously_invalidated_tokens", previouslyInvalidatedTokens)
.field("error_count", 0)
Expand All @@ -50,7 +49,6 @@ public void testFromXContent() throws IOException {

try (XContentParser parser = createParser(xContentType.xContent(), xContent)) {
final InvalidateTokenResponse response = InvalidateTokenResponse.fromXContent(parser);
assertThat(response.isCreated(), Matchers.equalTo(false));
assertThat(response.getInvalidatedTokens(), Matchers.equalTo(invalidatedTokens));
assertThat(response.getPreviouslyInvalidatedTokens(), Matchers.equalTo(previouslyInvalidatedTokens));
assertThat(response.getErrorsCount(), Matchers.equalTo(0));
Expand All @@ -64,7 +62,6 @@ public void testFromXContentWithErrors() throws IOException {
final int invalidatedTokens = randomInt(32);
final int previouslyInvalidatedTokens = randomInt(32);
builder.startObject()
.field("created", false)
.field("invalidated_tokens", invalidatedTokens)
.field("previously_invalidated_tokens", previouslyInvalidatedTokens)
.field("error_count", 0)
Expand All @@ -82,7 +79,6 @@ public void testFromXContentWithErrors() throws IOException {

try (XContentParser parser = createParser(xContentType.xContent(), xContent)) {
final InvalidateTokenResponse response = InvalidateTokenResponse.fromXContent(parser);
assertThat(response.isCreated(), Matchers.equalTo(false));
assertThat(response.getInvalidatedTokens(), Matchers.equalTo(invalidatedTokens));
assertThat(response.getPreviouslyInvalidatedTokens(), Matchers.equalTo(previouslyInvalidatedTokens));
assertThat(response.getErrorsCount(), Matchers.equalTo(2));
Expand Down
57 changes: 9 additions & 48 deletions .../main/java/org/elasticsearch/xpack/core/security/action/token/InvalidateTokenRequest.java
View file
Open in desktop
Expand Up @@ -5,7 +5,6 @@
*/
package org.elasticsearch.xpack.core.security.action.token;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.Nullable;
Expand Down Expand Up @@ -137,57 +136,19 @@ public void setUserName(String userName) {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (out.getVersion().before(Version.V_7_0_0)) {
if (Strings.isNullOrEmpty(tokenString)) {
throw new IllegalArgumentException("token is required for versions < v6.6.0");
}
out.writeString(tokenString);
} else {
out.writeOptionalString(tokenString);
}
if (out.getVersion().onOrAfter(Version.V_6_2_0)) {
if (out.getVersion().before(Version.V_7_0_0)) {
if (tokenType == null) {
throw new IllegalArgumentException("token type is not optional for versions > v6.2.0 and < v6.6.0");
}
out.writeVInt(tokenType.ordinal());
} else {
out.writeOptionalVInt(tokenType == null ? null : tokenType.ordinal());
}
} else if (tokenType == Type.REFRESH_TOKEN) {
throw new IllegalArgumentException("refresh token invalidation cannot be serialized with version [" + out.getVersion() + "]");
}
if (out.getVersion().onOrAfter(Version.V_7_0_0)) {
out.writeOptionalString(realmName);
out.writeOptionalString(userName);
} else if (realmName != null || userName != null) {
throw new IllegalArgumentException(
"realm or user token invalidation cannot be serialized with version [" + out.getVersion() + "]");
}
out.writeOptionalString(tokenString);
out.writeOptionalVInt(tokenType == null ? null : tokenType.ordinal());
out.writeOptionalString(realmName);
out.writeOptionalString(userName);
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
if (in.getVersion().before(Version.V_7_0_0)) {
tokenString = in.readString();
} else {
tokenString = in.readOptionalString();
}
if (in.getVersion().onOrAfter(Version.V_6_2_0)) {
if (in.getVersion().before(Version.V_7_0_0)) {
int type = in.readVInt();
tokenType = Type.values()[type];
} else {
Integer type = in.readOptionalVInt();
tokenType = type == null ? null : Type.values()[type];
}
} else {
tokenType = Type.ACCESS_TOKEN;
}
if (in.getVersion().onOrAfter(Version.V_7_0_0)) {
realmName = in.readOptionalString();
userName = in.readOptionalString();
}
tokenString = in.readOptionalString();
Integer type = in.readOptionalVInt();
tokenType = type == null ? null : Type.values()[type];
realmName = in.readOptionalString();
userName = in.readOptionalString();
}
}
26 changes: 2 additions & 24 deletions ...main/java/org/elasticsearch/xpack/core/security/action/token/InvalidateTokenResponse.java
View file
Open in desktop
Expand Up @@ -5,7 +5,6 @@
*/
package org.elasticsearch.xpack.core.security.action.token;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand All @@ -14,8 +13,6 @@
import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult;

import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.Objects;

/**
Expand All @@ -35,35 +32,16 @@ public TokensInvalidationResult getResult() {
return result;
}

private boolean isCreated() {
return result.getInvalidatedTokens().size() > 0
&& result.getPreviouslyInvalidatedTokens().isEmpty()
&& result.getErrors().isEmpty();
}

@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
if (out.getVersion().before(Version.V_7_0_0)) {
out.writeBoolean(isCreated());
} else {
result.writeTo(out);
}
result.writeTo(out);
}

@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
if (in.getVersion().before(Version.V_7_0_0)) {
final boolean created = in.readBoolean();
if (created) {
result = new TokensInvalidationResult(Arrays.asList(""), Collections.emptyList(), Collections.emptyList(), 0);
} else {
result = new TokensInvalidationResult(Collections.emptyList(), Arrays.asList(""), Collections.emptyList(), 0);
}
} else {
result = new TokensInvalidationResult(in);
}
result = new TokensInvalidationResult(in);
}

@Override
Expand Down
8 changes: 0 additions & 8 deletions ...in/java/org/elasticsearch/xpack/core/security/authc/support/TokensInvalidationResult.java
View file
Open in desktop
Expand Up @@ -79,8 +79,6 @@ public int getAttemptCount() {
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject()
//Remove created after PR is backported to 6.x
.field("created", isCreated())
.field("invalidated_tokens", invalidatedTokens.size())
.field("previously_invalidated_tokens", previouslyInvalidatedTokens.size())
.field("error_count", errors.size());
Expand All @@ -104,10 +102,4 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeCollection(errors, StreamOutput::writeException);
out.writeVInt(attemptCount);
}

private boolean isCreated() {
return this.getInvalidatedTokens().size() > 0
&& this.getPreviouslyInvalidatedTokens().isEmpty()
&& this.getErrors().isEmpty();
}
}
46 changes: 1 addition & 45 deletions ...java/org/elasticsearch/xpack/core/security/action/token/InvalidateTokenResponseTests.java
View file
Open in desktop
Expand Up @@ -6,15 +6,13 @@
package org.elasticsearch.xpack.core.security.action.token;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.Version;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.VersionUtils;
import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult;

import java.io.IOException;
Expand Down Expand Up @@ -65,48 +63,6 @@ public void testSerialization() throws IOException {
}
}

public void testSerializationToPre66Version() throws IOException{
final Version version = VersionUtils.randomVersionBetween(random(), Version.V_6_2_0, Version.V_6_5_1);
TokensInvalidationResult result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false, false)),
Arrays.asList(generateRandomStringArray(20, 15, false, false)),
Arrays.asList(new ElasticsearchException("foo", new IllegalArgumentException("this is an error message")),
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))),
randomIntBetween(0, 5));
InvalidateTokenResponse response = new InvalidateTokenResponse(result);
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.setVersion(version);
response.writeTo(output);
try (StreamInput input = output.bytes().streamInput()) {
// False as we have errors and previously invalidated tokens
assertThat(input.readBoolean(), equalTo(false));
}
}

result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false, false)),
Arrays.asList(generateRandomStringArray(20, 15, false, false)),
Collections.emptyList(), randomIntBetween(0, 5));
response = new InvalidateTokenResponse(result);
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.setVersion(version);
response.writeTo(output);
try (StreamInput input = output.bytes().streamInput()) {
// False as we have previously invalidated tokens
assertThat(input.readBoolean(), equalTo(false));
}
}

result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false, false)),
Collections.emptyList(), Collections.emptyList(), randomIntBetween(0, 5));
response = new InvalidateTokenResponse(result);
try (BytesStreamOutput output = new BytesStreamOutput()) {
output.setVersion(version);
response.writeTo(output);
try (StreamInput input = output.bytes().streamInput()) {
assertThat(input.readBoolean(), equalTo(true));
}
}
}

public void testToXContent() throws IOException {
List invalidatedTokens = Arrays.asList(generateRandomStringArray(20, 15, false));
List previouslyInvalidatedTokens = Arrays.asList(generateRandomStringArray(20, 15, false));
Expand All @@ -118,7 +74,7 @@ public void testToXContent() throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder();
response.toXContent(builder, ToXContent.EMPTY_PARAMS);
assertThat(Strings.toString(builder),
equalTo("{\"created\":false," +
equalTo("{" +
"\"invalidated_tokens\":" + invalidatedTokens.size() + "," +
"\"previously_invalidated_tokens\":" + previouslyInvalidatedTokens.size() + "," +
"\"error_count\":2," +
Expand Down
11 changes: 6 additions & 5 deletions ...in/security/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredTokenRemover.java
View file
Open in desktop
Expand Up @@ -33,7 +33,9 @@
import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin;

/**
* Responsible for cleaning the invalidated tokens from the invalidated tokens index.
* Responsible for cleaning the invalidated and expired tokens from the security index.
* The document gets deleted if it was created more than 24 hours which is the maximum
* lifetime of a refresh token
*/
final class ExpiredTokenRemover extends AbstractRunnable {
private static final Logger logger = LogManager.getLogger(ExpiredTokenRemover.class);
Expand All @@ -57,10 +59,9 @@ public void doRun() {
final Instant now = Instant.now();
expiredDbq
.setQuery(QueryBuilders.boolQuery()
.filter(QueryBuilders.termsQuery("doc_type", TokenService.INVALIDATED_TOKEN_DOC_TYPE, "token"))
.filter(QueryBuilders.boolQuery()
.should(QueryBuilders.rangeQuery("expiration_time").lte(now.toEpochMilli()))
.should(QueryBuilders.rangeQuery("creation_time").lte(now.minus(24L, ChronoUnit.HOURS).toEpochMilli()))));
.filter(QueryBuilders.termsQuery("doc_type", "token"))
.filter(QueryBuilders.boolQuery()
.must(QueryBuilders.rangeQuery("creation_time").lte(now.minus(24L, ChronoUnit.HOURS).toEpochMilli()))));
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a bool here? Can't we just add the range directly to the filter?

logger.trace(() -> new ParameterizedMessage("Removing old tokens: [{}]", Strings.toString(expiredDbq)));
executeAsyncWithOrigin(client, SECURITY_ORIGIN, DeleteByQueryAction.INSTANCE, expiredDbq,
ActionListener.wrap(r -> {
Expand Down