From ce57aada935d7e4ec675292bf2f8d7c3447c6b81 Mon Sep 17 00:00:00 2001 From: Dmitriy Kanaev Date: Fri, 4 Sep 2026 11:52:05 +0300 Subject: [PATCH 1/5] Redmine #167449: auto-set OTP type google_auth when OTP becomes mandatory Mandatory OTP (is_otp_optional='N') with otp_type=none was silently ignored at SSO login, so the flag alone did not enforce OTP. Now the invariant "OTP mandatory => type is not none" is maintained at write time in the service layer, covering both the admin UI and the Remote API without touching the authentication code: - UserServiceImpl.createUser/updateUser: assign google_auth when OTP is mandatory and the type is none/null, with an AUTO_SET_OTP_TYPE audit event - UserServiceImpl.updateUserIsOtpOptionalValue: same normalization after making OTP mandatory - UserServiceImpl.updateUserOtpType: reject unknown type codes (previously they silently reset the type) and reject 'none' for a user whose OTP is mandatory (SsoConflictException) - Create/EditUserPage: when the "Is OTP optional" checkbox is cleared and no type is chosen, the OTP type dropdown switches to Google Auth with an info message Existing DB records are intentionally not migrated: they get normalized as they are saved. Co-Authored-By: Claude Fable 5 --- .../payneteasy/superfly/api/SSOService.java | 14 ++ .../service/impl/UserServiceImpl.java | 33 ++- .../service/impl/UserServiceImplTest.java | 191 ++++++++++++++++++ .../service/impl/UserServiceLoggingTest.java | 4 + .../web/wicket/SuperflyApplication.properties | 3 +- .../web/wicket/page/user/CreateUserPage.java | 18 +- .../web/wicket/page/user/EditUserPage.java | 20 +- .../src/main/resources/release-notes.xml | 14 ++ 8 files changed, 291 insertions(+), 6 deletions(-) diff --git a/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/SSOService.java b/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/SSOService.java index 68dab1997..8a59a2097 100644 --- a/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/SSOService.java +++ b/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/SSOService.java @@ -57,6 +57,13 @@ public interface SSOService { */ List getUsersWithActions(GetUsersWithActionsRequest request); + /** + * Updates the OTP type of a user. + *

+ * An unknown OTP type code is rejected. Setting type 'none' for a user whose OTP + * is mandatory (is_otp_optional = false) is rejected with a conflict error: make + * OTP optional first. + */ void updateUserOtpType(UpdateUserOtpTypeRequest request); /** @@ -108,6 +115,13 @@ void registerUser(UserRegisterRequest request) */ String getUrlToGoogleAuthQrCode(GetGoogleAuthQrCodeRequest request); + /** + * Updates the "is OTP optional" flag of a user. + *

+ * When OTP is made mandatory (isOtpOptional = false) and the user's OTP type is + * 'none' or not set, the OTP type is automatically set to 'google_auth', so this + * call may change not only the flag. + */ void updateUserIsOtpOptionalValue(UpdateUserIsOtpOptionalValueRequest request); /** diff --git a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java index a19eef32f..de8bd41b2 100644 --- a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java +++ b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java @@ -2,6 +2,7 @@ import com.payneteasy.superfly.api.OTPType; import com.payneteasy.superfly.api.exceptions.PolicyValidationException; +import com.payneteasy.superfly.api.exceptions.SsoConflictException; import com.payneteasy.superfly.api.exceptions.SsoDecryptException; import com.payneteasy.superfly.dao.DaoConstants; import com.payneteasy.superfly.dao.UserDao; @@ -47,6 +48,8 @@ public class UserServiceImpl implements UserService { private static final Logger logger = LoggerFactory.getLogger(UserServiceImpl.class); + private static final OTPType DEFAULT_MANDATORY_OTP_TYPE = OTPType.GOOGLE_AUTH; + private UserDao userDao; private NotificationService notificationService; private LoggerSink loggerSink; @@ -165,6 +168,7 @@ public UIUserDetails getUser(long userId) { public RoutineResult updateUser(UIUser user) { UIUserForCreate userForDao = new UIUserForCreate(); copyUserAndEncryptPassword(user, userForDao); + assignDefaultOtpTypeIfOtpMandatory(userForDao); // password and salt are not updated here RoutineResult result = userDao.updateUser(userForDao); loggerSink.info(logger, "UPDATE_USER", result.isOk(), user.getUsername()); @@ -462,6 +466,7 @@ public String getUserSalt(String userName) { @Override public RoutineResult createUser(UIUserForCreate user) { + assignDefaultOtpTypeIfOtpMandatory(user); return userDao.createUser(user); } @@ -556,12 +561,38 @@ public void clearHOTPLoginsFailed(String username) { @Override public void updateUserOtpType(String username, String otpType) { - userDao.updateUserOtpType(username,otpType); + OTPType newOtpType = otpType == null ? OTPType.NONE : OTPType.strictFromCode(otpType); + if (newOtpType == OTPType.NONE) { + UserForDescription user = userDao.getUserForDescription(username); + if (user != null && !user.isOtpOptional()) { + throw new SsoConflictException("Cannot set OTP type 'none' for user '" + username + + "': OTP is mandatory for this user, make OTP optional first"); + } + } + userDao.updateUserOtpType(username, newOtpType.code()); } @Override public void updateUserIsOtpOptionalValue(String username, boolean isOtpOptional) { userDao.updateUserIsOtpOptionalValue(username,isOtpOptional); + if (!isOtpOptional) { + UserForDescription user = userDao.getUserForDescription(username); + if (user != null && user.getOtpType() == OTPType.NONE) { + userDao.updateUserOtpType(username, DEFAULT_MANDATORY_OTP_TYPE.code()); + loggerSink.info(logger, "AUTO_SET_OTP_TYPE", true, username); + } + } + } + + /** + * Authentication code enforces mandatory OTP only when the user has a concrete OTP type, + * so a user with mandatory OTP must never remain with type 'none'/null. + */ + private void assignDefaultOtpTypeIfOtpMandatory(UIUser user) { + if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { + user.setOtpType(DEFAULT_MANDATORY_OTP_TYPE.code()); + loggerSink.info(logger, "AUTO_SET_OTP_TYPE", true, user.getUsername()); + } } @Override diff --git a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java index 778a479b6..2a93e0000 100644 --- a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java +++ b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java @@ -1,6 +1,8 @@ package com.payneteasy.superfly.service.impl; +import com.payneteasy.superfly.api.OTPType; import com.payneteasy.superfly.api.exceptions.MessageSendException; +import com.payneteasy.superfly.api.exceptions.SsoConflictException; import com.payneteasy.superfly.dao.UserDao; import com.payneteasy.superfly.lockout.LockoutStrategy; import com.payneteasy.superfly.model.LockoutType; @@ -9,6 +11,7 @@ import com.payneteasy.superfly.model.ui.user.UICloneUserRequest; import com.payneteasy.superfly.model.ui.user.UIUser; import com.payneteasy.superfly.model.ui.user.UIUserForCreate; +import com.payneteasy.superfly.model.ui.user.UserForDescription; import com.payneteasy.superfly.password.ConstantSaltSource; import com.payneteasy.superfly.password.MessageDigestPasswordEncoder; import com.payneteasy.superfly.password.PlaintextPasswordEncoder; @@ -26,6 +29,7 @@ import static org.easymock.EasyMock.anyObject; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; public class UserServiceImplTest { @@ -166,6 +170,193 @@ public void testGetUserLoginStatusTemp() { EasyMock.verify(userDao); } + @Test + public void testUpdateUserIsOtpOptionalValueMandatorySetsDefaultOtpType() { + userDao.updateUserIsOtpOptionalValue("pete", false); + EasyMock.expect(userDao.getUserForDescription("pete")).andReturn(userForDescription("none", false)); + userDao.updateUserOtpType("pete", "google_auth"); + EasyMock.replay(userDao); + + userService.updateUserIsOtpOptionalValue("pete", false); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserIsOtpOptionalValueMandatoryKeepsExistingOtpType() { + userDao.updateUserIsOtpOptionalValue("pete", false); + EasyMock.expect(userDao.getUserForDescription("pete")).andReturn(userForDescription("google_auth", false)); + EasyMock.replay(userDao); + + userService.updateUserIsOtpOptionalValue("pete", false); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserIsOtpOptionalValueOptionalDoesNotTouchOtpType() { + userDao.updateUserIsOtpOptionalValue("pete", true); + EasyMock.replay(userDao); + + userService.updateUserIsOtpOptionalValue("pete", true); + + EasyMock.verify(userDao); + } + + @Test + public void testCreateUserWithMandatoryOtpGetsDefaultOtpType() { + EasyMock.expect(userDao.createUser(anyObject(UIUserForCreate.class))).andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUserForCreate user = (UIUserForCreate) EasyMock.getCurrentArguments()[0]; + assertEquals("google_auth", user.getOtpType()); + user.setId(1L); + return RoutineResult.okResult(); + } + }); + EasyMock.replay(userDao); + + UIUserForCreate user = new UIUserForCreate(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpType(null); + user.setOtpOptional(false); + userService.createUser(user, "subsystem"); + + EasyMock.verify(userDao); + } + + @Test + public void testCreateUserWithOptionalOtpKeepsNoneOtpType() { + EasyMock.expect(userDao.createUser(anyObject(UIUserForCreate.class))).andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUserForCreate user = (UIUserForCreate) EasyMock.getCurrentArguments()[0]; + assertEquals(OTPType.NONE.code(), user.getOtpType()); + user.setId(1L); + return RoutineResult.okResult(); + } + }); + EasyMock.replay(userDao); + + UIUserForCreate user = new UIUserForCreate(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpOptional(true); + userService.createUser(user, "subsystem"); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserWithMandatoryOtpGetsDefaultOtpType() { + EasyMock.expect(userDao.updateUser(anyObject(UIUser.class))).andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUser user = (UIUser) EasyMock.getCurrentArguments()[0]; + assertEquals("google_auth", user.getOtpType()); + return RoutineResult.okResult(); + } + }); + EasyMock.replay(userDao); + + UIUser user = new UIUser(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpType("none"); + user.setOtpOptional(false); + userService.updateUser(user); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserWithOptionalOtpKeepsNoneOtpType() { + EasyMock.expect(userDao.updateUser(anyObject(UIUser.class))).andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUser user = (UIUser) EasyMock.getCurrentArguments()[0]; + assertEquals("none", user.getOtpType()); + return RoutineResult.okResult(); + } + }); + EasyMock.replay(userDao); + + UIUser user = new UIUser(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpType("none"); + user.setOtpOptional(true); + userService.updateUser(user); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserWithMandatoryOtpKeepsChosenOtpType() { + EasyMock.expect(userDao.updateUser(anyObject(UIUser.class))).andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUser user = (UIUser) EasyMock.getCurrentArguments()[0]; + assertEquals("google_auth", user.getOtpType()); + return RoutineResult.okResult(); + } + }); + EasyMock.replay(userDao); + + UIUser user = new UIUser(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpType("google_auth"); + user.setOtpOptional(false); + userService.updateUser(user); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserOtpTypeNoneRejectedWhenOtpMandatory() { + EasyMock.expect(userDao.getUserForDescription("pete")).andReturn(userForDescription("none", false)); + EasyMock.replay(userDao); + + assertThrows(SsoConflictException.class, () -> userService.updateUserOtpType("pete", "none")); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserOtpTypeNoneAllowedWhenOtpOptional() { + EasyMock.expect(userDao.getUserForDescription("pete")).andReturn(userForDescription("google_auth", true)); + userDao.updateUserOtpType("pete", "none"); + EasyMock.replay(userDao); + + userService.updateUserOtpType("pete", "none"); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserOtpTypeUnknownCodeRejected() { + EasyMock.replay(userDao); + + assertThrows(IllegalStateException.class, () -> userService.updateUserOtpType("pete", "garbage")); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserOtpTypeGoogleAuth() { + userDao.updateUserOtpType("pete", "google_auth"); + EasyMock.replay(userDao); + + userService.updateUserOtpType("pete", "google_auth"); + + EasyMock.verify(userDao); + } + + private static UserForDescription userForDescription(String otpTypeCode, boolean isOtpOptional) { + UserForDescription user = new UserForDescription(); + user.setUsername("pete"); + user.setOtpTypeCode(otpTypeCode); + user.setOtpOptional(isOtpOptional); + return user; + } + private static class TestLockoutStrategy implements LockoutStrategy { public String username; public LockoutType lockoutType; diff --git a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java index ce2ef7ce5..6c2a38786 100644 --- a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java +++ b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java @@ -62,6 +62,7 @@ public RoutineResult answer() throws Throwable { UIUserForCreate user = new UIUserForCreate(); user.setUsername("test-user"); + user.setOtpOptional(true); userService.createUser(user, "subsystem"); EasyMock.verify(loggerSink); @@ -82,6 +83,7 @@ public RoutineResult answer() throws Throwable { UIUserForCreate user = new UIUserForCreate(); user.setUsername("test-user"); + user.setOtpOptional(true); userService.createUser(user, "subsystem"); EasyMock.verify(loggerSink); @@ -96,6 +98,7 @@ public void testUpdateUser() throws Exception { UIUser user = new UIUser(); user.setUsername("test-user"); + user.setOtpOptional(true); userService.updateUser(user); EasyMock.verify(loggerSink); @@ -110,6 +113,7 @@ public void testUpdateUserFail() throws Exception { UIUser user = new UIUser(); user.setUsername("test-user"); + user.setOtpOptional(true); userService.updateUser(user); EasyMock.verify(loggerSink); diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication.properties b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication.properties index db285ca2b..9e55643a4 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication.properties +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication.properties @@ -113,4 +113,5 @@ smtpServer.ssl=SSL # OTP TYPE # otpType.none=None -otpType.google_auth=Google Auth \ No newline at end of file +otpType.google_auth=Google Auth +user.otpTypeAutoSet=OTP type was automatically set to Google Auth \ No newline at end of file diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java index 13a4190d2..bd94fc605 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java @@ -142,9 +142,23 @@ protected void onUpdate(AjaxRequestTarget target) { form.add(new LabelTextFieldRow(user,"organization","user.create.organization", false)); - form.add(new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender())); + final LabelDropDownChoiceRow otpTypeRow = + new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender()); + otpTypeRow.setOutputMarkupId(true); + form.add(otpTypeRow); - form.add(new LabelCheckBoxRow("isOtpOptional", user, "user.create.isOtpOptional")); + LabelCheckBoxRow isOtpOptionalRow = new LabelCheckBoxRow("isOtpOptional", user, "user.create.isOtpOptional"); + isOtpOptionalRow.getCheckBox().add(new OnChangeAjaxBehavior() { + @Override + protected void onUpdate(AjaxRequestTarget target) { + if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { + user.setOtpType(OTPType.GOOGLE_AUTH.code()); + info(getString("user.otpTypeAutoSet")); + target.add(otpTypeRow, getFeedbackPanel()); + } + } + }); + form.add(isOtpOptionalRow); form.add(new Button("add") { @Override diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java index a73170f57..571b73807 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java @@ -11,6 +11,8 @@ import com.payneteasy.superfly.web.wicket.page.BasePage; import com.payneteasy.superfly.web.wicket.validation.PublicKeyValidator; import org.apache.wicket.Page; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.ajax.form.OnChangeAjaxBehavior; import org.apache.wicket.markup.html.form.ChoiceRenderer; import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.IChoiceRenderer; @@ -92,9 +94,23 @@ protected void onSubmit() { form.add(new LabelTextFieldRow(user,"organization","user.create.organization", false)); - form.add(new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender())); + final LabelDropDownChoiceRow otpTypeRow = + new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender()); + otpTypeRow.setOutputMarkupId(true); + form.add(otpTypeRow); - form.add(new LabelCheckBoxRow( "isOtpOptional",user, "user.create.isOtpOptional")); + LabelCheckBoxRow isOtpOptionalRow = new LabelCheckBoxRow("isOtpOptional", user, "user.create.isOtpOptional"); + isOtpOptionalRow.getCheckBox().add(new OnChangeAjaxBehavior() { + @Override + protected void onUpdate(AjaxRequestTarget target) { + if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { + user.setOtpType(OTPType.GOOGLE_AUTH.code()); + info(getString("user.otpTypeAutoSet")); + target.add(otpTypeRow, getFeedbackPanel()); + } + } + }); + form.add(isOtpOptionalRow); form.add(new BookmarkablePageLink("cancel", ListUsersPage.class)); } diff --git a/superfly-web/src/main/resources/release-notes.xml b/superfly-web/src/main/resources/release-notes.xml index 4362db884..8d25f6ce7 100644 --- a/superfly-web/src/main/resources/release-notes.xml +++ b/superfly-web/src/main/resources/release-notes.xml @@ -1,5 +1,19 @@ + + + OTP type is set automatically when OTP becomes mandatory + + When the "is OTP optional" flag is cleared for a user whose OTP type + is 'none' or not set, the type is automatically set to Google Auth + (both in the admin UI and in the service layer, so the Remote API is + covered too). Setting OTP type 'none' for a user with mandatory OTP + is now rejected with a conflict error, and an unknown OTP type code + is rejected instead of silently resetting the type. + + + + UI design and usability From f2ff843528f18b2d43ed62ff6cb29765e267ea0e Mon Sep 17 00:00:00 2001 From: Dmitriy Kanaev Date: Fri, 4 Sep 2026 12:10:20 +0300 Subject: [PATCH 2/5] Redmine #167449: default "Is OTP optional" to checked on the create user form UIUser.isOtpOptional defaults to false, so an untouched create form used to produce a user with mandatory OTP - and now that would also auto-set the google_auth type, sending every new user to Google Authenticator binding. Default the form to "optional", matching the DB column default. Co-Authored-By: Claude Fable 5 --- .../superfly/web/wicket/page/user/CreateUserPage.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java index bd94fc605..b8284afc8 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java @@ -54,6 +54,8 @@ public class CreateUserPage extends BasePage { public CreateUserPage() { super(ListUsersPage.class); final UIUserCheckPassword user = new UIUserCheckPassword(); + // matches the DB default 'Y', so an untouched form does not create a user with mandatory OTP + user.setOtpOptional(true); List listSub = subsystemService.getSubsystemsForFilter(); for (UISubsystemForFilter sub : listSub) { From 34b215a1fe26261d473bc161e192f63828132612 Mon Sep 17 00:00:00 2001 From: Dmitriy Kanaev Date: Fri, 4 Sep 2026 13:40:06 +0300 Subject: [PATCH 3/5] Redmine #167449: WicketTester coverage for OTP auto-set on user pages Renders CreateUserPage/EditUserPage with mocked Spring beans (no DB) and drives the "Is OTP optional" checkbox via an Ajax change event: - EditUserPage: clearing the flag auto-sets google_auth when the type is none, keeps an already-chosen type, and leaves the type alone when the flag stays optional - CreateUserPage: the flag is checked (OTP optional) by default, and clearing it auto-sets google_auth Co-Authored-By: Claude Fable 5 --- .../wicket/page/user/CreateUserPageTest.java | 102 ++++++++++++++++ .../wicket/page/user/EditUserPageTest.java | 113 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java create mode 100644 superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java diff --git a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java new file mode 100644 index 000000000..0b8d885c9 --- /dev/null +++ b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java @@ -0,0 +1,102 @@ +package com.payneteasy.superfly.web.wicket.page.user; + +import com.payneteasy.superfly.api.OTPType; +import com.payneteasy.superfly.crypto.PublicKeyCrypto; +import com.payneteasy.superfly.service.RoleService; +import com.payneteasy.superfly.service.SettingsService; +import com.payneteasy.superfly.service.SubsystemService; +import com.payneteasy.superfly.service.UserService; +import com.payneteasy.superfly.web.wicket.page.AbstractPageTest; +import org.apache.wicket.markup.html.form.CheckBox; +import org.apache.wicket.markup.html.form.DropDownChoice; +import org.apache.wicket.util.tester.FormTester; +import org.easymock.EasyMock; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContextHolder; + +import java.util.Collections; + +import static org.easymock.EasyMock.expect; +import static org.junit.Assert.assertEquals; + +public class CreateUserPageTest extends AbstractPageTest { + + private static final String OTP_TYPE_PATH = "form:otpType:container:field-id"; + private static final String OTP_OPTIONAL_PATH = "form:isOtpOptional:field-id"; + + private UserService userService; + private RoleService roleService; + private SubsystemService subsystemService; + private SettingsService settingsService; + private PublicKeyCrypto crypto; + + @Before + public void setUpBeans() { + userService = EasyMock.createNiceMock(UserService.class); + roleService = EasyMock.createNiceMock(RoleService.class); + subsystemService = EasyMock.createMock(SubsystemService.class); + settingsService = EasyMock.createNiceMock(SettingsService.class); + crypto = EasyMock.createNiceMock(PublicKeyCrypto.class); + + expect(subsystemService.getSubsystemsForFilter()).andReturn(Collections.emptyList()); + + SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken( + "admin", "admin", AuthorityUtils.createAuthorityList("ROLE_ADMIN"))); + } + + @After + public void tearDown() { + SecurityContextHolder.clearContext(); + } + + @Override + protected Object getBean(Class type) { + if (type == UserService.class) { + return userService; + } + if (type == RoleService.class) { + return roleService; + } + if (type == SubsystemService.class) { + return subsystemService; + } + if (type == SettingsService.class) { + return settingsService; + } + if (type == PublicKeyCrypto.class) { + return crypto; + } + return super.getBean(type); + } + + private void startPage() { + EasyMock.replay(userService, roleService, subsystemService, settingsService, crypto); + tester.getApplication().getResourceSettings().setThrowExceptionOnMissingResource(false); + tester.startPage(CreateUserPage.class); + tester.assertRenderedPage(CreateUserPage.class); + } + + @Test + public void otpIsOptionalByDefaultOnTheCreateForm() { + startPage(); + + CheckBox checkBox = (CheckBox) tester.getComponentFromLastRenderedPage(OTP_OPTIONAL_PATH); + assertEquals(Boolean.TRUE, checkBox.getDefaultModelObject()); + } + + @Test + public void clearingOptionalAutoSetsGoogleAuth() { + startPage(); + + FormTester formTester = tester.newFormTester("form"); + formTester.setValue("isOtpOptional:field-id", false); + tester.executeAjaxEvent(OTP_OPTIONAL_PATH, "change"); + + DropDownChoice otpType = (DropDownChoice) tester.getComponentFromLastRenderedPage(OTP_TYPE_PATH); + assertEquals(OTPType.GOOGLE_AUTH.code(), otpType.getDefaultModelObject()); + } +} diff --git a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java new file mode 100644 index 000000000..2a653000b --- /dev/null +++ b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java @@ -0,0 +1,113 @@ +package com.payneteasy.superfly.web.wicket.page.user; + +import com.payneteasy.superfly.api.OTPType; +import com.payneteasy.superfly.crypto.PublicKeyCrypto; +import com.payneteasy.superfly.model.ui.user.UIUserDetails; +import com.payneteasy.superfly.service.SettingsService; +import com.payneteasy.superfly.service.UserService; +import com.payneteasy.superfly.web.wicket.page.AbstractPageTest; +import org.apache.wicket.util.tester.FormTester; +import org.apache.wicket.request.mapper.parameter.PageParameters; +import org.easymock.EasyMock; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.AuthorityUtils; +import org.springframework.security.core.context.SecurityContextHolder; + +import static org.easymock.EasyMock.expect; +import static org.junit.Assert.assertEquals; + +public class EditUserPageTest extends AbstractPageTest { + + private static final String OTP_TYPE_PATH = "form:otpType:container:field-id"; + private static final String OTP_OPTIONAL_PATH = "form:isOtpOptional:field-id"; + + private UserService userService; + private SettingsService settingsService; + private PublicKeyCrypto crypto; + + @Before + public void setUpBeans() { + userService = EasyMock.createMock(UserService.class); + settingsService = EasyMock.createNiceMock(SettingsService.class); + crypto = EasyMock.createNiceMock(PublicKeyCrypto.class); + + // BasePage renders the logged-in user name from the security context + SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken( + "admin", "admin", AuthorityUtils.createAuthorityList("ROLE_ADMIN"))); + } + + @After + public void tearDown() { + SecurityContextHolder.clearContext(); + } + + @Override + protected Object getBean(Class type) { + if (type == UserService.class) { + return userService; + } + if (type == SettingsService.class) { + return settingsService; + } + if (type == PublicKeyCrypto.class) { + return crypto; + } + return super.getBean(type); + } + + private void startPageForUser(String otpTypeCode, boolean isOtpOptional) { + UIUserDetails user = new UIUserDetails(); + user.setId(1L); + user.setUsername("john"); + user.setOtpType(otpTypeCode); + user.setOtpOptional(isOtpOptional); + expect(userService.getUser(1L)).andReturn(user); + EasyMock.replay(userService, settingsService, crypto); + + // MockApplication does not load the app-level SuperflyApplication.properties bundle, + // and this test only cares about the OTP wiring, not label texts + tester.getApplication().getResourceSettings().setThrowExceptionOnMissingResource(false); + tester.startPage(EditUserPage.class, new PageParameters().add("userId", 1L)); + tester.assertRenderedPage(EditUserPage.class); + } + + private void setOtpOptionalAndFireAjax(boolean optional) { + FormTester formTester = tester.newFormTester("form"); + formTester.setValue("isOtpOptional:field-id", optional); + tester.executeAjaxEvent(OTP_OPTIONAL_PATH, "change"); + } + + private Object renderedOtpType() { + return tester.getComponentFromLastRenderedPage(OTP_TYPE_PATH).getDefaultModelObject(); + } + + @Test + public void makingOtpMandatoryAutoSetsGoogleAuthWhenTypeIsNone() { + startPageForUser(OTPType.NONE.code(), true); + + setOtpOptionalAndFireAjax(false); + + assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + } + + @Test + public void makingOtpMandatoryKeepsAlreadyChosenType() { + startPageForUser(OTPType.GOOGLE_AUTH.code(), true); + + setOtpOptionalAndFireAjax(false); + + assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + } + + @Test + public void keepingOtpOptionalDoesNotChangeType() { + startPageForUser(OTPType.NONE.code(), false); + + setOtpOptionalAndFireAjax(true); + + assertEquals(OTPType.NONE.code(), renderedOtpType()); + } +} From 1d8d36cf47d2f7747857c8f03ca04dd40880f53d Mon Sep 17 00:00:00 2001 From: Dmitriy Kanaev Date: Fri, 4 Sep 2026 22:34:47 +0300 Subject: [PATCH 4/5] Redmine #167449: address code review findings on the OTP auto-set change Remote API contract: - register SsoConflictException and SsoBadRequestException in ExceptionSerializationHelper. The server answers a thrown service exception with HTTP 202 + ExceptionWrapper, so the client rebuilt an unregistered class as an anonymous SsoException and a `catch (SsoConflictException)` never matched, contradicting the javadoc added for updateUserOtpType. - normalize OTP type codes in one place: an absent or blank code means "no OTP" (it used to clear the type before this branch), an unknown code is rejected with SsoBadRequestException instead of a raw IllegalStateException, and createUser/updateUser now use the same parsing as updateUserOtpType instead of the lenient fromCode. Audit trail: - log AUTO_SET_OTP_TYPE after the DAO call with its actual result, as every other event in the class does. It was logged as a success before the insert/update ran, so a duplicate username left an audit line claiming an OTP type was set on a user that was never created. Admin UI: - extract the auto-set rule into OtpTypeAutoSetBehavior and attach it to the OTP type drop down as well. An ajax request submits only the component it originates from, so the check box behavior decided on a stale OTP type: picking "None" and then clearing "Is OTP optional" showed None + mandatory, and the service silently rewrote it to google_auth on save. - clear the drop down input before repainting it, otherwise the value the component received (a previous failed submit, or the choice just overridden) wins over the model and the select keeps showing None. - add the missing Russian translation for user.otpTypeAutoSet. Tests: AUTO_SET_OTP_TYPE is now pinned by strict-mock logging tests (including the failed-DAO case), and the page tests assert the ajax repaint and the info message, not only the model - verified by removing each of them and watching the tests fail. Co-Authored-By: Claude Opus 5 (1M context) --- .../ExceptionSerializationHelper.java | 2 + .../service/impl/UserServiceImpl.java | 41 ++++++++-- .../service/impl/UserServiceImplTest.java | 29 ++++++- .../service/impl/UserServiceLoggingTest.java | 77 +++++++++++++++++++ .../web/wicket/SuperflyApplication_ru.xml | 1 + .../component/otp/OtpTypeAutoSetBehavior.java | 56 ++++++++++++++ .../web/wicket/page/user/CreateUserPage.java | 13 +--- .../web/wicket/page/user/EditUserPage.java | 15 +--- .../wicket/page/user/CreateUserPageTest.java | 23 ++++++ .../wicket/page/user/EditUserPageTest.java | 63 +++++++++++++++ 10 files changed, 290 insertions(+), 30 deletions(-) create mode 100644 superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java diff --git a/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/serialization/ExceptionSerializationHelper.java b/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/serialization/ExceptionSerializationHelper.java index 36d4f3ae9..49cb164a8 100644 --- a/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/serialization/ExceptionSerializationHelper.java +++ b/superfly-remote-api/src/main/java/com/payneteasy/superfly/api/serialization/ExceptionSerializationHelper.java @@ -26,6 +26,8 @@ public class ExceptionSerializationHelper { registerExceptionClass(SsoUserException.class); registerExceptionClass(SsoSystemException.class); registerExceptionClass(SsoDataException.class); + registerExceptionClass(SsoBadRequestException.class); + registerExceptionClass(SsoConflictException.class); } /** diff --git a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java index de8bd41b2..65c3a87d2 100644 --- a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java +++ b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java @@ -2,6 +2,7 @@ import com.payneteasy.superfly.api.OTPType; import com.payneteasy.superfly.api.exceptions.PolicyValidationException; +import com.payneteasy.superfly.api.exceptions.SsoBadRequestException; import com.payneteasy.superfly.api.exceptions.SsoConflictException; import com.payneteasy.superfly.api.exceptions.SsoDecryptException; import com.payneteasy.superfly.dao.DaoConstants; @@ -168,9 +169,12 @@ public UIUserDetails getUser(long userId) { public RoutineResult updateUser(UIUser user) { UIUserForCreate userForDao = new UIUserForCreate(); copyUserAndEncryptPassword(user, userForDao); - assignDefaultOtpTypeIfOtpMandatory(userForDao); + boolean otpTypeAssigned = assignDefaultOtpTypeIfOtpMandatory(userForDao); // password and salt are not updated here RoutineResult result = userDao.updateUser(userForDao); + if (otpTypeAssigned) { + loggerSink.info(logger, "AUTO_SET_OTP_TYPE", result.isOk(), user.getUsername()); + } loggerSink.info(logger, "UPDATE_USER", result.isOk(), user.getUsername()); return result; } @@ -466,8 +470,12 @@ public String getUserSalt(String userName) { @Override public RoutineResult createUser(UIUserForCreate user) { - assignDefaultOtpTypeIfOtpMandatory(user); - return userDao.createUser(user); + boolean otpTypeAssigned = assignDefaultOtpTypeIfOtpMandatory(user); + RoutineResult result = userDao.createUser(user); + if (otpTypeAssigned) { + loggerSink.info(logger, "AUTO_SET_OTP_TYPE", result.isOk(), user.getUsername()); + } + return result; } @Override @@ -561,7 +569,7 @@ public void clearHOTPLoginsFailed(String username) { @Override public void updateUserOtpType(String username, String otpType) { - OTPType newOtpType = otpType == null ? OTPType.NONE : OTPType.strictFromCode(otpType); + OTPType newOtpType = normalizeOtpType(otpType); if (newOtpType == OTPType.NONE) { UserForDescription user = userDao.getUserForDescription(username); if (user != null && !user.isOtpOptional()) { @@ -587,11 +595,30 @@ public void updateUserIsOtpOptionalValue(String username, boolean isOtpOptional) /** * Authentication code enforces mandatory OTP only when the user has a concrete OTP type, * so a user with mandatory OTP must never remain with type 'none'/null. + * + * @return true if the OTP type was assigned, so that the caller can audit it + * together with the result of the DAO call */ - private void assignDefaultOtpTypeIfOtpMandatory(UIUser user) { - if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { + private boolean assignDefaultOtpTypeIfOtpMandatory(UIUser user) { + if (!user.isOtpOptional() && normalizeOtpType(user.getOtpType()) == OTPType.NONE) { user.setOtpType(DEFAULT_MANDATORY_OTP_TYPE.code()); - loggerSink.info(logger, "AUTO_SET_OTP_TYPE", true, user.getUsername()); + return true; + } + return false; + } + + /** + * Maps an OTP type code to the enum: an absent or blank code means "no OTP", + * an unknown code is rejected instead of being silently stored as no OTP. + */ + private static OTPType normalizeOtpType(String otpTypeCode) { + if (otpTypeCode == null || otpTypeCode.trim().isEmpty()) { + return OTPType.NONE; + } + try { + return OTPType.strictFromCode(otpTypeCode); + } catch (IllegalStateException e) { + throw new SsoBadRequestException("Unknown OTP type code: '" + otpTypeCode + "'"); } } diff --git a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java index 2a93e0000..d3b0b6d7d 100644 --- a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java +++ b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceImplTest.java @@ -2,6 +2,7 @@ import com.payneteasy.superfly.api.OTPType; import com.payneteasy.superfly.api.exceptions.MessageSendException; +import com.payneteasy.superfly.api.exceptions.SsoBadRequestException; import com.payneteasy.superfly.api.exceptions.SsoConflictException; import com.payneteasy.superfly.dao.UserDao; import com.payneteasy.superfly.lockout.LockoutStrategy; @@ -334,7 +335,33 @@ public void testUpdateUserOtpTypeNoneAllowedWhenOtpOptional() { public void testUpdateUserOtpTypeUnknownCodeRejected() { EasyMock.replay(userDao); - assertThrows(IllegalStateException.class, () -> userService.updateUserOtpType("pete", "garbage")); + assertThrows(SsoBadRequestException.class, () -> userService.updateUserOtpType("pete", "garbage")); + + EasyMock.verify(userDao); + } + + @Test + public void testUpdateUserOtpTypeBlankCodeMeansNone() { + EasyMock.expect(userDao.getUserForDescription("pete")).andReturn(userForDescription("google_auth", true)); + userDao.updateUserOtpType("pete", "none"); + EasyMock.replay(userDao); + + userService.updateUserOtpType("pete", " "); + + EasyMock.verify(userDao); + } + + @Test + public void testCreateUserWithUnknownOtpTypeRejected() { + EasyMock.replay(userDao); + + UIUserForCreate user = new UIUserForCreate(); + user.setUsername("pete"); + user.setPassword("secret"); + user.setOtpType("garbage"); + user.setOtpOptional(false); + + assertThrows(SsoBadRequestException.class, () -> userService.createUser(user, "subsystem")); EasyMock.verify(userDao); } diff --git a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java index 6c2a38786..2d7e1f3b2 100644 --- a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java +++ b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java @@ -6,6 +6,7 @@ import com.payneteasy.superfly.model.ui.user.UICloneUserRequest; import com.payneteasy.superfly.model.ui.user.UIUser; import com.payneteasy.superfly.model.ui.user.UIUserForCreate; +import com.payneteasy.superfly.model.ui.user.UserForDescription; import com.payneteasy.superfly.password.NullSaltSource; import com.payneteasy.superfly.password.PlaintextPasswordEncoder; import com.payneteasy.superfly.password.SHA256RandomGUIDSaltGenerator; @@ -119,6 +120,82 @@ public void testUpdateUserFail() throws Exception { EasyMock.verify(loggerSink); } + @Test + public void testCreateUserAutoSetsOtpType() throws Exception { + userDao.createUser(anyObject(UIUserForCreate.class)); + EasyMock.expectLastCall().andAnswer(new IAnswer() { + public RoutineResult answer() throws Throwable { + UIUserForCreate user = (UIUserForCreate) EasyMock.getCurrentArguments()[0]; + user.setId(1L); + return okResult(); + } + }); + loggerSink.info(anyObject(Logger.class), eq("AUTO_SET_OTP_TYPE"), eq(true), eq("test-user")); + loggerSink.info(anyObject(Logger.class), eq("CREATE_USER"), eq(true), eq("test-user")); + EasyMock.replay(loggerSink, userDao); + + UIUserForCreate user = new UIUserForCreate(); + user.setUsername("test-user"); + user.setOtpOptional(false); + userService.createUser(user, "subsystem"); + + EasyMock.verify(loggerSink); + } + + /** + * The audit trail must not claim an OTP type was set on a user that was never created. + */ + @Test + public void testCreateUserAutoSetOtpTypeFail() throws Exception { + userDao.createUser(anyObject(UIUserForCreate.class)); + EasyMock.expectLastCall().andReturn(failureResult()); + loggerSink.info(anyObject(Logger.class), eq("AUTO_SET_OTP_TYPE"), eq(false), eq("test-user")); + loggerSink.info(anyObject(Logger.class), eq("CREATE_USER"), eq(false), eq("test-user")); + EasyMock.replay(loggerSink, userDao); + + UIUserForCreate user = new UIUserForCreate(); + user.setUsername("test-user"); + user.setOtpOptional(false); + userService.createUser(user, "subsystem"); + + EasyMock.verify(loggerSink); + } + + @Test + public void testUpdateUserAutoSetsOtpType() throws Exception { + userDao.updateUser(anyObject(UIUser.class)); + EasyMock.expectLastCall().andReturn(okResult()); + loggerSink.info(anyObject(Logger.class), eq("AUTO_SET_OTP_TYPE"), eq(true), eq("test-user")); + loggerSink.info(anyObject(Logger.class), eq("UPDATE_USER"), eq(true), eq("test-user")); + EasyMock.replay(loggerSink, userDao); + + UIUser user = new UIUser(); + user.setUsername("test-user"); + user.setOtpOptional(false); + userService.updateUser(user); + + EasyMock.verify(loggerSink); + } + + @Test + public void testUpdateUserIsOtpOptionalValueAutoSetsOtpType() throws Exception { + userDao.updateUserIsOtpOptionalValue("test-user", false); + EasyMock.expectLastCall(); + UserForDescription userForDescription = new UserForDescription(); + userForDescription.setUsername("test-user"); + userForDescription.setOtpTypeCode("none"); + userForDescription.setOtpOptional(false); + EasyMock.expect(userDao.getUserForDescription("test-user")).andReturn(userForDescription); + userDao.updateUserOtpType("test-user", "google_auth"); + EasyMock.expectLastCall(); + loggerSink.info(anyObject(Logger.class), eq("AUTO_SET_OTP_TYPE"), eq(true), eq("test-user")); + EasyMock.replay(loggerSink, userDao); + + userService.updateUserIsOtpOptionalValue("test-user", false); + + EasyMock.verify(loggerSink, userDao); + } + @Test public void testDeleteUser() throws Exception { userDao.deleteUser(anyLong()); diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication_ru.xml b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication_ru.xml index 1a1d59dec..077db08e0 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication_ru.xml +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/SuperflyApplication_ru.xml @@ -106,5 +106,6 @@ --> None Google Auth +Тип OTP автоматически установлен в Google Auth \ No newline at end of file diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java new file mode 100644 index 000000000..fcf2a542b --- /dev/null +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java @@ -0,0 +1,56 @@ +package com.payneteasy.superfly.web.wicket.component.otp; + +import com.payneteasy.superfly.api.OTPType; +import com.payneteasy.superfly.model.ui.user.UIUser; +import com.payneteasy.superfly.web.wicket.component.field.LabelDropDownChoiceRow; +import org.apache.wicket.Component; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.ajax.form.OnChangeAjaxBehavior; + +/** + * Keeps the invariant "OTP mandatory => OTP type is not none" visible in the form: + * as soon as the edited user would end up with mandatory OTP and no OTP type, the type + * is set to the default one and the administrator is told about it. + *

+ * Attach it to both the "is OTP optional" check box and the OTP type drop down: an ajax + * request submits only the component it originates from, so a behavior on the check box + * alone would decide on a stale OTP type (and vice versa). + * + * @see com.payneteasy.superfly.service.UserService#updateUser the same rule is enforced server-side + */ +public class OtpTypeAutoSetBehavior extends OnChangeAjaxBehavior { + + /** + * OTP type assigned when OTP becomes mandatory while no type is chosen. + * Kept in sync with the service layer, which applies the same rule on save. + */ + public static final OTPType DEFAULT_MANDATORY_OTP_TYPE = OTPType.GOOGLE_AUTH; + + private static final String MESSAGE_KEY = "user.otpTypeAutoSet"; + + private final UIUser user; + private final LabelDropDownChoiceRow otpTypeRow; + private final Component feedbackPanel; + + public OtpTypeAutoSetBehavior(UIUser user, LabelDropDownChoiceRow otpTypeRow, Component feedbackPanel) { + this.user = user; + this.otpTypeRow = otpTypeRow; + this.feedbackPanel = feedbackPanel; + } + + @Override + protected void onUpdate(AjaxRequestTarget target) { + if (user.isOtpOptional() || OTPType.fromCode(user.getOtpType()) != OTPType.NONE) { + return; + } + + user.setOtpType(DEFAULT_MANDATORY_OTP_TYPE.code()); + // a form component renders the value it received, not the model, so a stale + // selection (a previous failed submit, or the choice we have just overridden) + // would survive the re-render + otpTypeRow.clearInput(); + + getComponent().info(getComponent().getString(MESSAGE_KEY)); + target.add(otpTypeRow, feedbackPanel); + } +} diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java index b8284afc8..f75a4a171 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java @@ -15,6 +15,7 @@ import com.payneteasy.superfly.web.wicket.component.field.LabelPasswordTextFieldRow; import com.payneteasy.superfly.web.wicket.component.field.LabelTextAreaRow; import com.payneteasy.superfly.web.wicket.component.field.LabelTextFieldRow; +import com.payneteasy.superfly.web.wicket.component.otp.OtpTypeAutoSetBehavior; import com.payneteasy.superfly.web.wicket.page.BasePage; import com.payneteasy.superfly.web.wicket.validation.PasswordInputValidator; import com.payneteasy.superfly.web.wicket.validation.PublicKeyValidator; @@ -147,19 +148,11 @@ protected void onUpdate(AjaxRequestTarget target) { final LabelDropDownChoiceRow otpTypeRow = new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender()); otpTypeRow.setOutputMarkupId(true); + otpTypeRow.getDropDownChoice().add(new OtpTypeAutoSetBehavior(user, otpTypeRow, getFeedbackPanel())); form.add(otpTypeRow); LabelCheckBoxRow isOtpOptionalRow = new LabelCheckBoxRow("isOtpOptional", user, "user.create.isOtpOptional"); - isOtpOptionalRow.getCheckBox().add(new OnChangeAjaxBehavior() { - @Override - protected void onUpdate(AjaxRequestTarget target) { - if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { - user.setOtpType(OTPType.GOOGLE_AUTH.code()); - info(getString("user.otpTypeAutoSet")); - target.add(otpTypeRow, getFeedbackPanel()); - } - } - }); + isOtpOptionalRow.getCheckBox().add(new OtpTypeAutoSetBehavior(user, otpTypeRow, getFeedbackPanel())); form.add(isOtpOptionalRow); form.add(new Button("add") { diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java index 571b73807..fd91e0293 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPage.java @@ -8,11 +8,10 @@ import com.payneteasy.superfly.web.wicket.component.field.LabelDropDownChoiceRow; import com.payneteasy.superfly.web.wicket.component.field.LabelTextAreaRow; import com.payneteasy.superfly.web.wicket.component.field.LabelTextFieldRow; +import com.payneteasy.superfly.web.wicket.component.otp.OtpTypeAutoSetBehavior; import com.payneteasy.superfly.web.wicket.page.BasePage; import com.payneteasy.superfly.web.wicket.validation.PublicKeyValidator; import org.apache.wicket.Page; -import org.apache.wicket.ajax.AjaxRequestTarget; -import org.apache.wicket.ajax.form.OnChangeAjaxBehavior; import org.apache.wicket.markup.html.form.ChoiceRenderer; import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.IChoiceRenderer; @@ -97,19 +96,11 @@ protected void onSubmit() { final LabelDropDownChoiceRow otpTypeRow = new LabelDropDownChoiceRow<>("otpType", user, "user.create.otpTypeCode", otpTypes(), otpRender()); otpTypeRow.setOutputMarkupId(true); + otpTypeRow.getDropDownChoice().add(new OtpTypeAutoSetBehavior(user, otpTypeRow, getFeedbackPanel())); form.add(otpTypeRow); LabelCheckBoxRow isOtpOptionalRow = new LabelCheckBoxRow("isOtpOptional", user, "user.create.isOtpOptional"); - isOtpOptionalRow.getCheckBox().add(new OnChangeAjaxBehavior() { - @Override - protected void onUpdate(AjaxRequestTarget target) { - if (!user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE) { - user.setOtpType(OTPType.GOOGLE_AUTH.code()); - info(getString("user.otpTypeAutoSet")); - target.add(otpTypeRow, getFeedbackPanel()); - } - } - }); + isOtpOptionalRow.getCheckBox().add(new OtpTypeAutoSetBehavior(user, otpTypeRow, getFeedbackPanel())); form.add(isOtpOptionalRow); form.add(new BookmarkablePageLink("cancel", ListUsersPage.class)); diff --git a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java index 0b8d885c9..1c97ec8ac 100644 --- a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java +++ b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPageTest.java @@ -7,6 +7,7 @@ import com.payneteasy.superfly.service.SubsystemService; import com.payneteasy.superfly.service.UserService; import com.payneteasy.superfly.web.wicket.page.AbstractPageTest; +import org.apache.wicket.feedback.FeedbackMessage; import org.apache.wicket.markup.html.form.CheckBox; import org.apache.wicket.markup.html.form.DropDownChoice; import org.apache.wicket.util.tester.FormTester; @@ -22,10 +23,12 @@ import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; public class CreateUserPageTest extends AbstractPageTest { private static final String OTP_TYPE_PATH = "form:otpType:container:field-id"; + private static final String OTP_TYPE_ROW_PATH = "form:otpType"; private static final String OTP_OPTIONAL_PATH = "form:isOtpOptional:field-id"; private UserService userService; @@ -96,6 +99,26 @@ public void clearingOptionalAutoSetsGoogleAuth() { formTester.setValue("isOtpOptional:field-id", false); tester.executeAjaxEvent(OTP_OPTIONAL_PATH, "change"); + DropDownChoice otpType = (DropDownChoice) tester.getComponentFromLastRenderedPage(OTP_TYPE_PATH); + assertEquals(OTPType.GOOGLE_AUTH.code(), otpType.getDefaultModelObject()); + tester.assertComponentOnAjaxResponse(OTP_TYPE_ROW_PATH); + assertFalse("expected an info message about the automatically set OTP type", + tester.getMessages(FeedbackMessage.INFO).isEmpty()); + } + + @Test + public void choosingNoneWhileOtpIsMandatoryRevertsToGoogleAuth() { + startPage(); + + FormTester optionalTester = tester.newFormTester("form"); + optionalTester.setValue("isOtpOptional:field-id", false); + tester.executeAjaxEvent(OTP_OPTIONAL_PATH, "change"); + + FormTester typeTester = tester.newFormTester("form"); + // otpTypes() lists OTPType.values() in declaration order, so the ordinal is the choice index + typeTester.select("otpType:container:field-id", OTPType.NONE.ordinal()); + tester.executeAjaxEvent(OTP_TYPE_PATH, "change"); + DropDownChoice otpType = (DropDownChoice) tester.getComponentFromLastRenderedPage(OTP_TYPE_PATH); assertEquals(OTPType.GOOGLE_AUTH.code(), otpType.getDefaultModelObject()); } diff --git a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java index 2a653000b..e86f0d686 100644 --- a/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java +++ b/superfly-web/src/test/java/com/payneteasy/superfly/web/wicket/page/user/EditUserPageTest.java @@ -6,6 +6,7 @@ import com.payneteasy.superfly.service.SettingsService; import com.payneteasy.superfly.service.UserService; import com.payneteasy.superfly.web.wicket.page.AbstractPageTest; +import org.apache.wicket.feedback.FeedbackMessage; import org.apache.wicket.util.tester.FormTester; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.easymock.EasyMock; @@ -18,10 +19,13 @@ import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class EditUserPageTest extends AbstractPageTest { private static final String OTP_TYPE_PATH = "form:otpType:container:field-id"; + private static final String OTP_TYPE_ROW_PATH = "form:otpType"; private static final String OTP_OPTIONAL_PATH = "form:isOtpOptional:field-id"; private UserService userService; @@ -80,10 +84,31 @@ private void setOtpOptionalAndFireAjax(boolean optional) { tester.executeAjaxEvent(OTP_OPTIONAL_PATH, "change"); } + private void selectOtpTypeAndFireAjax(OTPType otpType) { + FormTester formTester = tester.newFormTester("form"); + // otpTypes() lists OTPType.values() in declaration order, so the ordinal is the choice index + formTester.select("otpType:container:field-id", otpType.ordinal()); + tester.executeAjaxEvent(OTP_TYPE_PATH, "change"); + } + private Object renderedOtpType() { return tester.getComponentFromLastRenderedPage(OTP_TYPE_PATH).getDefaultModelObject(); } + /** + * Asserting the model alone would still pass if the row were never repainted or the + * administrator never told, so pin both down. + */ + private void assertOtpTypeAutoSetReported() { + tester.assertComponentOnAjaxResponse(OTP_TYPE_ROW_PATH); + assertFalse("expected an info message about the automatically set OTP type", + tester.getMessages(FeedbackMessage.INFO).isEmpty()); + } + + private void assertNothingReported() { + assertTrue("expected no info message", tester.getMessages(FeedbackMessage.INFO).isEmpty()); + } + @Test public void makingOtpMandatoryAutoSetsGoogleAuthWhenTypeIsNone() { startPageForUser(OTPType.NONE.code(), true); @@ -91,6 +116,7 @@ public void makingOtpMandatoryAutoSetsGoogleAuthWhenTypeIsNone() { setOtpOptionalAndFireAjax(false); assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + assertOtpTypeAutoSetReported(); } @Test @@ -100,6 +126,7 @@ public void makingOtpMandatoryKeepsAlreadyChosenType() { setOtpOptionalAndFireAjax(false); assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + assertNothingReported(); } @Test @@ -109,5 +136,41 @@ public void keepingOtpOptionalDoesNotChangeType() { setOtpOptionalAndFireAjax(true); assertEquals(OTPType.NONE.code(), renderedOtpType()); + assertNothingReported(); + } + + /** + * The drop down is not submitted with the check box ajax request, so without a behavior + * of its own the rule would be decided on a stale OTP type. + */ + @Test + public void choosingNoneWhileOtpIsMandatoryRevertsToGoogleAuth() { + startPageForUser(OTPType.GOOGLE_AUTH.code(), false); + + selectOtpTypeAndFireAjax(OTPType.NONE); + + assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + assertOtpTypeAutoSetReported(); + } + + @Test + public void choosingNoneWhileOtpIsOptionalIsKept() { + startPageForUser(OTPType.GOOGLE_AUTH.code(), true); + + selectOtpTypeAndFireAjax(OTPType.NONE); + + assertEquals(OTPType.NONE.code(), renderedOtpType()); + assertNothingReported(); + } + + @Test + public void choosingNoneThenMakingOtpMandatoryAutoSetsGoogleAuth() { + startPageForUser(OTPType.GOOGLE_AUTH.code(), true); + + selectOtpTypeAndFireAjax(OTPType.NONE); + setOtpOptionalAndFireAjax(false); + + assertEquals(OTPType.GOOGLE_AUTH.code(), renderedOtpType()); + assertOtpTypeAutoSetReported(); } } From d99175b812f7246bbd11e6881c307b92217a1041 Mon Sep 17 00:00:00 2001 From: Dmitriy Kanaev Date: Fri, 4 Sep 2026 23:07:11 +0300 Subject: [PATCH 5/5] Redmine #167449: one source of truth for the OTP default, fix the UIUser default The default OTP type and the "mandatory OTP without a type" condition were spelled out separately in the service layer and in both admin pages, so changing the rule in one place would silently leave the other previewing something else. Move both into OtpTypeDefaults next to UIUser and reference it from the service and from the ajax behavior. UIUser.isOtpOptional defaulted to false while the DB column defaults to 'Y'. The previous commits worked around that in CreateUserPage and in four logging-test fixtures instead of fixing the field, which left the trap armed for the next object built in code: it would get mandatory OTP and, since this branch, a forced google_auth type as well. Default the field to true and drop both workarounds - the four logging tests now fail if the default is flipped back, which the workarounds used to hide. Verified by mutation: restoring the false default breaks exactly those four tests; the full reactor build stays green (156 service, 30 web). Co-Authored-By: Claude Opus 5 (1M context) --- .../model/ui/user/OtpTypeDefaults.java | 27 +++++++++++++++++++ .../superfly/model/ui/user/UIUser.java | 7 ++++- .../service/impl/UserServiceImpl.java | 15 ++++++----- .../service/impl/UserServiceLoggingTest.java | 4 --- .../component/otp/OtpTypeAutoSetBehavior.java | 12 +++------ .../web/wicket/page/user/CreateUserPage.java | 2 -- 6 files changed, 44 insertions(+), 23 deletions(-) create mode 100644 superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/OtpTypeDefaults.java diff --git a/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/OtpTypeDefaults.java b/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/OtpTypeDefaults.java new file mode 100644 index 000000000..2d3c0ae86 --- /dev/null +++ b/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/OtpTypeDefaults.java @@ -0,0 +1,27 @@ +package com.payneteasy.superfly.model.ui.user; + +import com.payneteasy.superfly.api.OTPType; + +/** + * The rule "OTP mandatory => OTP type is not none", in one place. + *

+ * The service layer applies it on save, and the admin pages apply it in the form so that + * the administrator sees the same outcome before saving. Keeping the default type and the + * condition here stops the two layers from drifting apart. + */ +public final class OtpTypeDefaults { + + /** OTP type assigned when OTP is mandatory but no type has been chosen. */ + public static final OTPType MANDATORY_DEFAULT = OTPType.GOOGLE_AUTH; + + private OtpTypeDefaults() { + } + + /** + * @return true when the user would end up with mandatory OTP and no OTP type, + * which the authentication code silently treats as "no second factor" + */ + public static boolean needsDefaultType(UIUser user) { + return !user.isOtpOptional() && OTPType.fromCode(user.getOtpType()) == OTPType.NONE; + } +} diff --git a/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/UIUser.java b/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/UIUser.java index e369ce100..ffec087cd 100644 --- a/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/UIUser.java +++ b/superfly-service/src/main/java/com/payneteasy/superfly/model/ui/user/UIUser.java @@ -26,7 +26,12 @@ public class UIUser implements Serializable { private String salt; private String publicKey; private String otpType = OTPType.NONE.code(); - private boolean isOtpOptional; + /** + * Mirrors the DB default {@code is_otp_optional='Y'}: a user built in code and not + * loaded from the DB must not silently end up with mandatory OTP (which would now + * also force an OTP type on them). + */ + private boolean isOtpOptional = true; private UISubsystemForFilter subsystemForEmail; private String organization; diff --git a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java index 65c3a87d2..3339554c4 100644 --- a/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java +++ b/superfly-service/src/main/java/com/payneteasy/superfly/service/impl/UserServiceImpl.java @@ -49,8 +49,6 @@ public class UserServiceImpl implements UserService { private static final Logger logger = LoggerFactory.getLogger(UserServiceImpl.class); - private static final OTPType DEFAULT_MANDATORY_OTP_TYPE = OTPType.GOOGLE_AUTH; - private UserDao userDao; private NotificationService notificationService; private LoggerSink loggerSink; @@ -586,7 +584,7 @@ public void updateUserIsOtpOptionalValue(String username, boolean isOtpOptional) if (!isOtpOptional) { UserForDescription user = userDao.getUserForDescription(username); if (user != null && user.getOtpType() == OTPType.NONE) { - userDao.updateUserOtpType(username, DEFAULT_MANDATORY_OTP_TYPE.code()); + userDao.updateUserOtpType(username, OtpTypeDefaults.MANDATORY_DEFAULT.code()); loggerSink.info(logger, "AUTO_SET_OTP_TYPE", true, username); } } @@ -600,11 +598,14 @@ public void updateUserIsOtpOptionalValue(String username, boolean isOtpOptional) * together with the result of the DAO call */ private boolean assignDefaultOtpTypeIfOtpMandatory(UIUser user) { - if (!user.isOtpOptional() && normalizeOtpType(user.getOtpType()) == OTPType.NONE) { - user.setOtpType(DEFAULT_MANDATORY_OTP_TYPE.code()); - return true; + // reject an unknown code instead of letting it pass as "no OTP" below + normalizeOtpType(user.getOtpType()); + + if (!OtpTypeDefaults.needsDefaultType(user)) { + return false; } - return false; + user.setOtpType(OtpTypeDefaults.MANDATORY_DEFAULT.code()); + return true; } /** diff --git a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java index 2d7e1f3b2..0f8a9ef12 100644 --- a/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java +++ b/superfly-service/src/test/java/com/payneteasy/superfly/service/impl/UserServiceLoggingTest.java @@ -63,7 +63,6 @@ public RoutineResult answer() throws Throwable { UIUserForCreate user = new UIUserForCreate(); user.setUsername("test-user"); - user.setOtpOptional(true); userService.createUser(user, "subsystem"); EasyMock.verify(loggerSink); @@ -84,7 +83,6 @@ public RoutineResult answer() throws Throwable { UIUserForCreate user = new UIUserForCreate(); user.setUsername("test-user"); - user.setOtpOptional(true); userService.createUser(user, "subsystem"); EasyMock.verify(loggerSink); @@ -99,7 +97,6 @@ public void testUpdateUser() throws Exception { UIUser user = new UIUser(); user.setUsername("test-user"); - user.setOtpOptional(true); userService.updateUser(user); EasyMock.verify(loggerSink); @@ -114,7 +111,6 @@ public void testUpdateUserFail() throws Exception { UIUser user = new UIUser(); user.setUsername("test-user"); - user.setOtpOptional(true); userService.updateUser(user); EasyMock.verify(loggerSink); diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java index fcf2a542b..aca2bb49d 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/component/otp/OtpTypeAutoSetBehavior.java @@ -1,6 +1,6 @@ package com.payneteasy.superfly.web.wicket.component.otp; -import com.payneteasy.superfly.api.OTPType; +import com.payneteasy.superfly.model.ui.user.OtpTypeDefaults; import com.payneteasy.superfly.model.ui.user.UIUser; import com.payneteasy.superfly.web.wicket.component.field.LabelDropDownChoiceRow; import org.apache.wicket.Component; @@ -20,12 +20,6 @@ */ public class OtpTypeAutoSetBehavior extends OnChangeAjaxBehavior { - /** - * OTP type assigned when OTP becomes mandatory while no type is chosen. - * Kept in sync with the service layer, which applies the same rule on save. - */ - public static final OTPType DEFAULT_MANDATORY_OTP_TYPE = OTPType.GOOGLE_AUTH; - private static final String MESSAGE_KEY = "user.otpTypeAutoSet"; private final UIUser user; @@ -40,11 +34,11 @@ public OtpTypeAutoSetBehavior(UIUser user, LabelDropDownChoiceRow otpTyp @Override protected void onUpdate(AjaxRequestTarget target) { - if (user.isOtpOptional() || OTPType.fromCode(user.getOtpType()) != OTPType.NONE) { + if (!OtpTypeDefaults.needsDefaultType(user)) { return; } - user.setOtpType(DEFAULT_MANDATORY_OTP_TYPE.code()); + user.setOtpType(OtpTypeDefaults.MANDATORY_DEFAULT.code()); // a form component renders the value it received, not the model, so a stale // selection (a previous failed submit, or the choice we have just overridden) // would survive the re-render diff --git a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java index f75a4a171..5c92ebbb5 100644 --- a/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java +++ b/superfly-web/src/main/java/com/payneteasy/superfly/web/wicket/page/user/CreateUserPage.java @@ -55,8 +55,6 @@ public class CreateUserPage extends BasePage { public CreateUserPage() { super(ListUsersPage.class); final UIUserCheckPassword user = new UIUserCheckPassword(); - // matches the DB default 'Y', so an untouched form does not create a user with mandatory OTP - user.setOtpOptional(true); List listSub = subsystemService.getSubsystemsForFilter(); for (UISubsystemForFilter sub : listSub) {