[java-identity-provider] branch master updated: IDP-1502 - Avoid object creation in the external web flows

Scott Cantor cantor.2 at osu.edu
Thu Sep 19 16:25:58 EDT 2019


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch master
in repository java-identity-provider.

View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=741498e27c053e27948763e35d991eff36ecdb74

The following commit(s) were added to refs/heads/master by this push:
       new  741498e   IDP-1502 - Avoid object creation in the external web flows
741498e is described below

commit 741498e27c053e27948763e35d991eff36ecdb74
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Sep 19 16:24:31 2019 -0400

    IDP-1502 - Avoid object creation in the external web flows
    
    https://issues.shibboleth.net/jira/browse/IDP-1502
    
    Adapt external interceptor flow design.
---
 .../main/resources/system/conf/webflow-config.xml  |   9 ++
 .../system/flows/intercept/external-flow.xml       |   3 +-
 .../context/ExternalInterceptorContext.java        |  24 +++-
 .../profile/interceptor/ExternalInterceptor.java   | 122 ++++++++++++++-------
 .../interceptor/impl/ExternalInterceptorImpl.java  |  54 +--------
 5 files changed, 119 insertions(+), 93 deletions(-)

diff --git a/idp-conf/src/main/resources/system/conf/webflow-config.xml b/idp-conf/src/main/resources/system/conf/webflow-config.xml
index 66ffbf7..7f75dd4 100644
--- a/idp-conf/src/main/resources/system/conf/webflow-config.xml
+++ b/idp-conf/src/main/resources/system/conf/webflow-config.xml
@@ -204,4 +204,13 @@
     <bean id="profileRequestContextFlowExecutionListener"
           class="net.shibboleth.idp.profile.support.ProfileRequestContextFlowExecutionListener" />
 
+    <!-- Expose the FlowExecutor for access via servlet context outside the webflows. -->
+	<bean class="org.springframework.web.context.support.ServletContextAttributeExporter">
+	   <property name="attributes">
+	       <map>
+	           <entry key="net.shibboleth.idp.flowExecutor" value-ref="flowExecutor"/>
+	       </map>
+	   </property>
+	</bean>
+
 </beans>
diff --git a/idp-conf/src/main/resources/system/flows/intercept/external-flow.xml b/idp-conf/src/main/resources/system/flows/intercept/external-flow.xml
index 49ff7fc..09c8fc1 100644
--- a/idp-conf/src/main/resources/system/flows/intercept/external-flow.xml
+++ b/idp-conf/src/main/resources/system/flows/intercept/external-flow.xml
@@ -10,8 +10,7 @@
     
     <view-state id="ExternalTransfer" view="externalRedirect:#{T(net.shibboleth.idp.profile.interceptor.ExternalInterceptor).getExternalRedirect(flowRequestContext.getActiveFlow().getApplicationContext().getBean('shibboleth.intercept.externalPathStrategy').apply(opensamlProfileRequestContext), flowExecutionContext.getKey().toString())}">
         <on-render>
-            <evaluate expression="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.profile.context.ProfileInterceptorContext)).addSubcontext(new net.shibboleth.idp.profile.context.ExternalInterceptorContext(), true).setFlowExecutionUrl(flowExecutionUrl + '&_eventId_proceed=1')" />
-            <evaluate expression="externalContext.getNativeRequest().getSession().setAttribute('conversation' + flowExecutionContext.getKey().toString(), new net.shibboleth.idp.profile.interceptor.impl.ExternalInterceptorImpl(opensamlProfileRequestContext))" />
+            <evaluate expression="opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.profile.context.ProfileInterceptorContext)).addSubcontext(new net.shibboleth.idp.profile.context.ExternalInterceptorContext(new net.shibboleth.idp.profile.interceptor.impl.ExternalInterceptorImpl()), true).setFlowExecutionUrl(flowExecutionUrl + '&_eventId_proceed=1')" />
         </on-render>
         <transition to="#{opensamlProfileRequestContext.getSubcontext(T(net.shibboleth.idp.profile.context.ProfileInterceptorContext)).getSubcontext(T(net.shibboleth.idp.profile.context.ExternalInterceptorContext)).getEventId()}" />
     </view-state>
diff --git a/idp-profile-api/src/main/java/net/shibboleth/idp/profile/context/ExternalInterceptorContext.java b/idp-profile-api/src/main/java/net/shibboleth/idp/profile/context/ExternalInterceptorContext.java
index 409522f..550c2d7 100644
--- a/idp-profile-api/src/main/java/net/shibboleth/idp/profile/context/ExternalInterceptorContext.java
+++ b/idp-profile-api/src/main/java/net/shibboleth/idp/profile/context/ExternalInterceptorContext.java
@@ -23,6 +23,9 @@ import javax.annotation.Nullable;
 import org.opensaml.messaging.context.BaseContext;
 import org.opensaml.profile.action.EventIds;
 
+import net.shibboleth.idp.profile.interceptor.ExternalInterceptor;
+import net.shibboleth.utilities.java.support.logic.Constraint;
+
 /**
  * A context representing the state of an externalized interceptor flow.
  * 
@@ -33,18 +36,35 @@ import org.opensaml.profile.action.EventIds;
  */
 public final class ExternalInterceptorContext extends BaseContext {
     
+    /** Implementation object. */
+    @Nonnull private final ExternalInterceptor externalInterceptor; 
+    
     /** Value of flowExecutionUrl on branching from flow. */
     @Nullable private String flowExecutionUrl;
 
     /** Event to signal. */
     @Nullable private String eventId;
     
-    /** Constructor. */
-    public ExternalInterceptorContext() {
+    /**
+     * Constructor.
+     * 
+     * @param interceptor implementation object
+     */
+    public ExternalInterceptorContext(@Nonnull final ExternalInterceptor interceptor) {
+        externalInterceptor = Constraint.isNotNull(interceptor, "ExternalInterceptor cannot be null");
         eventId = EventIds.PROCEED_EVENT_ID;
     }
     
     /**
+     * Get the {@link ExternalInterceptor} installed in the context.
+     * 
+     * @return the interceptor implementation
+     */
+    @Nonnull public ExternalInterceptor getExternalInterceptor() {
+        return externalInterceptor;
+    }
+    
+    /**
      * Get the flow execution URL to return control to.
      * 
      * @return return location
diff --git a/idp-profile-api/src/main/java/net/shibboleth/idp/profile/interceptor/ExternalInterceptor.java b/idp-profile-api/src/main/java/net/shibboleth/idp/profile/interceptor/ExternalInterceptor.java
index e0839ee..385e5f5 100644
--- a/idp-profile-api/src/main/java/net/shibboleth/idp/profile/interceptor/ExternalInterceptor.java
+++ b/idp-profile-api/src/main/java/net/shibboleth/idp/profile/interceptor/ExternalInterceptor.java
@@ -24,9 +24,17 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.opensaml.profile.context.ProfileRequestContext;
+import org.springframework.webflow.context.ExternalContextHolder;
+import org.springframework.webflow.context.servlet.ServletExternalContext;
+import org.springframework.webflow.execution.FlowExecution;
+import org.springframework.webflow.execution.repository.FlowExecutionRepository;
+import org.springframework.webflow.execution.repository.FlowExecutionRepositoryException;
+import org.springframework.webflow.executor.FlowExecutorImpl;
 
 import com.google.common.net.UrlEscapers;
 
+import net.shibboleth.idp.profile.context.ExternalInterceptorContext;
+import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
 import net.shibboleth.utilities.java.support.logic.Constraint;
 
@@ -35,8 +43,11 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  * 
  * @since 4.0.0
  */
-public class ExternalInterceptor {
+public abstract class ExternalInterceptor {
 
+    /** Parameter supplied to locate the SWF object needed in the session. */
+    @Nonnull @NotEmpty public static final String SWF_KEY = "net.shibboleth.idp.flowExecutor";
+    
     /** Parameter supplied to identify the per-conversation structure in the session. */
     @Nonnull @NotEmpty public static final String CONVERSATION_KEY = "conversation";
 
@@ -82,18 +93,16 @@ public class ExternalInterceptor {
      */
     @Nonnull @NotEmpty public static String startExternalInterceptor(@Nonnull final HttpServletRequest request)
             throws ExternalInterceptorException {
-        final String conv = request.getParameter(CONVERSATION_KEY);
-        if (conv == null || conv.isEmpty()) {
+        final String key = request.getParameter(CONVERSATION_KEY);
+        if (key == null || key.isEmpty()) {
             throw new ExternalInterceptorException("No conversation key found in request");
         }
         
-        final Object obj = request.getSession().getAttribute(CONVERSATION_KEY + conv);
-        if (obj == null || !(obj instanceof ExternalInterceptor)) {
-            throw new ExternalInterceptorException("No conversation state found in session for key (" + conv + ")");
-        }
+        final ProfileRequestContext profileRequestContext = getProfileRequestContext(key, request);
+        final ExternalInterceptorContext extContext = getExternalInterceptorContext(profileRequestContext);
+        extContext.getExternalInterceptor().doStart(request, profileRequestContext, extContext);
         
-        ((ExternalInterceptor) obj).doStart(request);
-        return conv;
+        return key;
     }
     
     /**
@@ -112,14 +121,9 @@ public class ExternalInterceptor {
             @Nonnull final HttpServletRequest request, @Nonnull final HttpServletResponse response)
             throws ExternalInterceptorException, IOException {
         
-        final Object obj = request.getSession().getAttribute(CONVERSATION_KEY + key);
-        if (obj == null || !(obj instanceof ExternalInterceptor)) {
-            throw new ExternalInterceptorException("No conversation state found in session for key (" + key + ")");
-        }
-        
-        request.getSession().removeAttribute(CONVERSATION_KEY + key);
-        
-        ((ExternalInterceptor) obj).doFinish(request, response);
+        final ProfileRequestContext profileRequestContext = getProfileRequestContext(key, request);
+        final ExternalInterceptorContext extContext = getExternalInterceptorContext(profileRequestContext);
+        extContext.getExternalInterceptor().doFinish(request, response, profileRequestContext, extContext);
     }
 
     /**
@@ -133,13 +137,56 @@ public class ExternalInterceptor {
      */
     @Nonnull public static ProfileRequestContext getProfileRequestContext(@Nonnull @NotEmpty final String key,
             @Nonnull final HttpServletRequest request) throws ExternalInterceptorException {
+
+        final Object obj = request.getServletContext().getAttribute(SWF_KEY);
+        if (!(obj instanceof FlowExecutorImpl)) {
+            throw new ExternalInterceptorException("No FlowExecutor available in servlet context");
+        }
+
+        try {
+            final FlowExecutionRepository repo = ((FlowExecutorImpl) obj).getExecutionRepository();
+            ExternalContextHolder.setExternalContext(
+                    new ServletExternalContext(request.getServletContext(), request, null));
+            
+            final FlowExecution execution = repo.getFlowExecution(repo.parseFlowExecutionKey(key));
+            final Object prc = execution.getConversationScope().get(ProfileRequestContext.BINDING_KEY);
+            if (!(prc instanceof ProfileRequestContext)) {
+                throw new ExternalInterceptorException(
+                        "ProfileRequestContext not available in webflow conversation scope");
+            }
+            
+            return (ProfileRequestContext) prc;
+        } catch (final FlowExecutionRepositoryException e) {
+            throw new ExternalInterceptorException("Error retrieving flow conversation", e);
+        } finally {
+            ExternalContextHolder.setExternalContext(null);
+        }
+    }
+    
+    /**
+     * Utility method to access the {@link ExternalInterceptorContext}.
+     * 
+     * @param profileRequestContext profile request context
+     * 
+     * @return the {@link ExternalInterceptorContext} to operate on
+     * 
+     * @throws ExternalInterceptorException if the context is missing
+     */
+    @Nonnull private static ExternalInterceptorContext getExternalInterceptorContext(
+            @Nonnull final ProfileRequestContext profileRequestContext) throws ExternalInterceptorException {
         
-        final Object obj = request.getSession().getAttribute(CONVERSATION_KEY + key);
-        if (obj == null || !(obj instanceof ExternalInterceptor)) {
-            throw new ExternalInterceptorException("No conversation state found in session");
+        final ProfileInterceptorContext piContext =
+                profileRequestContext.getSubcontext(ProfileInterceptorContext.class);
+        if (piContext == null) {
+            throw new ExternalInterceptorException("No ProfileInterceptorContext found");
         }
         
-        return ((ExternalInterceptor) obj).getProfileRequestContext(request);
+        final ExternalInterceptorContext extContext = piContext.getSubcontext(ExternalInterceptorContext.class);
+        if (extContext == null) {
+            throw new ExternalInterceptorException("No ExternalInterceptorContext found");
+        }
+        
+        return extContext;
     }
     
     /**
@@ -147,11 +194,15 @@ public class ExternalInterceptor {
      * the servlet session and exposing it as request attributes.
      * 
      * @param request servlet request
+     * @param profileRequestContext profile request context
+     * @param externalInterceptorContext external interceptor context
      * 
      * @throws ExternalInterceptorException if an error occurs
      */
-    protected void doStart(@Nonnull final HttpServletRequest request) throws ExternalInterceptorException {
-        throw new ExternalInterceptorException("Not implemented");
+    protected void doStart(@Nonnull final HttpServletRequest request,
+            @Nonnull final ProfileRequestContext profileRequestContext,
+            @Nonnull final ExternalInterceptorContext externalInterceptorContext) throws ExternalInterceptorException {
+        request.setAttribute(ProfileRequestContext.BINDING_KEY, profileRequestContext);
     }
 
     /**
@@ -161,26 +212,15 @@ public class ExternalInterceptor {
      * 
      * @param request servlet request
      * @param response servlet response
+     * @param profileRequestContext profile request context
+     * @param externalInterceptorContext external interceptor context
      * 
      * @throws ExternalInterceptorException if an error occurs
      * @throws IOException if the redirect cannot be issued
      */
-    protected void doFinish(@Nonnull final HttpServletRequest request, @Nonnull final HttpServletResponse response)
-            throws ExternalInterceptorException, IOException {
-        throw new ExternalInterceptorException("Not implemented");
-    }
-    
-    /**
-     * Get the {@link ProfileRequestContext} associated with a request.
-     * 
-     * @param request servlet request
-     * 
-     * @return the profile request context
-     * @throws ExternalInterceptorException if an error occurs
-     */
-    @Nonnull protected ProfileRequestContext getProfileRequestContext(@Nonnull final HttpServletRequest request)
-            throws ExternalInterceptorException {
-        throw new ExternalInterceptorException("Not implemented");
-    }
-    
+    protected abstract void doFinish(@Nonnull final HttpServletRequest request,
+            @Nonnull final HttpServletResponse response, @Nonnull final ProfileRequestContext profileRequestContext,
+            @Nonnull final ExternalInterceptorContext externalInterceptorContext)
+                    throws ExternalInterceptorException, IOException;
+
 }
\ No newline at end of file
diff --git a/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/interceptor/impl/ExternalInterceptorImpl.java b/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/interceptor/impl/ExternalInterceptorImpl.java
index 68ae604..3b2f098 100644
--- a/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/interceptor/impl/ExternalInterceptorImpl.java
+++ b/idp-profile-impl/src/main/java/net/shibboleth/idp/profile/interceptor/impl/ExternalInterceptorImpl.java
@@ -24,14 +24,10 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.opensaml.profile.context.ProfileRequestContext;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import net.shibboleth.idp.profile.context.ExternalInterceptorContext;
-import net.shibboleth.idp.profile.context.ProfileInterceptorContext;
 import net.shibboleth.idp.profile.interceptor.ExternalInterceptor;
 import net.shibboleth.idp.profile.interceptor.ExternalInterceptorException;
-import net.shibboleth.utilities.java.support.logic.Constraint;
 
 /**
  * Implementation of the {@link ExternalInterceptor} API that handles moving information in and out
@@ -41,57 +37,19 @@ import net.shibboleth.utilities.java.support.logic.Constraint;
  */
 public class ExternalInterceptorImpl extends ExternalInterceptor {
 
-    /** Class logger. */
-    @Nonnull private final Logger log = LoggerFactory.getLogger(ExternalInterceptorImpl.class);
-    
-    /** State of request to pull from. */
-    @Nonnull private final ProfileRequestContext profileRequestContext;
-
-    /**
-     * Constructor.
-     * 
-     * @param input profile request context to expose
-     */
-    public ExternalInterceptorImpl(@Nonnull final ProfileRequestContext input) {
-        profileRequestContext = Constraint.isNotNull(input, "ProfileRequestContext cannot be null");
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    protected void doStart(@Nonnull final HttpServletRequest request) {
-        request.setAttribute(ProfileRequestContext.BINDING_KEY, profileRequestContext);
-    }
-
     /** {@inheritDoc} */
     @Override
-    protected void doFinish(@Nonnull final HttpServletRequest request, @Nonnull final HttpServletResponse response)
-            throws IOException, ExternalInterceptorException {
-        final ProfileInterceptorContext piContext =
-                profileRequestContext.getSubcontext(ProfileInterceptorContext.class);
-        if (piContext == null) {
-            throw new ExternalInterceptorException("No ProfileInterceptorContext found");
-        }
-        
-        final ExternalInterceptorContext extContext = piContext.getSubcontext(ExternalInterceptorContext.class);
-        if (extContext == null) {
-            throw new ExternalInterceptorException("No ExternalInterceptorContext found");
-        } else if (extContext.getFlowExecutionUrl() == null) {
-            throw new ExternalInterceptorException("No flow execution URL found to return control");
-        }
+    protected void doFinish(@Nonnull final HttpServletRequest request, @Nonnull final HttpServletResponse response,
+            @Nonnull final ProfileRequestContext profileRequestContext,
+            @Nonnull final ExternalInterceptorContext externalContext)
+                    throws IOException, ExternalInterceptorException {
         
         final Object attr = request.getAttribute(EVENT_KEY);
         if (attr != null && attr instanceof String) {
-            extContext.setEventId((String) attr);
+            externalContext.setEventId((String) attr);
         }
                 
-        response.sendRedirect(extContext.getFlowExecutionUrl());
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    protected ProfileRequestContext getProfileRequestContext(@Nonnull final HttpServletRequest request)
-            throws ExternalInterceptorException {
-        return profileRequestContext;
+        response.sendRedirect(externalContext.getFlowExecutionUrl());
     }
     
 }
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list