[MGPG-105] Make possible backward compatibility (#74)

Back out a bit from original MGPG-105 PR that _fails_ if best practices not followed.

Introduce a "bestPractice" boolean configuration flag. By default, plugin enforces best practice (= `true`), and will refuse to operate if best practices violated. Still, user can explicitly configure value `false`, when plugin regains "old way" (unsecure) mode of secret configuration, but plugin will warn.

---

https://issues.apache.org/jira/browse/MGPG-105

Author: cstamas

pgp-keys-map.list

@@ -28,6 +28,8 @@ org.junit.platform:junit-platform-commons = 0xFF6E2C001948C5F2F38B0CC385911F425E
 org.opentest4j:opentest4j = 0xFF6E2C001948C5F2F38B0CC385911F425EC61B51
 org.apache.maven.resolver = 0x522CA055B326A636D833EF6A0551FD3684FCBBB7
 org.apache.maven.shared:maven-invoker = 0x84789D24DF77A32433CE1F079EB80E92EB2135B1
+org.codehaus.plexus:plexus-cipher = 0x6A814B1F869C2BBEAB7CB7271A2A1C94BDE89688
 org.codehaus.plexus:plexus-classworlds = 0xB91AB7D2121DC6B0A61AA182D7742D58455ECC7C
 org.codehaus.plexus:plexus-component-annotations = 0xFA77DCFEF2EE6EB2DEBEDD2C012579464D01C06A
 org.codehaus.plexus:plexus-utils = 0xF254B35617DC255D9344BCFA873A8E86B4372146
+org.codehaus.plexus:plexus-sec-dispatcher = 0x2BE13D052E9AA567D657D9791FD507154FB9BA39

pom.xml

@@ -137,6 +137,28 @@ under the License.
       <version>2.9.0</version>
       <type>pom</type>
     </dependency>
+    <dependency>
+      <groupId>org.codehaus.plexus</groupId>
+      <artifactId>plexus-sec-dispatcher</artifactId>
+      <version>2.0</version>
+      <exclusions>
+        <exclusion>
+          <groupId>*</groupId>
+          <artifactId>*</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+    <dependency>
+      <groupId>org.codehaus.plexus</groupId>
+      <artifactId>plexus-cipher</artifactId>
+      <version>2.0</version>
+      <exclusions>
+        <exclusion>
+          <groupId>*</groupId>
+          <artifactId>*</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
 
     <dependency>
       <groupId>org.junit.jupiter</groupId>

src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java

@@ -27,6 +27,11 @@
 import org.apache.maven.plugin.MojoFailureException;
 import org.apache.maven.plugins.annotations.Component;
 import org.apache.maven.plugins.annotations.Parameter;
+import org.apache.maven.project.MavenProject;
+import org.apache.maven.settings.Server;
+import org.apache.maven.settings.Settings;
+import org.sonatype.plexus.components.sec.dispatcher.SecDispatcher;
+import org.sonatype.plexus.components.sec.dispatcher.SecDispatcherException;
 
 /**
  * @author Benjamin Bentmann
@@ -248,14 +253,47 @@ public abstract class AbstractGpgMojo extends AbstractMojo {
     @Component
     protected MavenSession session;
 
+    // === Deprecated stuff
+
+    /**
+     * Switch to lax plugin enforcement of "best practices". If set to {@code false}, plugin will retain all the
+     * backward compatibility regarding getting secrets (but will warn). By default, plugin enforces "best practices"
+     * and in such cases plugin fails.
+     *
+     * @since 3.2.0
+     * @deprecated
+     */
+    @Deprecated
+    @Parameter(property = "gpg.bestPractices", defaultValue = "true")
+    private boolean bestPractices;
+
+    /**
+     * Current user system settings for use in Maven.
+     *
+     * @since 1.6
+     * @deprecated
+     */
+    @Deprecated
+    @Parameter(defaultValue = "${settings}", readonly = true, required = true)
+    private Settings settings;
+
+    /**
+     * Maven Security Dispatcher.
+     *
+     * @since 1.6
+     * @deprecated
+     */
+    @Deprecated
+    @Component
+    private SecDispatcher secDispatcher;
+
     @Override
     public final void execute() throws MojoExecutionException, MojoFailureException {
         if (skip) {
             // We're skipping the signing stuff
             return;
         }
-        if ((passphrase != null && !passphrase.trim().isEmpty())
-                || (passphraseServerId != null && !passphraseServerId.trim().isEmpty())) {
+        if (bestPractices && (isNotBlank(passphrase) || isNotBlank(passphraseServerId))) {
             // Stop propagating worst practices: passphrase MUST NOT be in any file on disk
             throw new MojoFailureException(
                     "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in "
@@ -267,7 +305,19 @@ public final void execute() throws MojoExecutionException, MojoFailureException
 
     protected abstract void doExecute() throws MojoExecutionException, MojoFailureException;
 
-    protected AbstractGpgSigner newSigner() throws MojoFailureException {
+    private void logBestPracticeWarning(String source) {
+        getLog().warn("");
+        getLog().warn("W A R N I N G");
+        getLog().warn("");
+        getLog().warn("Do not store passphrase in any file (disk or SCM repository),");
+        getLog().warn("instead rely on GnuPG agent in interactive sessions, or provide passphrase in ");
+        getLog().warn(passphraseEnvName + " environment variable for batch mode.");
+        getLog().warn("");
+        getLog().warn("Sensitive content loaded from " + source);
+        getLog().warn("");
+    }
+
+    protected AbstractGpgSigner newSigner(MavenProject mavenProject) throws MojoFailureException {
         AbstractGpgSigner signer;
         if (GpgSigner.NAME.equals(this.signer)) {
             signer = new GpgSigner(executable);
@@ -294,10 +344,32 @@ protected AbstractGpgSigner newSigner() throws MojoFailureException {
         signer.setLockMode(lockMode);
         signer.setArgs(gpgArguments);
 
+        // "new way": env prevails
         String passphrase =
                 (String) session.getRepositorySession().getConfigProperties().get("env." + passphraseEnvName);
-        if (passphrase != null) {
+        if (isNotBlank(passphrase)) {
             signer.setPassPhrase(passphrase);
+        } else if (!bestPractices) {
+            // "old way": mojo config
+            passphrase = this.passphrase;
+            if (isNotBlank(passphrase)) {
+                logBestPracticeWarning("Mojo configuration");
+                signer.setPassPhrase(passphrase);
+            } else {
+                // "old way": serverId + settings
+                passphrase = loadGpgPassphrase();
+                if (isNotBlank(passphrase)) {
+                    logBestPracticeWarning("settings.xml");
+                    signer.setPassPhrase(passphrase);
+                } else {
+                    // "old way": project properties
+                    passphrase = getPassphrase(mavenProject);
+                    if (isNotBlank(passphrase)) {
+                        logBestPracticeWarning("Project properties");
+                        signer.setPassPhrase(passphrase);
+                    }
+                }
+            }
         }
 
         // gpg signer: always failed if no passphrase and no agent and not interactive: retain this behavior
@@ -310,4 +382,56 @@ protected AbstractGpgSigner newSigner() throws MojoFailureException {
 
         return signer;
     }
+
+    private boolean isNotBlank(String string) {
+        return string != null && !string.trim().isEmpty();
+    }
+
+    // Below is attic, to be thrown out
+
+    @Deprecated
+    private static final String GPG_PASSPHRASE = "gpg.passphrase";
+
+    @Deprecated
+    private String loadGpgPassphrase() throws MojoFailureException {
+        if (isNotBlank(passphrase)) {
+            Server server = settings.getServer(passphraseServerId);
+            if (server != null) {
+                if (isNotBlank(server.getPassphrase())) {
+                    try {
+                        return secDispatcher.decrypt(server.getPassphrase());
+                    } catch (SecDispatcherException e) {
+                        throw new MojoFailureException("Unable to decrypt gpg passphrase", e);
+                    }
+                }
+            }
+        }
+        return null;
+    }
+
+    @Deprecated
+    public String getPassphrase(MavenProject project) {
+        String pass = null;
+        if (project != null) {
+            pass = project.getProperties().getProperty(GPG_PASSPHRASE);
+            if (pass == null) {
+                MavenProject prj2 = findReactorProject(project);
+                pass = prj2.getProperties().getProperty(GPG_PASSPHRASE);
+            }
+        }
+        if (project != null) {
+            findReactorProject(project).getProperties().setProperty(GPG_PASSPHRASE, pass);
+        }
+        return pass;
+    }
+
+    @Deprecated
+    private MavenProject findReactorProject(MavenProject prj) {
+        if (prj.getParent() != null
+                && prj.getParent().getBasedir() != null
+                && prj.getParent().getBasedir().exists()) {
+            return findReactorProject(prj.getParent());
+        }
+        return prj;
+    }
 }

src/main/java/org/apache/maven/plugins/gpg/GpgSignAttachedMojo.java

@@ -82,7 +82,7 @@ protected void doExecute() throws MojoExecutionException, MojoFailureException {
         // Sign collected files and attach all the signatures
         // ----------------------------------------------------------------------------
 
-        AbstractGpgSigner signer = newSigner();
+        AbstractGpgSigner signer = newSigner(project);
         signer.setOutputDirectory(ascDirectory);
         signer.setBuildDirectory(new File(project.getBuild().getDirectory()));
         signer.setBaseDirectory(project.getBasedir());

src/main/java/org/apache/maven/plugins/gpg/SignAndDeployFileMojo.java

@@ -349,7 +349,7 @@ protected void doExecute() throws MojoExecutionException, MojoFailureException {
         }
 
         // sign all
-        AbstractGpgSigner signer = newSigner();
+        AbstractGpgSigner signer = newSigner(null);
         signer.setOutputDirectory(ascDirectory);
         signer.setBaseDirectory(new File("").getAbsoluteFile());
 

src/test/java/org/apache/maven/plugins/gpg/it/GpgSignArtifactIT.java

@@ -23,6 +23,7 @@
 import java.util.Collection;
 
 import org.apache.maven.shared.invoker.InvocationRequest;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
@@ -80,4 +81,32 @@ void testPlacementOfArtifactInOutputDirectory(String pomPath, String expectedFil
         Arrays.sort(expectedFiles);
         assertEquals(Arrays.toString(expectedFiles), Arrays.toString(outputFiles));
     }
+
+    @Test
+    void testWorstPracticesStillWork() throws Exception {
+        // given
+        final File pomFile = InvokerTestUtils.getTestResource("/it/sign-release-in-same-dir/pom.xml");
+        final InvocationRequest request =
+                InvokerTestUtils.createRequest(pomFile, mavenUserSettings, gpgHome, "gpg", false);
+        request.addArg("-Dgpg.bestPractices=false");
+        request.addArg("-Dgpg.passphrase=TEST");
+
+        final File integrationTestRootDirectory = new File(pomFile.getParent());
+        final File expectedOutputDirectory = new File(integrationTestRootDirectory + "/target/tarballs/");
+
+        // when
+        InvokerTestUtils.executeRequest(request, mavenHome, localRepository);
+
+        // then
+        assertTrue(expectedOutputDirectory.isDirectory());
+
+        String[] outputFiles = expectedOutputDirectory.list();
+        assertNotNull(outputFiles);
+
+        String[] expectedFiles =
+                new String[] {"sign-release-in-same-dir-1.0.jar", "sign-release-in-same-dir-1.0.jar.asc"};
+        Arrays.sort(outputFiles);
+        Arrays.sort(expectedFiles);
+        assertEquals(Arrays.toString(expectedFiles), Arrays.toString(outputFiles));
+    }
 }