Alon Bar-Lev has posted comments on this change. Change subject: engine : Add user name to can do action error messages ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/36623/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LoginAdminUserCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LoginAdminUserCommand.java: Line 18 Line 19 Line 20 Line 21 Line 22 can't we send the parameters to the addCanDoActionMessage or something similar instead of holding the user name and profile name within command state? how can we make sure that we are not using leftovers from one command to another? http://gerrit.ovirt.org/#/c/36623/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java: Line 123: String username = null; Line 124: if (auditLogable.getUserId() != null) { Line 125: DbUser user = getDbFacadeInstance().getDbUserDao().get(auditLogable.getUserId()); Line 126: if (user != null) { Line 127: username = user.getLoginName() + "@" + user.getDomain(); this is authn name not profile name... Line 128: } Line 129: } Line 130: auditLog = new AuditLog(logType, Line 131: severity, Line 144: auditLogable.getCustomData()); Line 145: } else if ((message = messages.get(logType)) != null) { // Application log message from AuditLogMessages Line 146: resolvedMessage = resolveMessage(message, auditLogable); Line 147: auditLog = new AuditLog(logType, severity, resolvedMessage, auditLogable.getUserId(), Line 148: auditLogable.getUserName() + "@" + auditLogable.getProfileName(), auditLogable.getVmIdRef(), auditLogable.getVmName(), I could not follow, but sometime we have authz name and sometimes the profile name, can we be consistent? Line 149: auditLogable.getVdsIdRef(), auditLogable.getVdsName(), auditLogable.getVmTemplateIdRef(), Line 150: auditLogable.getVmTemplateName()); Line 151: } Line 152: if (auditLog != null) { http://gerrit.ovirt.org/#/c/36623/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java: Line 136: int index = mUserName.indexOf("@"); Line 137: if (index != -1) { Line 138: mProfileName = mUserName.substring(index + 1); Line 139: mUserName = mUserName.substring(0, index); Line 140: } user name within active directory for example can contain @ within... so this logic is incorrect, no? or if you always have authz name then you should get the last @. but best is to set the string of user@profile or user@authz with doing setUser, no? Line 141: this.mVdsGroupId = auditLog.getVdsGroupId(); Line 142: this.mVdsName = auditLog.getVdsName(); Line 143: this.mVmName = auditLog.getVmName(); Line 144: this.mVmTemplateId = auditLog.getVmTemplateId(); -- To view, visit http://gerrit.ovirt.org/36623 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7776f9f5b93aca96c84fb5a7672e10dded186d05 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
