Alon Bar-Lev has posted comments on this change.

Change subject: engine : Add user name to can do action error messages
......................................................................


Patch Set 4:

(3 comments)

Good!

last test... can you please make sure that if you use the new ldap provider you 
get the principal name as user within log?

for example for alonbl from qa ad, I expect: alo...@qa.lab....@profile

if you do not have time, I will check this tomorrow.

thanks!!!

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

Line 191:                 
SessionDataContainer.getInstance().getUser(cmdContext.getEngineContext().getSessionId(),
 true);
Line 192:         if (user != null) {
Line 193:             setCurrentUser(user);
Line 194:         }
Line 195:         if 
(SessionDataContainer.getInstance().getProfileName(cmdContext.getEngineContext().getSessionId())
 != null) {
.

 profile =  ...
 if (profile != null) {
 }

just like above user?
Line 196:             
setProfileName(SessionDataContainer.getInstance().getProfileName(cmdContext.getEngineContext().getSessionId()));
Line 197:         }
Line 198:         ExecutionContext executionContext = 
cmdContext.getExecutionContext();
Line 199:         if (executionContext.getJob() != null) {


http://gerrit.ovirt.org/#/c/36623/4/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 118:     protected void setProfileName() {
Line 119:         if (getParameters().getProfileName() != null) {
Line 120:             AuthenticationProfile profile = 
AuthenticationProfileRepository.getInstance().getProfile(getParameters().getProfileName());
Line 121:             if (profile != null) {
Line 122:                 setProfileName(profile.getName());
why not just return the getParameters().getProfileName() as is? why do we care 
if we actually support it?
Line 123:             }
Line 124:         }
Line 125:     }
Line 126: 


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

Line 26: 
Line 27:     @Override
Line 28:     protected void executeCommand() {
Line 29:         LogoutUserParameters params = new 
LogoutUserParameters(user.getId());
Line 30:         params.setSessionId(getParameters().getSessionId());
why not add the session id to constructor?
Line 31:         setReturnValue(Backend.getInstance().logoff(params));
Line 32:     }
Line 33: 
Line 34:     @Override


-- 
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: 4
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

Reply via email to