Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Introducing usage of Acct
......................................................................


Patch Set 10:

(4 comments)

missing report if invalid password at basic auth filter.

missing usage of message, please always forward human readable message in 
addition to the code.

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

how come we delete this here? maybe should be in other patch?


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

how come we delete this here? if it is unneeded anymore delete it in other 
patch?


http://gerrit.ovirt.org/#/c/27070/10/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 393:                 }
Line 394:                 addCanDoActionMessage(msg);
Line 395:             }
Line 396:             authRecord = null;
Line 397:         } else {
I am unsure I follow how this hank belongs to this patch.
Line 398:             authRecord = outputMap.<ExtMap> 
get(Authn.InvokeKeys.AUTH_RECORD);
Line 399:             if (mapper != null) {
Line 400:                 authRecord = mapper.invoke(new ExtMap().mput(
Line 401:                                 Base.InvokeKeys.COMMAND,


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

Line 204:                 );
Line 205:         removeSession(sessionId);
Line 206:     }
Line 207: 
Line 208:     public void removeSession(String sessionId) {
this cannot be public, without you call removeSessionImpl

we need to report every session removal, maybe with different reasons (expired, 
logout)...
Line 209:         sessionInfoMap.remove(sessionId);
Line 210:     }


-- 
To view, visit http://gerrit.ovirt.org/27070
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief13d233d11b7ab32b328735b4f58ec7cffff567
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@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