[ https://issues.apache.org/jira/browse/FEDIZ-217?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16477395#comment-16477395 ]
ASF GitHub Bot commented on FEDIZ-217: -------------------------------------- coheigea closed pull request #27: [FEDIZ-217] Fix SAML authentication in Plugin URL: https://github.com/apache/cxf-fediz/pull/27 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/plugins/core/src/main/java/org/apache/cxf/fediz/core/FederationConstants.java b/plugins/core/src/main/java/org/apache/cxf/fediz/core/FederationConstants.java index 6839ff50..88bd2730 100644 --- a/plugins/core/src/main/java/org/apache/cxf/fediz/core/FederationConstants.java +++ b/plugins/core/src/main/java/org/apache/cxf/fediz/core/FederationConstants.java @@ -150,6 +150,12 @@ * element. */ public static final String PARAM_RESULT_PTR = "wresultptr"; + + /** + * This OPTIONAL session attribute prefix append to request RelayState value specifies + * initial RequestState created before redirecting to IDP + */ + public static final String SESSION_SAVED_REQUEST_STATE_PREFIX = "SAVED_REQUEST_STATE_"; public static final Map<String, URI> AUTH_TYPE_MAP; static { diff --git a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java index 31aefcd0..125e9fc9 100644 --- a/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java +++ b/plugins/core/src/main/java/org/apache/cxf/fediz/core/handler/SigninHandler.java @@ -23,8 +23,10 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import org.apache.cxf.fediz.core.FederationConstants; +import org.apache.cxf.fediz.core.RequestState; import org.apache.cxf.fediz.core.SAMLSSOConstants; import org.apache.cxf.fediz.core.config.FederationProtocol; import org.apache.cxf.fediz.core.config.FedizContext; @@ -101,13 +103,22 @@ public FedizResponse processSigninRequest(String responseToken, HttpServletReque FedizRequest federationRequest = new FedizRequest(); String wa = req.getParameter(FederationConstants.PARAM_ACTION); + + String relayState = req.getParameter("RelayState"); federationRequest.setAction(wa); federationRequest.setResponseToken(responseToken); - federationRequest.setState(req.getParameter("RelayState")); + federationRequest.setState(relayState); federationRequest.setRequest(req); federationRequest.setCerts((X509Certificate[])req.getAttribute("javax.servlet.request.X509Certificate")); + if (relayState != null) { + HttpSession session = req.getSession(); + federationRequest.setRequestState((RequestState) + session.getAttribute(FederationConstants.SESSION_SAVED_REQUEST_STATE_PREFIX + relayState)); + session.removeAttribute(FederationConstants.SESSION_SAVED_REQUEST_STATE_PREFIX + relayState); + } + FedizProcessor processor = FedizProcessorFactory.newFedizProcessor(fedizContext.getProtocol()); return processor.processRequest(federationRequest, fedizContext); } diff --git a/plugins/core/src/main/java/org/apache/cxf/fediz/core/processor/SAMLProcessorImpl.java b/plugins/core/src/main/java/org/apache/cxf/fediz/core/processor/SAMLProcessorImpl.java index fc227e16..6f8d167a 100644 --- a/plugins/core/src/main/java/org/apache/cxf/fediz/core/processor/SAMLProcessorImpl.java +++ b/plugins/core/src/main/java/org/apache/cxf/fediz/core/processor/SAMLProcessorImpl.java @@ -134,6 +134,7 @@ protected FedizResponse processSignInRequest( tokenStream = CompressionUtils.inflate(deflatedToken); } } catch (DataFormatException ex) { + LOG.warn("Invalid data format", ex); throw new ProcessingException(TYPE.INVALID_REQUEST); } @@ -144,7 +145,7 @@ protected FedizResponse processSignInRequest( el = doc.getDocumentElement(); } catch (Exception e) { - LOG.warn("Failed to parse token: " + e.getMessage()); + LOG.warn("Failed to parse token", e); throw new ProcessingException(TYPE.INVALID_REQUEST); } diff --git a/plugins/tomcat8/src/main/java/org/apache/cxf/fediz/tomcat8/FederationAuthenticator.java b/plugins/tomcat8/src/main/java/org/apache/cxf/fediz/tomcat8/FederationAuthenticator.java index 1acd5514..ff92c695 100644 --- a/plugins/tomcat8/src/main/java/org/apache/cxf/fediz/tomcat8/FederationAuthenticator.java +++ b/plugins/tomcat8/src/main/java/org/apache/cxf/fediz/tomcat8/FederationAuthenticator.java @@ -43,6 +43,7 @@ import org.apache.catalina.connector.Response; import org.apache.cxf.fediz.core.FederationConstants; import org.apache.cxf.fediz.core.FedizPrincipal; +import org.apache.cxf.fediz.core.RequestState; import org.apache.cxf.fediz.core.config.FedizConfigurator; import org.apache.cxf.fediz.core.config.FedizContext; import org.apache.cxf.fediz.core.exception.ProcessingException; @@ -299,7 +300,7 @@ protected void redirectToIdp(Request request, HttpServletResponse response, Fedi // Save original request in our session try { - saveRequest(request, redirectionResponse.getRequestState().getState()); + saveRequest(request, redirectionResponse.getRequestState()); } catch (IOException ioe) { LOG.debug("Request body too big to save during authentication"); response.sendError(HttpServletResponse.SC_FORBIDDEN, sm @@ -333,7 +334,8 @@ protected boolean matchRequest(Request request) { return false; } - protected void saveRequest(Request request, String contextId) throws IOException { + protected void saveRequest(Request request, RequestState requestState) throws IOException { + String contextId = requestState.getState(); String uri = request.getDecodedRequestURI(); Session session = request.getSessionInternal(true); if (session != null) { @@ -352,6 +354,9 @@ protected void saveRequest(Request request, String contextId) throws IOException sb.append(saved.getQueryString()); } session.setNote(SESSION_SAVED_URI_PREFIX + contextId, sb.toString()); + //we set Request State as session attribute for later retrieval in SigninHandler + request.getSession().setAttribute( + FederationConstants.SESSION_SAVED_REQUEST_STATE_PREFIX + requestState.getState(), requestState); } } ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > SAML authentication fails in plugin > ----------------------------------- > > Key: FEDIZ-217 > URL: https://issues.apache.org/jira/browse/FEDIZ-217 > Project: CXF-Fediz > Issue Type: Bug > Components: Plugin > Affects Versions: 1.4.3 > Reporter: Arnaud MERGEY > Assignee: Colm O hEigeartaigh > Priority: Major > Fix For: 1.4.4 > > > On a tomcat hosting a SP application trying to authenticate against a SAML > IDP (OKTA) > authentication fails with this log: > May 11, 2018 11:22:14 AM > org.apache.cxf.fediz.core.processor.SAMLProcessorImpl processRelayState > SEVERE: Missing Request State > May 11, 2018 11:22:14 AM org.apache.cxf.fediz.core.handler.SigninHandler > handleRequest > SEVERE: Federation processing failed: The request was invalid or malformed > I checked in the code and it fails because request state in > org.apache.cxf.fediz.core.processor.FedizRequest is null, but it seems with > SAML protocol > org.apache.cxf.fediz.core.processor.FedizRequest.setRequestState(RequestState) > is never called, so I am wondering how it can be different from null and I > suspect a bug > I manage to patch fediz to have it working, I could propose a Pull request > for this if required > Additionally to OKTA I also tried with samling for a simple test setup, same > error > > {code:java} > <FedizConfig> > <contextConfig name="/myApp"> > <audienceUris> > <audienceItem>http://localhost:8080/myApp/</audienceItem> > </audienceUris> > <certificateStores> > <trustManager> > <keyStore file="/opt/tomcat/.keystore" password="changeit" > type="JKS" /> > </trustManager> > </certificateStores> > <trustedIssuers> > <issuer certificateValidation="PeerTrust" /> > </trustedIssuers> > <protocol xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" > xsi:type="samlProtocolType" version="2.0"> > <disableDeflateEncoding>true</disableDeflateEncoding> > <doNotEnforceKnownIssuer>true</doNotEnforceKnownIssuer> > <issuer>https://capriza.github.io/samling/samling.html</issuer> > <roleURI>groups</roleURI> > </protocol> > </contextConfig> > </FedizConfig> > {code} -- This message was sent by Atlassian JIRA (v7.6.3#76005)