From 154bce5693b8f3589ece7aa5880755705afc511b Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Mon, 3 Feb 2020 19:23:30 +0100 Subject: [PATCH] =?UTF-8?q?KEYCLOAK-12340=20KEYCLOAK-12386=20Regression=20?= =?UTF-8?q?in=20credential=20handling=20when=20=E2=80=A6=20(#6668)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../admin/client/resource/UserResource.java | 12 + .../models/UserCredentialManager.java | 8 + .../browser/OTPFormAuthenticator.java | 10 +- .../requiredactions/UpdateTotp.java | 10 +- .../credential/OTPCredentialProvider.java | 5 + .../UserCredentialStoreManager.java | 40 +++- .../resources/admin/UserResource.java | 25 +- .../DummyUserFederationProvider.java | 19 +- .../federation/ldap/AbstractLDAPTest.java | 4 + .../ldap/LDAPProvidersIntegrationTest.java | 97 +++++++- .../storage/UserStorageOTPTest.java | 222 ++++++++++++++++++ .../messages/admin-messages_en.properties | 4 + .../admin/resources/js/controllers/users.js | 96 +++++--- .../theme/base/admin/resources/js/services.js | 5 + .../resources/partials/user-credentials.html | 32 ++- 15 files changed, 533 insertions(+), 56 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageOTPTest.java diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java index 405c7e5127..131a4387ba 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java @@ -104,6 +104,18 @@ public interface UserResource { @Produces(MediaType.APPLICATION_JSON) List credentials(); + + /** + * Return credential types, which are provided by the user storage where user is stored. Returned values can contain for example "password", "otp" etc. + * This will always return empty list for "local" users, which are not backed by any user storage + * + * @return + */ + @GET + @Path("configured-user-storage-credential-types") + @Produces(MediaType.APPLICATION_JSON) + List getConfiguredUserStorageCredentialTypes(); + /** * Remove a credential for a user * diff --git a/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java b/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java index 84be238865..5fd98042eb 100644 --- a/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java +++ b/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java @@ -133,4 +133,12 @@ public interface UserCredentialManager extends UserCredentialStore { * @return */ CredentialValidationOutput authenticate(KeycloakSession session, RealmModel realm, CredentialInput input); + + /** + * Return credential types, which are provided by the user storage where user is stored. Returned values can contain for example "password", "otp" etc. + * This will always return empty list for "local" users, which are not backed by any user storage + * + * @return + */ + List getConfiguredUserStorageCredentialTypes(RealmModel realm, UserModel user); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java index 8426103a81..f64817a5b1 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java @@ -33,6 +33,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; +import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.services.messages.Messages; import javax.ws.rs.core.MultivaluedMap; @@ -74,8 +75,9 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl String credentialId = inputData.getFirst("selectedCredentialId"); if (credentialId == null || credentialId.isEmpty()) { - credentialId = getCredentialProvider(context.getSession()) - .getDefaultCredential(context.getSession(), context.getRealm(), context.getUser()).getId(); + OTPCredentialModel defaultOtpCredential = getCredentialProvider(context.getSession()) + .getDefaultCredential(context.getSession(), context.getRealm(), context.getUser()); + credentialId = defaultOtpCredential==null ? "" : defaultOtpCredential.getId(); } context.form().setAttribute(SELECTED_OTP_CREDENTIAL_ID, credentialId); @@ -90,7 +92,7 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl context.challenge(challengeResponse); return; } - boolean valid = getCredentialProvider(context.getSession()).isValid(context.getRealm(),context.getUser(), + boolean valid = context.getSession().userCredentialManager().isValid(context.getRealm(),context.getUser(), new UserCredentialModel(credentialId, getCredentialProvider(context.getSession()).getType(), otp)); if (!valid) { context.getEvent().user(userModel) @@ -119,7 +121,7 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl @Override public boolean configuredFor(KeycloakSession session, RealmModel realm, UserModel user) { - return getCredentialProvider(session).isConfiguredFor(realm, user); + return session.userCredentialManager().isConfiguredFor(realm, user, getCredentialProvider(session).getType()); } @Override diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java index 14d4b1ad21..837279d0ad 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java @@ -79,7 +79,7 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory .createResponse(UserModel.RequiredAction.CONFIGURE_TOTP); context.challenge(challenge); return; - } else if (!CredentialValidation.validOTP(challengeResponse, credentialModel, policy.getLookAheadWindow())) { + } else if (!validateOTPCredential(context, challengeResponse, credentialModel, policy)) { Response challenge = context.form() .setAttribute("mode", mode) .setError(Messages.INVALID_TOTP) @@ -91,7 +91,7 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory CredentialModel createdCredential = otpCredentialProvider.createCredential(context.getRealm(), context.getUser(), credentialModel); UserCredentialModel credential = new UserCredentialModel(createdCredential.getId(), otpCredentialProvider.getType(), challengeResponse); //If the type is HOTP, call verify once to consume the OTP used for registration and increase the counter. - if (OTPCredentialModel.HOTP.equals(credentialModel.getOTPCredentialData().getSubType()) && !otpCredentialProvider.isValid(context.getRealm(), context.getUser(), credential)) { + if (OTPCredentialModel.HOTP.equals(credentialModel.getOTPCredentialData().getSubType()) && !context.getSession().userCredentialManager().isValid(context.getRealm(), context.getUser(), credential)) { Response challenge = context.form() .setAttribute("mode", mode) .setError(Messages.INVALID_TOTP) @@ -103,6 +103,12 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory } + // Use separate method, so it's possible to override in the custom provider + protected boolean validateOTPCredential(RequiredActionContext context, String token, OTPCredentialModel credentialModel, OTPPolicy policy) { + return CredentialValidation.validOTP(token, credentialModel, policy.getLookAheadWindow()); + } + + @Override public void close() { diff --git a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java index 19c9d6ef4c..d65ba0c324 100644 --- a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java @@ -17,6 +17,7 @@ package org.keycloak.credential; import org.jboss.logging.Logger; +import org.keycloak.common.util.ObjectUtil; import org.keycloak.common.util.Time; import org.keycloak.models.RequiredActionProviderModel; import org.keycloak.models.credential.OTPCredentialModel; @@ -109,6 +110,10 @@ public class OTPCredentialProvider implements CredentialProvider getConfiguredUserStorageCredentialTypes(RealmModel realm, UserModel user) { + List credentialProviders = getCredentialProviders(session, realm, CredentialProvider.class); + + return credentialProviders.stream().map(CredentialProvider::getType) + .filter(credentialType -> UserStorageCredentialConfigured.CONFIGURED == isConfiguredThroughUserStorage(realm, user, credentialType)) + .collect(Collectors.toList()); + } + @Override public void close() { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java index 80f4e523e0..4df415fa62 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UserResource.java @@ -27,8 +27,6 @@ import org.keycloak.common.ClientConnection; import org.keycloak.common.Profile; import org.keycloak.common.util.Time; import org.keycloak.credential.CredentialModel; -import org.keycloak.credential.CredentialProvider; -import org.keycloak.credential.PasswordCredentialProvider; import org.keycloak.email.EmailException; import org.keycloak.email.EmailTemplateProvider; import org.keycloak.events.Details; @@ -47,6 +45,7 @@ import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.ModelException; import org.keycloak.models.RealmModel; import org.keycloak.models.UserConsentModel; +import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserLoginFailureModel; import org.keycloak.models.UserManager; import org.keycloak.models.UserModel; @@ -80,7 +79,6 @@ import javax.ws.rs.Consumes; import javax.ws.rs.DELETE; import javax.ws.rs.DefaultValue; import javax.ws.rs.GET; -import javax.ws.rs.NotFoundException; import javax.ws.rs.NotSupportedException; import javax.ws.rs.POST; import javax.ws.rs.PUT; @@ -598,8 +596,7 @@ public class UserResource { } try { - PasswordCredentialProvider provider = (PasswordCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-password"); - provider.createCredential(realm, user, cred.getValue()); + session.userCredentialManager().updateCredential(realm, user, UserCredentialModel.password(cred.getValue(), false)); } catch (IllegalStateException ise) { throw new BadRequestException("Resetting to N old passwords is not allowed."); } catch (ReadOnlyException mre) { @@ -627,6 +624,24 @@ public class UserResource { } + /** + * Return credential types, which are provided by the user storage where user is stored. Returned values can contain for example "password", "otp" etc. + * This will always return empty list for "local" users, which are not backed by any user storage + * + * @return + */ + @GET + @Path("configured-user-storage-credential-types") + @NoCache + @Produces(MediaType.APPLICATION_JSON) + public List getConfiguredUserStorageCredentialTypes() { + // This has "requireManage" due the compatibility with "credentials()" endpoint. Strictly said, it is reading endpoint, not writing, + // so may be revisited if to rather use "requireView" here in the future. + auth.users().requireManage(user); + return session.userCredentialManager().getConfiguredUserStorageCredentialTypes(realm, user); + } + + /** * Remove a credential for a user * diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/DummyUserFederationProvider.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/DummyUserFederationProvider.java index 1a6825ad22..e0795ed775 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/DummyUserFederationProvider.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/DummyUserFederationProvider.java @@ -27,12 +27,15 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; +import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.models.credential.PasswordCredentialModel; import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.user.UserLookupProvider; import org.keycloak.storage.user.UserRegistrationProvider; +import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -49,6 +52,12 @@ public class DummyUserFederationProvider implements UserStorageProvider, private KeycloakSession session; private ComponentModel component; + // Hardcoded password of test-user + public static final String HARDCODED_PASSWORD = "secret"; + + // Hardcoded otp code, which will be always considered valid for the test-user + public static final String HARDCODED_OTP = "123456"; + public DummyUserFederationProvider(KeycloakSession session, ComponentModel component, Map users) { @@ -104,7 +113,7 @@ public class DummyUserFederationProvider implements UserStorageProvider, } public Set getSupportedCredentialTypes() { - return Collections.singleton(PasswordCredentialModel.TYPE); + return new HashSet<>(Arrays.asList(PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE)); } @Override @@ -114,7 +123,7 @@ public class DummyUserFederationProvider implements UserStorageProvider, @Override public boolean isConfiguredFor(RealmModel realm, UserModel user, String credentialType) { - if (!PasswordCredentialModel.TYPE.equals(credentialType)) return false; + if (!supportsCredentialType(credentialType)) return false; if (user.getUsername().equals("test-user")) { return true; @@ -126,7 +135,11 @@ public class DummyUserFederationProvider implements UserStorageProvider, @Override public boolean isValid(RealmModel realm, UserModel user, CredentialInput credentialInput) { if (user.getUsername().equals("test-user")) { - return "secret".equals(credentialInput.getChallengeResponse()); + if (PasswordCredentialModel.TYPE.equals(credentialInput.getType())) { + return HARDCODED_PASSWORD.equals(credentialInput.getChallengeResponse()); + } else if (OTPCredentialModel.TYPE.equals(credentialInput.getType())) { + return HARDCODED_OTP.equals(credentialInput.getChallengeResponse()); + } } return false; } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/AbstractLDAPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/AbstractLDAPTest.java index c63b6e614b..441a83b7dd 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/AbstractLDAPTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/AbstractLDAPTest.java @@ -31,6 +31,7 @@ import org.keycloak.testsuite.pages.AccountPasswordPage; import org.keycloak.testsuite.pages.AccountUpdateProfilePage; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginPasswordUpdatePage; import org.keycloak.testsuite.pages.OAuthGrantPage; import org.keycloak.testsuite.pages.RegisterPage; import org.keycloak.testsuite.util.LDAPRule; @@ -64,6 +65,9 @@ public abstract class AbstractLDAPTest extends AbstractTestRealmKeycloakTest { @Page protected OAuthGrantPage grantPage; + @Page + protected LoginPasswordUpdatePage requiredActionChangePasswordPage; + @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java index a3b660f014..7ebdc3c256 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java @@ -23,6 +23,7 @@ import org.junit.FixMethodOrder; import org.junit.Test; import org.junit.runners.MethodSorters; import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.resource.UserResource; import org.keycloak.component.ComponentModel; import org.keycloak.credential.CredentialModel; import org.keycloak.models.GroupModel; @@ -38,6 +39,7 @@ import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.representations.AccessToken; import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.services.managers.RealmManager; import org.keycloak.storage.ReadOnlyException; @@ -66,8 +68,11 @@ import org.keycloak.testsuite.util.OAuthClient; import javax.naming.AuthenticationException; import javax.ws.rs.core.Response; + +import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -329,7 +334,7 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest { } @Test - public void passwordChangeLdap() throws Exception { + public void ldapPasswordChangeWithAccountConsole() throws Exception { changePasswordPage.open(); loginPage.login("johnkeycloak", "Password1"); changePasswordPage.changePassword("Password1", "New-password1", "New-password1"); @@ -352,6 +357,77 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest { Assert.assertEquals("Your password has been updated.", profilePage.getSuccess()); } + + // KEYCLOAK-12340 + @Test + public void ldapPasswordChangeWithAdminEndpointAndRequiredAction() throws Exception { + String username = "adminEndpointAndRequiredActionTest"; + String email = username + "@email.cz"; + + // Register new LDAP user with password, logout user + loginPage.open(); + loginPage.clickRegister(); + registerPage.assertCurrent(); + registerPage.register("firstName", "lastName", email, + username, "Password1", "Password1"); + + + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + + appPage.logout(); + + // Test admin endpoint. Assert federated endpoint returns password in LDAP "supportedCredentials", but there is no stored password + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), username); + assertPasswordConfiguredThroughLDAPOnly(user); + + // Update password through admin REST endpoint. Assert user can authenticate with the new password + ApiUtil.resetUserPassword(user, "Password1-updated1", false); + + loginPage.open(); + + loginSuccessAndLogout(username, "Password1-updated1"); + + // Test admin endpoint. Assert federated endpoint returns password in LDAP "supportedCredentials", but there is no stored password + assertPasswordConfiguredThroughLDAPOnly(user); + + // Test this just for the import mode. No-import mode doesn't support requiredActions right now + if (isImportEnabled()) { + // Update password through required action. + UserRepresentation user2 = user.toRepresentation(); + user2.setRequiredActions(Arrays.asList(UserModel.RequiredAction.UPDATE_PASSWORD.toString())); + user.update(user2); + + loginPage.open(); + loginPage.login(username, "Password1-updated1"); + requiredActionChangePasswordPage.assertCurrent(); + + requiredActionChangePasswordPage.changePassword("Password1-updated2", "Password1-updated2"); + + appPage.assertCurrent(); + appPage.logout(); + + // Assert user can authenticate with the new password + loginSuccessAndLogout(username, "Password1-updated2"); + + // Test admin endpoint. Assert federated endpoint returns password in LDAP "supportedCredentials", but there is no stored password + assertPasswordConfiguredThroughLDAPOnly(user); + } + } + + + // Use admin REST endpoints + private void assertPasswordConfiguredThroughLDAPOnly(UserResource user) { + // Assert password not stored locally + List storedCredentials = user.credentials(); + for (CredentialRepresentation credential : storedCredentials) { + Assert.assertFalse(PasswordCredentialModel.TYPE.equals(credential.getType())); + } + + // Assert password is stored in the LDAP + List userStorageCredentials = user.getConfiguredUserStorageCredentialTypes(); + Assert.assertTrue(userStorageCredentials.contains(PasswordCredentialModel.TYPE)); + } + @Test public void registerExistingLdapUser() { loginPage.open(); @@ -908,6 +984,25 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest { } catch (AuthenticationException ex) { throw new RuntimeException(ex); } + }); + + // Test admin REST endpoints + UserResource userResource = ApiUtil.findUserByUsernameId(testRealm(), "johnkeycloak"); + + // Assert password is stored locally + List storedCredentials = userResource.credentials().stream() + .map(CredentialRepresentation::getType) + .collect(Collectors.toList()); + Assert.assertTrue(storedCredentials.contains(PasswordCredentialModel.TYPE)); + + // Assert password is supported in the LDAP too. + List userStorageCredentials = userResource.getConfiguredUserStorageCredentialTypes(); + Assert.assertTrue(userStorageCredentials.contains(PasswordCredentialModel.TYPE)); + + testingClient.server().run(session -> { + LDAPTestContext ctx = LDAPTestContext.init(session); + RealmModel appRealm = ctx.getRealm(); + UserModel user = session.users().getUserByUsername("johnkeycloak", appRealm); // User is deleted just locally Assert.assertTrue(session.users().removeUser(appRealm, user)); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageOTPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageOTPTest.java new file mode 100644 index 0000000000..1f1591447c --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/UserStorageOTPTest.java @@ -0,0 +1,222 @@ +/* + * Copyright 2019 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.keycloak.testsuite.federation.storage; + +import java.io.IOException; +import java.net.URISyntaxException; +import java.util.Collections; +import java.util.List; + + +import org.jboss.arquillian.graphene.page.Page; +import org.junit.Before; +import org.junit.Test; +import org.keycloak.OAuth2Constants; +import org.keycloak.admin.client.resource.UserResource; +import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.models.UserModel; +import org.keycloak.models.credential.OTPCredentialModel; +import org.keycloak.models.credential.PasswordCredentialModel; +import org.keycloak.models.utils.TimeBasedOTP; +import org.keycloak.representations.idm.ComponentRepresentation; +import org.keycloak.representations.idm.RealmRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.storage.UserStorageProvider; +import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; +import org.keycloak.testsuite.Assert; +import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.federation.DummyUserFederationProvider; +import org.keycloak.testsuite.federation.DummyUserFederationProviderFactory; +import org.keycloak.testsuite.pages.AppPage; +import org.keycloak.testsuite.pages.LoginConfigTotpPage; +import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginTotpPage; +import org.keycloak.testsuite.util.UserBuilder; + +import static org.keycloak.storage.UserStorageProviderModel.IMPORT_ENABLED; +import static org.keycloak.testsuite.federation.storage.UserStorageTest.addComponent; + +/** + * @author Marek Posolda + */ +public class UserStorageOTPTest extends AbstractTestRealmKeycloakTest { + + + @Page + protected LoginPage loginPage; + + @Page + protected LoginTotpPage loginTotpPage; + + @Page + protected LoginConfigTotpPage loginConfigTotpPage; + + @Page + protected AppPage appPage; + + protected TimeBasedOTP totp = new TimeBasedOTP(); + + + + @Override + public void configureTestRealm(RealmRepresentation testRealm) { + + } + + @Before + public void addProvidersBeforeTest() throws URISyntaxException, IOException { + ComponentRepresentation dummyProvider = new ComponentRepresentation(); + dummyProvider.setName("dummy"); + dummyProvider.setId(DummyUserFederationProviderFactory.PROVIDER_NAME); + dummyProvider.setProviderId(DummyUserFederationProviderFactory.PROVIDER_NAME); + dummyProvider.setProviderType(UserStorageProvider.class.getName()); + dummyProvider.setConfig(new MultivaluedHashMap<>()); + dummyProvider.getConfig().putSingle("priority", Integer.toString(0)); + dummyProvider.getConfig().putSingle(IMPORT_ENABLED, Boolean.toString(false)); + + addComponent(testRealm(), getCleanup(), dummyProvider); + + UserRepresentation user = UserBuilder.create() + .username("test-user") + .email("test-user@something.org") + .build(); + String testUserId = ApiUtil.createUserWithAdminClient(testRealm(), user); + + getCleanup().addUserId(testUserId); + } + + + @Test + public void testCredentialsThroughRESTAPI() { + // Test that test-user has federation link on him + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user"); + Assert.assertEquals(DummyUserFederationProviderFactory.PROVIDER_NAME, user.toRepresentation().getFederationLink()); + + // Test that both "password" and "otp" are configured for the test-user + List userStorageCredentialTypes = user.getConfiguredUserStorageCredentialTypes(); + Assert.assertNames(userStorageCredentialTypes, PasswordCredentialModel.TYPE, OTPCredentialModel.TYPE); + } + + + @Test + public void testAuthentication() { + // Test that user is required to provide OTP credential during authentication + loginPage.open(); + loginPage.login("test-user", DummyUserFederationProvider.HARDCODED_PASSWORD); + + loginTotpPage.assertCurrent(); + + loginTotpPage.login("654321"); + loginTotpPage.assertCurrent(); + Assert.assertEquals("Invalid authenticator code.", loginPage.getError()); + + loginTotpPage.login(DummyUserFederationProvider.HARDCODED_OTP); + + appPage.assertCurrent(); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + } + + + @Test + public void testUpdateOTP() { + // Add requiredAction to the user for update OTP + UserResource user = ApiUtil.findUserByUsernameId(testRealm(), "test-user"); + UserRepresentation userRep = user.toRepresentation(); + userRep.setRequiredActions(Collections.singletonList(UserModel.RequiredAction.CONFIGURE_TOTP.toString())); + user.update(userRep); + + // Authenticate as the user + loginPage.open(); + loginPage.login("test-user", DummyUserFederationProvider.HARDCODED_PASSWORD); + loginTotpPage.assertCurrent(); + loginTotpPage.login(DummyUserFederationProvider.HARDCODED_OTP); + + // User should be required to update OTP + loginConfigTotpPage.assertCurrent(); + + // Dummy OTP code won't work when configure new OTP + loginConfigTotpPage.configure(DummyUserFederationProvider.HARDCODED_OTP); + Assert.assertEquals("Invalid authenticator code.", loginConfigTotpPage.getError()); + + // This will save the credential to the local DB + String totpSecret = loginConfigTotpPage.getTotpSecret(); + log.infof("Totp Secret: %s", totpSecret); + String totpCode = totp.generateTOTP(totpSecret); + loginConfigTotpPage.configure(totpCode); + + appPage.assertCurrent(); + + // Logout + appPage.logout(); + + // Authenticate as the user again with the dummy OTP should still work + loginPage.open(); + loginPage.login("test-user", DummyUserFederationProvider.HARDCODED_PASSWORD); + loginTotpPage.assertCurrent(); + loginTotpPage.login(DummyUserFederationProvider.HARDCODED_OTP); + + appPage.assertCurrent(); + appPage.logout(); + + // Authenticate with the new OTP code should work as well + loginPage.open(); + loginPage.login("test-user", DummyUserFederationProvider.HARDCODED_PASSWORD); + loginTotpPage.assertCurrent(); + loginTotpPage.login(totp.generateTOTP(totpSecret)); + + appPage.assertCurrent(); + appPage.logout(); + } + + @Test + public void testNormalUser() { + // Add some other user to the KEycloak + UserRepresentation user = UserBuilder.create() + .username("test-user2") + .email("test-user2@something.org") + .build(); + String testUserId = ApiUtil.createUserWithAdminClient(testRealm(), user); + getCleanup().addUserId(testUserId); + + // Assert he has federation link on him + UserResource userResource = ApiUtil.findUserByUsernameId(testRealm(), "test-user2"); + Assert.assertEquals(DummyUserFederationProviderFactory.PROVIDER_NAME, userResource.toRepresentation().getFederationLink()); + + // Assert no userStorage supported credentials shown through admin REST API for that user. For this user, the validation of password and OTP is not delegated + // to the dummy user storage provider + Assert.assertTrue(userResource.getConfiguredUserStorageCredentialTypes().isEmpty()); + + // Update password + ApiUtil.resetUserPassword(userResource, "pass", false); + + // Authenticate as the user. Only the password will be required for him + loginPage.open(); + loginPage.login("test-user2", "pass"); + + appPage.assertCurrent(); + Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType()); + Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE)); + } + + + + + +} diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties index 1ca35c593f..472d669d6a 100644 --- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties +++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties @@ -1548,7 +1548,11 @@ disable-credential-types=Disable Credential Types credentials.disable.tooltip=Click button to disable selected credential types credential-types=Credential Types manage-user-password=Manage Password +supported-user-storage-credential-types=Supported User Storage Credential Types +supported-user-storage-credential-types.tooltip=Credential types, which are provided by User Storage Provider. Validation and eventually update of the credentials of those types can be delegated to the User Storage Provider based on the configuration and implementation of the particular provider. +provided-by=Provided By manage-credentials=Manage Credentials +manage-credentials.tooltip=Credentials, which are not provided by the user storage. They are saved in the local database. disable-credentials=Disable Credentials credential-reset-actions=Credential Reset credential-reset-actions-timeout=Expires In diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js index f909a47059..b8f4764e59 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js @@ -331,6 +331,46 @@ module.controller('UserTabCtrl', function($scope, $location, Dialog, Notificatio }; }); +function loadUserStorageLink(realm, user, console, Components, UserStorageOperations, $scope, $location) { + if(user.federationLink) { + console.log("federationLink is not null. It is " + user.federationLink); + + if ($scope.access.viewRealm) { + Components.get({realm: realm.realm, componentId: user.federationLink}, function (link) { + $scope.federationLinkName = link.name; + $scope.federationLink = "#/realms/" + realm.realm + "/user-storage/providers/" + link.providerId + "/" + link.id; + }); + } else { + // KEYCLOAK-4328 + UserStorageOperations.simpleName.get({realm: realm.realm, componentId: user.federationLink}, function (link) { + $scope.federationLinkName = link.name; + $scope.federationLink = $location.absUrl(); + }) + } + + } else { + console.log("federationLink is null"); + } + + if(user.origin) { + if ($scope.access.viewRealm) { + Components.get({realm: realm.realm, componentId: user.origin}, function (link) { + $scope.originName = link.name; + $scope.originLink = "#/realms/" + realm.realm + "/user-storage/providers/" + link.providerId + "/" + link.id; + }) + } + else { + // KEYCLOAK-4328 + UserStorageOperations.simpleName.get({realm: realm.realm, componentId: user.origin}, function (link) { + $scope.originName = link.name; + $scope.originLink = $location.absUrl(); + }) + } + } else { + console.log("origin is null"); + } +}; + module.controller('UserDetailCtrl', function($scope, realm, user, BruteForceUser, User, Components, UserImpersonation, RequiredActions, @@ -359,42 +399,9 @@ module.controller('UserDetailCtrl', function($scope, realm, user, BruteForceUser } }); }; - if(user.federationLink) { - console.log("federationLink is not null. It is " + user.federationLink); - if ($scope.access.viewRealm) { - Components.get({realm: realm.realm, componentId: user.federationLink}, function (link) { - $scope.federationLinkName = link.name; - $scope.federationLink = "#/realms/" + realm.realm + "/user-storage/providers/" + link.providerId + "/" + link.id; - }); - } else { - // KEYCLOAK-4328 - UserStorageOperations.simpleName.get({realm: realm.realm, componentId: user.federationLink}, function (link) { - $scope.federationLinkName = link.name; - $scope.federationLink = $location.absUrl(); - }) - } + loadUserStorageLink(realm, user, console, Components, UserStorageOperations, $scope, $location); - } else { - console.log("federationLink is null"); - } - if(user.origin) { - if ($scope.access.viewRealm) { - Components.get({realm: realm.realm, componentId: user.origin}, function (link) { - $scope.originName = link.name; - $scope.originLink = "#/realms/" + realm.realm + "/user-storage/providers/" + link.providerId + "/" + link.id; - }) - } - else { - // KEYCLOAK-4328 - UserStorageOperations.simpleName.get({realm: realm.realm, componentId: user.origin}, function (link) { - $scope.originName = link.name; - $scope.originLink = $location.absUrl(); - }) - } - } else { - console.log("origin is null"); - } console.log('realm brute force? ' + realm.bruteForceProtected) $scope.temporarilyDisabled = false; var isDisabled = function () { @@ -514,7 +521,8 @@ module.controller('UserDetailCtrl', function($scope, realm, user, BruteForceUser } }); -module.controller('UserCredentialsCtrl', function($scope, realm, user, $route, RequiredActions, User, UserExecuteActionsEmail, UserCredentials, Notifications, Dialog, TimeUnit2) { +module.controller('UserCredentialsCtrl', function($scope, realm, user, $route, $location, RequiredActions, User, UserExecuteActionsEmail, + UserCredentials, Notifications, Dialog, TimeUnit2, Components, UserStorageOperations) { console.log('UserCredentialsCtrl'); $scope.hasPassword = false; @@ -523,6 +531,16 @@ module.controller('UserCredentialsCtrl', function($scope, realm, user, $route, R loadCredentials(); + loadUserStorageLink(realm, user, console, Components, UserStorageOperations, $scope, $location); + + $scope.getUserStorageProviderName = function() { + return user.federationLink ? $scope.federationLinkName : $scope.originName; + } + + $scope.getUserStorageProviderLink = function() { + return user.federationLink ? $scope.federationLink : $scope.originLink; + } + $scope.keys = function(object) { return object ? Object.keys(object) : []; } @@ -648,6 +666,14 @@ module.controller('UserCredentialsCtrl', function($scope, realm, user, $route, R Notifications.error("Error while loading user credentials. See console for more information."); console.log(err); }); + + UserCredentials.getConfiguredUserStorageCredentialTypes({ realm: realm.realm, userId: user.id }, null, function(userStorageCredentialTypes) { + $scope.userStorageCredentialTypes = userStorageCredentialTypes; + $scope.hasPassword = $scope.hasPassword || userStorageCredentialTypes.lastIndexOf("password") > -1; + }, function(err) { + Notifications.error("Error while loading user storage credentials. See console for more information."); + console.log(err); + }); } $scope.resetPassword = function() { diff --git a/themes/src/main/resources/theme/base/admin/resources/js/services.js b/themes/src/main/resources/theme/base/admin/resources/js/services.js index 1ceda107bd..cb976dfbc9 100755 --- a/themes/src/main/resources/theme/base/admin/resources/js/services.js +++ b/themes/src/main/resources/theme/base/admin/resources/js/services.js @@ -659,6 +659,11 @@ module.factory('UserCredentials', function($resource) { userId : '@userId' }).query; + credentials.getConfiguredUserStorageCredentialTypes = $resource(authUrl + '/admin/realms/:realm/users/:userId/configured-user-storage-credential-types', { + realm : '@realm', + userId : '@userId' + }).query; + credentials.deleteCredential = $resource(authUrl + '/admin/realms/:realm/users/:userId/credentials/:credentialId', { realm : '@realm', userId : '@userId', diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/user-credentials.html b/themes/src/main/resources/theme/base/admin/resources/partials/user-credentials.html index 6ea62ce30a..9015660f11 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/user-credentials.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/user-credentials.html @@ -7,8 +7,38 @@
+ +
+ + {{:: 'supported-user-storage-credential-types' | translate}} + {{:: 'supported-user-storage-credential-types.tooltip' | translate}} + + + + + + + + + + + + +
{{:: 'type' | translate}}{{:: 'provided-by' | translate}}
+ {{credential}} + + {{getUserStorageProviderName()}} +
+
+
- {{:: 'manage-credentials' | translate}} + + {{:: 'manage-credentials' | translate}} + {{:: 'manage-credentials.tooltip' | translate}} + + + {{:: 'manage-credentials' | translate}} +