Alon Bar-Lev has posted comments on this change. Change subject: aaa: Intorduce filters ......................................................................
Patch Set 27: (9 comments) http://gerrit.ovirt.org/#/c/28022/27/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 84: result = translateUserRestApiSpecific(translateFrom); Line 85: } Line 86: if (result == null) { Line 87: result = new UserProfile(translateFrom, null); Line 88: } > what about the case of "user" in user name format? (no domain, no profile). we agreed that basic authentication cannot be done without suffix. Line 89: return result; Line 90: } Line 91: Line 92: private UserProfile translateUserProfileUpn(String translateFrom) { http://gerrit.ovirt.org/#/c/28022/27/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/EnforceAuthFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/EnforceAuthFilter.java: Line 21: private List<String> additionalSchemes; Line 22: Line 23: @Override Line 24: public void init(FilterConfig filterConfig) throws ServletException { Line 25: String schemes = filterConfig.getInitParameter("schemes"); > where do we need comma within the name? Realm="alon, balon" Line 26: if (schemes == null) { Line 27: schemes = ""; Line 28: } Line 29: additionalSchemes = Arrays.asList(schemes.split(",")); http://gerrit.ovirt.org/#/c/28022/27/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 13: public final static String HTTP_SESSION_ENGINE_SESSION_ID_KEY = "engineSessionId"; Line 14: public final static String REQUEST_AUTH_RECORD_KEY = "auth_record"; Line 15: public final static String REQUEST_SCHEMES_KEY = "schemes"; Line 16: public final static String REQUEST_PROFILE_KEY = "profile"; Line 17: public final static String REQUEST_PERSISTENT_AUTH = "presist_auth"; > done. I think we do not need it. Line 18: } Line 19: Line 20: public static BackendLocal getBackend(Context context) { Line 21: http://gerrit.ovirt.org/#/c/28022/27/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 44: ExtMap authRecord = (ExtMap) request.getAttribute(FiltersHelper.Constants.REQUEST_AUTH_RECORD_KEY); Line 45: if (authRecord != null) { Line 46: String engineSessionId = UUID.randomUUID().toString(); Line 47: InitialContext context = new InitialContext(); Line 48: try { move here... at least, if not eliminate. Line 49: VdcReturnValueBase returnValue = FiltersHelper.getBackend(context).login(new Line 50: LoginUserParameters( Line 51: profileName, Line 52: authRecord, Line 56: if (returnValue.getSucceeded()) { Line 57: HttpSession session = req.getSession(true); Line 58: session.setAttribute( Line 59: FiltersHelper.Constants.HTTP_SESSION_ENGINE_SESSION_ID_KEY, Line 60: engineSessionId > I expect to write here returnValue.getSessionId() and get rid of me trying is it possible? Line 61: ); Line 62: } Line 63: } finally { Line 64: context.close(); http://gerrit.ovirt.org/#/c/28022/27/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtEpilogueFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtEpilogueFilter.java: Line 17: import org.ovirt.engine.core.common.action.VdcActionParametersBase; Line 18: import org.ovirt.engine.core.common.action.VdcActionType; Line 19: Line 20: Line 21: public class RestApiSessionMgmtEpilogueFilter implements Filter { > please pay attention that this is the 2nd part... the original code of Rest yes, but we do not drag legacy... Line 22: Line 23: private static Log log = LogFactory.getLog(RestApiSessionMgmtEpilogueFilter.class); Line 24: Line 25: private static final int MINIMAL_SESSION_TTL = 1; http://gerrit.ovirt.org/#/c/28022/27/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtPrologueFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtPrologueFilter.java: Line 22: public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, Line 23: ServletException { Line 24: HttpServletRequest req = (HttpServletRequest) request; Line 25: Enumeration<String> preferValues = req.getHeaders("Prefer"); Line 26: while (preferValues.hasMoreElements()) { please consider: for (String value : Collections.list(req.getHeaders("Prefer")) { } Line 27: if ("persistent-auth".equals(preferValues.nextElement())) { Line 28: request.setAttribute(FiltersHelper.Constants.REQUEST_PERSISTENT_AUTH, true); Line 29: HttpSession session = req.getSession(false); Line 30: if ((req.getHeader("Authorization") != null)) { Line 25: Enumeration<String> preferValues = req.getHeaders("Prefer"); Line 26: while (preferValues.hasMoreElements()) { Line 27: if ("persistent-auth".equals(preferValues.nextElement())) { Line 28: request.setAttribute(FiltersHelper.Constants.REQUEST_PERSISTENT_AUTH, true); Line 29: HttpSession session = req.getSession(false); > which previous filter? this is first in chain... the previous in review :) the last in chain, the session management. Line 30: if ((req.getHeader("Authorization") != null)) { Line 31: // No need to pass credentials again - if passed, login should be called Line 32: if (session != null) { Line 33: session.invalidate(); Line 32: if (session != null) { Line 33: session.invalidate(); Line 34: session = null; Line 35: } Line 36: } > see my above comment, not sure i understand. this is the only statement that belongs to the 1st filter as far as I understand. the rest can go into the session management filter. Line 37: break; Line 38: } Line 39: } Line 40: chain.doFilter(request, response); -- 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: 27 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
