Yair Zaslavsky has posted comments on this change. Change subject: Introduction of filters to unify AAA flows for UI and REST-API ......................................................................
Patch Set 47: (7 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); > No session should be created if the user didn't request persistent authenti Done 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/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 43: HttpSession session = req.getSession(); Line 44: try { Line 45: int ttlValue = Integer.parseInt(req.getHeader("Session-TTL")) * SECONDS_IN_MINUTE; Line 46: if (ttlValue >= MINIMAL_SESSION_TTL) { Line 47: session = req.getSession(true); > I don't see any problem with setting the inactive interval with each call. Ok, thanks. Line 48: session.setMaxInactiveInterval(ttlValue); Line 49: } Line 50: } catch (NumberFormatException ex) { Line 51: log.error("Session-TTL header was not passed. Not setting TTL value"); 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 > The first advantage is that slf4j is basically an interface, without implem thanks for the explanation. http://gerrit.ovirt.org/#/c/28022/47/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/SessionUtils.java File backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/SessionUtils.java: Line 14: /* Line 15: * This class contains useful session utils Line 16: */ Line 17: public class SessionUtils { Line 18: public final static String ENGINE_SESSION_ID_KEY = "ovirt_aaa_engineSessionId"; > If it isn't used in the aaa subsystem then why rename it? i'll rename back Line 19: public final static String PREFER_HEADER_FIELD = "Prefer"; Line 20: public final static String SESSION_TTL_HEADER_FIELD = "Session-TTL"; Line 21: public final static String PERSIST_FIELD_VALUE = "persistent-auth"; Line 22: public final static String JSESSIONID_HEADER = "JSESSIONID"; http://gerrit.ovirt.org/#/c/28022/47/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/preprocessors/CurrentPreProcessor.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/preprocessors/CurrentPreProcessor.java: Line 8: import org.jboss.resteasy.core.ResourceMethod; Line 9: import org.jboss.resteasy.core.ServerResponse; Line 10: import org.jboss.resteasy.spi.Failure; Line 11: import org.jboss.resteasy.spi.HttpRequest; Line 12: import org.jboss.resteasy.spi.interception.PreProcessInterceptor; > I mean any "import org.jboss.resteasy....". ok, then how do you suggest to handle this ? that is , setting to current the values i set here at the pre processor? Line 13: import org.ovirt.engine.api.common.invocation.Current; Line 14: import org.ovirt.engine.api.common.security.auth.SessionUtils; Line 15: import org.ovirt.engine.api.restapi.util.SessionHelper; Line 16: import org.ovirt.engine.core.common.config.ConfigCommon; 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: } > Well, this code runs outside of the control of the web container, in one of Done Line 287: } Line 288: sh.clean(); Line 289: } Line 290: } http://gerrit.ovirt.org/#/c/28022/47/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 163 Line 164 Line 165 Line 166 Line 167 > You are right. But I'm asking what happens to the treatment of "async" that Ok, if the session is not persistent (regardless to the value of "async" ) - we should issue a logoff command, right? So this is handled at the RestApiSessionMgmtFilter. Regarding the setting of JESSIONID_HEADER - You're correct, this is missing(I thought that the fact I see the JESSIONID cookie is enough but I guess i was wrong and missed that point). -- 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 <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
