Support wild cards in allowed external redirect hosts

api/src/org/labkey/api/action/BaseApiAction.java

@@ -92,13 +92,13 @@ private Marshaller findMarshaller()
         {
             Class<?> superClass = getClass().getSuperclass();
             if (null != superClass)
-                marshal = (Marshal) superClass.getAnnotation(Marshal.class);
+                marshal = superClass.getAnnotation(Marshal.class);
         }
         if (marshal == null)
         {
             Class<?> declaringClass = getClass().getDeclaringClass();
             if (declaringClass != null)
-                marshal = (Marshal)declaringClass.getAnnotation(Marshal.class);
+                marshal = declaringClass.getAnnotation(Marshal.class);
         }
 
         if (marshal != null)

api/src/org/labkey/api/action/ConfirmAction.java

@@ -77,7 +77,7 @@ public final ModelAndView handleRequest() throws Exception
                     ModelAndView mv = getSuccessView(form);
                     if (null != mv)
                         return mv;
-                    throw new RedirectException(getSuccessURL(form), false);
+                    throw new RedirectException(getSuccessURL(form));
                 }
             }
             else

api/src/org/labkey/api/action/FormViewAction.java

@@ -95,7 +95,7 @@ public ModelAndView handleRequest(FORM form, BindException errors) throws Except
             {
                 URLHelper url = getSuccessURL(form);
                 if (null != url)
-                    return HttpView.redirect(url, false);
+                    return HttpView.redirect(url);
                 try (Timing ignored = MiniProfiler.step("createView"))
                 {
                     ModelAndView successView = getSuccessView(form);

api/src/org/labkey/api/action/SimpleRedirectAction.java

@@ -15,10 +15,9 @@
  */
 package org.labkey.api.action;
 
-import org.labkey.api.util.URLHelper;
 import org.labkey.api.view.ActionURL;
-import org.labkey.api.view.HttpView;
 import org.labkey.api.view.NavTree;
+import org.labkey.api.view.RedirectException;
 import org.springframework.validation.BindException;
 import org.springframework.web.servlet.ModelAndView;
 
@@ -31,7 +30,7 @@ public abstract class SimpleRedirectAction<FORM> extends SimpleViewAction<FORM>
     @Override
     public final ModelAndView getView(FORM form, BindException errors) throws Exception
     {
-        URLHelper url;
+        ActionURL url;
 
         try
         {
@@ -45,15 +44,15 @@ public final ModelAndView getView(FORM form, BindException errors) throws Except
         if (null != getViewContext().getRequest().getHeader("template"))
             url.addParameter(ActionURL.Param._template.name(), getViewContext().getRequest().getHeader("template"));
 
-        return HttpView.redirect(url, url.isAllowableHost());
+        throw new RedirectException(url);
     }
 
     @Override
     public final void addNavTrail(NavTree root)
     {
     }
 
-    public abstract URLHelper getRedirectURL(FORM form) throws Exception;
+    public abstract ActionURL getRedirectURL(FORM form) throws Exception;
 
     // Called whenever getRedirectURL() throws an exception.  Standard code rethrows the exception.  Override this
     // method to customize the handling of certain exceptions (e.g., display an error view). 

api/src/org/labkey/api/action/SpringActionController.java

@@ -79,7 +79,6 @@
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.ViewResolver;
 import org.springframework.web.servlet.mvc.Controller;
-import org.springframework.web.servlet.view.RedirectView;
 
 import java.io.File;
 import java.io.IOException;
@@ -439,9 +438,8 @@ public ModelAndView handleRequest(HttpServletRequest request, @NotNull HttpServl
 
             if (null != redirectURL)
             {
-                _log.debug("URL " + url + " was redirected to " + redirectURL + " instead");
-                response.sendRedirect(redirectURL.toString());
-                return null;
+                _log.debug("URL {} was redirected to {} instead", url, redirectURL);
+                throw new RedirectException(redirectURL);
             }
 
             // This container check used to be in checkPermissions(), but that meant actions with lenient permissions checking
@@ -534,11 +532,6 @@ public ModelAndView handleRequest(HttpServletRequest request, @NotNull HttpServl
             ModelAndView mv = controller.handleRequest(request, response);
             if (mv != null)
             {
-                if (mv.getView() instanceof RedirectView)
-                {
-                    // treat same as a throw redirect
-                    throw new RedirectException(((RedirectView)mv.getView()).getUrl());
-                }
                 renderInTemplate(context, controller, pageConfig, mv);
             }
         }
@@ -596,14 +589,14 @@ protected void handleException(Throwable x, ViewContext context, PageConfig page
             if (!"1".equals(getViewContext().getActionURL().getParameter("_retry_")))
             {
                 ActionURL url = getViewContext().cloneActionURL().addParameter("_retry_", "1");
-                ExceptionUtil.doErrorRedirect(response, url.getLocalURIString());
+                ExceptionUtil.unsafeRedirect(response, url.getLocalURIString());
                 return;
             }
         }
 
         ActionURL errorURL = ExceptionUtil.handleException(request, response, x, null, false, context, pageConfig);
         if (null != errorURL)
-            ExceptionUtil.doErrorRedirect(response, errorURL.toString());
+            ExceptionUtil.unsafeRedirect(response, errorURL.toString());
     }
 
 

api/src/org/labkey/api/assay/actions/ReimportRedirectAction.java

@@ -22,7 +22,6 @@
 import org.labkey.api.exp.api.ExperimentService;
 import org.labkey.api.security.RequiresPermission;
 import org.labkey.api.security.permissions.InsertPermission;
-import org.labkey.api.util.URLHelper;
 import org.labkey.api.view.ActionURL;
 import org.labkey.api.view.NotFoundException;
 
@@ -36,7 +35,7 @@
 public class ReimportRedirectAction extends SimpleRedirectAction<ProtocolIdForm>
 {
     @Override
-    public URLHelper getRedirectURL(ProtocolIdForm form)
+    public ActionURL getRedirectURL(ProtocolIdForm form)
     {
         Set<String> selectedRunIds = DataRegionSelection.getSelected(getViewContext(), true);
         if (selectedRunIds.isEmpty())

api/src/org/labkey/api/reports/report/RedirectReport.java

@@ -15,7 +15,6 @@
  */
 package org.labkey.api.reports.report;
 
-import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
@@ -24,17 +23,12 @@
 import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.HttpView;
 import org.labkey.api.view.JspView;
-import org.labkey.api.view.RedirectException;
 import org.labkey.api.view.ViewContext;
 
 import java.net.MalformedURLException;
+import java.net.URISyntaxException;
 import java.net.URL;
 
-/**
- * User: migra
- * Date: Mar 7, 2006
- * Time: 3:00:54 PM
- */
 public abstract class RedirectReport extends AbstractReport
 {
     private static final Logger LOG = LogHelper.getLogger(RedirectReport.class, "Reports that send the user to some other URL");
@@ -49,15 +43,17 @@ public RedirectReport()
     @Override
     public HttpView<?> renderReport(ViewContext context)
     {
-        String url = getUrl(context.getContainer());
-
         // When rendering in a portal webpart, render the redirect link and thumbnail
         if (context.get(renderParam.reportWebPart.name()) != null)
             return new JspView<>("/org/labkey/api/reports/report/view/redirectReportWebPart.jsp", this);
 
-        throw new RedirectException(url);
+        redirect(context);
+
+        return null;
     }
 
+    protected abstract void redirect(ViewContext context);
+
     @Override
     public String getRunReportTarget()
     {
@@ -85,11 +81,29 @@ public void setUrl(URLHelper url)
         getDescriptor().setProperty(REDIRECT_URL, redirectUrl);
     }
 
+    // TODO: Returning URLHelper from here would clean up a lot of messy code in these classes
     public @Nullable String getUrl(Container c)
     {
         return getDescriptor().getProperty(REDIRECT_URL);
     }
 
+    protected @Nullable URLHelper getURLHelper(ViewContext ctx)
+    {
+        String urlString = getUrl(ctx.getContainer());
+        if (urlString != null)
+        {
+            try
+            {
+                return new URLHelper(urlString);
+            }
+            catch (URISyntaxException e)
+            {
+                LOG.warn("Bad URL in report {}: {}", getReportId(), urlString);
+            }
+        }
+        return null;
+    }
+
     public @Nullable URL getURL()
     {
         String urlString = getUrl(null);
@@ -111,20 +125,6 @@ public void setUrl(URLHelper url)
         }
     }
 
-    /**
-     * URL is not a LabKey local URL or a host local URL.
-     *
-     */
-    public boolean isExternalLink()
-    {
-        String url = getUrl(null);
-        if (url == null)
-            return false;
-
-        return (url.startsWith("http://") || url.startsWith("https://")) &&
-               !isInternalLink(url) && !isLocalLink(url);
-    }
-
     /**
      * URL has same hostname as this LabKey server, but is not under the LabKey webapp.
      *

api/src/org/labkey/api/security/AuthFilter.java

@@ -110,27 +110,6 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
             return;
         }
 
-        // Versions of tomcat prior to 7.0.67 would automatically redirect if a URL with just a context path without
-        // a trailing slash was entered ie: http://www.labkey.org/labkey would automatically redirect to: http://www.labkey.org/labkey/
-        // We need to handle it here otherwise the begin page redirect in index.html will fail: issue 25395
-
-        // getServletPath() will return an empty String when a URL with a context path but no trailing slash is requested
-        if (req.getServletPath().isEmpty())
-        {
-            // now check if this is the case where the request URL contains only the context path
-            if (req.getContextPath() != null && req.getRequestURL().toString().endsWith(req.getContextPath()))
-            {
-                StringBuilder redirectURL = new StringBuilder();
-                redirectURL.append(req.getRequestURL()).append("/");
-                if (!StringUtils.isBlank(req.getQueryString()))
-                {
-                    redirectURL.append("?").append(req.getQueryString());
-                }
-                resp.sendRedirect(redirectURL.toString());
-                return;
-            }
-        }
-
         if (AppProps.getInstance().isSSLRequired())
         {
             // No startup failure, so check for SSL redirection
@@ -167,8 +146,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
                 }
                 url = new URL("https", url.getHost(), port, url.getFile());
                 // Use 301 redirect instead of a 302 to indicate it's a permanent move
-                resp.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
-                resp.setHeader("Location", resp.encodeRedirectURL(url.toString()));
+                ExceptionUtil.unsafeRedirect(resp, url.toString(), HttpServletResponse.SC_MOVED_PERMANENTLY);
                 return;
             }
             else if (!AppProps.getInstance().isDevMode())

api/src/org/labkey/api/security/AuthenticationConfiguration.java

@@ -98,7 +98,7 @@ interface LoginFormAuthenticationConfiguration<AP extends LoginFormAuthenticatio
     interface SSOAuthenticationConfiguration<AP extends SSOAuthenticationProvider<?>> extends PrimaryAuthenticationConfiguration<AP>
     {
         LinkFactory getLinkFactory();
-        URLHelper getUrl(String secret, ViewContext ctx);
+        URLHelper getUrl(ViewContext ctx);
 
         /**
          * Allows an SSO auth configuration to specify that it should be used automatically instead of showing the standard

api/src/org/labkey/api/security/AuthenticationManager.java

@@ -95,7 +95,7 @@
 import org.labkey.api.util.URLHelper;
 import org.labkey.api.util.logging.LogHelper;
 import org.labkey.api.view.ActionURL;
-import org.labkey.api.view.HttpView;
+import org.labkey.api.view.ExternalRedirectException;
 import org.labkey.api.view.NavTree;
 import org.labkey.api.view.NotFoundException;
 import org.labkey.api.view.RedirectException;
@@ -474,7 +474,7 @@ public ModelAndView getView(FORM form, BindException errors) throws Exception
                     AuthenticationManager.setPrimaryAuthenticationResult(request, primaryResult);
                     AuthenticationResult result = AuthenticationManager.handleAuthentication(request, getContainer());
 
-                    return HttpView.redirect(result.getRedirectURL(), true);
+                    throw new ExternalRedirectException(result.getRedirectURL());
                 }
 
                 primaryResult.getStatus().addUserErrorMessage(errors, primaryResult, null, null);

api/src/org/labkey/api/util/ExceptionUtil.java

@@ -816,7 +816,7 @@ static ActionURL handleException(@NotNull HttpServletRequest request, @NotNull H
         if (ex instanceof RedirectException rex)
         {
             String url = rex.getURL();
-            doErrorRedirect(response, url, rex.getHttpStatusCode());
+            unsafeRedirect(response, url, rex.getHttpStatusCode());
             return null;
         }
 
@@ -1207,18 +1207,19 @@ private static void addDependenciesAndRender(int responseStatus, PageConfig page
         errorView.getView().render(errorView.getModel(), request, response);
     }
 
-    // Temporary redirect
-    public static void doErrorRedirect(HttpServletResponse response, String url)
+    // Temporary redirect. "Unsafe" in the sense that the URL is not checked against the admin-configured allow list.
+    // Callers are responsible for ensuring any external redirect is safe.
+    public static void unsafeRedirect(HttpServletResponse response, String url)
     {
-        doErrorRedirect(response, url, HttpServletResponse.SC_MOVED_TEMPORARILY);
+        unsafeRedirect(response, url, HttpServletResponse.SC_MOVED_TEMPORARILY);
     }
 
     // Pass in HTTP status code to designate temporary vs. permanent redirect
-    private static void doErrorRedirect(HttpServletResponse response, String url, int httpStatusCode)
+    public static void unsafeRedirect(HttpServletResponse response, String url, int httpStatusCode)
     {
         response.setStatus(httpStatusCode);
         response.setDateHeader("Expires", 0);
-        response.setHeader("Location", url);
+        response.setHeader("Location", response.encodeRedirectURL(url));
         response.setContentType("text/html; charset=UTF-8");
 
         // backup strategy!
@@ -1234,7 +1235,7 @@ private static void doErrorRedirect(HttpServletResponse response, String url, in
         }
         catch (IOException x)
         {
-            LOG.error("doErrorRedirect", x);
+            LOG.error("unsafeRedirect", x);
         }
     }
 
@@ -1587,7 +1588,7 @@ public String encodeURL(String s)
         @Override
         public String encodeRedirectURL(String s)
         {
-            throw new IllegalStateException();
+            return s;
         }
 
         @Override