Alon Bar-Lev has posted comments on this change. Change subject: aaa: Stop using proxies ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/26602/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 301: "Can't login user \"{0}\" with authentication profile \"{1}\" because the authentication failed.", Line 302: user, Line 303: getParameters().getProfileName()); Line 304: Line 305: AuditLogType auditLogType = auditLogMap.get(authResult); > Well, see but what if we add new codes in future? Line 306: // if found matching audit log type, and it's not general login failure audit log (which will be logged Line 307: // anyway due to CommandBase.log) Line 308: if (auditLogType != null && auditLogType != AuditLogType.USER_VDC_LOGIN_FAILED) { Line 309: logEventForUser(user, auditLogType); Line 304: Line 305: AuditLogType auditLogType = auditLogMap.get(authResult); Line 306: // if found matching audit log type, and it's not general login failure audit log (which will be logged Line 307: // anyway due to CommandBase.log) Line 308: if (auditLogType != null && auditLogType != AuditLogType.USER_VDC_LOGIN_FAILED) { > not sure i understood, this has to do with audit log, what is wrong with th I would have checked authResult above instead of anything related to audit log. I also do not understand why you do not log an event in any case... can you please explain? Line 309: logEventForUser(user, auditLogType); Line 310: } Line 311: Line 312: if (authResult == Authn.AuthResult.CREDENTIALS_EXPIRED) { Line 316: getReturnValue().getCanDoActionMessages().add(String.format("$URL %1$s", Line 317: outputMap.<String> get(Authn.InvokeKeys.CREDENTIALS_CHANGE_URL))); Line 318: addedUserPasswordExpiredCDA = true; Line 319: } Line 320: if (outputMap.<String> get(Authn.InvokeKeys.USER_MESSAGE) != null) { > I don't think so - well... I would prefer URL over that message. in future this message should be presented anyway and have nothing to do with password change... it should be presented if user passes authentication (expiration is included in this definition). Line 321: addCanDoActionMessage(VdcBllMessages.USER_PASSWORD_EXPIRED_CHANGE_MSG_PROVIDED); Line 322: getReturnValue().getCanDoActionMessages().add(String.format("$MSG %1$s", Line 323: outputMap.<String> get(Authn.InvokeKeys.USER_MESSAGE))); Line 324: addedUserPasswordExpiredCDA = true; -- To view, visit http://gerrit.ovirt.org/26602 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I916012eab61a96bdb0f366d9dc8462325d7f726f 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: 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
