Alon Bar-Lev has posted comments on this change. Change subject: aaa: Fix Audit log and can do action msgs handling in ldap broker. ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/25529/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java: Line 202: } Line 203: Line 204: VdcBllMessages canDoActionMsg = vdcBllMessagesMap.get(ex.getError()); Line 205: Line 206: String passwordChangeMsg = passwordChangeMsgPerDomain.get(getParameters().getProfileName()); > Just to be sure - What should be? moved the whole password change msg/url? each authentication provider should have property of URL when changing password, the built-in should set this property based on the vdc_options. Line 207: getReturnValue().setSucceeded(false); Line 208: if (canDoActionMsg == VdcBllMessages.USER_PASSWORD_EXPIRED && passwordChangeMsg != null) { Line 209: if (passwordChangeMsg.indexOf("http") == 0 || passwordChangeMsg.indexOf("https") == 0) { Line 210: addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED); Line 205: Line 206: String passwordChangeMsg = passwordChangeMsgPerDomain.get(getParameters().getProfileName()); Line 207: getReturnValue().setSucceeded(false); Line 208: if (canDoActionMsg == VdcBllMessages.USER_PASSWORD_EXPIRED && passwordChangeMsg != null) { Line 209: if (passwordChangeMsg.indexOf("http") == 0 || passwordChangeMsg.indexOf("https") == 0) { > This code was moved here from ProvisionalAuthenticationResult that was remo if we expose it to provider we must attend to it in this series. if you want to have hyperlinks, why only 1? if you have messages why not workout the entire URL within messages? anyway, this must be consistent and provided by the provider in simple format. Line 210: addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_URL_PROVIDED); Line 211: getReturnValue().getCanDoActionMessages().add(String.format("$URL %1$s", passwordChangeMsg)); Line 212: } else { Line 213: addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED); http://gerrit.ovirt.org/#/c/25529/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerCommandBase.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerCommandBase.java: Line 57: new AAAExtensionException(ExtensionError.CREDENTIALS_EXPIRED, "")); Line 58: authResultToExceptionMap.put(AuthenticationResult.USER_ACCOUNT_DISABLED_OR_LOCKED, Line 59: new AAAExtensionException(ExtensionError.LOCKED_OR_DISABLED_ACCOUNT, "")); Line 60: authResultToExceptionMap.put(AuthenticationResult.WRONG_REALM, Line 61: new AAAExtensionException(ExtensionError.INCORRET_CREDENTIALS, "")); > Meaning? introducing it to AuthenticationResult as another field? Meaning: new AAAExtensionException(ExtensionError.SERVER_IS_NOT_AVAILABLE, "") -> ExtensionError.SERVER_IS_NOT_AVAILABLE and BTW: ExtensionError -> AAAExtensionError Line 62: } Line 63: Line 64: @Override Line 65: protected String getPROTOCOL() { -- To view, visit http://gerrit.ovirt.org/25529 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b0b024d27a92f620bb60e4689264bc6b3c3eda1 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
