[KEYCLOAK-11804] - Block service accounts to authenticate or manage credentials

This commit is contained in:
Pedro Igor 2020-02-19 14:27:15 -03:00 committed by Stian Thorgersen
parent 3fa8a5aa88
commit 49b1dbba68
4 changed files with 81 additions and 1 deletions

View file

@ -1061,6 +1061,7 @@ public class AuthenticationProcessor {
public void validateUser(UserModel authenticatedUser) { public void validateUser(UserModel authenticatedUser) {
if (authenticatedUser == null) return; if (authenticatedUser == null) return;
if (!authenticatedUser.isEnabled()) throw new AuthenticationFlowException(AuthenticationFlowError.USER_DISABLED); if (!authenticatedUser.isEnabled()) throw new AuthenticationFlowException(AuthenticationFlowError.USER_DISABLED);
if (authenticatedUser.getServiceAccountClientLink() != null) throw new AuthenticationFlowException(AuthenticationFlowError.UNKNOWN_USER);
} }
protected Response authenticationComplete() { protected Response authenticationComplete() {

View file

@ -60,16 +60,19 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
@Override @Override
public void updateCredential(RealmModel realm, UserModel user, CredentialModel cred) { public void updateCredential(RealmModel realm, UserModel user, CredentialModel cred) {
throwExceptionIfInvalidUser(user);
getStoreForUser(user).updateCredential(realm, user, cred); getStoreForUser(user).updateCredential(realm, user, cred);
} }
@Override @Override
public CredentialModel createCredential(RealmModel realm, UserModel user, CredentialModel cred) { public CredentialModel createCredential(RealmModel realm, UserModel user, CredentialModel cred) {
throwExceptionIfInvalidUser(user);
return getStoreForUser(user).createCredential(realm, user, cred); return getStoreForUser(user).createCredential(realm, user, cred);
} }
@Override @Override
public boolean removeStoredCredential(RealmModel realm, UserModel user, String id) { public boolean removeStoredCredential(RealmModel realm, UserModel user, String id) {
throwExceptionIfInvalidUser(user);
boolean removalResult = getStoreForUser(user).removeStoredCredential(realm, user, id); boolean removalResult = getStoreForUser(user).removeStoredCredential(realm, user, id);
session.userCache().evict(realm, user); session.userCache().evict(realm, user);
return removalResult; return removalResult;
@ -97,6 +100,7 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
@Override @Override
public boolean moveCredentialTo(RealmModel realm, UserModel user, String id, String newPreviousCredentialId){ public boolean moveCredentialTo(RealmModel realm, UserModel user, String id, String newPreviousCredentialId){
throwExceptionIfInvalidUser(user);
return getStoreForUser(user).moveCredentialTo(realm, user, id, newPreviousCredentialId); return getStoreForUser(user).moveCredentialTo(realm, user, id, newPreviousCredentialId);
} }
@ -107,6 +111,7 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
@Override @Override
public CredentialModel createCredentialThroughProvider(RealmModel realm, UserModel user, CredentialModel model){ public CredentialModel createCredentialThroughProvider(RealmModel realm, UserModel user, CredentialModel model){
throwExceptionIfInvalidUser(user);
List <CredentialProvider> credentialProviders = session.getKeycloakSessionFactory().getProviderFactories(CredentialProvider.class) List <CredentialProvider> credentialProviders = session.getKeycloakSessionFactory().getProviderFactories(CredentialProvider.class)
.stream() .stream()
.map(f -> session.getProvider(CredentialProvider.class, f.getId())) .map(f -> session.getProvider(CredentialProvider.class, f.getId()))
@ -121,6 +126,7 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
@Override @Override
public void updateCredentialLabel(RealmModel realm, UserModel user, String credentialId, String userLabel){ public void updateCredentialLabel(RealmModel realm, UserModel user, String credentialId, String userLabel){
throwExceptionIfInvalidUser(user);
CredentialModel credential = getStoredCredentialById(realm, user, credentialId); CredentialModel credential = getStoredCredentialById(realm, user, credentialId);
credential.setUserLabel(userLabel); credential.setUserLabel(userLabel);
getStoreForUser(user).updateCredential(realm, user, credential); getStoreForUser(user).updateCredential(realm, user, credential);
@ -132,7 +138,9 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
@Override @Override
public boolean isValid(RealmModel realm, UserModel user, List<CredentialInput> inputs) { public boolean isValid(RealmModel realm, UserModel user, List<CredentialInput> inputs) {
if (!isValid(user)) {
return false;
}
List<CredentialInput> toValidate = new LinkedList<>(); List<CredentialInput> toValidate = new LinkedList<>();
toValidate.addAll(inputs); toValidate.addAll(inputs);
if (!StorageId.isLocalStorage(user)) { if (!StorageId.isLocalStorage(user)) {
@ -203,6 +211,7 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
} }
} else { } else {
throwExceptionIfInvalidUser(user);
if (user.getFederationLink() != null) { if (user.getFederationLink() != null) {
UserStorageProvider provider = UserStorageManager.getStorageProvider(session, realm, user.getFederationLink()); UserStorageProvider provider = UserStorageManager.getStorageProvider(session, realm, user.getFederationLink());
if (provider instanceof CredentialInputUpdater) { if (provider instanceof CredentialInputUpdater) {
@ -234,6 +243,7 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
} }
} else { } else {
throwExceptionIfInvalidUser(user);
if (user.getFederationLink() != null) { if (user.getFederationLink() != null) {
UserStorageProvider provider = UserStorageManager.getStorageProvider(session, realm, user.getFederationLink()); UserStorageProvider provider = UserStorageManager.getStorageProvider(session, realm, user.getFederationLink());
if (provider instanceof CredentialInputUpdater) { if (provider instanceof CredentialInputUpdater) {
@ -384,4 +394,15 @@ public class UserCredentialStoreManager implements UserCredentialManager, OnUser
public void close() { public void close() {
} }
private boolean isValid(UserModel user) {
return user != null && user.getServiceAccountClientLink() == null;
}
private void throwExceptionIfInvalidUser(UserModel user) {
if (user == null || isValid(user)) {
return;
}
throw new RuntimeException("You can not manage credentials for this user");
}
} }

View file

@ -18,8 +18,10 @@ package org.keycloak.testsuite.forms;
import org.hamcrest.Matchers; import org.hamcrest.Matchers;
import org.jboss.arquillian.drone.api.annotation.Drone; import org.jboss.arquillian.drone.api.annotation.Drone;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.authentication.actiontoken.resetcred.ResetCredentialsActionToken; import org.keycloak.authentication.actiontoken.resetcred.ResetCredentialsActionToken;
import org.jboss.arquillian.graphene.page.Page; import org.jboss.arquillian.graphene.page.Page;
import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
import org.keycloak.events.EventType; import org.keycloak.events.EventType;
@ -44,6 +46,7 @@ import org.keycloak.testsuite.updaters.ClientAttributeUpdater;
import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.GreenMailRule;
import org.keycloak.testsuite.util.MailUtils; import org.keycloak.testsuite.util.MailUtils;
import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.OAuthClient;
import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.SecondBrowser; import org.keycloak.testsuite.util.SecondBrowser;
import org.keycloak.testsuite.util.UserActionTokenBuilder; import org.keycloak.testsuite.util.UserActionTokenBuilder;
import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserBuilder;
@ -82,6 +85,8 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
@Override @Override
public void configureTestRealm(RealmRepresentation testRealm) { public void configureTestRealm(RealmRepresentation testRealm) {
RealmBuilder.edit(testRealm)
.client(org.keycloak.testsuite.util.ClientBuilder.create().clientId("client-user").serviceAccount());
} }
@Before @Before
@ -1055,4 +1060,28 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest {
assertThat(driver2.getPageSource(), Matchers.containsString("Your account has been updated.")); assertThat(driver2.getPageSource(), Matchers.containsString("Your account has been updated."));
} }
@Test
public void failResetPasswordServiceAccount() {
String username = ServiceAccountConstants.SERVICE_ACCOUNT_USER_PREFIX + "client-user";
UserRepresentation serviceAccount = testRealm().users()
.search(username).get(0);
serviceAccount.toString();
UserResource serviceAccount1 = testRealm().users().get(serviceAccount.getId());
serviceAccount = serviceAccount1.toRepresentation();
serviceAccount.setEmail("client-user@test.com");
serviceAccount1.update(serviceAccount);
String resetUri = oauth.AUTH_SERVER_ROOT + "/realms/test/login-actions/reset-credentials";
driver.navigate().to(resetUri);
resetPasswordPage.assertCurrent();
resetPasswordPage.changePassword(username);
loginPage.assertCurrent();
assertEquals("Invalid username or password.", errorPage.getError());
}
} }

View file

@ -18,9 +18,12 @@
package org.keycloak.testsuite.oauth; package org.keycloak.testsuite.oauth;
import org.apache.http.HttpResponse; import org.apache.http.HttpResponse;
import org.hamcrest.Matchers;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.keycloak.admin.client.resource.UserResource;
import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator; import org.keycloak.authentication.authenticators.client.ClientIdAndSecretAuthenticator;
import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.crypto.Algorithm; import org.keycloak.crypto.Algorithm;
@ -32,6 +35,7 @@ import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.representations.AccessToken; import org.keycloak.representations.AccessToken;
import org.keycloak.representations.RefreshToken; import org.keycloak.representations.RefreshToken;
import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.CredentialRepresentation;
import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.AbstractKeycloakTest;
@ -44,11 +48,14 @@ import org.keycloak.testsuite.util.RealmBuilder;
import org.keycloak.testsuite.util.TokenSignatureUtil; import org.keycloak.testsuite.util.TokenSignatureUtil;
import org.keycloak.testsuite.util.UserBuilder; import org.keycloak.testsuite.util.UserBuilder;
import java.util.Arrays;
import java.util.List; import java.util.List;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import javax.ws.rs.ClientErrorException;
/** /**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a> * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/ */
@ -60,6 +67,9 @@ public class ServiceAccountTest extends AbstractKeycloakTest {
@Rule @Rule
public AssertEvents events = new AssertEvents(this); public AssertEvents events = new AssertEvents(this);
@Rule
public ExpectedException expectedException = ExpectedException.none();
@Override @Override
public void beforeAbstractKeycloakTest() throws Exception { public void beforeAbstractKeycloakTest() throws Exception {
@ -281,6 +291,25 @@ public class ServiceAccountTest extends AbstractKeycloakTest {
conductClientCredentialsAuthRequest(Algorithm.HS256, Algorithm.ES256, Algorithm.PS256); conductClientCredentialsAuthRequest(Algorithm.HS256, Algorithm.ES256, Algorithm.PS256);
} }
@Test
public void failManagePassword() {
UserResource serviceAccount = adminClient.realm("test").users().get(userId);
UserRepresentation representation = serviceAccount.toRepresentation();
CredentialRepresentation password = new CredentialRepresentation();
password.setValue("password");
password.setType(CredentialRepresentation.PASSWORD);
password.setTemporary(false);
representation.setCredentials(Arrays.asList(password));
this.expectedException.expect(Matchers.allOf(Matchers.instanceOf(ClientErrorException.class),
Matchers.hasProperty("response", Matchers.hasProperty("status", Matchers.is(409)))));
this.expectedException.reportMissingExceptionWithMessage("Should fail, should not be possible to manage credentials for service accounts");
serviceAccount.update(representation);
}
private void conductClientCredentialsAuthRequest(String expectedRefreshAlg, String expectedAccessAlg, String realmTokenAlg) throws Exception { private void conductClientCredentialsAuthRequest(String expectedRefreshAlg, String expectedAccessAlg, String realmTokenAlg) throws Exception {
try { try {
/// Realm Setting is used for ID Token Signature Algorithm /// Realm Setting is used for ID Token Signature Algorithm