[java-identity-provider] 02/04: IDP-1683 Plugin Handling. Remove methods.

Rod Widdowson rdw at steadingsoftware.com
Sun Oct 4 13:51:49 UTC 2020


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

rdw pushed a commit to branch main
in repository java-identity-provider.

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

commit c87195a079619d895ef13473aaaaf1f3d1576013
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Sun Oct 4 13:23:17 2020 +0100

    IDP-1683  Plugin Handling. Remove methods.
    
    https://issues.shibboleth.net/jira/browse/IDP-1683
    
    The plugin installer can no longer be told to move files to
    anywhere except webapp (moving these elsewhere is the module's job).
    
    The plugin installer can no longer be told to move download files.
---
 .../shibboleth/idp/plugin/AbstractIdPPlugin.java   |  19 +--
 .../java/net/shibboleth/idp/plugin/IdPPlugin.java  |  30 -----
 .../idp/installer/plugin/impl/PluginInstaller.java | 131 ---------------------
 3 files changed, 2 insertions(+), 178 deletions(-)

diff --git a/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/AbstractIdPPlugin.java b/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/AbstractIdPPlugin.java
index 2dd027a90..8c2eea9b7 100644
--- a/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/AbstractIdPPlugin.java
+++ b/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/AbstractIdPPlugin.java
@@ -17,11 +17,7 @@
 
 package net.shibboleth.idp.plugin;
 
-import java.io.IOException;
-import java.net.URL;
-import java.nio.file.Path;
 import java.util.Collections;
-import java.util.List;
 import java.util.Set;
 
 import javax.annotation.Nonnegative;
@@ -30,7 +26,6 @@ import javax.annotation.Nonnull;
 import net.shibboleth.idp.module.IdPModule;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.collection.Pair;
 
 /**
  * A base class implementing {@link IdPPlugin} that defaults common settings.
@@ -50,22 +45,12 @@ public abstract class AbstractIdPPlugin implements IdPPlugin {
         enableModules = Collections.emptySet();
         disableModules = Collections.emptySet();
     }
-    
+
     /** {@inheritDoc} */
     @Nonnull @NotEmpty public String getPluginId() {
         return getClass().getPackageName();
     }
-    
-    /** {@inheritDoc} */
-    @Nonnull @NonnullElements public List<Path> getFilePathsToCopy() {
-        return Collections.emptyList();
-    }
-    
-    /** {@inheritDoc} */
-    @Nonnull @NonnullElements public List<Pair<URL, Path>> getExternalFilePathsToCopy() throws IOException {
-        return Collections.emptyList();
-    }
-    
+
     /** {@inheritDoc} */
     @Nonnegative public int getPatchVersion() {
         return 0;
diff --git a/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/IdPPlugin.java b/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/IdPPlugin.java
index 29378b446..bd1d23210 100644
--- a/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/IdPPlugin.java
+++ b/idp-admin-api/src/main/java/net/shibboleth/idp/plugin/IdPPlugin.java
@@ -19,7 +19,6 @@ package net.shibboleth.idp.plugin;
 
 import java.io.IOException;
 import java.net.URL;
-import java.nio.file.Path;
 import java.util.List;
 import java.util.Set;
 
@@ -29,7 +28,6 @@ import javax.annotation.Nonnull;
 import net.shibboleth.idp.module.IdPModule;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullElements;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.collection.Pair;
 
 /**
  * This interface is exported (via the service API) by every IdP plugin.
@@ -49,34 +47,6 @@ public interface IdPPlugin {
      */
     @Nonnull @NotEmpty String getPluginId();
 
-    /** Return the list of (idp.home) relative paths (of files, <em>not directories </em>) 
-     * to copy from the distribution into the IdP installation.
-     *
-     * <em>Not currently supported</em>
-     * 
-     * <p>These files are copied non-destructively (if the file already exists
-     * then it is not copied).  Some paths are disallowed (for instance dist and system).
-     * Directories are created if needed</p>
-     * <p>
-     * The dist folder is always copied, so no files from it should be included.</p>
-     *
-     * @return The list of paths.
-     */
-    @Nonnull @NonnullElements List<Path> getFilePathsToCopy();
-    
-    /** <p>Return the list of files <em>not directories </em> to get from 'external'
-     * sources. This allows external content to be downloaded during installation.</p>
-     *
-     * <p>The first part of the pair is the source URL,
-     * the second is a path relative to idp.home.  These can include files
-     * going to dist\edit-webapp in which case the path is expected to have the
-     * plugin id appended. Sub directories are created if needed</p>
-     *
-     * @return The list.
-     * @throws IOException if the resource construction failed.
-     */
-    @Nonnull @NonnullElements List<Pair<URL, Path>> getExternalFilePathsToCopy() throws IOException;
-
     /** Return the places to look for information for this plugin package.
      * The format of the (property) file at this location is fixed.
      * 
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 234955312..466417c15 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -72,7 +72,6 @@ import net.shibboleth.idp.plugin.IdPPlugin;
 import net.shibboleth.idp.plugin.PluginVersion;
 import net.shibboleth.utilities.java.support.annotation.constraint.NonnullAfterInit;
 import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
-import net.shibboleth.utilities.java.support.collection.Pair;
 import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
 import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.component.ComponentSupport;
@@ -86,9 +85,6 @@ import net.shibboleth.utilities.java.support.resource.Resource;
  */
 public final class PluginInstaller extends AbstractInitializableComponent implements AutoCloseable {
 
-    /** Where we cannot install. */
-    private static List<String> disallowedPaths = List.of("dist", "system", "webapp");
-
     /** Class logger. */
     @Nonnull
     private static final Logger LOG = LoggerFactory.getLogger(PluginInstaller.class);
@@ -169,40 +165,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         httpClient = Constraint.isNotNull(what, "HttpClient should be non-null");
     }
 
-    /** Check that the provide path is inside {@link #idpHome}.
-     * @param to the path to check.
-     * @throws BuildException if it isn't
-     */
-    private void policeTo(final Path to) throws BuildException {
-        try {
-            final Path canonicalTo = PluginInstallerSupport.canonicalPath(to);
-            if (!canonicalTo.startsWith(idpHome)) {
-                LOG.error("File destination {} ({}) was illegal (not inside {}", to, canonicalTo, idpHome);
-                throw new BuildException("Illegal file destination");
-            }
-        } catch (final IOException e) {
-            LOG.error("Error checking destination {}", to, e);
-            throw new BuildException(e);
-        }
-    }
-
-    /** Check that the provide path is inside {@link #distribution}.
-     * @param from the path to check.
-     * @throws BuildException if it isn't
-     */
-    private void policeFrom(final Path from) throws BuildException {
-        try {
-            final Path canonicalFrom = PluginInstallerSupport.canonicalPath(from);
-            if (!canonicalFrom.startsWith(distribution)) {
-                LOG.error("File source {} ({}) was illegal (not inside {}", from, canonicalFrom, distribution);
-                throw new BuildException("Illegal file source");
-            }
-        } catch (final IOException e) {
-            LOG.error("Error checking destination {}", from, e);
-            throw new BuildException(e);
-        }
-    }
-
     /** Install the plugin from the provided URL.  Involves downloading
      *  the file and then doing a {@link #installPlugin(Path, String)}.
      * @param baseURL where we get the files from
@@ -247,8 +209,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
 
         uninstallOld(myWebApp);
         installWebapp(myWebApp);
-        installFiles();
-        downloadExternals();
         InstallerSupport.setReadOnly(myWebApp, true);
         saveCopiedFiles();
 
@@ -280,41 +240,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         LOG.info("Removed resources for {} from the war", pluginId);
     }
 
-    /** Download any files that should not be shipped.
-     * @throws BuildException if badness is detected.
-     */
-    private void downloadExternals() throws BuildException {
-        try {
-            for (final Pair<URL, Path> pair : description.getExternalFilePathsToCopy()) {
-                final Path to = idpHome.resolve(pair.getSecond());
-                policeTo(to);
-                if (Files.exists(to)) {
-                    LOG.warn("{} exists, not copied", to);
-                    continue;
-                }
-                if (!acceptDownload.test(pair.getFirst().toExternalForm())) {
-                    LOG.info("Did not download {} to {}", pair.getFirst(), to);
-                    continue;
-                }
-                buildHttpClient();
-                createParent(to);
-                LOG.debug("Copying from {} to {}", pair.getFirst(), to);
-                final Resource from  = new HTTPResource(httpClient, pair.getFirst());
-                try (final InputStream in = new BufferedInputStream(from.getInputStream());
-                     final OutputStream out =  new ProgressReportingOutputStream(new FileOutputStream(to.toFile()))) {
-
-                    in.transferTo(out);
-
-                } catch (final IOException e) {
-                    LOG.error("Could not copy from {} to {}",  from, to, e);
-                    throw new BuildException(e);
-                }
-            }
-        } catch (final IOException e) {
-            throw new BuildException(e);
-        }
-    }
-
     /** Get hold of the {@link IdPPlugin} for this plugin.
      * @throws BuildException if badness is happens.
      */
@@ -386,62 +311,6 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
         }
     }
 
-    /** Copy the files the distribution tells us to.
-     * @throws BuildException if badness is happens.
-     */
-    private void installFiles() throws BuildException {
-        for (final Path p : description.getFilePathsToCopy()) {
-            for (final String disallowedPath : disallowedPaths) {
-                if (p.startsWith(disallowedPath)) {
-                    LOG.error("Path {} contained disallowed location", p);
-                    throw new BuildException("Copy to banned location");
-                }
-            }
-
-            final Path from = distribution.resolve(p);
-            policeFrom(from);
-            final Path to = idpHome.resolve(p);
-            policeTo(to);
-            if (Files.exists(to)) {
-                LOG.debug("File {} exists, skipping", to);
-                continue;
-            }
-            if (!Files.exists(from)) {
-                LOG.warn("Source File {} does not exists, skipping", from);
-                continue;
-            }
-            try {
-                createParent(to);
-                LOG.debug("Copying from {} to {}", from, to);
-                try (final InputStream in = new BufferedInputStream(new FileInputStream(from.toFile()));
-                     final OutputStream out =  new ProgressReportingOutputStream(new FileOutputStream(to.toFile()))) {
-                    in.transferTo(out);
-                }
-            } catch (final IOException e) {
-                LOG.error("Could not copy from {} to {}",  from, to, e);
-                throw new BuildException(e);
-            }
-        }
-    }
-
-    /** If the parent dir of the provided path doesn't exist, create it.
-     * @param file where the file will go
-     * @throws IOException if the directory couldn't be created
-     * @throws BuildException if the parent wasnt a directory
-     */
-    private void createParent(final Path file) throws IOException, BuildException {
-        final Path parent = file.resolve("..");
-        if (!Files.exists(parent)) {
-            LOG.debug("Creating parent directory {}", parent);
-            Files.createDirectories(parent);
-        } else if (!Files.isDirectory(parent)) {
-            LOG.error("{} exists and is not a directory", parent);
-            throw new BuildException("Parent of target file was not a directory");
-        } else {
-            LOG.trace("Parent directory {} existed", parent);
-        }  
-    }
-
     /** Copy the webapp folder from the distribution to the per plugin
      * location inside dist.
      * @param myWebApp Where to put it.

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


More information about the commits mailing list