Vojtech Szocs has posted comments on this change. Change subject: Introduction of filters to unify AAA flows for UI and REST-API ......................................................................
Patch Set 47: >> 2, InvalidateSessionIfAuthorizationHeaderFilter - potential issue: typical >> web browser _always and unconditionally_ sends HTTP "Authorization" header >> once it's set for given origin, what will happen if this HTTP header will be >> present in each request? Will this make REST API persistent session >> mechanism unusable? > as far as we understand there is no change in behaviour. InvalidateSessionIfAuthorizationHeaderFilter is present in REST API web.xml so I assume that any request to REST API application containing "Authorization" header will cause the current REST API HttpSession to be invalidated, which in turn prevents reusing that (now invalidated) HttpSession through REST API persistent management feature (RestApiSessionMgmtFilter). Yair/Alon, is this assumption correct? If so, web applications running in browser will NOT be able to use REST API persistent management feature in following scenario: * web application acquires auth token via REST API (i.e. NOT via interactive login page) by making request with "Authorization: Basic xxx" + "Prefer: persistent-auth" headers, as described in [1] * web application attempts to reuse existing REST API session by passing auth token + "Prefer: persistent-auth" header, but browser will also include "Authorization: Basic xxx" header unconditionally * InvalidateSessionIfAuthorizationHeaderFilter invalidates REST API session, subsequent filters will cause re-login and re-creation of REST API session * web application is confused, because even though it sends "Prefer: persistent-auth" header, it always gets new auth token [1] http://www.ovirt.org/Features/RESTSessionManagement Is above scenario likely to happen? Did you test REST API changes with UI plugins that interact with REST API backend? >> 10, what is the purpose of BasicAuthenticationFilter & NegotiationFilter in >> context of WebAdmin & UserPortal web application? (I think I'm missing >> something) > 1. allow basic authentication at that context as well, sync the entire > feature set across application. > 2. allow negotiation authentication, such as SPNEGO, OpenID, apache context, > SSOs. So in practice, client applications will be able to make Basic / negotiation authentication requests also against WebAdmin / UserPortal application context? I don't see any use case for this. Can you please provide some use case to justify this? In other words, BasicAuthenticationFilter & NegotiationFilter is already applied to REST API application context, why have it also for specific GWT UI applications? > 3. "working with InitialContext like this..." - I disagree here, IMHO we > should use the bean in the JNDI context. I was just suggesting that "make InitialContext, try { use context to do something } finally { close context }" is sort of extra complexity to deal with, it's not a big deal (I agree with bean used in JNDI context, my comment was rather focused on how we work with that context in code). > 5. Thanks for the tip, IMHO this will be an overkill here + the > initialization depends on the caps field which is retreived by the request. I > do not want to pass getInstance the "caps" value. Point accepted :-) > 6, "usage of java.lang.Object in LoginUserParameters ... " - No need for this > when used from GUI, the field is not passed. Client-side GWT code works with LoginUserParameters (CommunicationProvider.login) but you're right that "Object authRecord" field is not passed via GWT RPC. Technically, LoginUserParameters is subclass of VdcActionParametersBase and VdcActionParametersBase is part of GWT RPC interface signature (GenericApiGWTService) so it should have custom field serializer. Anyway, I think that "Object authRecord" field in LoginUserParameters will be excluded in target JavaScript output, which as I understand, isn't a big deal. -- 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: No _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches