Alon Bar-Lev has posted comments on this change.

Change subject: aaa: fix audit/acct/log messages without profile/authn names
......................................................................


Patch Set 10:

(2 comments)

> I am guessing it is the UI that runs the command to take the user back to 
> login screen and clear its state

it is not important... as the case of ui has valid session will always exist, 
example: engine is restarted.

I think the logout command should be non public, but not part of this patch.

http://gerrit.ovirt.org/#/c/36623/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java:

Line 338:     public void processOnClientIpChange(final Guid vmId) {
Line 339:         final VmDynamic vmDynamic = 
DbFacade.getInstance().getVmDynamicDao().get(vmId);
Line 340:         final AuditLogableBase event = new AuditLogableBase();
Line 341:         event.setVmId(vmId);
Line 342:         event.setUserName(vmDynamic.getConsoleCurrentUserName());
> Are we sure this includes authz name as well?
it is not as this is the user of remote. I think this setting is a bug... using 
the same field for local and remote user but  not part of this patch.
Line 343: 
Line 344:         // in case of empty clientIp we clear the logged in user.
Line 345:         // (this happened when user close the console to spice/vnc)
Line 346:         if (StringUtils.isEmpty(vmDynamic.getClientIp())) {


http://gerrit.ovirt.org/#/c/36623/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LoginBaseCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/LoginBaseCommand.java:

Line 114:             }
Line 115:         }
Line 116:         String profile = getParameters().getProfileName();
Line 117:         if (StringUtils.isEmpty(profile)) {
Line 118:             profile = "UNKNOWN";
> Hmm, UKNOWN or N/A, as in other place you perform String.format and when pr
N/A it is.
Line 119:         }
Line 120:         setUserName(String.format("%s@%s", user, profile));
Line 121: 
Line 122:         boolean result = isUserCanBeAuthenticated();


-- 
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: 10
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: Yair Zaslavsky <yzasl...@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

Reply via email to