Alon Bar-Lev has posted comments on this change. Change subject: aaa: Rename Authentication filter ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/27284/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-05-01 08:13:05 +0300 Line 4: Commit: Yair Zaslavsky <[email protected]> Line 5: CommitDate: 2014-05-01 08:17:58 +0300 Line 6: Line 7: aaa: Rename Authentication filter this is far more than rename :) Line 8: Line 9: Change-Id: Iee6d0c5805e4509f4011dd9ebc994fba0f419f55 Line 10: Topic: AAA http://gerrit.ovirt.org/#/c/27284/1/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/NegotiationAuthnFilter.java File backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/NegotiationAuthnFilter.java: Line 61: * stacks of profiles later when processing requests. Line 62: */ Line 63: private void findNegotiatingProfiles() { Line 64: if (profiles == null) { Line 65: synchronized(this) { why is this code synchronized? oh... it happens one time... also need to move to init of servlet, no? or your problem is that the init bean is called after? Line 66: if (profiles == null) { Line 67: profiles = new ArrayList<AuthenticationProfile>(1); Line 68: for (AuthenticationProfile profile : AuthenticationProfileRepository.getInstance().getProfiles()) { Line 69: if (profile != null) { Line 112: chain.doFilter(req, rsp); Line 113: return; Line 114: } Line 115: Line 116: caps = 0; you should not change object state at doFilter... it should have been passed as a parameter. Line 117: for (String nego : req.getServletContext().getInitParameter(INTERACTIVE_NEGOTIATION_PARAMETER).split(",")) { Line 118: if (nego.equals("interactive")) { Line 119: caps |= Authn.Capabilities.AUTHENTICATE_NEGOTIATE_INTERACTIVE; Line 120: } else if (nego.equals("non-interactive")) { Line 113: return; Line 114: } Line 115: Line 116: caps = 0; Line 117: for (String nego : req.getServletContext().getInitParameter(INTERACTIVE_NEGOTIATION_PARAMETER).split(",")) { please get and parse parameter at servlet init. this gives you the option to perform validation before application starts. Line 118: if (nego.equals("interactive")) { Line 119: caps |= Authn.Capabilities.AUTHENTICATE_NEGOTIATE_INTERACTIVE; Line 120: } else if (nego.equals("non-interactive")) { Line 121: caps |= Authn.Capabilities.AUTHENTICATE_NEGOTIATE_NON_INTERACTIVE; Line 114: } Line 115: Line 116: caps = 0; Line 117: for (String nego : req.getServletContext().getInitParameter(INTERACTIVE_NEGOTIATION_PARAMETER).split(",")) { Line 118: if (nego.equals("interactive")) { please use capabilities terms. parameter should be AUTHN_CAPABILITIES, values AUTHENTICATE_NEGOTIATE_INTERACTIVE, AUTHENTICATE_NEGOTIATE_NON_INTERACTIVE Line 119: caps |= Authn.Capabilities.AUTHENTICATE_NEGOTIATE_INTERACTIVE; Line 120: } else if (nego.equals("non-interactive")) { Line 121: caps |= Authn.Capabilities.AUTHENTICATE_NEGOTIATE_NON_INTERACTIVE; Line 122: } http://gerrit.ovirt.org/#/c/27284/1/frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml File frontend/webadmin/modules/webadmin/src/main/webapp/WEB-INF/web.xml: Line 25: <filter> Line 26: <filter-name>NegotiationAuthnFilter</filter-name> Line 27: <filter-class>org.ovirt.engine.core.aaa.NegotiationAuthnFilter</filter-class> Line 28: <init-param> Line 29: <param-name>negotiation</param-name> please use capabilities terms and symbols, it will be easier to understand. Line 30: <param-value>non-interactive,interactive</param-value> Line 31: </init-param> Line 32: </filter> Line 33: -- To view, visit http://gerrit.ovirt.org/27284 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iee6d0c5805e4509f4011dd9ebc994fba0f419f55 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[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
