Skip to content

Support stored authentication headers prior to version 6.7 #92221

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

Merged
merged 4 commits into from
Dec 12, 2022
Merged

Support stored authentication headers prior to version 6.7 #92221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c3815cf
Support stored authentication headers prior to version 6.7
ywangd Dec 8, 2022
7d5147f
Update docs/changelog/92221.yaml
ywangd Dec 8, 2022
5e18c72
Merge remote-tracking branch 'origin/main' into support-v6-stored-aut…
ywangd Dec 12, 2022
9b3a82c
Support write for 6.x authc header
ywangd Dec 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/92221.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 92221
summary: Support stored authentication headers prior to version 6.7
area: Authentication
type: bug
issues: []
28 changes: 24 additions & 4 deletions ...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.common.xcontent.XContentHelper;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.core.Strings;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.ToXContentObject;
Expand Down Expand Up @@ -90,6 +91,7 @@
public final class Authentication implements ToXContentObject {

private static final Logger logger = LogManager.getLogger(Authentication.class);
private static final Version VERSION_AUTHENTICATION_TYPE = Version.fromString("6.7.0");

public static final Version VERSION_API_KEY_ROLES_AS_BYTES = Version.V_7_9_0;
public static final Version VERSION_REALM_DOMAINS = Version.V_8_2_0;
Expand Down Expand Up @@ -141,8 +143,14 @@ public Authentication(StreamInput in) throws IOException {
assert innerUser != null || lookedUpBy == null : "Authentication has no inner-user, but looked-up-by is [" + lookedUpBy + "]";

final Version version = in.getVersion();
type = AuthenticationType.values()[in.readVInt()];
final Map<String, Object> metadata = in.readMap();
final Map<String, Object> metadata;
if (version.onOrAfter(VERSION_AUTHENTICATION_TYPE)) {
type = AuthenticationType.values()[in.readVInt()];
metadata = in.readMap();
} else {
type = AuthenticationType.REALM;
metadata = Map.of();
}
Comment on lines +147 to +153
Copy link
Member Author

@ywangd ywangd Dec 12, 2022

Choose a reason for hiding this comment

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

We should have corresponding logic on the writeTo side. But I could not decide whether it should be:

  1. Simply throwing an error if version is 6.x
  2. Drop type and metadata if version is 6.x
  3. A combination of the two, ie. drop type and metadata if type is REALM and metadata is empty, otherwise throw.

I think the answer might be 3?

Copy link
Member Author

@ywangd ywangd Dec 12, 2022

Choose a reason for hiding this comment

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

I pushed a commit for option 2. Because option 1 is not viable as it prevents authentication to be sent across wire. Option 3 is likely overkill and hard to test (need to hack an invalid 6.x header). Please consider it as a strawman implementation. It is easier to discuss with something concrete.

if (innerUser != null) {
authenticatingSubject = new Subject(innerUser, authenticatedBy, version, metadata);
// The lookup user for run-as currently doesn't have authentication metadata associated with them because
Expand Down Expand Up @@ -469,8 +477,20 @@ public void writeTo(StreamOutput out) throws IOException {
} else {
out.writeBoolean(false);
}
out.writeVInt(type.ordinal());
out.writeGenericMap(getAuthenticatingSubject().getMetadata());
final Map<String, Object> metadata = getAuthenticatingSubject().getMetadata();
if (out.getVersion().onOrAfter(VERSION_AUTHENTICATION_TYPE)) {
out.writeVInt(type.ordinal());
out.writeGenericMap(metadata);
} else {
assert type == AuthenticationType.REALM && metadata.isEmpty()
: Strings.format(
"authentication with version [%s] must have authentication type %s and empty metadata, but got [%s] and [%s]",
out.getVersion(),
AuthenticationType.REALM,
type,
metadata
);
}
}

/**
Expand Down
30 changes: 30 additions & 0 deletions ...n/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;
import org.elasticsearch.xpack.core.security.authc.service.ServiceAccountSettings;
import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer;
import org.elasticsearch.xpack.core.security.user.AnonymousUser;
import org.elasticsearch.xpack.core.security.user.User;

Expand All @@ -32,6 +33,7 @@
import java.util.function.Consumer;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -474,6 +476,34 @@ public void testToXContentWithServiceAccount() throws IOException {
);
}

public void testBwcWithStoredAuthenticationHeaders() throws IOException {
// Version 6.6.1
final String headerV6 = "p/HxAgANZWxhc3RpYy1hZG1pbgEJc3VwZXJ1c2VyCgAAAAEABG5vZGUFZmlsZTEEZmlsZQA=";
final Authentication authenticationV6 = AuthenticationContextSerializer.decode(headerV6);
assertThat(authenticationV6.getEffectiveSubject().getVersion(), equalTo(Version.fromString("6.6.1")));
assertThat(authenticationV6.getEffectiveSubject().getUser(), equalTo(new User("elastic-admin", "superuser")));
assertThat(authenticationV6.getAuthenticationType(), equalTo(Authentication.AuthenticationType.REALM));
assertThat(authenticationV6.isRunAs(), is(false));
assertThat(authenticationV6.encode(), equalTo(headerV6));

// Rewrite for a different version
final Version nodeVersion = VersionUtils.randomIndexCompatibleVersion(random());
final Authentication rewrittenAuthentication = authenticationV6.maybeRewriteForOlderVersion(nodeVersion);
assertThat(rewrittenAuthentication.getEffectiveSubject().getVersion(), equalTo(nodeVersion));
assertThat(rewrittenAuthentication.getEffectiveSubject().getUser(), equalTo(authenticationV6.getEffectiveSubject().getUser()));
assertThat(rewrittenAuthentication.getAuthenticationType(), equalTo(Authentication.AuthenticationType.REALM));
assertThat(rewrittenAuthentication.isRunAs(), is(false));

// Version 7.2.1
final String headerV7 = "p72sAwANZWxhc3RpYy1hZG1pbgENX2VzX3Rlc3Rfcm9vdAoAAAABAARub2RlBWZpbGUxBGZpbGUAAAoA";
final Authentication authenticationV7 = AuthenticationContextSerializer.decode(headerV7);
assertThat(authenticationV7.getEffectiveSubject().getVersion(), equalTo(Version.fromString("7.2.1")));
assertThat(authenticationV7.getEffectiveSubject().getUser(), equalTo(new User("elastic-admin", "_es_test_root")));
assertThat(authenticationV7.getAuthenticationType(), equalTo(Authentication.AuthenticationType.REALM));
assertThat(authenticationV7.isRunAs(), is(false));
assertThat(authenticationV7.encode(), equalTo(headerV7));
}

private void runWithAuthenticationToXContent(Authentication authentication, Consumer<Map<String, Object>> consumer) throws IOException {
try (XContentBuilder builder = XContentBuilder.builder(XContentType.JSON.xContent())) {
authentication.toXContent(builder, ToXContent.EMPTY_PARAMS);
Expand Down