Review

elastic · Jul 6, 2020 · 103609e · 103609e
1 parent b21a797
commit 103609e
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 36 deletions.
diff --git a/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
@@ -230,9 +230,9 @@ private void createApiKeyAndIndexIt(Authentication authentication, CreateApiKeyR
     }
 
     /**
-     * public for testing
+     * package-private for testing
      */
-    public XContentBuilder newDocument(SecureString apiKey, String name, Authentication authentication, Set<RoleDescriptor> userRoles,
+    XContentBuilder newDocument(SecureString apiKey, String name, Authentication authentication, Set<RoleDescriptor> userRoles,
                                         Instant created, Instant expiration, List<RoleDescriptor> keyRoles,
                                         Version version) throws IOException {
         XContentBuilder builder = XContentFactory.jsonBuilder();
@@ -505,8 +505,8 @@ CachedApiKeyHashResult getFromCache(String id) {
         return apiKeyAuthCache == null ? null : FutureUtils.get(apiKeyAuthCache.get(id), 0L, TimeUnit.MILLISECONDS);
     }
 
-    // public for testing
-    public void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
+    // package-private for testing
+    void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredentials credentials, Clock clock,
                                   ActionListener<AuthenticationResult> listener) {
         final Long expirationEpochMilli = (Long) source.get("expiration_time");
         if (expirationEpochMilli == null || Instant.ofEpochMilli(expirationEpochMilli).isAfter(clock.instant())) {

diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityTestsUtils.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecurityTestsUtils.java
@@ -7,28 +7,10 @@
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.elasticsearch.ElasticsearchSecurityException;
-import org.elasticsearch.Version;
-import org.elasticsearch.action.support.PlainActionFuture;
-import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.settings.SecureString;
-import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.common.xcontent.XContentHelper;
-import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.license.LicenseUtils;
 import org.elasticsearch.rest.RestStatus;
-import org.elasticsearch.xpack.core.security.authc.Authentication;
-import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
-import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
-import org.elasticsearch.xpack.security.authc.ApiKeyService;
 import org.hamcrest.Matcher;
 
-import java.time.Clock;
-import java.time.Duration;
-import java.time.Instant;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 import static org.apache.lucene.util.LuceneTestCase.expectThrows;
 import static org.elasticsearch.xpack.core.security.test.SecurityAssertions.assertContainsWWWAuthenticateHeader;
 import static org.hamcrest.CoreMatchers.containsString;
@@ -83,17 +65,6 @@ public static void assertThrowsAuthorizationException(LuceneTestCase.ThrowingRun
         assertAuthorizationException(securityException, messageMatcher);
     }
 
-    public static Authentication createApiKeyAuthentication(ApiKeyService apiKeyService, Authentication authentication,
-                                                            Set<RoleDescriptor> userRoles, List<RoleDescriptor> keyRoles) throws Exception {
-        XContentBuilder keyDocSource = apiKeyService.newDocument(new SecureString("secret".toCharArray()), "test", authentication,
-                userRoles, Instant.now(), Instant.now().plus(Duration.ofSeconds(3600)), keyRoles, Version.CURRENT);
-        Map<String, Object> keyDocMap = XContentHelper.convertToMap(BytesReference.bytes(keyDocSource), true, XContentType.JSON).v2();
-        PlainActionFuture<AuthenticationResult> authenticationResultFuture = PlainActionFuture.newFuture();
-        apiKeyService.validateApiKeyExpiration(keyDocMap, new ApiKeyService.ApiKeyCredentials("id", new SecureString("pass".toCharArray())),
-                Clock.systemUTC(), authenticationResultFuture);
-        return apiKeyService.createApiKeyAuthentication(authenticationResultFuture.get(), "node01");
-    }
-
     private static void assertAuthorizationException(Throwable throwable, Matcher<String> messageMatcher) {
         assertThat(throwable, instanceOf(ElasticsearchSecurityException.class));
         ElasticsearchSecurityException securityException = (ElasticsearchSecurityException) throwable;

diff --git a/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java b/...gin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java
@@ -58,7 +58,9 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -602,6 +604,21 @@ public void testWillGetLookedUpByRealmTypeIfExists() {
         assertEquals("looked_up_by_type", ApiKeyService.getCreatorRealmType(authentication));
     }
 
+    public static class Utils {
+
+        public static Authentication createApiKeyAuthentication(ApiKeyService apiKeyService, Authentication authentication,
+                                                                Set<RoleDescriptor> userRoles, List<RoleDescriptor> keyRoles) throws Exception {
+            XContentBuilder keyDocSource = apiKeyService.newDocument(new SecureString("secret".toCharArray()), "test", authentication,
+                    userRoles, Instant.now(), Instant.now().plus(Duration.ofSeconds(3600)), keyRoles, Version.CURRENT);
+            Map<String, Object> keyDocMap = XContentHelper.convertToMap(BytesReference.bytes(keyDocSource), true, XContentType.JSON).v2();
+            PlainActionFuture<AuthenticationResult> authenticationResultFuture = PlainActionFuture.newFuture();
+            apiKeyService.validateApiKeyExpiration(keyDocMap, new ApiKeyService.ApiKeyCredentials("id",
+                            new SecureString("pass".toCharArray())),
+                    Clock.systemUTC(), authenticationResultFuture);
+            return apiKeyService.createApiKeyAuthentication(authenticationResultFuture.get(), "node01");
+        }
+    }
+
     private ApiKeyService createApiKeyService(Settings baseSettings) {
         final Settings settings = Settings.builder()
             .put(XPackSettings.API_KEY_SERVICE_ENABLED_SETTING.getKey(), true)

diff --git a/.../src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/.../src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java
@@ -32,7 +32,6 @@
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.license.XPackLicenseState.Feature;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.test.SecurityTestsUtils;
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportRequest;
 import org.elasticsearch.transport.TransportRequest.Empty;
@@ -91,6 +90,7 @@
 
 import static org.elasticsearch.mock.orig.Mockito.times;
 import static org.elasticsearch.mock.orig.Mockito.verifyNoMoreInteractions;
+import static org.elasticsearch.xpack.security.authc.ApiKeyServiceTests.Utils.createApiKeyAuthentication;
 import static org.hamcrest.Matchers.anyOf;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
@@ -1046,7 +1046,7 @@ public void testApiKeyAuthUsesApiKeyService() throws Exception {
                 rds -> effectiveRoleDescriptors.set(rds));
         AuditUtil.getOrGenerateRequestId(threadContext);
 
-        final Authentication authentication = SecurityTestsUtils.createApiKeyAuthentication(apiKeyService, createAuthentication(),
+        final Authentication authentication = createApiKeyAuthentication(apiKeyService, createAuthentication(),
                 Collections.singleton(new RoleDescriptor("user_role_" + randomAlphaOfLength(4), new String[]{"manage"}, null, null)), null);
 
         PlainActionFuture<Role> roleFuture = new PlainActionFuture<>();
@@ -1091,7 +1091,7 @@ public void testApiKeyAuthUsesApiKeyServiceWithScopedRole() throws Exception {
                 rds -> effectiveRoleDescriptors.set(rds));
         AuditUtil.getOrGenerateRequestId(threadContext);
 
-        final Authentication authentication = SecurityTestsUtils.createApiKeyAuthentication(apiKeyService, createAuthentication(),
+        final Authentication authentication = createApiKeyAuthentication(apiKeyService, createAuthentication(),
                 Collections.singleton(new RoleDescriptor("user_role_" + randomAlphaOfLength(4), new String[]{"manage"}, null, null)),
                 Collections.singletonList(new RoleDescriptor("key_role_" + randomAlphaOfLength(8), new String[]{"monitor"}, null, null)));