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: (5 comments) http://gerrit.ovirt.org/#/c/25529/2/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/PasswordAuthenticator.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/PasswordAuthenticator.java: Line 16: * the name of user being authenticated Line 17: * @param password Line 18: * @return true if authenticated Line 19: */ Line 20: public abstract boolean authenticate(String name, String password); why do you need boolean? if exception it is a failure. 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()); this should be moved to provider as well 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) { why do you check http or https? 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, "")); why can't you just map the code? Line 62: } Line 63: Line 64: @Override Line 65: protected String getPROTOCOL() { http://gerrit.ovirt.org/#/c/25529/2/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/AuthenticationResult.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/AuthenticationResult.java: Line 1 this should be moved to the built-in provider module, right? -- 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: [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
