Alon Bar-Lev has posted comments on this change. Change subject: aaa: Intorduce filters ......................................................................
Patch Set 22: (7 comments) http://gerrit.ovirt.org/#/c/28022/22/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java: Line 40: try { Line 41: userNameFormat = UserNameFormat.valueOf(filterConfig.getInitParameter("user-name-format")); Line 42: } catch (Exception ex) { Line 43: log.error(String.format("The value %1$s is not a valid UserNameFormat. setting UPN as default", filterConfig.getInitParameter("user-name-format"))); Line 44: userNameFormat = UserNameFormat.UPN; > what do you mean ? if it gets the variable from init param, why not here? . private UserNameFormat userNameFormat = UserNameFormat.UPN; Line 45: } Line 46: Line 47: } Line 48: Line 78: } else if (userNameFormat == UserNameFormat.RESTAPI_SPECIFIC) { Line 79: separator = translateFrom.indexOf("\\"); Line 80: result.profile = translateFrom.substring(0, separator); Line 81: result.userName = translateFrom.substring(separator+1); Line 82: } > ok, so the init variable should be diffrerent - I do not care that if you are restapi specific then both are considered. Line 83: return result; Line 84: } Line 85: Line 86: private void handleCredentials(HttpServletRequest request, String user, String password) { 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: > renamed to ForceAuthFilter - what do you think? ok Line 1: package org.ovirt.engine.core.aaa.filters; Line 2: Line 3: import java.io.IOException; Line 4: import java.util.Arrays; 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 64: FiltersHelper.Constants.SESSION_ENGINE_SESSION_ID_KEY, Line 65: engineSessionId Line 66: ); Line 67: req.setAttribute(FiltersHelper.Constants.REQUEST_USER_KEY, Line 68: returnValue.getActionReturnValue()); > required by rest-api. but why? why can't the rest-api acquire the information out of the engine session? Line 69: VdcQueryReturnValue result = Line 70: FiltersHelper.getBackend(context) Line 71: .runPublicQuery(VdcQueryType.GetConfigurationValue, Line 72: new GetConfigurationValueParameters(ConfigurationValues.ApplicationMode, http://gerrit.ovirt.org/#/c/28022/22/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/NegotiationFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/NegotiationFilter.java: Line 28: /** Line 29: * This filter should be added to the {@code web.xml} file to the applications that want to use the authentication Line 30: * mechanism implemented in this package. Line 31: */ Line 32: public class NegotiationFilter implements Filter { > Hi, write a simple nego extension that just return auth_record using hardcoded user. Line 33: Line 34: private static Log log = LogFactory.getLog(NegotiationFilter.class); Line 35: /** Line 36: * The authentication profiles used to perform the authentication process. 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 48: if (!returnValue.getSucceeded()) { Line 49: httpSession.removeAttribute(FiltersHelper.Constants.SESSION_ENGINE_SESSION_ID_KEY); Line 50: } else { Line 51: request.setAttribute(FiltersHelper.Constants.REQUEST_USER_KEY, Line 52: returnValue.getReturnValue()); > i disagree. if not placing the user, then at least placing the UUID. but wh because it is out of the scope of the aaa to handle something special for restapi. if you need something special there, either create a new filter last in chain or do this within the webapplication. there is no need to set this if application is behaving nicely. Line 53: } Line 54: } finally { Line 55: ctx.close(); Line 56: } http://gerrit.ovirt.org/#/c/28022/22/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml File backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml: Line 21: <url-pattern>/*</url-pattern> Line 22: </filter-mapping> Line 23: Line 24: <filter> Line 25: <filter-name>RestApiSessionMgmtFilter</filter-name> > please do not use the term j2ee session :) as the logic can be simple: 1. if we have sticky cookie, set session with cookie value 2. validate session (2) now does not care if the session was from previous request or because the sticky cookie. and (1) can be simpler. Line 26: <filter-class>org.ovirt.engine.core.aaa.filters.RestApiSessionMgmtFilter</filter-class> Line 27: </filter> Line 28: <filter-mapping> Line 29: <filter-name>RestApiSessionMgmtFilter</filter-name> -- 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
