Disallow Password Change when authenticated by Token (#49694) (#53614)

Password changes are only allowed when the user is currently
authenticated by a realm (that permits the password to be changed)
and not when authenticated by a bearer token or an API key.
This commit is contained in:
Ioannis Kakavas 2020-03-17 09:45:35 +02:00 committed by GitHub
parent 7f21ade924
commit 23af171cf8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 123 additions and 6 deletions

View File

@ -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 {

View File

@ -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);
}

View File

@ -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"
}