Alon Bar-Lev has posted comments on this change.

Change subject: Introduction of filters to unify AAA flows for UI and REST-API
......................................................................


Patch Set 47:

(3 comments)

http://gerrit.ovirt.org/#/c/28022/47/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/LoginFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/LoginFilter.java:

Line 51:                                         authRecord,
Line 52:                                         loginAsAdmin ? 
VdcActionType.LoginAdminUser : VdcActionType.LoginUser)
Line 53:                                 );
Line 54:                         if (returnValue.getSucceeded()) {
Line 55:                             HttpSession session = req.getSession(true);
> This creates a session even if the user didn't sent "Prefer: persistent-aut
the session is created and destroyed later when logout is executed for restapi.
Line 56:                             session.setAttribute(
Line 57:                                     
SessionConstants.HTTP_SESSION_ENGINE_SESSION_ID_KEY,
Line 58:                                     returnValue.getSessionId()
Line 59:                                     );


http://gerrit.ovirt.org/#/c/28022/47/backend/manager/modules/aaa/src/main/modules/org/ovirt/engine/core/aaa/main/module.xml
File 
backend/manager/modules/aaa/src/main/modules/org/ovirt/engine/core/aaa/main/module.xml:

Line 13
Line 14
Line 15
Line 16
Line 17
> Slf4j is better than log4j and commons logging in many aspects. Why are you
the engine is a mess, sometimes the slf4j is used and sometimes the 
commons-logging. as we took several classes into the aaa, the majority used the 
common-loggins, so we synced into it.

if slf4j is to be used, I think that cross-engine patch should be applied.


http://gerrit.ovirt.org/#/c/28022/47/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java:

Line 282:                         backend.logoff(sh.sessionize(new 
LogoutUserParameters(user.getId())));
Line 283:                         HttpSession session = 
SessionUtils.getCurrentSession(false);
Line 284:                         if (session != null) {
Line 285:                             session.invalidate();
Line 286:                         }
> Assuming that the session is available here makes it more difficult to migr
please explain, what makes more difficult to migrate? what to avoid?
Line 287:                     }
Line 288:                     sh.clean();
Line 289:                 }
Line 290:             }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5536d123b6407acf41b6946dde796bd67d1e073
Gerrit-PatchSet: 47
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@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