Improve role cache efficiency for API key roles #58156

ywangd · 2020-06-16T08:39:45Z

This PR ensure that same roles are cached only once even when they are from different API keys.

API key role descriptors and limited role descriptors are now saved in Authentication#metadata as raw bytes instead of deserialised Map<String, Object>. Hashes of these bytes are used as keys for API key roles. Only when the required role is not found in the cache, they will be deserialised to build the RoleDescriptors. The deserialisation is directly from raw bytes to RoleDescriptors without going through the current detour of "bytes -> Map -> bytes -> RoleDescriptors".

A major aspect worth attention is BWC, we need:

Originally we planned to make v7.9.0 the shim for BWC and ultimately get rid of the BWC code for v8.0.0 and onwards. Unfortunately, this idea of shim does not work for Authentication object. This is because: though we only need to guarantee compatibility between "current major" and "last minor of last major" for wire communication, we have to cater for much wider version gaps when it comes to persisted data, e.g. at least "current major" and "last major". Authentication object can be persisted in an index as part of some background job definition, e.g. watcher, CCR etc. This means v8.0.0 may deserialise an old Authentication object from the index and it must be supported. Additionally, when it comes to reindex, it is possible that Authentication objects from even older versions are reindexed into newer ES versions without change. This basically means we have to support older Authentication objects forever. It is an issue that should be re-visited in future.

Resolves: #53939

ywangd · 2020-06-16T08:41:40Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

@@ -145,6 +145,7 @@ public void executeAsUser(User user, Consumer<StoredContext> consumer, Version v
     * The original context is provided to the consumer. When this method returns, the original context is restored.
     */
    public void executeAfterRewritingAuthentication(Consumer<StoredContext> consumer, Version version) {
+        // TODO: bwc here for API key metadata changes? i.e. deserialize the raw bytes into Map for older node.


I think BWC code is required here so that: if target node is an old version, rewrite the roleDescriptor and limitedRoleDescriptors to be Map<String, Object>.

ywangd · 2020-06-16T08:54:45Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

+            InstantiatingObjectParser.Builder<ApiKeyDoc, Void> builder =
+                InstantiatingObjectParser.builder("api_key_doc", true, ApiKeyDoc.class);
+            builder.declareString(constructorArg(), new ParseField("doc_type"));
+            builder.declareLong(constructorArg(), new ParseField("creation_time"));
+            builder.declareLongOrNull(constructorArg(), -1, new ParseField("expiration_time"));
+            builder.declareBoolean(constructorArg(), new ParseField("api_key_invalidated"));
+            builder.declareString(optionalConstructorArg(), new ParseField("api_key_hash"));
+            builder.declareString(constructorArg(), new ParseField("name"));
+            builder.declareInt(constructorArg(), new ParseField("version"));
+            ObjectParserHelper<ApiKeyDoc, Void> parserHelper = new ObjectParserHelper<>();
+            parserHelper.declareRawObject(builder, optionalConstructorArg(), new ParseField("role_descriptors"));
+            parserHelper.declareRawObject(builder, constructorArg(), new ParseField("limited_by_role_descriptors"));
+            parserHelper.declareRawObject(builder, constructorArg(), new ParseField("creator"));
+            PARSER = builder.build();


Is it possible that some fields, e.g. version or name, are completely missing from the source document? This parser requires all fields to present (null value is ok but it must present). In current code, all fields do present. I wonder whether this is true for older versions.

I suspect it's not true for all versions, or at least it might not be.
We'd probably want to generate an API key from v6.7 and check, but it might be safer to change the parser to be more accommodating of missing fields.

I checked v6.7 and it looks fine, i.e. it has the same fields as in the master branch. But it might still be safer to have more leniency for missing fields? I'll see how it can be done.

It's actually fairly easily to make fields optional by using

builder.declareString(ApiKeyDoc::setName, new ParseField("name"))

instead of

builder.declareString(constructorArg(), new ParseField("name")).

But I ended up didn't make any of the fields optional because:

All of them exist from 6.7. So not sure which one I should pick for optional. (The only thing missing in 6.7 is creator.realm_type. But it is still wrapped inside a Map<String, Object> in the new implementation so it is not an issue).

Once a field made optional, it cannot be final and becomes an anomaly in current implementation. It's not a strong argument, just a personal preference.

Please let me know if you have concerns.

ywangd · 2020-06-16T12:30:56Z

Added bwc for outgoing requests. It rewrites the API key auth metadata when target node is on an old version.

Unfortunately, I think the idea of using v7.9.0 as a shim and make v8.0.0 free of bwc code does not work. The shim idea will only works for wire communication since we only guarantee compatibility between "current major" and "last minor of last major". But in the case of Authentication object, it can be serialised in an index as part of some background job definition. Since we guarantee compatibility for indexed data between "current major" and "last major", this means v8.0.0 may deserialise an old Authentication ojbect from the index and it must be supported.

ywangd · 2020-06-16T13:10:58Z

@elasticmachine run elasticsearch-ci/2

elasticmachine · 2020-06-17T02:08:03Z

Pinging @elastic/es-security (:Security/Authorization)

ywangd · 2020-06-17T02:34:20Z

Added a yaml test in rolling upgrade test suite to ensure BWC layer is working. It is currently disabled and will be enabled once the changes are backported to v7.9.0.

This PR is now ready for review.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

...security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

tvernum · 2020-06-25T06:02:09Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

@@ -295,6 +318,21 @@ private void buildThenMaybeCacheRole(RoleKey roleKey, Collection<RoleDescriptor>
        }, listener::onFailure));
    }

+    private void getOrBuildRoleForApiKey(Authentication authentication, boolean limitedBy, ActionListener<Role> roleActionListener) {
+        final BytesReference roleDescriptorsBytes = apiKeyService.getRoleDescriptorsBytesForApiKey(authentication, limitedBy);
+        final String roleDescriptorsHash = new String(Hasher.SHA256.hash(new SecureString(roleDescriptorsBytes.toString().toCharArray())));


BytesReference.toString() is not guaranteed to be what you want here.
You want either utf8ToString or just use the bytes directly.

I'm not sure whether we really even need to hash these - we can, but it might not be necessary.
I think the options are either:

Don't bother hashing, but that will mean using a separate cache for API key roles (because the cache key is different)

Suggested change

final String roleDescriptorsHash = new String(Hasher.SHA256.hash(new SecureString(roleDescriptorsBytes.toString().toCharArray())));

final BytesKey cacheKey = new BytesKey(BytesReference.toBytes(roleDescriptorsBytes));

Hash using MessageDigest

Suggested change

final String roleDescriptorsHash = new String(Hasher.SHA256.hash(new SecureString(roleDescriptorsBytes.toString().toCharArray())));

MessageDigest digest = MessageDigests.sha256();

digest.update(BytesReference.toBytes(roleDescriptorsBytes));

final String roleDescriptorsHash = MessageDigests.toHexString(digest.digest());

I really like BytesKey, unfortunately it is not compatible with the current cache and I don't want to add another cache and associated overhead just because of this incompatibility. So I'll prefer to your second suggestion, i.e. MessageDigest.sha256 where raw bytes are used.

Alternatively, we could potential change RoleKey to have either Set<byte[]> names and byte[] source, so it can accomodate both regular roles and API key roles. But I am not sure whether it really gives us much unless we have a real concern for sha256 collision.

I'm happy with the sha256 option. I also like BytesKey - it's simple and sensible, but I don't think it's the right fit here unless we change the cache to have aSet<?> as the key, or follow your idea of converting a Set<String> into a BytesKey

Cool let's keep the sha256 option then. It works and is simpler.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

tvernum · 2020-06-25T06:20:43Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

+        return (BytesReference) metadata.get(limitedBy ? API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY : API_KEY_ROLE_DESCRIPTORS_KEY);
+    }
+
+    public List<RoleDescriptor> getRoleDescriptorsForApiKey(Authentication authentication, boolean limitedBy) {


I think having getRoleForApiKey, getRoleDescriptorsBytesForApiKey and getRoleDescriptorsForApiKey is confusing and likely to get us into trouble in the future.

Is it necessary for the caller to have to know which method to use depending on the version of the Authentication object it has?
I worry about the fragility of code that calls getRoleDescriptorsForApiKey and works on 7.9+ but then fails in a mixed cluster.
Would it be possible to merge getRoleDescriptorsForApiKey and getRoleForApiKey into a single method, or at least make it save to call getRoleDescriptorsForApiKey on an old style Authentication object?

Short version: I removed getRoleDescriptorsForApiKey and renamed getRoleDescriptorsBytesForApiKey to getApiKeyIdAndBytes.

Longer version: I agree the names are rather confusing. Initially, I completely removed getRoleForApiKey and List<RoleDescriptor> parseRoleDescriptors(String, Map<String, Object>) because they are used only for old style Authentication. But I had to add them back when I realised there were BWC issues. Anyway, we can and should remove them in 9.0.

Unfortunately, it is not feasible to merge getRoleForApiKey and getRoleDescriptorsForApiKey even though they both return something related to role descriptors. The old getRoleForApiKey returns a ApiKeyRoleDescriptors object, which internally has two List<RoleDescriptors>, one for from-role and the other for limited-by. But this would be non-ideal for the new getRoleDescriptorsForApiKey which returns either from-role-descriptors or limited-by-role-descriptors. By returning them separately, we can cache and lookup them separately. That is, if multiple keys are created by the same user, their limited-by roles are only computed once and cached for all other keys. The reverse is also true (though maybe less likely), if multiple keys happen to have the from-role-descriptors even when they are created by different users, we only need compute the from-roles once.

So there is no shared code path between getRoleForApiKey and getRoleDescriptorsForApiKey they work like the follows:

Old Authentication -> getRoleForApiKey -> parseRoleDescriptors(String, Map<String, Object>)

New Authentication -> getRoleDescriptorsBytesForApiKey -> if not cached -> getRoleDescriptorsForApiKey -> parseRoleDescriptors(String, ByteReference)

I intentionally kept them separate so the old code path can be easily removed when the time comes.

With above being said, I do agree that the names are confusing. So I renamed getRoleDescriptorsBytesForApiKey to getApiKeyIdAndBytes and removed getRoleDescriptorsForApiKey by a direct call to parseRoleDescriptors(String, ByteReference) in the caller.

If I understand correctly, getRoleForApiKey will only do the right thing for old style authentication.
If so, can we:

Add that to the javadoc

Add an assert authentication.getVersion().before(Version.V7_9_0) : "This method only applies to authentication objects created before v7.9.0";

These are both good ideas. Will update accordingly. Thanks!

tvernum · 2020-07-01T05:54:36Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

+            InstantiatingObjectParser.Builder<ApiKeyDoc, Void> builder =
+                InstantiatingObjectParser.builder("api_key_doc", true, ApiKeyDoc.class);
+            builder.declareString(constructorArg(), new ParseField("doc_type"));
+            builder.declareLong(constructorArg(), new ParseField("creation_time"));
+            builder.declareLongOrNull(constructorArg(), -1, new ParseField("expiration_time"));
+            builder.declareBoolean(constructorArg(), new ParseField("api_key_invalidated"));
+            builder.declareString(optionalConstructorArg(), new ParseField("api_key_hash"));
+            builder.declareString(constructorArg(), new ParseField("name"));
+            builder.declareInt(constructorArg(), new ParseField("version"));
+            ObjectParserHelper<ApiKeyDoc, Void> parserHelper = new ObjectParserHelper<>();
+            parserHelper.declareRawObject(builder, optionalConstructorArg(), new ParseField("role_descriptors"));
+            parserHelper.declareRawObject(builder, constructorArg(), new ParseField("limited_by_role_descriptors"));
+            parserHelper.declareRawObject(builder, constructorArg(), new ParseField("creator"));
+            PARSER = builder.build();


I suspect it's not true for all versions, or at least it might not be.
We'd probably want to generate an API key from v6.7 and check, but it might be safer to change the parser to be more accommodating of missing fields.

Co-authored-by: Tim Vernum <tim@adjective.org>

…les-cache

…ecurity/authz/store/CompositeRolesStore.java Co-authored-by: Tim Vernum <tim@adjective.org>

…les-cache

albertzaharovits

I think this would work, but it feels like you're trying to force it into the existing code, and the interfaces lose consistency. I think that if you were to introduce a more moderate change, that is the cache key be based on the RoleDescriptor list, you could fit that into the CompositeRolesStore.

If we really need to get this merged before FF, I think we can address the refactoring in a follow-up.

I haven't looked at the tests too closely.

albertzaharovits · 2020-07-12T16:57:18Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

+        Map<String, Object> metadata = authentication.getMetadata();
+        if (authentication.getAuthenticationType() == AuthenticationType.API_KEY
+            && authentication.getVersion().onOrAfter(Version.V_7_9_0)
+            && streamVersion.before(Version.V_7_9_0)) {


The name rewriteMetadataIfNecessary is not very inspired; I have to read the code to know what this rewriting is about.

My suggestion: rewriteApiKeyAuthenticationRoleMetadata (drop "if necessary")
I would also slightly prefer to move this to the Authentication class, and maybe have a static method there that takes an authentication and a version as parameters and returns a new authentication with that version, and any rewritings done. I would also define a new static constant on Version.V_7_9_0 inside Authentication as VERSION_API_KEY_ROLES_AS_STRING (see TokenService for examples).

These are all good suggestions. Will address accordingly.

I renamed the method to rewriteMetadataForApiKeyRoleDescriptors. But after some thoughts, I decided to keep this method inside SecurityContext. Because I need to make it public if moved to Authentication and that feels a bit uneasy to me since SecurityContext is the only consumer and will probably keep it that way in future. So I feel it is better to stay within it and closer to the actual context. It is an unnecessary detail to put inside Authentication if people is not concerned of BWC. Similarly I added the new version constant to SecurityContext as well. Sorry for the back and forth.

Okay, but the constant can sit inside Authentication to be referenced in the CompositeRolesStore too.

Moved the new version constant to Authentication as suggested.

albertzaharovits · 2020-07-12T17:48:43Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

@@ -974,4 +1018,66 @@ private boolean verify(SecureString password) {
            return hash != null && cacheHasher.verify(password, hash);
        }
    }
+
+    public static final class ApiKeyDoc {


Why have you introduced this new class in this PR? Is it required for something?
I'd obviously vote to get rid of the Map<String, Object> source any time of day, but this doesn't look like a great improvement, and it makes this PR harder to follow than necessary.

Yes, it is intentionally introduced and is more or less a centre piece of this PR, which is about improve the role cache efficiency for API keys, i.e. same role descriptors should only be cached once even when they are from different API keys. The current cache is basically keyed by ApiKey ID, which is unfit for the goal. The obvious choice is to use role descriptors themselves as the key, but it has a few downsides and also not compatible the existing cache. So alternatively, we could use the string representation of role descriptors as the key. To further reduce the memory cost, we actually use sha256 of the string representation as the key. More details can be found here

So this new class has roleDescriptors and limitedByRoeDescriptors as ByteReference to faciliate the calculation. By storing them as bytes instead of Map, it also helps avoid quite a few unncessary serialisation and deserialisation. So instead of four de/serialisation, it now only deserialises once. When security index is local to a node, all these de/serialisation operations are a noticable part of the total authentication time (> 20%).

Okay, thank you for the explanation!

I would've thought that you can get the BytesReference for a source field but haven't checked before asking and it turns out you can't, you must use a parser.

albertzaharovits · 2020-07-12T20:01:21Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

+            if (authentication.getVersion().onOrAfter(Version.V_7_9_0)) {
+                getOrBuildRoleForApiKey(authentication, false, ActionListener.wrap(
+                    role -> {
+                        if (role == Role.EMPTY) {


When can this be true?

When a list of role descriptor is empty, the role building method buildRoleFromDescriptors returns Role.EMPTY.

Also when an API key does not have its own role descriptors, its document will have {} for the role_descriptors field, which in turn will be converted to an empty list of RoleDescriptor.

When the two work together, you can get Role.EMPTY.

Got it! I have to check the code every single time to tell if the role_descriptors field or the limited_by_role_descriptors can be empty, the meanings are reversed in ApiKeyService#getRoleForApiKey.

Since limitedByRole can never be null I would be building that first, for a minor branching simplification:

final Authentication.AuthenticationType authType = authentication.getAuthenticationType(); if (authType == Authentication.AuthenticationType.API_KEY) { if (authentication.getVersion().onOrAfter(Version.V_7_9_0)) { - buildAndCacheRoleForApiKey(authentication, false, ActionListener.wrap( - role -> { - if (role == Role.EMPTY) { - buildAndCacheRoleForApiKey(authentication, true, roleActionListener); - } else { - buildAndCacheRoleForApiKey(authentication, true, ActionListener.wrap( - limitedByRole -> roleActionListener.onResponse( - limitedByRole == Role.EMPTY ? role : LimitedRole.createLimitedRole(role, limitedByRole)), - roleActionListener::onFailure + buildAndCacheRoleForApiKey(authentication, true, ActionListener.wrap( + limitedByRole -> { + buildAndCacheRoleForApiKey(authentication, false, ActionListener.wrap( + role -> roleActionListener.onResponse( + role == Role.EMPTY ? limitedByRole : LimitedRole.createLimitedRole(role, limitedByRole)), + roleActionListener::onFailure )); - } - }, - roleActionListener::onFailure + }, + roleActionListener::onFailure )); } else { apiKeyService.getRoleForApiKey(authentication, ActionListener.wrap(apiKeyRoleDescriptors -> {

Feel free to follow this suggestion or not.

You may be right. But I am not entirely sure when it comes to derived API keys. A derived keys will have limited by role descriptors from parent API key's roles. Since we now zero out API key's roles during authentication, we may end up having an empty limited by role descriptors as well. I have not tested, but I feel it is probably safer to keep the code as is for now.

albertzaharovits · 2020-07-12T20:08:00Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

+        final Tuple<String, BytesReference> apiKeyIdAndBytes = apiKeyService.getApiKeyIdAndRoleBytes(authentication, limitedBy);
+        final MessageDigest digest = MessageDigests.sha256();
+        digest.update(BytesReference.toBytes(apiKeyIdAndBytes.v2()));
+        final String roleDescriptorsHash = MessageDigests.toHexString(digest.digest());


Nit: you can can compute the digest in one line.

Even more succinct:

final String roleDescriptorsHash = MessageDigests.toHexString(MessageDigests.sha256().digest(BytesReference.toBytes(apiKeyIdAndBytes.v2())));

MessageDigests.sha256() is thread local and it's also reset before every use, so my objection was actually about minimising the scope of the digest instance.

Updated as suggested. Thanks!

albertzaharovits · 2020-07-12T20:32:32Z

...security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java

@@ -295,6 +318,23 @@ private void buildThenMaybeCacheRole(RoleKey roleKey, Collection<RoleDescriptor>
        }, listener::onFailure));
    }

+    private void getOrBuildRoleForApiKey(Authentication authentication, boolean limitedBy, ActionListener<Role> roleActionListener) {
+        final Tuple<String, BytesReference> apiKeyIdAndBytes = apiKeyService.getApiKeyIdAndRoleBytes(authentication, limitedBy);


Methods here don't have a clear interface, it could just as well not be methods at all; the limitedBy flag is really ruining every structure.

I would move the full logic that obtains the Role for an API Keys from the CompositeRolesStore class to the ApiKeyService class.
But I realise I'm late to the party, we can do this in a follow-up.
@tvernum would you agree merging this PR, and following up with a refactoring?

Role building has always been done inside CompositeRolesStore and I think it fits here since role building and caching are the main purposes of CompositeRolesStore. It only delegates retrieval of role descriptors to different roles providers, including ApiKeyService, which is the case for this method as well. Overall, I think the method name could use some help (maybe buildAndCacheRoleForApiKey to mirror the existing buildAndCacheRoleFromDescriptors?), but its content looks fine to me.

Good point! Yes, Role should stay inside the CompositeRolesStore.

Then I think we can make ApiKeyRoleDescriptors lazily parse RoleDescriptor. Not sure what to do about the cache key. Let's leave this for the future us.

tvernum

LGTM.

I think what we end up with at the end is still more complex than necessary - there's a lot of scope for refactoring within these classes - but I think this change covers the right things and is the right size.
I'd be very happy to see additional PRs focused on refactoring & cleaning up the interaction of all these overlapping concerns.

ywangd · 2020-07-13T06:52:08Z

I think this would work, but it feels like you're trying to force it into the existing code, and the interfaces lose consistency. I think that if you were to introduce a more moderate change, that is the cache key be based on the RoleDescriptor list, you could fit that into the CompositeRolesStore.

Thanks Albert. You are right that I did try fit the changes to the exisitng role cache, which leads to some friction. But I think it's overall a reasonable approach:

The RoleKeys#names is meaningless for API key anyway. So it is arguable that a singletonList of role-descriptors string is a close enough approximation. Please note that I am suggesting a singletonList of string where the string is the representation of the entire role descriptors instead of a single role descriptor. This is better in terms of de/serialisation efficiency. Basically nothing is deseirialised unless it is really needed, i.e. when building the Role.
I did consider using the above singletonList of role-descriptors string as the key. But decided to use its sha256 to be extra safe because the role descriptors is an user input and can be arbitrarily large.
Adding another cache has non-negligible overhead, I'd prefer reuse existing ones unless there are serious issues.

…les-cache

albertzaharovits

LGTM, after I got a bit familiar with the code it become easier to follow.

This PR ensure that same roles are cached only once even when they are from different API keys. API key role descriptors and limited role descriptors are now saved in Authentication#metadata as raw bytes instead of deserialised Map<String, Object>. Hashes of these bytes are used as keys for API key roles. Only when the required role is not found in the cache, they will be deserialised to build the RoleDescriptors. The deserialisation is directly from raw bytes to RoleDescriptors without going through the current detour of "bytes -> Map -> bytes -> RoleDescriptors".

tylersmalley · 2020-07-13T23:43:49Z

This commit is resulting in a regression within Kibana which being tracked here: elastic/kibana#71555

   │ info [o.e.x.s.a.AuthenticationService] [desktop] Authentication using apikey failed - apikey authentication for id NB-BSnMBuA9FcpEJrQtR encountered a failure
   │      org.elasticsearch.common.xcontent.XContentParseException: [1:532] [api_key_doc] name doesn't support values of type: VALUE_NULL

ywangd added 4 commits June 16, 2020 14:05

Only partially deserialise api key doc during authentication

390c499

Improve role cache efficiency and add bwc for API key auth metadata

c353aef

Fix tests

46cf4fd

Update bwc version

1319909

ywangd added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 v7.9.0 labels Jun 16, 2020

ywangd requested a review from tvernum June 16, 2020 08:39

ywangd commented Jun 16, 2020

View reviewed changes

checkstyle

b1688f9

ywangd commented Jun 16, 2020

View reviewed changes

ywangd added 2 commits June 16, 2020 19:55

Fix more tests

0f9532b

Add bwc for outgoing requests

f1e1285

ywangd added 2 commits June 17, 2020 11:07

Add more tests for bwc and role cache reuse

9832f50

Add mixed cluster bwc test

d2560d8

ywangd marked this pull request as ready for review June 17, 2020 02:08

elasticmachine added the Team:Security Meta label for security team label Jun 17, 2020

tvernum reviewed Jul 1, 2020

View reviewed changes

ywangd and others added 7 commits July 1, 2020 17:14

Apply suggestions from code review

ca698d7

Co-authored-by: Tim Vernum <tim@adjective.org>

fix variable name

cf785c9

Merge remote-tracking branch 'origin/master' into es-53939-api-key-ro…

abcd485

…les-cache

Avoid using deprecated API

d62d8a2

Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/s…

8148050

…ecurity/authz/store/CompositeRolesStore.java Co-authored-by: Tim Vernum <tim@adjective.org>

Address feedback

1440bfe

Fix tests

cfca9cc

ywangd requested a review from albertzaharovits July 8, 2020 07:29

ywangd added 5 commits July 8, 2020 17:58

Merge remote-tracking branch 'origin/master' into es-53939-api-key-ro…

9395a83

…les-cache

Address feedback

0f08a3c

Simplify creator parsing of ApiKeyDoc

5b75e1d

Merge remote-tracking branch 'origin/master' into es-53939-api-key-ro…

bedb46f

…les-cache

Fix merge

52698f4

albertzaharovits reviewed Jul 12, 2020

View reviewed changes

ywangd mentioned this pull request Jul 13, 2020

Cache API key doc to reduce traffic to the security index #59376

Merged

Address feedback

8543c18

tvernum approved these changes Jul 13, 2020

View reviewed changes

ywangd added 2 commits July 13, 2020 16:55

Address more feedback

c1fba5a

Merge remote-tracking branch 'origin/master' into es-53939-api-key-ro…

0c991a6

…les-cache

albertzaharovits self-requested a review July 13, 2020 09:27

albertzaharovits approved these changes Jul 13, 2020

View reviewed changes

ywangd added 5 commits July 13, 2020 19:57

Address feedback

0ac2a84

checkstyle

70307e7

Use new version constants where it is applicable

8b1153c

more checkstyle

121134b

Add one test for ApiKeyDoc.fromXContent

2b20b48

ywangd merged commit a28ce1e into elastic:master Jul 13, 2020

tylersmalley mentioned this pull request Jul 14, 2020

Regression when an API key has no name #59481

Closed

dhurley14 mentioned this pull request Jul 31, 2020

[SIEM] Executes security application cypress tests on every PR elastic/kibana#73939

Closed

FrankHassanabad mentioned this pull request Aug 26, 2020

Alerts and security solutions rules with spaces have validation issues elastic/kibana#76035

Closed

ywangd mentioned this pull request Sep 25, 2020

Wrong format of API key role descriptors used for node requests after cluster upgrade #62911

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

	final String roleDescriptorsHash = new String(Hasher.SHA256.hash(new SecureString(roleDescriptorsBytes.toString().toCharArray())));
	final BytesKey cacheKey = new BytesKey(BytesReference.toBytes(roleDescriptorsBytes));

Improve role cache efficiency for API key roles #58156

Improve role cache efficiency for API key roles #58156

Conversation

ywangd commented Jun 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywangd commented Jun 16, 2020

ywangd commented Jun 16, 2020

elasticmachine commented Jun 17, 2020

ywangd commented Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertzaharovits left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywangd Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

albertzaharovits Jul 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tvernum left a comment

Choose a reason for hiding this comment

ywangd commented Jul 13, 2020

albertzaharovits left a comment

Choose a reason for hiding this comment

tylersmalley commented Jul 13, 2020

ywangd commented Jun 16, 2020 •

edited

Loading

ywangd commented Jun 17, 2020 •

edited

Loading

ywangd Jul 13, 2020 •

edited

Loading

albertzaharovits Jul 13, 2020 •

edited

Loading