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 801bcb400d5..6113cf4dfd3 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 @@ -539,9 +539,12 @@ public class RBACEngine implements AuthorizationEngine { } assert realmType != null; - // ensure 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 - return ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType); + // 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) + && (ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType))); } static class RBACAuthorizationInfo implements AuthorizationInfo { 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 1fedbffc76e..42092a60d77 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 @@ -102,6 +102,7 @@ public class RBACEngineTests extends ESTestCase { final String action = changePasswordRequest ? ChangePasswordAction.NAME : AuthenticateAction.NAME; final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM); when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authenticatedBy.getType()) @@ -125,9 +126,10 @@ 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(changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealmSettings.TYPE) : - randomAlphaOfLengthBetween(4, 12)); + final String authenticationType = changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealmSettings.TYPE) : + randomAlphaOfLengthBetween(4, 12); + when(authenticatedBy.getType()).thenReturn(authenticationType); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM); assertThat(request, instanceOf(UserRequest.class)); assertFalse(engine.checkSameUserPermissions(action, request, authentication)); @@ -180,6 +182,7 @@ public class RBACEngineTests extends ESTestCase { final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); final Authentication.RealmRef lookedUpBy = mock(Authentication.RealmRef.class); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM); when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getLookedUpBy()).thenReturn(lookedUpBy); @@ -198,6 +201,7 @@ public class RBACEngineTests extends ESTestCase { final String action = ChangePasswordAction.NAME; final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM); when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authenticatedBy.getType()).thenReturn(randomFrom(LdapRealmSettings.LDAP_TYPE, FileRealmSettings.TYPE, @@ -209,6 +213,47 @@ public class RBACEngineTests extends ESTestCase { verify(authenticatedBy).getType(); verify(authentication).getAuthenticatedBy(); verify(authentication, times(2)).getUser(); + verify(authentication).getAuthenticationType(); + verifyNoMoreInteractions(authenticatedBy, authentication); + } + + public void testSameUserPermissionDoesNotAllowChangePasswordForApiKey() { + final User user = new User("joe"); + final ChangePasswordRequest request = new ChangePasswordRequestBuilder(mock(Client.class)).username(user.principal()).request(); + final String action = ChangePasswordAction.NAME; + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.API_KEY); + when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE); + + assertThat(request, instanceOf(UserRequest.class)); + assertFalse(engine.checkSameUserPermissions(action, request, authentication)); + verify(authenticatedBy).getType(); + verify(authentication).getAuthenticatedBy(); + verify(authentication, times(2)).getUser(); + verify(authentication).getAuthenticationType(); + verifyNoMoreInteractions(authenticatedBy, authentication); + } + + public void testSameUserPermissionDoesNotAllowChangePasswordForAccessToken() { + final User user = new User("joe"); + final ChangePasswordRequest request = new ChangePasswordRequestBuilder(mock(Client.class)).username(user.principal()).request(); + final String action = ChangePasswordAction.NAME; + final Authentication authentication = mock(Authentication.class); + final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); + when(authentication.getUser()).thenReturn(user); + when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.TOKEN); + when(authenticatedBy.getType()).thenReturn(NativeRealmSettings.TYPE); + + assertThat(request, instanceOf(UserRequest.class)); + assertFalse(engine.checkSameUserPermissions(action, request, authentication)); + verify(authenticatedBy).getType(); + verify(authentication).getAuthenticatedBy(); + verify(authentication, times(2)).getUser(); + verify(authentication).getAuthenticationType(); verifyNoMoreInteractions(authenticatedBy, authentication); } @@ -220,6 +265,7 @@ public class RBACEngineTests extends ESTestCase { final Authentication authentication = mock(Authentication.class); final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class); final Authentication.RealmRef lookedUpBy = mock(Authentication.RealmRef.class); + when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM); when(authentication.getUser()).thenReturn(user); when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy); when(authentication.getLookedUpBy()).thenReturn(lookedUpBy); @@ -232,6 +278,7 @@ public class RBACEngineTests extends ESTestCase { verify(authentication).getLookedUpBy(); verify(authentication, times(2)).getUser(); verify(lookedUpBy).getType(); + verify(authentication).getAuthenticationType(); verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/change_password/11_token.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/change_password/11_token.yml new file mode 100644 index 00000000000..fcbbb1c257b --- /dev/null +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/change_password/11_token.yml @@ -0,0 +1,67 @@ +--- +setup: + - skip: + features: headers + - do: + cluster.health: + wait_for_status: yellow + - do: + security.put_user: + username: "token_joe" + body: > + { + "password": "s3krit", + "roles" : [ "token_admin" ] + } + - do: + security.put_role: + name: "token_admin" + body: > + { + "cluster": ["manage_token"], + "indices": [ + { + "names": "*", + "privileges": ["all"] + } + ] + } +--- +teardown: + - do: + security.delete_user: + username: "token_joe" + ignore: 404 + - do: + security.delete_role: + name: "token_admin" + ignore: 404 + +--- +"Test user changing their password authenticating with token not allowed": + + - do: + headers: + Authorization: "Basic dG9rZW5fam9lOnMza3JpdA==" + security.get_token: + body: + grant_type: "password" + username: "token_joe" + password: "s3krit" + + - match: { type: "Bearer" } + - is_true: access_token + - set: { access_token: token } + - match: { expires_in: 1200 } + - is_false: scope + + - do: + headers: + Authorization: Bearer ${token} + catch: forbidden + security.change_password: + username: "joe" + body: > + { + "password" : "s3krit2" + }