Yair Zaslavsky has posted comments on this change. Change subject: aaa: Intorduce filters ......................................................................
Patch Set 27: (8 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: } > we agreed that basic authentication cannot be done without suffix. i looked at the log of our conversation, and you mentioned that "user" will be used in ui only. isn't UI going to "cross" this filter as well? how would you like to handle that? 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"); > Realm="alon, balon" Well, currently from what I see at REST-API code (the original, before changes) the REAL was hardcoded to be ENGINE. Do you see any reason this should be changed? 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"; > I think we do not need it. i removed it as you suggested. 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. once again? eliminate what? 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 > is it possible? Hmm, The original rest-api code was in charge of generating the engine session id. I'm not against changing that. Currently in the code it is not possible. In addition I saw some differentiation at backend to whether UI or API addressed the backend when it comes to session. I would like to defer this point to later on if 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 { > yes, but we do not drag legacy... legacy ?:) The RestApiSessionMgmtFilter is not legacy :) I wrote the code several days ago :) Line 22: Line 23: private static Log log = LogFactory.getLog(RestApiSessionMgmtEpilogueFilter.class); Line 24: Line 25: private static final int MINIMAL_SESSION_TTL = 1; Line 36: Line 37: chain.doFilter(request, response); Line 38: HttpServletRequest req = (HttpServletRequest)request; Line 39: Line 40: if (request.getAttribute(FiltersHelper.Constants.REQUEST_PERSISTENT_AUTH) != null) { > why do you need this? extract the prefer header here, do not assume anythin Done Line 41: HttpSession session = req.getSession(); Line 42: try { Line 43: int ttlValue = Integer.parseInt(req.getHeader("Session-TTL")) * SECONDS_IN_MINUTE; Line 44: if (ttlValue >= MINIMAL_SESSION_TTL) { 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 32: if (session != null) { Line 33: session.invalidate(); Line 34: session = null; Line 35: } Line 36: } > this is the only statement that belongs to the 1st filter as far as I under but all this code checks when to invalidate the session, no? 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
