From 5e7dc579ab1c226a27f33dd59c0c88211527875f Mon Sep 17 00:00:00 2001
From: Andrew Oberstar <ajoberstar@gmail.com>
Date: Sat, 17 Dec 2016 11:08:47 -0600
Subject: [PATCH] Authentication creds can be provided via env vars

This makes it easier in some contexts like ci tools
to provide the credentials to the build.

This resolves #135.
---
 .../ajoberstar/grgit/auth/AuthConfig.groovy   | 83 ++++++++++++-------
 .../grgit/auth/TransportOpUtil.groovy         |  2 +-
 .../grgit/auth/AuthConfigSpec.groovy          | 28 ++++---
 3 files changed, 69 insertions(+), 44 deletions(-)

diff --git a/src/main/groovy/org/ajoberstar/grgit/auth/AuthConfig.groovy b/src/main/groovy/org/ajoberstar/grgit/auth/AuthConfig.groovy
index af4d018f..21dbe834 100644
--- a/src/main/groovy/org/ajoberstar/grgit/auth/AuthConfig.groovy
+++ b/src/main/groovy/org/ajoberstar/grgit/auth/AuthConfig.groovy
@@ -50,6 +50,16 @@ import org.ajoberstar.grgit.exception.GrgitException
  * </ul>
  *
  * <p>
+ *   Hardcoded credentials can alternately be provided with environment variables.
+ *   These take a lower precedence than the system properties, but all other
+ *   considerations are the same.
+ * </p>
+ * <ul>
+ *   <li>{@code GRGIT_USER=<username>}</li>
+ *   <li>{@code GRGIT_PASS=<password>}</li>
+ * </ul>
+ *
+ * <p>
  *   In order to add a non-standard SSH key to use as your credentials,
  *   use the following property.
  * </p>
@@ -90,14 +100,35 @@ class AuthConfig {
     static final String SSH_PRIVATE_KEY_OPTION = 'org.ajoberstar.grgit.auth.ssh.private'
     static final String SSH_SESSION_CONFIG_OPTION_PREFIX = 'org.ajoberstar.grgit.auth.session.config.'
 
+    static final String USERNAME_ENV_VAR = 'GRGIT_USER'
+    static final String PASSWORD_ENV_VAR = 'GRGIT_PASS'
+
+    private final Map<String, String> props
+    private final Map<String, String> env
+
+    private AuthConfig(Map<String, String> props, Map<String, String> env) {
+        this.props = props
+        this.env = env
+    }
+
     /**
      * Set of all authentication options that are allowed in this
      * configuration.
      */
-    final Set<Option> allowed
-
-    private AuthConfig(Set<Option> allowed) {
-        this.allowed = allowed.asImmutable()
+    Set<Option> getAllowed() {
+        String forceSetting = props[FORCE_OPTION]
+        if (forceSetting) {
+            try {
+                return [Option.valueOf(forceSetting.toUpperCase())]
+            } catch (IllegalArgumentException e) {
+                throw new GrgitException("${FORCE_OPTION} must be set to one of ${Option.values() as List}. Currently set to: ${forceSetting}", e)
+            }
+        } else {
+            return (Option.values() as Set).findAll {
+                String setting = props[it.systemPropertyName]
+                setting == null ? true : Boolean.valueOf(setting)
+            }
+        }
     }
 
     /**
@@ -108,7 +139,7 @@ class AuthConfig {
      * otherwise
      */
     boolean allows(Option option) {
-        return allowed.contains(option)
+        return getAllowed().contains(option)
     }
 
     /**
@@ -118,10 +149,14 @@ class AuthConfig {
      * properties, or, if the username isn't set, {@code null}
      */
     Credentials getHardcodedCreds() {
-        String username = System.properties[USERNAME_OPTION]
-        String password = System.properties[PASSWORD_OPTION]
-        if (username) {
-            return new Credentials(username, password)
+        if (allows(Option.HARDCODED)) {
+            String username = props[USERNAME_OPTION] ?: env[USERNAME_ENV_VAR]
+            String password = props[PASSWORD_OPTION] ?: env[PASSWORD_ENV_VAR]
+            if (username) {
+                return new Credentials(username, password)
+            } else {
+                return null
+            }
         } else {
             return null
         }
@@ -133,7 +168,7 @@ class AuthConfig {
      * @return the path to the SSH key, if set, otherwise {@code null}
      */
     String getSshPrivateKeyPath() {
-        return System.properties[SSH_PRIVATE_KEY_OPTION]
+        return props[SSH_PRIVATE_KEY_OPTION]
     }
 
     /**
@@ -141,43 +176,31 @@ class AuthConfig {
      * @return map with configuration or empty if nothing was specified in system property
      */
     Map<String, String> getSessionConfig() {
-        return System.properties
+        return props
             .findAll { key, value -> key.startsWith(SSH_SESSION_CONFIG_OPTION_PREFIX) }
             .collectEntries { key, value -> [key.substring(SSH_SESSION_CONFIG_OPTION_PREFIX.length()), value] }
     }
 
     /**
      * Factory method to construct an authentication configuration from the
-     * given properties.
+     * given properties and environment.
      * @param properties the properties to use in this configuration
+     * @param env the environment vars to use in this configuration
      * @return the constructed configuration
      * @throws GrgitException if force is set to an invalid option
      */
-    static AuthConfig fromMap(Map properties) {
-        String forceSetting = properties[FORCE_OPTION]
-        if (forceSetting) {
-            try {
-                return new AuthConfig([Option.valueOf(forceSetting.toUpperCase())] as Set)
-            } catch (IllegalArgumentException e) {
-                throw new GrgitException("${FORCE_OPTION} must be set to one of ${Option.values() as List}. Currently set to: ${forceSetting}", e)
-            }
-        } else {
-            Set<Option> allowed = (Option.values() as Set).findAll {
-                String setting = properties[it.systemPropertyName]
-                setting == null ? true : Boolean.valueOf(setting)
-            }
-            return new AuthConfig(allowed)
-        }
+    static AuthConfig fromMap(Map props, Map env = [:]) {
+        return new AuthConfig(props, env)
     }
 
     /**
      * Factory method to construct an authentication configuration from the
-     * current system properties.
+     * current system properties and environment variables.
      * @return the constructed configuration
      * @throws GrgitException if force is set to an invalid option
      */
-    static AuthConfig fromSystemProperties() {
-        return fromMap(System.properties)
+    static AuthConfig fromSystem() {
+        return fromMap(System.properties, System.env)
     }
 
     /**
diff --git a/src/main/groovy/org/ajoberstar/grgit/auth/TransportOpUtil.groovy b/src/main/groovy/org/ajoberstar/grgit/auth/TransportOpUtil.groovy
index 5e46e347..dbb368eb 100644
--- a/src/main/groovy/org/ajoberstar/grgit/auth/TransportOpUtil.groovy
+++ b/src/main/groovy/org/ajoberstar/grgit/auth/TransportOpUtil.groovy
@@ -44,7 +44,7 @@ final class TransportOpUtil {
      * @param creds the hardcoded credentials to use, if not {@code null}
      */
     static void configure(TransportCommand cmd, Credentials creds) {
-        AuthConfig config = AuthConfig.fromSystemProperties()
+        AuthConfig config = AuthConfig.fromSystem()
         logger.info('The following authentication options are allowed (though they may not be available): {}', config.allowed)
         cmd.credentialsProvider = determineCredentialsProvider(config, creds)
         cmd.transportConfigCallback = new JschAgentProxyConfigCallback(config)
diff --git a/src/test/groovy/org/ajoberstar/grgit/auth/AuthConfigSpec.groovy b/src/test/groovy/org/ajoberstar/grgit/auth/AuthConfigSpec.groovy
index 17bbf9b8..61d80e0f 100644
--- a/src/test/groovy/org/ajoberstar/grgit/auth/AuthConfigSpec.groovy
+++ b/src/test/groovy/org/ajoberstar/grgit/auth/AuthConfigSpec.groovy
@@ -46,39 +46,41 @@ class AuthConfigSpec extends Specification {
         ['pageant.allow': 'true', 'sshagent.allow': 'false']          | [HARDCODED, INTERACTIVE, PAGEANT]
     }
 
-    def 'getHardcodedCreds returns creds if username and password are set'() {
+    def 'getHardcodedCreds returns creds if username and password are set with properties'() {
         given:
-        System.setProperty(AuthConfig.USERNAME_OPTION, 'myuser')
-        System.setProperty(AuthConfig.PASSWORD_OPTION, 'mypass')
+        def props = [(AuthConfig.USERNAME_OPTION): 'myuser', (AuthConfig.PASSWORD_OPTION): 'mypass']
         expect:
-        AuthConfig.fromMap([:]).getHardcodedCreds() == new Credentials('myuser', 'mypass')
+        AuthConfig.fromMap(props).getHardcodedCreds() == new Credentials('myuser', 'mypass')
+    }
+
+    def 'getHardcodedCreds returns creds if username and password are set with env'() {
+        given:
+        def env = [(AuthConfig.USERNAME_ENV_VAR): 'myuser', (AuthConfig.PASSWORD_ENV_VAR): 'mypass']
+        expect:
+        AuthConfig.fromMap([:], env).getHardcodedCreds() == new Credentials('myuser', 'mypass')
     }
 
     def 'getHardcodedCreds returns creds if username is set and password is not'() {
         given:
-        System.setProperty(AuthConfig.USERNAME_OPTION, 'myuser')
-        System.clearProperty(AuthConfig.PASSWORD_OPTION)
+        def props = [(AuthConfig.USERNAME_OPTION): 'myuser']
         expect:
-        AuthConfig.fromMap([:]).getHardcodedCreds() == new Credentials('myuser', null)
+        AuthConfig.fromMap(props).getHardcodedCreds() == new Credentials('myuser', null)
     }
 
     def 'getHardcodedCreds returns null if username is not set'() {
-        given:
-        System.clearProperty(AuthConfig.USERNAME_OPTION)
-        System.clearProperty(AuthConfig.PASSWORD_OPTION)
         expect:
         AuthConfig.fromMap([:]).getHardcodedCreds() == null
     }
 
     def 'getSessionConfig returns empty map if nothing specified'() {
         expect:
-        AuthConfig.fromMap(Collections.emptyMap()).sessionConfig.isEmpty()
+        AuthConfig.fromMap([:]).sessionConfig.isEmpty()
     }
 
     def 'getSessionConfig returns session config based on system property'() {
         given:
-        System.setProperty(AuthConfig.SSH_SESSION_CONFIG_OPTION_PREFIX + 'StrictHostKeyChecking', 'no')
+        def props = [(AuthConfig.SSH_SESSION_CONFIG_OPTION_PREFIX + 'StrictHostKeyChecking'): 'no']
         expect:
-        AuthConfig.fromMap(Collections.emptyMap()).sessionConfig == [ StrictHostKeyChecking: 'no' ]
+        AuthConfig.fromMap(props).sessionConfig == [StrictHostKeyChecking: 'no']
     }
 }