Avoid regenerating the totpSecret on every reload of the OTP configuration page

Using an auth note to store the totpSecret and passing its value in the TotpBean constructor to keep the totpSecret on page reload

Closes #26052

Signed-off-by: graziang <g.graziano94@gmail.com>
This commit is contained in:
graziang 2024-02-21 15:13:27 +01:00 committed by Marek Posolda
parent 35537d6289
commit cecce40aa5
5 changed files with 25 additions and 2 deletions

View file

@ -165,4 +165,6 @@ public final class Constants {
public static final String SESSION_NOTE_LIGHTWEIGHT_USER = "keycloak.userModel";
public static final String USE_LIGHTWEIGHT_ACCESS_TOKEN_ENABLED = "client.use.lightweight.access.token.enabled";
public static final String TOTP_SECRET_KEY = "TOTP_SECRET_KEY";
}

View file

@ -33,6 +33,7 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.OTPPolicy;
import org.keycloak.models.UserModel;
import org.keycloak.models.Constants;
import org.keycloak.models.credential.OTPCredentialModel;
import org.keycloak.models.utils.CredentialValidation;
import org.keycloak.models.utils.FormMessage;
@ -118,6 +119,7 @@ public class UpdateTotp implements RequiredActionProvider, RequiredActionFactory
context.challenge(challenge);
return;
}
context.getAuthenticationSession().removeAuthNote(Constants.TOTP_SECRET_KEY);
context.success();
}

View file

@ -238,7 +238,9 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
switch (page) {
case LOGIN_CONFIG_TOTP:
attributes.put("totp", new TotpBean(session, realm, user, getTotpUriBuilder()));
TotpBean totpBean = new TotpBean(session, realm, user, getTotpUriBuilder(), authenticationSession.getAuthNote(Constants.TOTP_SECRET_KEY));
authenticationSession.setAuthNote(Constants.TOTP_SECRET_KEY, totpBean.getTotpSecret());
attributes.put("totp", totpBean);
break;
case LOGIN_RECOVERY_AUTHN_CODES_CONFIG:
attributes.put("recoveryAuthnCodesConfigBean", new RecoveryAuthnCodesBean());

View file

@ -50,6 +50,10 @@ public class TotpBean {
private final UserModel user;
public TotpBean(KeycloakSession session, RealmModel realm, UserModel user, UriBuilder uriBuilder) {
this(session, realm, user, uriBuilder, null);
}
public TotpBean(KeycloakSession session, RealmModel realm, UserModel user, UriBuilder uriBuilder, String secret) {
this.session = session;
this.realm = realm;
this.user = user;
@ -61,7 +65,11 @@ public class TotpBean {
} else {
otpCredentials = Collections.EMPTY_LIST;
}
this.totpSecret = HmacOTP.generateSecret(20);
if (secret == null) {
this.totpSecret = HmacOTP.generateSecret(20);
} else {
this.totpSecret = secret;
}
this.totpSecretEncoded = TotpUtils.encode(totpSecret);
this.totpSecretQrCode = TotpUtils.qrCode(totpSecret, realm, user);

View file

@ -160,6 +160,15 @@ public class RequiredActionTotpSetupTest extends AbstractTestRealmKeycloakTest {
// KEYCLOAK-11753 - Verify OTP label element present on "Configure OTP" required action form
driver.findElement(By.id("userLabel"));
String totpSecret = totpPage.getTotpSecret();
//submit with wrong otp
totpPage.configure("wrongOtp");
totpPage.assertCurrent();
//assert totpSecret doesn't change after a wrong submit
assertEquals(totpSecret, totpPage.getTotpSecret());
totpPage.configure(totp.generateTOTP(totpPage.getTotpSecret()));
String authSessionId = events.expectRequiredAction(EventType.UPDATE_TOTP).user(userId).detail(Details.USERNAME, "setuptotp").assertEvent()