Skip to content

Commit 6543e18

Browse files
jkakavasjkakavas
authored
Disallow Password Change when authenticated by Token (#49694)
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.
1 parent 06cec76 commit 6543e18

File tree

3 files changed

+123
-6
lines changed

3 files changed

+123
-6
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -524,9 +524,12 @@ private static boolean checkChangePasswordAction(Authentication authentication)
524524
}
525525

526526
assert realmType != null;
527-
// ensure the user was authenticated by a realm that we can change a password for. The native realm is an internal realm and
528-
// right now only one can exist in the realm configuration - if this changes we should update this check
529-
return ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType);
527+
// Ensure that the user is not authenticated with an access token or an API key.
528+
// Also ensure that the user was authenticated by a realm that we can change a password for. The native realm is an internal realm
529+
// and right now only one can exist in the realm configuration - if this changes we should update this check
530+
final Authentication.AuthenticationType authType = authentication.getAuthenticationType();
531+
return (authType.equals(Authentication.AuthenticationType.REALM)
532+
&& (ReservedRealm.TYPE.equals(realmType) || NativeRealmSettings.TYPE.equals(realmType)));
530533
}
531534

532535
static class RBACAuthorizationInfo implements AuthorizationInfo {

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ public void testSameUserPermission() {
103103
final String action = changePasswordRequest ? ChangePasswordAction.NAME : AuthenticateAction.NAME;
104104
final Authentication authentication = mock(Authentication.class);
105105
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
106+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM);
106107
when(authentication.getUser()).thenReturn(user);
107108
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
108109
when(authenticatedBy.getType())
@@ -126,9 +127,10 @@ public void testSameUserPermissionDoesNotAllowNonMatchingUsername() {
126127
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
127128
when(authentication.getUser()).thenReturn(user);
128129
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
129-
when(authenticatedBy.getType())
130-
.thenReturn(changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealmSettings.TYPE) :
131-
randomAlphaOfLengthBetween(4, 12));
130+
final String authenticationType = changePasswordRequest ? randomFrom(ReservedRealm.TYPE, NativeRealmSettings.TYPE) :
131+
randomAlphaOfLengthBetween(4, 12);
132+
when(authenticatedBy.getType()).thenReturn(authenticationType);
133+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM);
132134

133135
assertThat(request, instanceOf(UserRequest.class));
134136
assertFalse(engine.checkSameUserPermissions(action, request, authentication));
@@ -181,6 +183,7 @@ public void testSameUserPermissionRunAsChecksAuthenticatedBy() {
181183
final Authentication authentication = mock(Authentication.class);
182184
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
183185
final Authentication.RealmRef lookedUpBy = mock(Authentication.RealmRef.class);
186+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM);
184187
when(authentication.getUser()).thenReturn(user);
185188
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
186189
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
@@ -199,6 +202,7 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForOtherRealms() {
199202
final String action = ChangePasswordAction.NAME;
200203
final Authentication authentication = mock(Authentication.class);
201204
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
205+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM);
202206
when(authentication.getUser()).thenReturn(user);
203207
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
204208
when(authenticatedBy.getType()).thenReturn(randomFrom(LdapRealmSettings.LDAP_TYPE, FileRealmSettings.TYPE,
@@ -210,6 +214,47 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForOtherRealms() {
210214
verify(authenticatedBy).getType();
211215
verify(authentication).getAuthenticatedBy();
212216
verify(authentication, times(2)).getUser();
217+
verify(authentication).getAuthenticationType();
218+
verifyNoMoreInteractions(authenticatedBy, authentication);
219+
}
220+
221+
public void testSameUserPermissionDoesNotAllowChangePasswordForApiKey() {
222+
final User user = new User("joe");
223+
final ChangePasswordRequest request = new ChangePasswordRequestBuilder(mock(Client.class)).username(user.principal()).request();
224+
final String action = ChangePasswordAction.NAME;
225+
final Authentication authentication = mock(Authentication.class);
226+
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
227+
when(authentication.getUser()).thenReturn(user);
228+
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
229+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.API_KEY);
230+
when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE);
231+
232+
assertThat(request, instanceOf(UserRequest.class));
233+
assertFalse(engine.checkSameUserPermissions(action, request, authentication));
234+
verify(authenticatedBy).getType();
235+
verify(authentication).getAuthenticatedBy();
236+
verify(authentication, times(2)).getUser();
237+
verify(authentication).getAuthenticationType();
238+
verifyNoMoreInteractions(authenticatedBy, authentication);
239+
}
240+
241+
public void testSameUserPermissionDoesNotAllowChangePasswordForAccessToken() {
242+
final User user = new User("joe");
243+
final ChangePasswordRequest request = new ChangePasswordRequestBuilder(mock(Client.class)).username(user.principal()).request();
244+
final String action = ChangePasswordAction.NAME;
245+
final Authentication authentication = mock(Authentication.class);
246+
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
247+
when(authentication.getUser()).thenReturn(user);
248+
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
249+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.TOKEN);
250+
when(authenticatedBy.getType()).thenReturn(NativeRealmSettings.TYPE);
251+
252+
assertThat(request, instanceOf(UserRequest.class));
253+
assertFalse(engine.checkSameUserPermissions(action, request, authentication));
254+
verify(authenticatedBy).getType();
255+
verify(authentication).getAuthenticatedBy();
256+
verify(authentication, times(2)).getUser();
257+
verify(authentication).getAuthenticationType();
213258
verifyNoMoreInteractions(authenticatedBy, authentication);
214259
}
215260

@@ -221,6 +266,7 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForLookedUpByOtherRe
221266
final Authentication authentication = mock(Authentication.class);
222267
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
223268
final Authentication.RealmRef lookedUpBy = mock(Authentication.RealmRef.class);
269+
when(authentication.getAuthenticationType()).thenReturn(Authentication.AuthenticationType.REALM);
224270
when(authentication.getUser()).thenReturn(user);
225271
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
226272
when(authentication.getLookedUpBy()).thenReturn(lookedUpBy);
@@ -233,6 +279,7 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForLookedUpByOtherRe
233279
verify(authentication).getLookedUpBy();
234280
verify(authentication, times(2)).getUser();
235281
verify(lookedUpBy).getType();
282+
verify(authentication).getAuthenticationType();
236283
verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy);
237284
}
238285

‎x-pack/plugin/src/test/resources/rest-api-spec/test/change_password/11_token.yml‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
---
2+
setup:
3+
- skip:
4+
features: headers
5+
- do:
6+
cluster.health:
7+
wait_for_status: yellow
8+
- do:
9+
security.put_user:
10+
username: "token_joe"
11+
body: >
12+
{
13+
"password": "s3krit",
14+
"roles" : [ "token_admin" ]
15+
}
16+
- do:
17+
security.put_role:
18+
name: "token_admin"
19+
body: >
20+
{
21+
"cluster": ["manage_token"],
22+
"indices": [
23+
{
24+
"names": "*",
25+
"privileges": ["all"]
26+
}
27+
]
28+
}
29+
---
30+
teardown:
31+
- do:
32+
security.delete_user:
33+
username: "token_joe"
34+
ignore: 404
35+
- do:
36+
security.delete_role:
37+
name: "token_admin"
38+
ignore: 404
39+
40+
---
41+
"Test user changing their password authenticating with token not allowed":
42+
43+
- do:
44+
headers:
45+
Authorization: "Basic dG9rZW5fam9lOnMza3JpdA=="
46+
security.get_token:
47+
body:
48+
grant_type: "password"
49+
username: "token_joe"
50+
password: "s3krit"
51+
52+
- match: { type: "Bearer" }
53+
- is_true: access_token
54+
- set: { access_token: token }
55+
- match: { expires_in: 1200 }
56+
- is_false: scope
57+
58+
- do:
59+
headers:
60+
Authorization: Bearer ${token}
61+
catch: forbidden
62+
security.change_password:
63+
username: "joe"
64+
body: >
65+
{
66+
"password" : "s3krit2"
67+
}

0 commit comments

Comments
 (0)