From 72e4690248a28c8296c78e1fa95726877f894b8f Mon Sep 17 00:00:00 2001 From: mposolda Date: Thu, 5 Mar 2020 18:07:52 +0100 Subject: [PATCH] KEYCLOAK-13174 Not possible to delegate creating or deleting OTP credential to userStorage --- .../admin/client/resource/UserResource.java | 4 +- .../org/keycloak/utils/CredentialHelper.java | 62 ++++++ .../keycloak/credential/CredentialModel.java | 10 + .../credential/CredentialProvider.java | 2 +- .../models/UserCredentialManager.java | 4 +- .../keycloak/models/UserCredentialModel.java | 7 +- .../requiredactions/ConsoleUpdateTotp.java | 6 +- .../requiredactions/UpdateTotp.java | 7 +- .../credential/OTPCredentialProvider.java | 4 +- .../PasswordCredentialProvider.java | 4 +- .../UserCredentialStoreManager.java | 14 +- .../WebAuthnCredentialProvider.java | 4 +- .../account/freemarker/model/TotpBean.java | 18 +- .../account/AccountCredentialResource.java | 7 +- .../resources/account/AccountFormService.java | 9 +- .../resources/admin/UserResource.java | 7 +- .../BackwardsCompatibilityUserStorage.java | 101 ++++++++- ...kwardsCompatibilityUserStorageFactory.java | 6 + ...BackwardsCompatibilityUserStorageTest.java | 191 +++++++++++++++++- .../messages/admin-messages_en.properties | 2 +- 20 files changed, 410 insertions(+), 59 deletions(-) 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 131a4387ba..d6b323e37e 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 @@ -154,14 +154,14 @@ public interface UserResource { * Disables or deletes all credentials for specific types. * Type examples "otp", "password" * - * This endpoint is deprecated as it is not supported to disable credentials, just delete them + * This is typically supported just for the users backed by user storage providers. See {@link UserRepresentation#getDisableableCredentialTypes()} + * to see what credential types can be disabled for the particular user * * @param credentialTypes */ @Path("disable-credential-types") @PUT @Consumes(MediaType.APPLICATION_JSON) - @Deprecated void disableCredentialType(List credentialTypes); @PUT diff --git a/server-spi-private/src/main/java/org/keycloak/utils/CredentialHelper.java b/server-spi-private/src/main/java/org/keycloak/utils/CredentialHelper.java index b3f96785c0..bc7476c11e 100755 --- a/server-spi-private/src/main/java/org/keycloak/utils/CredentialHelper.java +++ b/server-spi-private/src/main/java/org/keycloak/utils/CredentialHelper.java @@ -17,6 +17,8 @@ package org.keycloak.utils; +import javax.ws.rs.core.Response; + import org.jboss.logging.Logger; import org.keycloak.authentication.Authenticator; import org.keycloak.authentication.AuthenticatorFactory; @@ -25,10 +27,18 @@ import org.keycloak.authentication.ClientAuthenticatorFactory; import org.keycloak.authentication.ConfigurableAuthenticatorFactory; import org.keycloak.authentication.FormAction; import org.keycloak.authentication.FormActionFactory; +import org.keycloak.credential.CredentialModel; +import org.keycloak.credential.CredentialProvider; +import org.keycloak.forms.account.AccountPages; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.AuthenticationFlowModel; 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.models.utils.CredentialValidation; +import org.keycloak.representations.idm.CredentialRepresentation; /** * used to set an execution a state based on type. @@ -79,4 +89,56 @@ public class CredentialHelper { } return factory; } + + /** + * Create OTP credential either in userStorage or local storage (Keycloak DB) + * + * @return true if credential was successfully created either in the user storage or Keycloak DB. False if error happened (EG. during HOTP validation) + */ + public static boolean createOTPCredential(KeycloakSession session, RealmModel realm, UserModel user, String totpCode, OTPCredentialModel credentialModel) { + CredentialProvider otpCredentialProvider = session.getProvider(CredentialProvider.class, "keycloak-otp"); + String totpSecret = credentialModel.getOTPSecretData().getValue(); + + UserCredentialModel otpUserCredential = new UserCredentialModel("", realm.getOTPPolicy().getType(), totpSecret); + boolean userStorageCreated = session.userCredentialManager().updateCredential(realm, user, otpUserCredential); + + String credentialId = null; + if (userStorageCreated) { + logger.debugf("Created OTP credential for user '%s' in the user storage", user.getUsername()); + } else { + CredentialModel createdCredential = otpCredentialProvider.createCredential(realm, user, credentialModel); + credentialId = createdCredential.getId(); + } + + //If the type is HOTP, call verify once to consume the OTP used for registration and increase the counter. + UserCredentialModel credential = new UserCredentialModel(credentialId, otpCredentialProvider.getType(), totpCode); + return session.userCredentialManager().isValid(realm, user, credential); + } + + public static void deleteOTPCredential(KeycloakSession session, RealmModel realm, UserModel user, String credentialId) { + CredentialProvider otpCredentialProvider = session.getProvider(CredentialProvider.class, "keycloak-otp"); + boolean removed = otpCredentialProvider.deleteCredential(realm, user, credentialId); + + // This can usually happened when credential is stored in the userStorage. Propagate to "disable" credential in the userStorage + if (!removed) { + logger.debug("Removing OTP credential from userStorage"); + session.userCredentialManager().disableCredentialType(realm, user, OTPCredentialModel.TYPE); + } + } + + /** + * Create "dummy" representation of the credential. Typically used when credential is provided by userStorage and we don't know further + * details about the credential besides the type + * + * @param credentialProviderType + * @return dummy credential + */ + public static CredentialRepresentation createUserStorageCredentialRepresentation(String credentialProviderType) { + CredentialRepresentation credential = new CredentialRepresentation(); + credential.setId(credentialProviderType + "-id"); + credential.setType(credentialProviderType); + credential.setCreatedDate(-1L); + credential.setPriority(0); + return credential; + } } diff --git a/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java b/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java index 16b0481bd6..07d33bf1d7 100755 --- a/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java +++ b/server-spi/src/main/java/org/keycloak/credential/CredentialModel.java @@ -23,6 +23,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.Map; +import com.fasterxml.jackson.annotation.JsonIgnore; import org.keycloak.common.util.Base64; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.models.credential.PasswordCredentialModel; @@ -134,6 +135,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use PasswordCredentialModel.getSecretData().getValue() or OTPCredentialModel.getSecretData().getValue() */ @Deprecated + @JsonIgnore public String getValue() { return readString("value", true); } @@ -150,6 +152,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use OTPCredentialModel.getCredentialData().getDevice() */ @Deprecated + @JsonIgnore public String getDevice() { return readString("device", false); } @@ -166,6 +169,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use PasswordCredentialModel.getSecretData().getSalt() */ @Deprecated + @JsonIgnore public byte[] getSalt() { try { String saltStr = readString("salt", true); @@ -188,6 +192,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use PasswordCredentialModel.getCredentialData().getHashIterations() */ @Deprecated + @JsonIgnore public int getHashIterations() { return readInt("hashIterations", false); } @@ -204,6 +209,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use OTPCredentialModel.getCredentialData().getCounter() */ @Deprecated + @JsonIgnore public int getCounter() { return readInt("counter", false); } @@ -220,6 +226,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use PasswordCredentialModel.getCredentialData().getAlgorithm() or OTPCredentialModel.getCredentialData().getAlgorithm() */ @Deprecated + @JsonIgnore public String getAlgorithm() { return readString("algorithm", false); } @@ -236,6 +243,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use OTPCredentialModel.getCredentialData().getDigits() */ @Deprecated + @JsonIgnore public int getDigits() { return readInt("digits", false); } @@ -252,6 +260,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use OTPCredentialModel.getCredentialData().getPeriod() */ @Deprecated + @JsonIgnore public int getPeriod() { return readInt("period", false); } @@ -268,6 +277,7 @@ public class CredentialModel implements Serializable { * @deprecated Recommended to use {@link #getCredentialData()} instead and use the subtype of CredentialData specific to your credential */ @Deprecated + @JsonIgnore public MultivaluedHashMap getConfig() { Map credentialData = readMapFromJson(false); if (credentialData == null) { diff --git a/server-spi/src/main/java/org/keycloak/credential/CredentialProvider.java b/server-spi/src/main/java/org/keycloak/credential/CredentialProvider.java index c1d41bde83..327d7110e6 100644 --- a/server-spi/src/main/java/org/keycloak/credential/CredentialProvider.java +++ b/server-spi/src/main/java/org/keycloak/credential/CredentialProvider.java @@ -38,7 +38,7 @@ public interface CredentialProvider extends Provider CredentialModel createCredential(RealmModel realm, UserModel user, T credentialModel); - void deleteCredential(RealmModel realm, UserModel user, String credentialId); + boolean deleteCredential(RealmModel realm, UserModel user, String credentialId); T getCredentialFromModel(CredentialModel model); 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 5fd98042eb..d555f15dfd 100644 --- a/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java +++ b/server-spi/src/main/java/org/keycloak/models/UserCredentialManager.java @@ -57,9 +57,9 @@ public interface UserCredentialManager extends UserCredentialStore { * * @param realm * @param user - * @return + * @return true if credential was successfully updated by UserStorage or any CredentialInputUpdater */ - void updateCredential(RealmModel realm, UserModel user, CredentialInput input); + boolean updateCredential(RealmModel realm, UserModel user, CredentialInput input); /** * Creates a credential from the credentialModel, by looping through the providers to find a match for the type diff --git a/server-spi/src/main/java/org/keycloak/models/UserCredentialModel.java b/server-spi/src/main/java/org/keycloak/models/UserCredentialModel.java index 4a0346688d..87af69316f 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserCredentialModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserCredentialModel.java @@ -52,16 +52,19 @@ public class UserCredentialModel implements CredentialInput { public static final String KERBEROS = CredentialModel.KERBEROS; public static final String CLIENT_CERT = CredentialModel.CLIENT_CERT; - private final String credentialId; + private String credentialId; private String type; private String challengeResponse; private String device; private String algorithm; - private final boolean adminRequest; + private boolean adminRequest; // Additional context informations protected Map notes = new HashMap<>(); + public UserCredentialModel() { + } + public UserCredentialModel(String credentialId, String type, String challengeResponse) { this.credentialId = credentialId; this.type = type; diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/ConsoleUpdateTotp.java b/services/src/main/java/org/keycloak/authentication/requiredactions/ConsoleUpdateTotp.java index 0b4e57dbb7..9b2c03248a 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/ConsoleUpdateTotp.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/ConsoleUpdateTotp.java @@ -32,6 +32,7 @@ import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.models.utils.CredentialValidation; import org.keycloak.services.messages.Messages; import org.keycloak.services.validation.Validation; +import org.keycloak.utils.CredentialHelper; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; @@ -85,10 +86,7 @@ public class ConsoleUpdateTotp implements RequiredActionProvider { return; } - OTPCredentialProvider otpCredentialProvider = (OTPCredentialProvider) context.getSession().getProvider(CredentialProvider.class, "keycloak-otp"); - CredentialModel createdCredential = otpCredentialProvider.createCredential(context.getRealm(), context.getUser(), credentialModel); - UserCredentialModel credential = new UserCredentialModel(createdCredential.getId(), otpCredentialProvider.getType(), challengeResponse); - if (!otpCredentialProvider.isValid(context.getRealm(), context.getUser(), credential)) { + if (!CredentialHelper.createOTPCredential(context.getSession(), context.getRealm(), context.getUser(), challengeResponse, credentialModel)) { context.challenge(challenge(context).message(Messages.INVALID_TOTP)); return; } 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 ea2d6de497..bf1e3288e8 100644 --- a/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java +++ b/services/src/main/java/org/keycloak/authentication/requiredactions/UpdateTotp.java @@ -34,6 +34,7 @@ import org.keycloak.models.credential.OTPCredentialModel; import org.keycloak.models.utils.CredentialValidation; import org.keycloak.services.messages.Messages; import org.keycloak.services.validation.Validation; +import org.keycloak.utils.CredentialHelper; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; @@ -102,10 +103,8 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory context.challenge(challenge); return; } - 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()) && !context.getSession().userCredentialManager().isValid(context.getRealm(), context.getUser(), credential)) { + + if (!CredentialHelper.createOTPCredential(context.getSession(), context.getRealm(), context.getUser(), challengeResponse, credentialModel)) { Response challenge = context.form() .setAttribute("mode", mode) .setError(Messages.INVALID_TOTP) diff --git a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java index f6c2e412aa..f30a65e66f 100644 --- a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java @@ -75,8 +75,8 @@ public class OTPCredentialProvider implements CredentialProvider credentialProviders = getCredentialProviders(session, realm, CredentialInputUpdater.class); for (CredentialInputUpdater updater : credentialProviders) { if (!updater.supportsCredentialType(input.getType())) continue; - if (updater.updateCredential(realm, user, input)) return; + if (updater.updateCredential(realm, user, input)) return true; } + + return false; } @Override diff --git a/services/src/main/java/org/keycloak/credential/WebAuthnCredentialProvider.java b/services/src/main/java/org/keycloak/credential/WebAuthnCredentialProvider.java index d821ef12da..972a02525c 100644 --- a/services/src/main/java/org/keycloak/credential/WebAuthnCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/WebAuthnCredentialProvider.java @@ -75,9 +75,9 @@ public class WebAuthnCredentialProvider implements CredentialProviderStian Thorgersen @@ -48,11 +52,19 @@ public class TotpBean { public TotpBean(KeycloakSession session, RealmModel realm, UserModel user, UriBuilder uriBuilder) { this.uriBuilder = uriBuilder; - this.enabled = ((OTPCredentialProvider)session.getProvider(CredentialProvider.class, "keycloak-otp")).isConfiguredFor(realm, user); + this.enabled = session.userCredentialManager().isConfiguredFor(realm, user, OTPCredentialModel.TYPE); if (enabled) { - otpCredentials = session.userCredentialManager().getStoredCredentialsByType(realm, user, OTPCredentialModel.TYPE); + List otpCredentials = session.userCredentialManager().getStoredCredentialsByType(realm, user, OTPCredentialModel.TYPE); + + if (otpCredentials.isEmpty()) { + // Credential is configured on userStorage side. Create the "fake" credential similar like we do for the new account console + CredentialRepresentation credential = createUserStorageCredentialRepresentation(OTPCredentialModel.TYPE); + this.otpCredentials = Collections.singletonList(RepresentationToModel.toModel(credential)); + } else { + this.otpCredentials = otpCredentials; + } } else { - otpCredentials = Collections.EMPTY_LIST; + this.otpCredentials = Collections.EMPTY_LIST; } this.realm = realm; diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountCredentialResource.java b/services/src/main/java/org/keycloak/services/resources/account/AccountCredentialResource.java index 4981581652..2b06adecc8 100644 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountCredentialResource.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountCredentialResource.java @@ -41,6 +41,7 @@ import java.util.Set; import java.util.stream.Collectors; import static org.keycloak.models.AuthenticationExecutionModel.Requirement.DISABLED; +import static org.keycloak.utils.CredentialHelper.createUserStorageCredentialRepresentation; public class AccountCredentialResource { @@ -201,11 +202,7 @@ public class AccountCredentialResource { session.userCredentialManager().isConfiguredFor(realm, user, credentialProviderType)) { // In case user is federated in the userStorage, he may have credential configured on the userStorage side. We're // creating "dummy" credential representing the credential provided by userStorage - CredentialRepresentation credential = new CredentialRepresentation(); - credential.setId(credentialProviderType + "-id"); - credential.setType(credentialProviderType); - credential.setCreatedDate(-1L); - credential.setPriority(0); + CredentialRepresentation credential = createUserStorageCredentialRepresentation(credentialProviderType); userCredentialModels = Collections.singletonList(credential); } diff --git a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java index 9b562f909f..e416a117af 100755 --- a/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java +++ b/services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java @@ -77,6 +77,7 @@ import org.keycloak.services.validation.Validation; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.storage.ReadOnlyException; import org.keycloak.util.JsonSerialization; +import org.keycloak.utils.CredentialHelper; import javax.ws.rs.Consumes; import javax.ws.rs.FormParam; @@ -494,14 +495,13 @@ public class AccountFormService extends AbstractSecuredLocalService { UserModel user = auth.getUser(); - OTPCredentialProvider otpCredentialProvider = (OTPCredentialProvider) session.getProvider(CredentialProvider.class, "keycloak-otp"); if (action != null && action.equals("Delete")) { String credentialId = formData.getFirst("credentialId"); if (credentialId == null) { setReferrerOnPage(); return account.setError(Status.OK, Messages.UNEXPECTED_ERROR_HANDLING_REQUEST).createResponse(AccountPages.TOTP); } - otpCredentialProvider.deleteCredential(realm, user, credentialId); + CredentialHelper.deleteOTPCredential(session, realm, user, credentialId); event.event(EventType.REMOVE_TOTP).client(auth.getClient()).user(auth.getUser()).success(); setReferrerOnPage(); return account.setSuccess(Messages.SUCCESS_TOTP_REMOVED).createResponse(AccountPages.TOTP); @@ -520,10 +520,7 @@ public class AccountFormService extends AbstractSecuredLocalService { return account.setError(Status.OK, Messages.INVALID_TOTP).createResponse(AccountPages.TOTP); } - - CredentialModel createdCredential = otpCredentialProvider.createCredential(realm, user, credentialModel); - UserCredentialModel credential = new UserCredentialModel(createdCredential.getId(), otpCredentialProvider.getType(), challengeResponse); - if (!otpCredentialProvider.isValid(realm, user, credential)) { + if (!CredentialHelper.createOTPCredential(session, realm, user, challengeResponse, credentialModel)) { setReferrerOnPage(); return account.setError(Status.OK, Messages.INVALID_TOTP).createResponse(AccountPages.TOTP); } 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 2da72a6c3c..724c3e4c8b 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 @@ -581,7 +581,12 @@ public class UserResource { @PUT @Consumes(MediaType.APPLICATION_JSON) public void disableCredentialType(List credentialTypes) { - throw new NotSupportedException("Not supported to disable credentials. Only credentials removal is supported"); + auth.users().requireManage(user); + if (credentialTypes == null) return; + for (String type : credentialTypes) { + session.userCredentialManager().disableCredentialType(realm, user, type); + + } } /** diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorage.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorage.java index 647f62b529..492589f427 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorage.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorage.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.federation; import java.util.Collections; +import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -31,12 +32,14 @@ import org.keycloak.credential.CredentialInputValidator; import org.keycloak.credential.CredentialModel; import org.keycloak.credential.hash.PasswordHashProvider; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.OTPPolicy; import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.cache.UserCache; import org.keycloak.models.credential.PasswordUserCredentialModel; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; import org.keycloak.storage.adapter.AbstractUserAdapterFederatedStorage; @@ -94,7 +97,19 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us @Override public boolean supportsCredentialType(String credentialType) { - return CredentialModel.PASSWORD.equals(credentialType); + if (CredentialModel.PASSWORD.equals(credentialType) + || isOTPType(credentialType)) { + return true; + } else { + log.infof("Unsupported credential type: %s", credentialType); + return false; + } + } + + private boolean isOTPType(String credentialType) { + return CredentialModel.OTP.equals(credentialType) + || CredentialModel.HOTP.equals(credentialType) + || CredentialModel.TOTP.equals(credentialType); } @Override @@ -137,8 +152,32 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us if (userCache != null) { userCache.evict(realm, user); } + return true; + } else if (isOTPType(input.getType())) { + UserCredentialModel otpCredential = (UserCredentialModel) input; + + // Those are not supposed to be set when calling this method in Keycloak 4.8.3 for password credential + assertNull(otpCredential.getDevice()); + assertNull(otpCredential.getAlgorithm()); + + OTPPolicy otpPolicy = session.getContext().getRealm().getOTPPolicy(); + + CredentialModel newOTP = new CredentialModel(); + newOTP.setType(input.getType()); + long createdDate = Time.currentTimeMillis(); + newOTP.setCreatedDate(createdDate); + newOTP.setValue(otpCredential.getValue()); + + newOTP.setCounter(otpPolicy.getInitialCounter()); + newOTP.setDigits(otpPolicy.getDigits()); + newOTP.setAlgorithm(otpPolicy.getAlgorithm()); + newOTP.setPeriod(otpPolicy.getPeriod()); + + users.get(user.getUsername()).otp = newOTP; + return true; } else { + log.infof("Attempt to update unsupported credential of type: %s", input.getType()); return false; } } @@ -154,24 +193,53 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us @Override public void disableCredentialType(RealmModel realm, UserModel user, String credentialType) { + if (isOTPType(credentialType)) { + MyUser myUser = getMyUser(user); + myUser.otp = null; + } else { + log.infof("Unsupported to disable credential of type: %s", credentialType); + } + } + private MyUser getMyUser(UserModel user) { + return users.get(user.getUsername()); } @Override public Set getDisableableCredentialTypes(RealmModel realm, UserModel user) { - return Collections.EMPTY_SET; + Set types = new HashSet<>(); + + MyUser myUser = getMyUser(user); + if (myUser != null && myUser.otp != null) { + types.add(CredentialModel.OTP); + } + + return types; } @Override public boolean isConfiguredFor(RealmModel realm, UserModel user, String credentialType) { - return CredentialModel.PASSWORD.equals(credentialType); + // Always assume that password is supported + if (CredentialModel.PASSWORD.equals(credentialType)) return true; + MyUser myUser = getMyUser(user); + if (myUser == null) return false; + + if (isOTPType(credentialType) && myUser.otp != null) { + return true; + } else { + log.infof("Not supported credentialType '%s' for user '%s'", credentialType, user.getUsername()); + return false; + } } @Override public boolean isValid(RealmModel realm, UserModel user, CredentialInput input) { - if (!(input instanceof PasswordUserCredentialModel)) return false; + MyUser myUser = users.get(user.getUsername()); + if (myUser == null) return false; + if (input.getType().equals(UserCredentialModel.PASSWORD)) { - CredentialModel hashedPassword = users.get(user.getUsername()).hashedPassword; + if (!(input instanceof PasswordUserCredentialModel)) return false; + CredentialModel hashedPassword = myUser.hashedPassword; if (hashedPassword == null) { log.warnf("Password not set for user %s", user.getUsername()); return false; @@ -190,7 +258,25 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us // Compatibility with 4.8.3 - using "legacy" signature of this method return hashProvider.verify(rawPassword, hashedPassword); + } else if (isOTPType(input.getType())) { + UserCredentialModel otpCredential = (UserCredentialModel) input; + + // Special hardcoded OTP, which is always considered valid + if ("123456".equals(otpCredential.getValue())) { + return true; + } + + CredentialModel storedOTPCredential = myUser.otp; + if (storedOTPCredential == null) { + log.warnf("Not found credential for the user %s", user.getUsername()); + return false; + } + + TimeBasedOTP validator = new TimeBasedOTP(storedOTPCredential.getAlgorithm(), storedOTPCredential.getDigits(), + storedOTPCredential.getPeriod(), realm.getOTPPolicy().getLookAheadWindow()); + return validator.validateTOTP(otpCredential.getValue(), storedOTPCredential.getValue().getBytes()); } else { + log.infof("Not supported to validate credential of type '%s' for user '%s'", input.getType(), user.getUsername()); return false; } } @@ -227,11 +313,15 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us private String username; private CredentialModel hashedPassword; + private CredentialModel otp; private MyUser(String username) { this.username = username; } + public CredentialModel getOtp() { + return otp; + } } @@ -254,4 +344,3 @@ public class BackwardsCompatibilityUserStorage implements UserLookupProvider, Us } } - diff --git a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorageFactory.java b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorageFactory.java index 84095067f2..7e9c43f1e7 100644 --- a/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorageFactory.java +++ b/testsuite/integration-arquillian/servers/auth-server/services/testsuite-providers/src/main/java/org/keycloak/testsuite/federation/BackwardsCompatibilityUserStorageFactory.java @@ -44,4 +44,10 @@ public class BackwardsCompatibilityUserStorageFactory implements UserStorageProv return PROVIDER_ID; } + public boolean hasUserOTP(String username) { + BackwardsCompatibilityUserStorage.MyUser user = userPasswords.get(username); + if (user == null) return false; + return user.getOtp() != null; + } + } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/BackwardsCompatibilityUserStorageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/BackwardsCompatibilityUserStorageTest.java index 3f218bf532..cf15fc65df 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/BackwardsCompatibilityUserStorageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/storage/BackwardsCompatibilityUserStorageTest.java @@ -20,25 +20,36 @@ package org.keycloak.testsuite.federation.storage; import java.io.IOException; import java.net.URISyntaxException; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import javax.ws.rs.core.Response; import org.jboss.arquillian.graphene.page.Page; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.credential.CredentialModel; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.CredentialRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.storage.StorageId; import org.keycloak.storage.UserStorageProvider; import org.keycloak.testsuite.AbstractAuthTest; +import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.federation.BackwardsCompatibilityUserStorageFactory; +import org.keycloak.testsuite.pages.AccountTotpPage; 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 static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlDoesntStartWith; import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWith; @@ -48,10 +59,29 @@ import static org.keycloak.testsuite.util.URLAssert.assertCurrentUrlStartsWith; * * @author Marek Posolda */ +@AuthServerContainerExclude(AuthServerContainerExclude.AuthServer.REMOTE) public class BackwardsCompatibilityUserStorageTest extends AbstractAuthTest { private String backwardsCompProviderId; + @Page + protected AppPage appPage; + + @Page + protected LoginPage loginPage; + + @Page + protected LoginTotpPage loginTotpPage; + + @Page + protected AccountTotpPage accountTotpSetupPage; + + @Page + protected LoginConfigTotpPage configureTotpRequiredActionPage; + + + private TimeBasedOTP totp = new TimeBasedOTP(); + @Before public void addProvidersBeforeTest() throws URISyntaxException, IOException { ComponentRepresentation memProvider = new ComponentRepresentation(); @@ -72,13 +102,6 @@ public class BackwardsCompatibilityUserStorageTest extends AbstractAuthTest { return id; } - - @Page - protected AppPage appPage; - - @Page - protected LoginPage loginPage; - private void loginSuccessAndLogout(String username, String password) { testRealmAccountPage.navigateTo(); loginPage.login(username, password); @@ -102,7 +125,7 @@ public class BackwardsCompatibilityUserStorageTest extends AbstractAuthTest { loginBadPassword("tbrady"); } - private void addUserAndResetPassword(String username, String password) { + private String addUserAndResetPassword(String username, String password) { // Save user and assert he is saved in the new storage UserRepresentation user = new UserRepresentation(); user.setEnabled(true); @@ -119,5 +142,153 @@ public class BackwardsCompatibilityUserStorageTest extends AbstractAuthTest { passwordRep.setTemporary(false); testRealmResource().users().get(userId).resetPassword(passwordRep); + + return userId; } -} \ No newline at end of file + + + @Test + public void testOTPUpdateAndLogin() { + String userId = addUserAndResetPassword("otp1", "pass"); + getCleanup().addUserId(userId); + + // Setup OTP for the user + String totpSecret = setupOTPForUserWithRequiredAction(userId); + + // Assert user has OTP in the userStorage + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(true); + + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(true); + + // Authenticate as the user with the hardcoded OTP. Should be supported + loginPage.login("otp1", "pass"); + loginTotpPage.assertCurrent(); + loginTotpPage.login("123456"); + + assertCurrentUrlStartsWith(testRealmAccountPage); + testRealmAccountPage.logOut(); + + // Authenticate as the user with bad OTP + loginPage.login("otp1", "pass"); + loginTotpPage.assertCurrent(); + loginTotpPage.login("7123456"); + assertCurrentUrlDoesntStartWith(testRealmAccountPage); + Assert.assertNotNull(loginTotpPage.getError()); + + // Authenticate as the user with correct OTP + loginTotpPage.login(totp.generateTOTP(totpSecret)); + assertCurrentUrlStartsWith(testRealmAccountPage); + testRealmAccountPage.logOut(); + } + + @Test + public void testOTPSetupThroughAccountMgmtAndLogin() { + String userId = addUserAndResetPassword("otp1", "pass"); + getCleanup().addUserId(userId); + + // Login as user to account mgmt + accountTotpSetupPage.open(); + loginPage.login("otp1", "pass"); + + // Setup OTP + String totpSecret = accountTotpSetupPage.getTotpSecret(); + accountTotpSetupPage.configure(totp.generateTOTP(totpSecret)); + + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(true); + + // Logout and assert user can login with hardcoded OTP + accountTotpSetupPage.logout(); + loginPage.login("otp1", "pass"); + loginTotpPage.login("123456"); + assertCurrentUrlStartsWith(testRealmAccountPage); + + // Logout and assert user can login with valid credential + accountTotpSetupPage.logout(); + loginPage.login("otp1", "pass"); + loginTotpPage.login(totp.generateTOTP(totpSecret)); + assertCurrentUrlStartsWith(testRealmAccountPage); + + // Delete OTP credential in account console + accountTotpSetupPage.removeTotp(); + accountTotpSetupPage.logout(); + + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(false); + + // Assert user can login without OTP + loginSuccessAndLogout("otp1", "pass"); + } + + @Test + public void testDisableCredentialsInUserStorage() { + String userId = addUserAndResetPassword("otp1", "pass"); + getCleanup().addUserId(userId); + + // Setup OTP for the user + setupOTPForUserWithRequiredAction(userId); + + // Assert user has OTP in the userStorage + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(true); + + UserResource user = testRealmResource().users().get(userId); + + // Disable OTP credential for the user through REST endpoint + UserRepresentation userRep = user.toRepresentation(); + Assert.assertNames(userRep.getDisableableCredentialTypes(), CredentialModel.OTP); + + user.disableCredentialType(Collections.singletonList(CredentialModel.OTP)); + + // User don't have OTP credential in userStorage anymore + assertUserDontHaveDBCredentials(); + assertUserHasOTPCredentialInUserStorage(false); + + // Assert user can login without OTP + loginSuccessAndLogout("otp1", "pass"); + } + + // return created totpSecret + private String setupOTPForUserWithRequiredAction(String userId) { + // Add required action to the user to reset OTP + UserResource user = testRealmResource().users().get(userId); + UserRepresentation userRep = user.toRepresentation(); + userRep.setRequiredActions(Arrays.asList(UserModel.RequiredAction.CONFIGURE_TOTP.toString())); + user.update(userRep); + + // Login as the user and setup OTP + testRealmAccountPage.navigateTo(); + loginPage.login("otp1", "pass"); + + configureTotpRequiredActionPage.assertCurrent(); + String totpSecret = configureTotpRequiredActionPage.getTotpSecret(); + configureTotpRequiredActionPage.configure(totp.generateTOTP(totpSecret)); + assertCurrentUrlStartsWith(testRealmAccountPage); + + // Logout + testRealmAccountPage.logOut(); + + return totpSecret; + } + + + private void assertUserDontHaveDBCredentials() { + testingClient.server().run(session -> { + RealmModel realm1 = session.realms().getRealmByName("test"); + UserModel user1 = session.users().getUserByUsername("otp1", realm1); + List keycloakDBCredentials = session.userCredentialManager().getStoredCredentials(realm1, user1); + Assert.assertTrue(keycloakDBCredentials.isEmpty()); + }); + } + + private void assertUserHasOTPCredentialInUserStorage(boolean expectedUserHasOTP) { + boolean hasUserOTP = testingClient.server().fetch(session -> { + BackwardsCompatibilityUserStorageFactory storageFactory = (BackwardsCompatibilityUserStorageFactory) session.getKeycloakSessionFactory() + .getProviderFactory(UserStorageProvider.class, BackwardsCompatibilityUserStorageFactory.PROVIDER_ID); + return storageFactory.hasUserOTP("otp1"); + }, Boolean.class); + Assert.assertEquals(expectedUserHasOTP, hasUserOTP); + } +} 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 909d4e118b..e54b06c89a 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 @@ -1554,7 +1554,7 @@ 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. +supported-user-storage-credential-types.tooltip=Credential types, which are provided by User Storage Provider and which are configured for this user. 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.