From fa899a0379ee16026a798722c14412b2b564a69a Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Wed, 25 Mar 2020 14:05:38 +1100 Subject: [PATCH] Check authentication type using enum instead of string --- .../core/security/authc/Authentication.java | 4 ++++ .../ManageOwnApiKeyClusterPrivilege.java | 6 ++--- .../ManageOwnApiKeyClusterPrivilegeTests.java | 24 ++++++++++++------- .../xpack/security/authc/ApiKeyService.java | 7 +++--- .../security/authz/AuthorizationService.java | 3 ++- .../xpack/security/authz/RBACEngine.java | 7 +++--- .../xpack/security/authz/RBACEngineTests.java | 5 ++-- 7 files changed, 35 insertions(+), 21 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index 3f1e49098ff5e..4aa850e1aa60d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -77,6 +77,10 @@ public RealmRef getLookedUpBy() { return lookedUpBy; } + /** + * Get the realm where the effective user comes from. + * The effective user is the es-security-runas-user if present or the authenticated user. + */ public RealmRef getSourceRealm() { return lookedUpBy == null ? authenticatedBy : lookedUpBy; } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java index b1e6c117a04be..51ae560be1d3b 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java @@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest; import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.support.Automatons; @@ -23,7 +24,6 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege { public static final ManageOwnApiKeyClusterPrivilege INSTANCE = new ManageOwnApiKeyClusterPrivilege(); private static final String PRIVILEGE_NAME = "manage_own_api_key"; - private static final String API_KEY_REALM_TYPE = "_es_api_key"; private static final String API_KEY_ID_KEY = "_security_api_key_id"; private ManageOwnApiKeyClusterPrivilege() { @@ -78,7 +78,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin * TODO bizybot we need to think on how we can propagate appropriate error message to the end user when username, realm name * is missing. This is similar to the problem of propagating right error messages in case of access denied. */ - if (authentication.getSourceRealm().getType().equals(API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { // API key cannot own any other API key so deny access return false; } else if (ownedByAuthenticatedUser) { @@ -93,7 +93,7 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin } private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) { - if (authentication.getSourceRealm().getType().equals(API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { // API key id from authentication must match the id from request final String authenticatedApiKeyId = (String) authentication.getMetadata().get(API_KEY_ID_KEY); if (Strings.hasText(apiKeyId)) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java index 0f043035b79c6..64b040e247b2a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilegeTests.java @@ -13,6 +13,7 @@ import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest; import org.elasticsearch.xpack.core.security.action.InvalidateApiKeyRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authz.permission.ClusterPermission; import org.elasticsearch.xpack.core.security.user.User; @@ -28,8 +29,8 @@ public void testAuthenticationWithApiKeyAllowsAccessToApiKeyActionsWhenItIsOwner ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key", - Map.of("_security_api_key_id", apiKeyId)); + final Authentication authentication = createMockAuthentication("joe","_es_api_key", + AuthenticationType.API_KEY, Map.of("_security_api_key_id", apiKeyId)); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); @@ -43,8 +44,8 @@ public void testAuthenticationWithApiKeyDeniesAccessToApiKeyActionsWhenItIsNotOw ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); final String apiKeyId = randomAlphaOfLengthBetween(4, 7); - final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key", - Map.of("_security_api_key_id", randomAlphaOfLength(7))); + final Authentication authentication = createMockAuthentication("joe","_es_api_key", + AuthenticationType.API_KEY, Map.of("_security_api_key_id", randomAlphaOfLength(7))); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean()); @@ -56,7 +57,8 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner() final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); + final Authentication authentication = createMockAuthentication("joe","realm1", + AuthenticationType.REALM, Map.of()); final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe"); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe"); @@ -69,7 +71,8 @@ public void testAuthenticationWithUserAllowsAccessToApiKeyActionsWhenItIsOwner_W final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - final Authentication authentication = createMockAuthentication("joe","realm1", "native", Map.of()); + final Authentication authentication = createMockAuthentication("joe","realm1", + AuthenticationType.REALM, Map.of()); final TransportRequest getApiKeyRequest = GetApiKeyRequest.forOwnedApiKeys(); final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.forOwnedApiKeys(); @@ -82,7 +85,8 @@ public void testAuthenticationWithUserDeniesAccessToApiKeyActionsWhenItIsNotOwne final ClusterPermission clusterPermission = ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build(); - final Authentication authentication = createMockAuthentication("joe", "realm1", "native", Map.of()); + final Authentication authentication = createMockAuthentication("joe", "realm1", + AuthenticationType.REALM, Map.of()); final TransportRequest getApiKeyRequest = randomFrom( GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)), GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"), @@ -110,14 +114,15 @@ public void testGetAndInvalidateApiKeyWillRespectRunAsUser() { InvalidateApiKeyRequest.usingRealmAndUserName("realm_b", "user_b"), authentication)); } - private Authentication createMockAuthentication(String username, String realmName, String realmType, Map metadata) { + private Authentication createMockAuthentication(String username, String realmName, + AuthenticationType authenticationType, Map metadata) { final User user = new User(username); final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); when(authentication.getUser()).thenReturn(user); when(authentication.getSourceRealm()).thenReturn(authenticatedBy); + when(authentication.getAuthenticationType()).thenReturn(authenticationType); when(authenticatedBy.getName()).thenReturn(realmName); - when(authenticatedBy.getType()).thenReturn(realmType); when(authentication.getMetadata()).thenReturn(metadata); return authentication; } @@ -136,6 +141,7 @@ private Authentication createMockRunAsAuthentication(String username, String rea when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getSourceRealm()).thenReturn(lookedUpBy); when(authentication.getMetadata()).thenReturn(Map.of()); + when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.REALM); return authentication; } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index c363ad1253509..6ac7f03595ec7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -102,6 +102,7 @@ import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING; import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.executeAsyncWithOrigin; +import static org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS; public class ApiKeyService { @@ -343,7 +344,7 @@ private void loadApiKeyAndValidateCredentials(ThreadContext ctx, ApiKeyCredentia * retrieval of role descriptors that are associated with the api key */ public void getRoleForApiKey(Authentication authentication, ActionListener listener) { - if (authentication.getAuthenticationType() != Authentication.AuthenticationType.API_KEY) { + if (authentication.getAuthenticationType() != AuthenticationType.API_KEY) { throw new IllegalStateException("authentication type must be api key but is " + authentication.getAuthenticationType()); } @@ -894,7 +895,7 @@ public void getApiKeys(String realmName, String username, String apiKeyName, Str * @return realm name */ public static String getCreatorRealmName(final Authentication authentication) { - if (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { return (String) authentication.getMetadata().get(API_KEY_CREATOR_REALM_NAME); } else { return authentication.getSourceRealm().getName(); @@ -909,7 +910,7 @@ public static String getCreatorRealmName(final Authentication authentication) { * @return realm type */ public static String getCreatorRealmType(final Authentication authentication) { - if (authentication.getAuthenticatedBy().getType().equals(API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { return (String) authentication.getMetadata().get(API_KEY_CREATOR_REALM_TYPE); } else { return authentication.getSourceRealm().getType(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 67d88efc0bc1d..783f31f8ce855 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -42,6 +42,7 @@ import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authc.AuthenticationFailureHandler; import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm; import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine; @@ -585,7 +586,7 @@ private ElasticsearchSecurityException denialException(Authentication authentica authentication.getUser().principal()); } // check for authentication by API key - if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { final String apiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY); assert apiKeyId != null : "api key id must be present in the metadata"; logger.debug("action [{}] is unauthorized for API key id [{}] of user [{}]", action, apiKeyId, authUser.principal()); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java index 6113cf4dfd305..38091ffe94fc6 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java @@ -47,6 +47,7 @@ import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; import org.elasticsearch.xpack.core.security.action.user.UserRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings; import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine; import org.elasticsearch.xpack.core.security.authz.ResolvedIndices; @@ -179,7 +180,7 @@ boolean checkSameUserPermissions(String action, TransportRequest request, Authen return sameUsername; } else if (request instanceof GetApiKeyRequest) { GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request; - if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) { + if (AuthenticationType.API_KEY == authentication.getAuthenticationType()) { assert authentication.getLookedUpBy() == null : "runAs not supported for api key authentication"; // if authenticated by API key then the request must also contain same API key id String authenticatedApiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY); @@ -542,8 +543,8 @@ private static boolean checkChangePasswordAction(Authentication authentication) // Ensure that the user is not authenticated with an access token or an API key. // Also ensure that the user was authenticated by a realm that we can change a password for. The native realm is an internal realm // and right now only one can exist in the realm configuration - if this changes we should update this check - final Authentication.AuthenticationType authType = authentication.getAuthenticationType(); - return (authType.equals(Authentication.AuthenticationType.REALM) + final AuthenticationType authType = authentication.getAuthenticationType(); + return (authType.equals(AuthenticationType.REALM) && (ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType))); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java index 3066ee2732a5a..dd160f2c8e6ec 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java @@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.security.action.user.PutUserAction; import org.elasticsearch.xpack.core.security.action.user.UserRequest; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; 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.ldap.LdapRealmSettings; @@ -291,7 +292,7 @@ public void testSameUserPermissionAllowsSelfApiKeyInfoRetrievalWhenAuthenticated final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); - when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.API_KEY); when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, apiKeyId)); assertTrue(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication)); @@ -321,7 +322,7 @@ public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenLookedupByIsPrese when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getLookedUpBy()).thenReturn(lookedupBy); - when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.API_KEY); when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7))); final AssertionError assertionError = expectThrows(AssertionError.class, () -> engine.checkSameUserPermissions(GetApiKeyAction.NAME,