Yair Zaslavsky has posted comments on this change. Change subject: aaa: Intorduce filters ......................................................................
Patch Set 22: (11 comments) http://gerrit.ovirt.org/#/c/28022/22/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 15: public final static String REQUEST_SCHEMES_KEY = "schemes"; Line 16: public final static String REQUEST_PROFILE_KEY = "profile"; Line 17: public static final String SESSION_APPLICATION_MODE_KEY = "app_mode"; Line 18: public static final String REQUEST_USER_KEY = "user"; Line 19: public static final String REQUEST_ALREADY_LOGGED_OUT_KEY = "already_logged_out"; > I'll check. Done Line 20: } Line 21: Line 22: public static BackendLocal getBackend(Context context) { Line 23: http://gerrit.ovirt.org/#/c/28022/22/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/ForceLoginFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/ForceLoginFilter.java: > it is not force login, but enforce authenticated or anything similar. it do Done Line 1: package org.ovirt.engine.core.aaa.filters; Line 2: Line 3: import java.io.IOException; Line 4: import java.util.Arrays; Line 49: for (String scheme: allSchemes) { Line 50: if (scheme.equals("Basic")) { Line 51: if (realmInfo != null) { Line 52: scheme += realmInfo; Line 53: } > why can't we have the entire string as init parameter? Done Line 54: } Line 55: res.setHeader("WWW-Authenticate", scheme); Line 56: } Line 57: res.sendError(HttpServletResponse.SC_UNAUTHORIZED); Line 55: res.setHeader("WWW-Authenticate", scheme); Line 56: } Line 57: res.sendError(HttpServletResponse.SC_UNAUTHORIZED); Line 58: } else { Line 59: chain.doFilter(request, response); > i'll do that, but don't we prefer to have the code of chain.doFilter as lat Done Line 60: } Line 61: Line 62: Line 63: } http://gerrit.ovirt.org/#/c/28022/22/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 72: new GetConfigurationValueParameters(ConfigurationValues.ApplicationMode, Line 73: ConfigCommon.defaultConfigurationVersion)); Line 74: if (result.getSucceeded()) { Line 75: SetDataOnSessionParameters setDataParams = new SetDataOnSessionParameters( Line 76: FiltersHelper.Constants.SESSION_APPLICATION_MODE_KEY, > hmm... this is different session, use different prefix Done Line 77: ApplicationMode.from((Integer) result.getReturnValue() Line 78: )); Line 79: setDataParams.setSessionId(engineSessionId); Line 80: FiltersHelper.getBackend(context).runAction(VdcActionType.SetDataOnSession, Line 73: ConfigCommon.defaultConfigurationVersion)); Line 74: if (result.getSucceeded()) { Line 75: SetDataOnSessionParameters setDataParams = new SetDataOnSessionParameters( Line 76: FiltersHelper.Constants.SESSION_APPLICATION_MODE_KEY, Line 77: ApplicationMode.from((Integer) result.getReturnValue() > why can't the login do that and store it in session? what's so special that i'll change. Line 78: )); Line 79: setDataParams.setSessionId(engineSessionId); Line 80: FiltersHelper.getBackend(context).runAction(VdcActionType.SetDataOnSession, Line 81: setDataParams); Line 75: SetDataOnSessionParameters setDataParams = new SetDataOnSessionParameters( Line 76: FiltersHelper.Constants.SESSION_APPLICATION_MODE_KEY, Line 77: ApplicationMode.from((Integer) result.getReturnValue() Line 78: )); Line 79: setDataParams.setSessionId(engineSessionId); > why isn't this in constructor as well? Done Line 80: FiltersHelper.getBackend(context).runAction(VdcActionType.SetDataOnSession, Line 81: setDataParams); Line 82: } Line 83: Line 83: Line 84: } Line 85: } finally { Line 86: context.close(); Line 87: } > doFilter=true Done Line 88: } Line 89: } Line 90: } catch (Exception ex) { Line 91: exceptionOccurred = true; http://gerrit.ovirt.org/#/c/28022/22/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 70: try { Line 71: VdcActionParametersBase params = new VdcActionParametersBase(); Line 72: params.setSessionId(engineSessionId); Line 73: FiltersHelper.getBackend(ctx).runAction(VdcActionType.LogoutBySession, params); Line 74: session.invalidate(); > just invalidate session and logout, remove the attribute of session id, no Well, with REST-API logout may also be called (regardless of persistent auth or not) when there are "async commands" running (what we know as commands running SPM async tasks). i'll fix the code there (BackendResource.doNonBlockingAction) to perform session invalidation at that point. Here I will check if the session exists (both http and engine) adnd only if so - i will perform logoff. Line 75: } finally { Line 76: ctx.close(); Line 77: } Line 78: } http://gerrit.ovirt.org/#/c/28022/22/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SessionValidationFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/SessionValidationFilter.java: Line 40: if (engineSession != null) { Line 41: ctx = new InitialContext(); Line 42: try { Line 43: VdcQueryParametersBase parameters = new VdcQueryParametersBase(); Line 44: parameters.setSessionId(engineSession); > why not in constructor? Done Line 45: Line 46: VdcQueryReturnValue returnValue = FiltersHelper.getBackend(ctx) Line 47: .runPublicQuery(VdcQueryType.ValidateSession, parameters); Line 48: if (!returnValue.getSucceeded()) { Line 53: } Line 54: } finally { Line 55: ctx.close(); Line 56: } Line 57: } > doFilter = true Done Line 58: } Line 59: } catch (Exception ex) { Line 60: exceptionOccurred = true; Line 61: String msg = -- 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: 22 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@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: 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
