[ 
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)

Reply via email to