Check authentication type using enum instead of string (#54145) (#54246)

Avoid string comparison when we can use safer enums.
This refactor is a follow up for #52178.

Resolves: #52511
This commit is contained in:
Yang Wang 2020-03-26 15:45:10 +11:00 committed by GitHub
parent 6af89e62d1
commit 1afd510721
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 35 additions and 21 deletions

View File

@ -82,6 +82,10 @@ public class Authentication implements ToXContentObject {
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;
}

View File

@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.security.action.CreateApiKeyRequest;
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 @@ import org.elasticsearch.xpack.core.security.support.Automatons;
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 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
* 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 @@ public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
}
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)) {

View File

@ -13,6 +13,7 @@ import org.elasticsearch.transport.TransportRequest;
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;
@ -29,8 +30,8 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build();
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key",
Collections.singletonMap("_security_api_key_id", apiKeyId));
final Authentication authentication = createMockAuthentication("joe","_es_api_key",
AuthenticationType.API_KEY, Collections.singletonMap("_security_api_key_id", apiKeyId));
final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
@ -44,8 +45,8 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build();
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final Authentication authentication = createMockAuthentication("joe","_es_api_key", "_es_api_key",
Collections.singletonMap("_security_api_key_id", randomAlphaOfLength(7)));
final Authentication authentication = createMockAuthentication("joe","_es_api_key",
AuthenticationType.API_KEY, Collections.singletonMap("_security_api_key_id", randomAlphaOfLength(7)));
final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean());
@ -57,7 +58,8 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
final ClusterPermission clusterPermission =
ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build();
final Authentication authentication = createMockAuthentication("joe","realm1", "native", Collections.emptyMap());
final Authentication authentication = createMockAuthentication("joe","realm1",
AuthenticationType.REALM, Collections.emptyMap());
final TransportRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("realm1", "joe");
final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.usingRealmAndUserName("realm1", "joe");
@ -70,7 +72,8 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
final ClusterPermission clusterPermission =
ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build();
final Authentication authentication = createMockAuthentication("joe","realm1", "native", Collections.emptyMap());
final Authentication authentication = createMockAuthentication("joe","realm1",
AuthenticationType.REALM, Collections.emptyMap());
final TransportRequest getApiKeyRequest = GetApiKeyRequest.forOwnedApiKeys();
final TransportRequest invalidateApiKeyRequest = InvalidateApiKeyRequest.forOwnedApiKeys();
@ -83,7 +86,8 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
final ClusterPermission clusterPermission =
ManageOwnApiKeyClusterPrivilege.INSTANCE.buildPermission(ClusterPermission.builder()).build();
final Authentication authentication = createMockAuthentication("joe", "realm1", "native", Collections.emptyMap());
final Authentication authentication = createMockAuthentication("joe", "realm1",
AuthenticationType.REALM, Collections.emptyMap());
final TransportRequest getApiKeyRequest = randomFrom(
GetApiKeyRequest.usingRealmAndUserName("realm1", randomAlphaOfLength(7)),
GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), "joe"),
@ -111,14 +115,15 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
InvalidateApiKeyRequest.usingRealmAndUserName("realm_b", "user_b"), authentication));
}
private Authentication createMockAuthentication(String username, String realmName, String realmType, Map<String, Object> metadata) {
private Authentication createMockAuthentication(String username, String realmName,
AuthenticationType authenticationType, Map<String, Object> 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;
}
@ -137,6 +142,7 @@ public class ManageOwnApiKeyClusterPrivilegeTests extends ESTestCase {
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
when(authentication.getSourceRealm()).thenReturn(lookedUpBy);
when(authentication.getMetadata()).thenReturn(Collections.emptyMap());
when(authentication.getAuthenticationType()).thenReturn(AuthenticationType.REALM);
return authentication;
}
}

View File

@ -104,6 +104,7 @@ import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
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 {
@ -351,7 +352,7 @@ public class ApiKeyService {
* retrieval of role descriptors that are associated with the api key
*/
public void getRoleForApiKey(Authentication authentication, ActionListener<ApiKeyRoleDescriptors> 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());
}
@ -902,7 +903,7 @@ public class ApiKeyService {
* @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();
@ -917,7 +918,7 @@ public class ApiKeyService {
* @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();

View File

@ -42,6 +42,7 @@ import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRespon
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 @@ public class AuthorizationService {
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());

View File

@ -47,6 +47,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.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 @@ public class RBACEngine implements AuthorizationEngine {
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 @@ public class RBACEngine implements AuthorizationEngine {
// 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)));
}

View File

@ -36,6 +36,7 @@ import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse;
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;
@ -290,7 +291,7 @@ public class RBACEngineTests extends ESTestCase {
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(Collections.singletonMap(ApiKeyService.API_KEY_ID_KEY, apiKeyId));
assertTrue(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication));
@ -321,7 +322,7 @@ public class RBACEngineTests extends ESTestCase {
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(
Collections.singletonMap(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7)));