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 52: (3 comments) http://gerrit.ovirt.org/#/c/28022/52/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/FiltersHelper.java: Line 17: public static final String REQUEST_PROFILE_KEY = "ovirt_aaa_profile"; Line 18: public static final String HEADER_AUTHORIZATION = "Authorization"; Line 19: public static final String HEADER_WWW_AUTHENTICATE = "WWW-Authenticate"; Line 20: public static final String HEADER_PREFER = "Prefer"; Line 21: public static final String JSESSIONID_COOKIE = "JSESSIONID"; > can be both header, and the cookie name, what do u suggest? (i didnt put he you put it as header, then i is header. the cookie should have its own constant if you like. the fact that we use JSESSIONID as header name is a very bad practise anyway. Line 22: } Line 23: Line 24: public static BackendLocal getBackend(Context context) { Line 25: http://gerrit.ovirt.org/#/c/28022/52/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtFilter.java: Line 72: ctx.close(); Line 73: } Line 74: } else { Line 75: HttpSession session = req.getSession(false); Line 76: if (session != null && session.isNew() && session.getAttribute("async") != null && !Boolean.valueOf((boolean)session.getAttribute("async"))) { > this has to do with rest-api behavior, i dont think we should prefix it wit this is relation between the aaa module and the restapi application. it is the application to instruct the aaa module what to do, so as long as this code lives at aaa we should use the ovirt_aaa_ prefix for all artifacts within session/request. Line 77: ((HttpServletResponse)response).addHeader(FiltersHelper.Constants.JSESSIONID_COOKIE, session.getId()); Line 78: } Line 79: } Line 80: } catch (Exception ex) { http://gerrit.ovirt.org/#/c/28022/52/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/auth/LoginValidator.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/auth/LoginValidator.java: Line 36: public void postProcess(ServerResponse response) { Line 37: HttpSession httpSession = getCurrentSession(false); Line 38: if (!current.get(MetaData.class).hasKey("async") || Line 39: (!(Boolean.TRUE.equals((Boolean) current.get(MetaData.class).get("async"))))) { Line 40: httpSession.setAttribute("async", false); > see previous comment on constant value. ok Line 41: } Line 42: sessionHelper.clean(); Line 43: } Line 44: -- 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: 52 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: Einav Cohen <eco...@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