From 0fcf5d39366e62fc402cc5666397f120710969ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Barto=C5=A1?= Date: Wed, 24 Aug 2022 16:48:26 +0200 Subject: [PATCH] Reuse of token in TOTP is possible Fixes #13607 --- .../idm/RealmRepresentation.java | 10 +- .../org/keycloak/models/jpa/RealmAdapter.java | 2 + .../datastore/LegacyExportImportManager.java | 2 +- .../realm/entity/HotRodOTPPolicyEntity.java | 2 + .../map/datastore/MapExportImportManager.java | 2 +- .../map/realm/entity/MapOTPPolicyEntity.java | 12 +++ .../models/utils/ModelToRepresentation.java | 1 + .../java/org/keycloak/models/OTPPolicy.java | 19 ++++ .../credential/OTPCredentialProvider.java | 20 +++- .../updaters/RealmAttributeUpdater.java | 37 +++++++ .../testsuite/AbstractKeycloakTest.java | 20 +++- .../account/custom/CustomAuthFlowOTPTest.java | 72 +++++++++++++- .../AppInitiatedActionTotpSetupTest.java | 8 +- .../actions/RequiredActionTotpSetupTest.java | 37 ++++++- .../testsuite/admin/realm/RealmTest.java | 38 +++++-- .../broker/AbstractAdvancedBrokerTest.java | 8 ++ .../testsuite/broker/KcOidcBrokerTest.java | 91 +++++++++-------- .../KcOidcFirstBrokerLoginNewAuthTest.java | 5 + .../storage/UserStorageOTPTest.java | 99 ++++++++++--------- .../testsuite/forms/BruteForceTest.java | 3 + .../forms/LevelOfAssuranceFlowTest.java | 17 ++++ .../testsuite/forms/LoginTotpTest.java | 7 +- .../otppolicy/OTPPolicyForm.java | 18 +++- .../console/authentication/OTPPolicyTest.java | 15 ++- .../messages/admin-messages_en.properties | 2 + .../admin/resources/partials/otp-policy.html | 10 ++ 26 files changed, 434 insertions(+), 123 deletions(-) diff --git a/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java index 7c820bc7eb..7a9300ed90 100755 --- a/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/RealmRepresentation.java @@ -24,7 +24,6 @@ import org.jboss.logging.Logger; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.util.JsonSerialization; -import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; @@ -122,6 +121,7 @@ public class RealmRepresentation { protected Integer otpPolicyDigits; protected Integer otpPolicyLookAheadWindow; protected Integer otpPolicyPeriod; + protected Boolean otpPolicyCodeReusable; protected List otpSupportedApplications; // WebAuthn 2-factor properties below @@ -1025,6 +1025,14 @@ public class RealmRepresentation { this.otpSupportedApplications = otpSupportedApplications; } + public Boolean isOtpPolicyCodeReusable() { + return otpPolicyCodeReusable; + } + + public void setOtpPolicyCodeReusable(Boolean isCodeReusable) { + this.otpPolicyCodeReusable = isCodeReusable; + } + // WebAuthn 2-factor properties below public String getWebAuthnPolicyRpEntityName() { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 41fe39dd30..7eb9afeece 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -894,6 +894,7 @@ public class RealmAdapter implements LegacyRealmModel, JpaModel { otpPolicy.setLookAheadWindow(realm.getOtpPolicyLookAheadWindow()); otpPolicy.setType(realm.getOtpPolicyType()); otpPolicy.setPeriod(realm.getOtpPolicyPeriod()); + otpPolicy.setCodeReusable(getAttribute(OTPPolicy.REALM_REUSABLE_CODE_ATTRIBUTE, OTPPolicy.DEFAULT_IS_REUSABLE)); } return otpPolicy; } @@ -906,6 +907,7 @@ public class RealmAdapter implements LegacyRealmModel, JpaModel { realm.setOtpPolicyLookAheadWindow(policy.getLookAheadWindow()); realm.setOtpPolicyType(policy.getType()); realm.setOtpPolicyPeriod(policy.getPeriod()); + setAttribute(OTPPolicy.REALM_REUSABLE_CODE_ATTRIBUTE, policy.isCodeReusable()); em.flush(); } diff --git a/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java b/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java index 19245e82bd..9c2ad4227d 100644 --- a/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java +++ b/model/legacy-private/src/main/java/org/keycloak/storage/datastore/LegacyExportImportManager.java @@ -1374,8 +1374,8 @@ public class LegacyExportImportManager implements ExportImportManager { if (rep.getOtpPolicyAlgorithm() != null) policy.setAlgorithm(rep.getOtpPolicyAlgorithm()); if (rep.getOtpPolicyDigits() != null) policy.setDigits(rep.getOtpPolicyDigits()); if (rep.getOtpPolicyPeriod() != null) policy.setPeriod(rep.getOtpPolicyPeriod()); + if (rep.isOtpPolicyCodeReusable() != null) policy.setCodeReusable(rep.isOtpPolicyCodeReusable()); return policy; - } diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/realm/entity/HotRodOTPPolicyEntity.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/realm/entity/HotRodOTPPolicyEntity.java index 793b2a6084..1fd0bc72d5 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/realm/entity/HotRodOTPPolicyEntity.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/realm/entity/HotRodOTPPolicyEntity.java @@ -20,6 +20,8 @@ public class HotRodOTPPolicyEntity extends AbstractHotRodEntity { public String otpPolicyAlgorithm; @ProtoField(number = 6) public String otpPolicyType; + @ProtoField(number = 7) + public Boolean otpPolicyCodeReusable; @Override public boolean equals(Object o) { return HotRodOTPPolicyEntityDelegate.entityEquals(this, o); diff --git a/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java b/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java index 2c3756e78c..4463e1dc2b 100644 --- a/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java +++ b/model/map/src/main/java/org/keycloak/models/map/datastore/MapExportImportManager.java @@ -1280,8 +1280,8 @@ public class MapExportImportManager implements ExportImportManager { if (rep.getOtpPolicyAlgorithm() != null) policy.setAlgorithm(rep.getOtpPolicyAlgorithm()); if (rep.getOtpPolicyDigits() != null) policy.setDigits(rep.getOtpPolicyDigits()); if (rep.getOtpPolicyPeriod() != null) policy.setPeriod(rep.getOtpPolicyPeriod()); + if (rep.isOtpPolicyCodeReusable() != null) policy.setCodeReusable(rep.isOtpPolicyCodeReusable()); return policy; - } diff --git a/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapOTPPolicyEntity.java b/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapOTPPolicyEntity.java index e93266a7b6..cf90610dba 100644 --- a/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapOTPPolicyEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/realm/entity/MapOTPPolicyEntity.java @@ -34,22 +34,31 @@ public interface MapOTPPolicyEntity extends UpdatableEntity { entity.setOtpPolicyLookAheadWindow(model.getLookAheadWindow()); entity.setOtpPolicyType(model.getType()); entity.setOtpPolicyPeriod(model.getPeriod()); + entity.setOtpPolicyCodeReusable(model.isCodeReusable()); return entity; } static OTPPolicy toModel(MapOTPPolicyEntity entity) { if (entity == null) return null; OTPPolicy model = new OTPPolicy(); + Integer otpPolicyDigits = entity.getOtpPolicyDigits(); model.setDigits(otpPolicyDigits == null ? 0 : otpPolicyDigits); model.setAlgorithm(entity.getOtpPolicyAlgorithm()); + Integer otpPolicyInitialCounter = entity.getOtpPolicyInitialCounter(); model.setInitialCounter(otpPolicyInitialCounter == null ? 0 : otpPolicyInitialCounter); + Integer otpPolicyLookAheadWindow = entity.getOtpPolicyLookAheadWindow(); model.setLookAheadWindow(otpPolicyLookAheadWindow == null ? 0 : otpPolicyLookAheadWindow); model.setType(entity.getOtpPolicyType()); + Integer otpPolicyPeriod = entity.getOtpPolicyPeriod(); model.setPeriod(otpPolicyPeriod == null ? 0 : otpPolicyPeriod); + + Boolean isOtpPolicyReusable = entity.isOtpPolicyCodeReusable(); + model.setCodeReusable(isOtpPolicyReusable == null ? OTPPolicy.DEFAULT_IS_REUSABLE : isOtpPolicyReusable); + return model; } @@ -70,4 +79,7 @@ public interface MapOTPPolicyEntity extends UpdatableEntity { String getOtpPolicyAlgorithm(); void setOtpPolicyAlgorithm(String otpPolicyAlgorithm); + + Boolean isOtpPolicyCodeReusable(); + void setOtpPolicyCodeReusable(Boolean isOtpPolicyCodeReusable); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 2e5a1369fc..81a72bba7b 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -418,6 +418,7 @@ public class ModelToRepresentation { rep.setOtpPolicyType(otpPolicy.getType()); rep.setOtpPolicyLookAheadWindow(otpPolicy.getLookAheadWindow()); rep.setOtpSupportedApplications(otpPolicy.getSupportedApplications()); + rep.setOtpPolicyCodeReusable(otpPolicy.isCodeReusable()); WebAuthnPolicy webAuthnPolicy = realm.getWebAuthnPolicy(); rep.setWebAuthnPolicyRpEntityName(webAuthnPolicy.getRpEntityName()); diff --git a/server-spi/src/main/java/org/keycloak/models/OTPPolicy.java b/server-spi/src/main/java/org/keycloak/models/OTPPolicy.java index b163cc8007..f70f88e960 100755 --- a/server-spi/src/main/java/org/keycloak/models/OTPPolicy.java +++ b/server-spi/src/main/java/org/keycloak/models/OTPPolicy.java @@ -44,6 +44,7 @@ public class OTPPolicy implements Serializable { protected int digits; protected int lookAheadWindow; protected int period; + protected boolean isCodeReusable; private static final Map algToKeyUriAlg = new HashMap<>(); @@ -59,15 +60,24 @@ public class OTPPolicy implements Serializable { } public OTPPolicy(String type, String algorithm, int initialCounter, int digits, int lookAheadWindow, int period) { + this(type, algorithm, initialCounter, digits, lookAheadWindow, period, DEFAULT_IS_REUSABLE); + } + + public OTPPolicy(String type, String algorithm, int initialCounter, int digits, int lookAheadWindow, int period, boolean isCodeReusable) { this.type = type; this.algorithm = algorithm; this.initialCounter = initialCounter; this.digits = digits; this.lookAheadWindow = lookAheadWindow; this.period = period; + this.isCodeReusable = isCodeReusable; } public static OTPPolicy DEFAULT_POLICY = new OTPPolicy(OTPCredentialModel.TOTP, HmacOTP.HMAC_SHA1, 0, 6, 1, 30); + public static final boolean DEFAULT_IS_REUSABLE = false; + + // Realm attributes + public static final String REALM_REUSABLE_CODE_ATTRIBUTE = "realmReusableOtpCode"; public String getAlgorithmKey() { return algToKeyUriAlg.containsKey(algorithm) ? algToKeyUriAlg.get(algorithm) : algorithm; @@ -121,8 +131,17 @@ public class OTPPolicy implements Serializable { this.period = period; } + public boolean isCodeReusable() { + return isCodeReusable; + } + + public void setCodeReusable(boolean isReusable) { + isCodeReusable = isReusable; + } + /** * Constructs the otpauth:// URI based on the Key-Uri-Format. + * * @param realm * @param user * @param secret diff --git a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java index f2c79ef2c0..0f73630974 100644 --- a/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java +++ b/services/src/main/java/org/keycloak/credential/OTPCredentialProvider.java @@ -19,14 +19,15 @@ 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.credential.OTPCredentialModel; -import org.keycloak.models.credential.dto.OTPCredentialData; -import org.keycloak.models.credential.dto.OTPSecretData; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OTPPolicy; import org.keycloak.models.RealmModel; +import org.keycloak.models.SingleUseObjectProvider; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; +import org.keycloak.models.credential.OTPCredentialModel; +import org.keycloak.models.credential.dto.OTPCredentialData; +import org.keycloak.models.credential.dto.OTPSecretData; import org.keycloak.models.utils.HmacOTP; import org.keycloak.models.utils.TimeBasedOTP; @@ -99,6 +100,7 @@ public class OTPCredentialProvider implements CredentialProvider authExec.getDisplayName().equals("Browser - Conditional OTP")); + testRealmAccountManagementPage.navigateTo(); + testRealmLoginPage.form().login(testUser); + assertTrue(loginConfigTotpPage.isCurrent()); + + //configure OTP for test user + testRealmAccountManagementPage.navigateTo(); + testRealmLoginPage.form().login(testUser); + + final String totpSecret = testRealmLoginPage.form().totpForm().getTotpSecret(); + assertThat(totpSecret, notNullValue()); + + final String generatedOtp = totp.generateTOTP(totpSecret); + assertThat(generatedOtp, notNullValue()); + + testRealmLoginPage.form().totpForm().setTotp(generatedOtp); + testRealmLoginPage.form().totpForm().submit(); + testRealmAccountManagementPage.signOut(); + + testRealmAccountManagementPage.navigateTo(); + testRealmLoginPage.form().login(testUser); + + loginTotpPage.assertCurrent(); + loginTotpPage.login(generatedOtp); + + assertThat(loginTotpPage.isCurrent(), is(!allowReusingExistingOtp)); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + @Test @AuthServerContainerExclude(AuthServer.REMOTE) public void conditionalOTPNoDefault() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java index ceb277942f..36dae497bb 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/AppInitiatedActionTotpSetupTest.java @@ -363,9 +363,11 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT events.expectLogout(authSessionId).assertEvent(); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + loginPage.open(); loginPage.login("test-user@localhost", "password"); - + loginTotpPage.login(totp.generateTOTP(totpSecret)); events.expectLogin().assertEvent(); @@ -412,6 +414,8 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT assertTrue(loginPage.isCurrent()); Assert.assertFalse(totpPage.isCurrent()); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + // Login with one-time password loginTotpPage.login(totp.generateTOTP(totpCode)); @@ -472,6 +476,8 @@ public class AppInitiatedActionTotpSetupTest extends AbstractAppInitiatedActionT events.expectLogout(loginEvent.getSessionId()).assertEvent(); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, timeBased); + loginPage.open(); loginPage.login("test-user@localhost", "password"); String src = driver.getPageSource(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java index 0a6f63275e..4051c75838 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionTotpSetupTest.java @@ -47,14 +47,15 @@ import org.keycloak.testsuite.pages.LoginConfigTotpPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginTotpPage; import org.keycloak.testsuite.pages.RegisterPage; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.UserBuilder; import org.openqa.selenium.By; +import java.io.IOException; import java.util.LinkedList; import java.util.List; -import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -281,6 +282,8 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { String customOtpLabel = "my-custom-otp-label"; + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + // Set OTP label to a custom value totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret()), customOtpLabel); @@ -325,7 +328,18 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { } @Test - public void setupTotpExisting() { + public void setupTotpExistingReusableCodeEnabled() throws IOException { + try (RealmAttributeUpdater rau = new RealmAttributeUpdater(testRealm()).setOtpPolicyCodeReusable(true).update()) { + setupTotpExisting(true); + } + } + + @Test + public void setupTotpExistingReusableCodeDisabled() { + setupTotpExisting(false); // Default value + } + + public void setupTotpExisting(boolean reusableCodesEnabled) { loginPage.open(); loginPage.login("test-user@localhost", "password"); @@ -348,14 +362,21 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { events.expectLogout(authSessionId).assertEvent(); + if (!reusableCodesEnabled) { + setTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS); + } + loginPage.open(); loginPage.login("test-user@localhost", "password"); String src = driver.getPageSource(); loginTotpPage.login(totp.generateTOTP(totpSecret)); - assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); - - events.expectLogin().assertEvent(); + if (!reusableCodesEnabled) { + loginTotpPage.assertCurrent(); + } else { + assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + events.expectLogin().assertEvent(); + } } //KEYCLOAK-15511 @@ -409,6 +430,8 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { oauth.idTokenHint(tokenResponse.getIdToken()).openLogout(); events.expectLogout(loginEvent.getSessionId()).user(userId).assertEvent(); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + // Try to login after logout loginPage.open(); loginPage.login("setupTotp2", "password2"); @@ -437,6 +460,8 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { accountTotpPage.logout(); events.expectLogout(loginEvent.getSessionId()).user(userId).detail(Details.REDIRECT_URI, oauth.AUTH_SERVER_ROOT + "/realms/test/account/totp").assertEvent(); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + // Try to login loginPage.open(); loginPage.login("setupTotp2", "password2"); @@ -489,6 +514,8 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest { events.expectLogout(loginEvent.getSessionId()).assertEvent(); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, timeBased); + loginPage.open(); loginPage.login("test-user@localhost", "password"); String src = driver.getPageSource(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java index 26c3c96e0e..c4f8eee99e 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/realm/RealmTest.java @@ -17,7 +17,9 @@ package org.keycloak.testsuite.admin.realm; +import com.google.common.collect.Sets; import org.apache.commons.io.IOUtils; +import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; import org.junit.Assume; import org.junit.Rule; @@ -35,6 +37,7 @@ import org.keycloak.events.log.JBossLoggingEventListenerProviderFactory; import org.keycloak.models.CibaConfig; import org.keycloak.models.Constants; import org.keycloak.models.OAuth2DeviceConfig; +import org.keycloak.models.OTPPolicy; import org.keycloak.models.ParConfig; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.protocol.oidc.OIDCLoginProtocol; @@ -59,6 +62,7 @@ import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude.A import org.keycloak.testsuite.auth.page.AuthRealm; import org.keycloak.testsuite.client.KeycloakTestingClient; import org.keycloak.testsuite.events.TestEventsListenerProviderFactory; +import org.keycloak.testsuite.model.StoreProvider; import org.keycloak.testsuite.runonserver.RunHelpers; import org.keycloak.testsuite.updaters.Creator; import org.keycloak.testsuite.util.AdminEventPaths; @@ -74,16 +78,24 @@ import javax.ws.rs.BadRequestException; import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Response; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.keycloak.testsuite.util.ServerURLs.getAuthServerContextRoot; @@ -192,11 +204,23 @@ public class RealmTest extends AbstractAdminTest { CibaConfig.CIBA_AUTH_REQUESTED_USER_HINT).stream().forEach(i -> rep2.getAttributes().remove(i)); } - Map attributes = rep2.getAttributes(); - assertTrue("Attributes expected to be present oauth2DeviceCodeLifespan, oauth2DevicePollingInterval, found: " + String.join(", ", attributes.keySet()), - attributes.size() == 3 && attributes.containsKey(OAuth2DeviceConfig.OAUTH2_DEVICE_CODE_LIFESPAN) - && attributes.containsKey(OAuth2DeviceConfig.OAUTH2_DEVICE_POLLING_INTERVAL) - && attributes.containsKey(ParConfig.PAR_REQUEST_URI_LIFESPAN)); + Set attributesKeys = rep2.getAttributes().keySet(); + + int expectedAttributesCount = 3; + final Set expectedAttributes = Sets.newHashSet( + OAuth2DeviceConfig.OAUTH2_DEVICE_CODE_LIFESPAN, + OAuth2DeviceConfig.OAUTH2_DEVICE_POLLING_INTERVAL, + ParConfig.PAR_REQUEST_URI_LIFESPAN + ); + + // This attribute is represented in Legacy store as attribute and for Map store as a field + if (!StoreProvider.getCurrentProvider().isMapStore()) { + expectedAttributes.add(OTPPolicy.REALM_REUSABLE_CODE_ATTRIBUTE); + expectedAttributesCount++; + } + + assertThat(attributesKeys.size(), CoreMatchers.is(expectedAttributesCount)); + assertThat(attributesKeys, CoreMatchers.is(expectedAttributes)); } finally { adminClient.realm("attributes").remove(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java index a3b296a55b..d0267c8c9e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractAdvancedBrokerTest.java @@ -8,6 +8,8 @@ import org.keycloak.common.Profile; import org.keycloak.common.util.Time; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.models.IdentityProviderSyncMode; +import org.keycloak.models.OTPPolicy; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; @@ -473,6 +475,8 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { assertNumFederatedIdentities(realm.users().search(bc.getUserLogin()).get(0).getId(), 1); logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + logInWithBroker(bc); loginTotpPage.assertCurrent(); @@ -512,6 +516,8 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { assertNumFederatedIdentities(realm.users().search(bc.getUserLogin()).get(0).getId(), 1); logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + logInWithBroker(bc); loginTotpPage.assertCurrent(); @@ -531,6 +537,8 @@ public abstract class AbstractAdvancedBrokerTest extends AbstractBrokerTest { String userId = ApiUtil.findUserByUsername(realm, bc.getUserLogin()).getId(); realm.attackDetection().clearBruteForceForUser(userId); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + loginTotpPage.login(totp.generateTOTP(totpSecret)); waitForAccountManagementTitle(); logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java index 8b5fdf6478..17749e1136 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java @@ -5,6 +5,7 @@ import com.google.common.collect.Lists; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.hamcrest.Matchers; +import org.junit.Before; import org.junit.Test; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientsResource; @@ -21,20 +22,18 @@ import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.IdentityProviderMapperSyncMode; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderSyncMode; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.protocol.oidc.OIDCConfigAttributes; -import org.keycloak.protocol.oidc.representations.OIDCConfigurationRepresentation; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.representations.idm.ClientRepresentation; -import org.keycloak.representations.idm.ErrorRepresentation; import org.keycloak.representations.idm.IdentityProviderMapperRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; -import org.keycloak.services.Urls; import org.keycloak.testsuite.Assert; -import org.keycloak.testsuite.admin.ApiUtil; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.WaitUtils; @@ -51,6 +50,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThat; +import static org.keycloak.models.utils.TimeBasedOTP.DEFAULT_INTERVAL_SECONDS; import static org.keycloak.testsuite.admin.ApiUtil.removeUserByUsername; import static org.keycloak.testsuite.broker.BrokerRunOnServerUtil.configurePostBrokerLoginWithOTP; import static org.keycloak.testsuite.broker.BrokerTestConstants.REALM_PROV_NAME; @@ -69,12 +69,17 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest { return KcOidcBrokerConfiguration.INSTANCE; } + @Before + public void setUpTotp() { + totp = new TimeBasedOTP(); + } + @Override protected Iterable createIdentityProviderMappers(IdentityProviderMapperSyncMode syncMode) { IdentityProviderMapperRepresentation attrMapper1 = new IdentityProviderMapperRepresentation(); attrMapper1.setName("manager-role-mapper"); attrMapper1.setIdentityProviderMapper(ExternalKeycloakRoleToRoleMapper.PROVIDER_ID); - attrMapper1.setConfig(ImmutableMap.builder() + attrMapper1.setConfig(ImmutableMap.builder() .put(IdentityProviderMapperModel.SYNC_MODE, syncMode.toString()) .put("external.role", ROLE_MANAGER) .put("role", ROLE_MANAGER) @@ -258,6 +263,8 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest { totpPage.configure(totp.generateTOTP(totpSecret)); logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + setOtpTimeOffset(DEFAULT_INTERVAL_SECONDS, totp); + logInWithBroker(bc); waitForPage(driver, "account already exists", false); @@ -320,48 +327,54 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest { */ @Test public void testReauthenticationBothBrokersWithOTPRequired() throws Exception { - KcSamlBrokerConfiguration samlBrokerConfig = KcSamlBrokerConfiguration.INSTANCE; - ClientRepresentation samlClient = samlBrokerConfig.createProviderClients().get(0); - IdentityProviderRepresentation samlBroker = samlBrokerConfig.setUpIdentityProvider(); - RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName()); + final RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName()); + final RealmResource providerRealm = adminClient.realm(bc.providerRealmName()); - try { - updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); - adminClient.realm(bc.providerRealmName()).clients().create(samlClient); - consumerRealm.identityProviders().create(samlBroker); + try (RealmAttributeUpdater rauConsumer = new RealmAttributeUpdater(consumerRealm).setOtpPolicyCodeReusable(true).update(); + RealmAttributeUpdater rauProvider = new RealmAttributeUpdater(providerRealm).setOtpPolicyCodeReusable(true).update()) { - driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); - testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(samlBrokerConfig.getIDPAlias())); - logInWithBroker(samlBrokerConfig); - totpPage.assertCurrent(); - String totpSecret = totpPage.getTotpSecret(); - totpPage.configure(totp.generateTOTP(totpSecret)); - logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + KcSamlBrokerConfiguration samlBrokerConfig = KcSamlBrokerConfiguration.INSTANCE; + ClientRepresentation samlClient = samlBrokerConfig.createProviderClients().get(0); + IdentityProviderRepresentation samlBroker = samlBrokerConfig.setUpIdentityProvider(); - testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(bc.getIDPAlias())); - logInWithBroker(bc); + try { + updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin); + providerRealm.clients().create(samlClient); + consumerRealm.identityProviders().create(samlBroker); - waitForPage(driver, "account already exists", false); - idpConfirmLinkPage.assertCurrent(); - idpConfirmLinkPage.clickLinkAccount(); - loginPage.clickSocial(samlBrokerConfig.getIDPAlias()); + driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName())); + testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(samlBrokerConfig.getIDPAlias())); + logInWithBroker(samlBrokerConfig); + totpPage.assertCurrent(); + String totpSecret = totpPage.getTotpSecret(); + totpPage.configure(totp.generateTOTP(totpSecret)); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); - loginTotpPage.assertCurrent(); - loginTotpPage.login(totp.generateTOTP(totpSecret)); - logoutFromRealm(getProviderRoot(), bc.providerRealmName()); - logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); + testingClient.server(bc.consumerRealmName()).run(configurePostBrokerLoginWithOTP(bc.getIDPAlias())); + logInWithBroker(bc); - logInWithBroker(bc); + waitForPage(driver, "account already exists", false); + idpConfirmLinkPage.assertCurrent(); + idpConfirmLinkPage.clickLinkAccount(); + loginPage.clickSocial(samlBrokerConfig.getIDPAlias()); - loginTotpPage.assertCurrent(); - loginTotpPage.login(totp.generateTOTP(totpSecret)); - waitForAccountManagementTitle(); - accountUpdateProfilePage.assertCurrent(); + loginTotpPage.assertCurrent(); + loginTotpPage.login(totp.generateTOTP(totpSecret)); + logoutFromRealm(getProviderRoot(), bc.providerRealmName()); + logoutFromRealm(getConsumerRoot(), bc.consumerRealmName()); - assertNumFederatedIdentities(consumerRealm.users().search(samlBrokerConfig.getUserLogin()).get(0).getId(), 2); - } finally { - updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin); - removeUserByUsername(consumerRealm, "consumer"); + logInWithBroker(bc); + + loginTotpPage.assertCurrent(); + loginTotpPage.login(totp.generateTOTP(totpSecret)); + waitForAccountManagementTitle(); + accountUpdateProfilePage.assertCurrent(); + + assertNumFederatedIdentities(consumerRealm.users().search(samlBrokerConfig.getUserLogin()).get(0).getId(), 2); + } finally { + updateExecutions(AbstractBrokerTest::setUpMissingUpdateProfileOnFirstLogin); + removeUserByUsername(consumerRealm, "consumer"); + } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java index fdcc59bf3e..32221002ca 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcFirstBrokerLoginNewAuthTest.java @@ -6,6 +6,7 @@ import org.junit.Test; import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.UserResource; import org.keycloak.models.UserModel; +import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.admin.ApiUtil; @@ -81,6 +82,8 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr String consumerRealmUserId = createUser("consumer"); String totpSecret = addTOTPToUser("consumer"); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + loginWithBrokerAndConfirmLinkAccount(); // Login with password @@ -164,6 +167,8 @@ public class KcOidcFirstBrokerLoginNewAuthTest extends AbstractInitializedBaseBr String consumerRealmUserId = createUser("consumer"); String totpSecret = addTOTPToUser("consumer"); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + loginWithBrokerAndConfirmLinkAccount(); // Assert that user can see credentials combobox. Password and OTP are available credentials. Password should be selected. 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 index 103aef3814..eafb244f81 100644 --- 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 @@ -53,6 +53,7 @@ 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.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.UserBuilder; import static org.keycloak.storage.UserStorageProviderModel.IMPORT_ENABLED; @@ -146,64 +147,66 @@ public class UserStorageOTPTest extends AbstractTestRealmKeycloakTest { @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); + public void testUpdateOTP() throws IOException { + try (RealmAttributeUpdater rau = new RealmAttributeUpdater(testRealm()).setOtpPolicyCodeReusable(true).update()) { + // 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); + // 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(); + // 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.getInputCodeError()); + // Dummy OTP code won't work when configure new OTP + loginConfigTotpPage.configure(DummyUserFederationProvider.HARDCODED_OTP); + Assert.assertEquals("Invalid authenticator code.", loginConfigTotpPage.getInputCodeError()); - // 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); + // 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(); + appPage.assertCurrent(); - // Logout - events.expect(EventType.UPDATE_TOTP).user(userRep.getId()).assertEvent(); //remove the UPDATE_TOTP event - EventRepresentation loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); - String idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); - appPage.logout(idTokenHint); - events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); + // Logout + events.expect(EventType.UPDATE_TOTP).user(userRep.getId()).assertEvent(); //remove the UPDATE_TOTP event + EventRepresentation loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); + String idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); + appPage.logout(idTokenHint); + events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); - // 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); + // 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(); - loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); - idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); - appPage.logout(idTokenHint); - events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); + appPage.assertCurrent(); + loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); + idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); + appPage.logout(idTokenHint); + events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); - // 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)); + // 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(); - loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); - idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); - appPage.logout(idTokenHint); - events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); + appPage.assertCurrent(); + loginEvent = events.expectLogin().user(userRep.getId()).assertEvent(); + idTokenHint = sendTokenRequestAndGetResponse(loginEvent).getIdToken(); + appPage.logout(idTokenHint); + events.expectLogout(loginEvent.getSessionId()).user(userRep.getId()).assertEvent(); + } } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java index faf17641e8..29aa4b565b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BruteForceTest.java @@ -53,6 +53,7 @@ import org.keycloak.testsuite.util.UserBuilder; import javax.mail.internet.MimeMessage; import java.net.MalformedURLException; +import java.util.Calendar; import java.util.Collections; import java.util.Map; @@ -112,6 +113,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { testRealm.setMaxDeltaTimeSeconds(20); testRealm.setMaxFailureWaitSeconds(100); testRealm.setWaitIncrementSeconds(5); + testRealm.setOtpPolicyCodeReusable(true); //testRealm.setQuickLoginCheckMilliSeconds(0L); userId = user.getId(); @@ -130,6 +132,7 @@ public class BruteForceTest extends AbstractTestRealmKeycloakTest { realm.setMaxDeltaTimeSeconds(20); realm.setMaxFailureWaitSeconds(100); realm.setWaitIncrementSeconds(5); + realm.setOtpPolicyCodeReusable(true); adminClient.realm("test").update(realm); } catch (Exception e) { throw new RuntimeException(e); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java index c52029b680..7e509b25c0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java @@ -27,6 +27,7 @@ import javax.ws.rs.BadRequestException; import javax.ws.rs.core.UriBuilder; import org.jboss.arquillian.graphene.page.Page; import org.junit.After; +import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; import org.junit.Rule; @@ -62,11 +63,13 @@ import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginTotpPage; import org.keycloak.testsuite.pages.PushTheButtonPage; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.FlowUtil; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmBuilder; import org.keycloak.testsuite.util.RealmRepUtil; import org.keycloak.testsuite.util.UserBuilder; +import org.keycloak.testsuite.util.WaitUtils; import org.keycloak.util.JsonSerialization; import static org.hamcrest.CoreMatchers.is; @@ -100,6 +103,7 @@ public class LevelOfAssuranceFlowTest extends AbstractTestRealmKeycloakTest { @Override public void configureTestRealm(RealmRepresentation testRealm) { try { + testRealm.setOtpPolicyCodeReusable(true); findTestApp(testRealm).setAttributes(Collections.singletonMap(Constants.ACR_LOA_MAP, getAcrToLoaMappingForClient())); UserRepresentation user = RealmRepUtil.findUser(testRealm, "test-user@localhost"); UserBuilder.edit(user) @@ -121,6 +125,19 @@ public class LevelOfAssuranceFlowTest extends AbstractTestRealmKeycloakTest { @Before public void setupFlow() { configureStepUpFlow(testingClient); + canBeOtpCodesReusable(true); + } + + @After + public void tearDown() { + canBeOtpCodesReusable(false); + } + + // Fixing this test with not reusable OTP codes would bring additional unwanted workarounds; not scope of this test + private void canBeOtpCodesReusable(boolean state) { + new RealmAttributeUpdater(testRealm()) + .setOtpPolicyCodeReusable(state) + .update(); } public static void configureStepUpFlow(KeycloakTestingClient testingClient) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java index 49bbaa5d60..c43d81e837 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/LoginTotpTest.java @@ -24,6 +24,7 @@ import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.events.Details; import org.keycloak.models.Constants; +import org.keycloak.models.OTPPolicy; import org.keycloak.models.utils.TimeBasedOTP; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -33,11 +34,13 @@ import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.LoginTotpPage; +import org.keycloak.testsuite.updaters.RealmAttributeUpdater; import org.keycloak.testsuite.util.AdminClientUtil; import org.keycloak.testsuite.util.GreenMailRule; import org.keycloak.testsuite.util.OAuthClient; import org.keycloak.testsuite.util.RealmRepUtil; import org.keycloak.testsuite.util.UserBuilder; +import org.keycloak.testsuite.util.WaitUtils; import javax.ws.rs.client.Client; import javax.ws.rs.client.Entity; @@ -132,6 +135,8 @@ public class LoginTotpTest extends AbstractTestRealmKeycloakTest { Assert.assertTrue(loginTotpPage.isCurrent()); + setOtpTimeOffset(TimeBasedOTP.DEFAULT_INTERVAL_SECONDS, totp); + loginTotpPage.login(totp.generateTOTP("totpSecret")); Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); @@ -199,7 +204,7 @@ public class LoginTotpTest extends AbstractTestRealmKeycloakTest { @Test public void loginWithTotp_getToken_checkCompatibilityCLI() throws IOException { Client httpClient = AdminClientUtil.createResteasyClient(); - try { + try (RealmAttributeUpdater rau = new RealmAttributeUpdater(testRealm()).setOtpPolicyCodeReusable(true).update()) { WebTarget exchangeUrl = httpClient.target(OAuthClient.AUTH_SERVER_ROOT) .path("/realms") .path(TEST) diff --git a/testsuite/integration-arquillian/tests/other/old-admin-console/src/main/java/org/keycloak/testsuite/console/page/authentication/otppolicy/OTPPolicyForm.java b/testsuite/integration-arquillian/tests/other/old-admin-console/src/main/java/org/keycloak/testsuite/console/page/authentication/otppolicy/OTPPolicyForm.java index 4e135f0e97..37f93c362e 100644 --- a/testsuite/integration-arquillian/tests/other/old-admin-console/src/main/java/org/keycloak/testsuite/console/page/authentication/otppolicy/OTPPolicyForm.java +++ b/testsuite/integration-arquillian/tests/other/old-admin-console/src/main/java/org/keycloak/testsuite/console/page/authentication/otppolicy/OTPPolicyForm.java @@ -21,6 +21,8 @@ */ package org.keycloak.testsuite.console.page.authentication.otppolicy; +import org.keycloak.models.OTPPolicy; +import org.keycloak.testsuite.console.page.fragment.OnOffSwitch; import org.keycloak.testsuite.page.Form; import org.keycloak.testsuite.util.UIUtils; import org.openqa.selenium.WebElement; @@ -47,22 +49,30 @@ public class OTPPolicyForm extends Form { @FindBy(id = "lookAround") private WebElement lookAround; - + @FindBy(id = "period") private WebElement period; - + @FindBy(id = "counter") private WebElement counter; - + + @FindBy(xpath = ".//div[@class='onoffswitch' and ./input[@id='reusableCode']]") + private OnOffSwitch reusableCode; + public void setValues(OTPType otpType, OTPHashAlg otpHashAlg, Digits digits, String lookAheadOrAround, String periodOrCounter) { + setValues(otpType, otpHashAlg, digits, lookAheadOrAround, periodOrCounter, OTPPolicy.DEFAULT_IS_REUSABLE); + } + + public void setValues(OTPType otpType, OTPHashAlg otpHashAlg, Digits digits, String lookAheadOrAround, String periodOrCounter, boolean isReusableCode) { this.otpType.selectByValue(otpType.getName()); this.otpHashAlg.selectByValue(otpHashAlg.getName()); this.digits.selectByVisibleText("" + digits.getName()); - + switch (otpType) { case TIME_BASED: UIUtils.setTextInputValue(this.lookAround, lookAheadOrAround); UIUtils.setTextInputValue(period, periodOrCounter); + reusableCode.setOn(isReusableCode); break; case COUNTER_BASED: UIUtils.setTextInputValue(this.lookAhead, lookAheadOrAround); diff --git a/testsuite/integration-arquillian/tests/other/old-admin-console/src/test/java/org/keycloak/testsuite/console/authentication/OTPPolicyTest.java b/testsuite/integration-arquillian/tests/other/old-admin-console/src/test/java/org/keycloak/testsuite/console/authentication/OTPPolicyTest.java index dea138375f..6c46051998 100644 --- a/testsuite/integration-arquillian/tests/other/old-admin-console/src/test/java/org/keycloak/testsuite/console/authentication/OTPPolicyTest.java +++ b/testsuite/integration-arquillian/tests/other/old-admin-console/src/test/java/org/keycloak/testsuite/console/authentication/OTPPolicyTest.java @@ -32,7 +32,9 @@ import org.keycloak.testsuite.console.page.authentication.otppolicy.OTPPolicyFor import org.keycloak.testsuite.console.page.authentication.otppolicy.OTPPolicyForm.OTPType; import org.keycloak.testsuite.util.WaitUtils; -import static org.junit.Assert.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; /** * @@ -60,14 +62,17 @@ public class OTPPolicyTest extends AbstractConsoleTest { assertEquals(Integer.valueOf(8), realm.getOtpPolicyDigits()); assertEquals(Integer.valueOf(10), realm.getOtpPolicyLookAheadWindow()); assertEquals(Integer.valueOf(50), realm.getOtpPolicyInitialCounter()); - - otpPolicyPage.form().setValues(OTPType.TIME_BASED, OTPHashAlg.SHA512, Digits.EIGHT, "10", "40"); + assertThat(realm.isOtpPolicyCodeReusable(), is(org.keycloak.models.OTPPolicy.DEFAULT_IS_REUSABLE)); + + otpPolicyPage.form().setValues(OTPType.TIME_BASED, OTPHashAlg.SHA512, Digits.EIGHT, "10", "40", false); assertAlertSuccess(); - + realm = testRealmResource().toRepresentation(); assertEquals("totp", realm.getOtpPolicyType()); assertEquals(Integer.valueOf(40), realm.getOtpPolicyPeriod()); - } + + assertThat(realm.isOtpPolicyCodeReusable(), is(false)); + } @Test public void invalidValuesTest() { 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 179180f478..f32ccb4fc8 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 @@ -1371,6 +1371,8 @@ otp-token-period=OTP Token Period otp-token-period.tooltip=How many seconds should an OTP token be valid? Defaults to 30 seconds. otp-supported-applications=Supported Applications otp-supported-applications.tooltip=Applications that are known to work with the current OTP policy +otp-reusable-code=Reusable token +otp-reusable-code.tooltip=Possibility to use the same OTP code again after successful authentication. loa-level=Level of Authentication loa-level.tooltip=Sets the Level of Authentication to the specified value. loa-max-age=Max Age diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/otp-policy.html b/themes/src/main/resources/theme/base/admin/resources/partials/otp-policy.html index e44afa91ad..59037a12a2 100755 --- a/themes/src/main/resources/theme/base/admin/resources/partials/otp-policy.html +++ b/themes/src/main/resources/theme/base/admin/resources/partials/otp-policy.html @@ -81,6 +81,16 @@ {{:: 'otp-supported-applications.tooltip' | translate}} +
+ +
+
+ +
+
+ {{:: 'otp-reusable-code.tooltip' | translate}} +
+