Merge pull request #1160 from bitwiseman/bug/withCreds

Fix withCredetials() to correctly detect missing creds

Author: bitwiseman

src/main/java/org/kohsuke/github/GitHubBuilder.java

@@ -24,6 +24,9 @@
  */
 public class GitHubBuilder implements Cloneable {
 
+    // for testing
+    static File HOME_DIRECTORY = null;
+
     // default scoped so unit tests can read them.
     /* private */ String endpoint = GitHubClient.GITHUB_URL;
 
@@ -60,13 +63,13 @@ static GitHubBuilder fromCredentials() throws IOException {
 
         builder = fromEnvironment();
 
-        if (builder.authorizationProvider != null)
+        if (builder.authorizationProvider != AuthorizationProvider.ANONYMOUS)
             return builder;
 
         try {
             builder = fromPropertyFile();
 
-            if (builder.authorizationProvider != null)
+            if (builder.authorizationProvider != AuthorizationProvider.ANONYMOUS)
                 return builder;
         } catch (FileNotFoundException e) {
             // fall through
@@ -178,7 +181,7 @@ public static GitHubBuilder fromEnvironment() throws IOException {
      *             the io exception
      */
     public static GitHubBuilder fromPropertyFile() throws IOException {
-        File homeDir = new File(System.getProperty("user.home"));
+        File homeDir = HOME_DIRECTORY != null ? HOME_DIRECTORY : new File(System.getProperty("user.home"));
         File propertyFile = new File(homeDir, ".github");
         return fromPropertyFile(propertyFile.getPath());
     }

src/test/java/org/kohsuke/github/GitHubConnectionTest.java

@@ -1,14 +1,19 @@
 package org.kohsuke.github;
 
+import org.apache.commons.lang3.SystemUtils;
 import org.junit.Assume;
 import org.junit.Test;
+import org.kohsuke.github.authorization.AuthorizationProvider;
 import org.kohsuke.github.authorization.UserAuthorizationProvider;
 
+import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.lang.reflect.Field;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Properties;
 
 import static org.hamcrest.Matchers.*;
 
@@ -137,6 +142,147 @@ public void testGitHubBuilderFromCustomEnvironment() throws IOException {
         assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), equalTo("bogus login"));
     }
 
+    @Test
+    public void testGitHubBuilderFromCredentialsWithEnvironment() throws IOException {
+        // we disable this test for JDK 16+ as the current hacks in setupEnvironment() don't work with JDK 16+
+        Assume.assumeThat(Double.valueOf(System.getProperty("java.specification.version")), lessThan(16.0));
+        Assume.assumeFalse(SystemUtils.IS_OS_WINDOWS);
+
+        Map<String, String> props = new HashMap<String, String>();
+
+        props.put("endpoint", "bogus endpoint url");
+        props.put("oauth", "bogus oauth token string");
+        setupEnvironment(props);
+        GitHubBuilder builder = GitHubBuilder.fromCredentials();
+
+        assertThat(builder.endpoint, equalTo("bogus endpoint url"));
+
+        assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+        assertThat(builder.authorizationProvider.getEncodedAuthorization(), equalTo("token bogus oauth token string"));
+        assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), nullValue());
+
+        props.put("login", "bogus login");
+        setupEnvironment(props);
+        builder = GitHubBuilder.fromCredentials();
+
+        assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+        assertThat(builder.authorizationProvider.getEncodedAuthorization(), equalTo("token bogus oauth token string"));
+        assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), equalTo("bogus login"));
+
+        props.put("jwt", "bogus jwt token string");
+        setupEnvironment(props);
+        builder = GitHubBuilder.fromCredentials();
+
+        assertThat(builder.authorizationProvider, not(instanceOf(UserAuthorizationProvider.class)));
+        assertThat(builder.authorizationProvider.getEncodedAuthorization(), equalTo("Bearer bogus jwt token string"));
+
+        props.put("password", "bogus weak password");
+        setupEnvironment(props);
+        builder = GitHubBuilder.fromCredentials();
+
+        assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+        assertThat(builder.authorizationProvider.getEncodedAuthorization(),
+                equalTo("Basic Ym9ndXMgbG9naW46Ym9ndXMgd2VhayBwYXNzd29yZA=="));
+        assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), equalTo("bogus login"));
+    }
+
+    @Test
+    public void testGitHubBuilderFromCredentialsWithPropertyFile() throws IOException {
+        // we disable this test for JDK 16+ as the current hacks in setupEnvironment() don't work with JDK 16+
+        Assume.assumeThat(Double.valueOf(System.getProperty("java.specification.version")), lessThan(16.0));
+        Assume.assumeFalse(SystemUtils.IS_OS_WINDOWS);
+
+        Map<String, String> props = new HashMap<String, String>();
+
+        // Clear the environment
+        setupEnvironment(props);
+        try {
+            GitHubBuilder.HOME_DIRECTORY = new File(getTestDirectory());
+            try {
+                GitHubBuilder builder = GitHubBuilder.fromCredentials();
+                fail();
+            } catch (Exception e) {
+                assertThat(e, instanceOf(IOException.class));
+                assertThat(e.getMessage(), equalTo("Failed to resolve credentials from ~/.github or the environment."));
+            }
+
+            props = new HashMap<String, String>();
+
+            props.put("endpoint", "bogus endpoint url");
+            props.put("oauth", "bogus oauth token string");
+
+            setupPropertyFile(props);
+
+            GitHubBuilder builder = GitHubBuilder.fromCredentials();
+
+            assertThat(builder.endpoint, equalTo("bogus endpoint url"));
+
+            assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+            assertThat(builder.authorizationProvider.getEncodedAuthorization(),
+                    equalTo("token bogus oauth token string"));
+            assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), nullValue());
+
+            props.put("login", "bogus login");
+            setupPropertyFile(props);
+            builder = GitHubBuilder.fromCredentials();
+
+            assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+            assertThat(builder.authorizationProvider.getEncodedAuthorization(),
+                    equalTo("token bogus oauth token string"));
+            assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), equalTo("bogus login"));
+
+            props.put("jwt", "bogus jwt token string");
+            setupPropertyFile(props);
+            builder = GitHubBuilder.fromCredentials();
+
+            assertThat(builder.authorizationProvider, not(instanceOf(UserAuthorizationProvider.class)));
+            assertThat(builder.authorizationProvider.getEncodedAuthorization(),
+                    equalTo("Bearer bogus jwt token string"));
+
+            props.put("password", "bogus weak password");
+            setupPropertyFile(props);
+            builder = GitHubBuilder.fromCredentials();
+
+            assertThat(builder.authorizationProvider, instanceOf(UserAuthorizationProvider.class));
+            assertThat(builder.authorizationProvider.getEncodedAuthorization(),
+                    equalTo("Basic Ym9ndXMgbG9naW46Ym9ndXMgd2VhayBwYXNzd29yZA=="));
+            assertThat(((UserAuthorizationProvider) builder.authorizationProvider).getLogin(), equalTo("bogus login"));
+        } finally {
+            GitHubBuilder.HOME_DIRECTORY = null;
+            File propertyFile = new File(getTestDirectory(), ".github");
+            propertyFile.delete();
+        }
+    }
+
+    private void setupPropertyFile(Map<String, String> props) throws IOException {
+        File propertyFile = new File(getTestDirectory(), ".github");
+        Properties properties = new Properties();
+        properties.putAll(props);
+        properties.store(new FileOutputStream(propertyFile), "");
+    }
+
+    private String getTestDirectory() {
+        return new File("target").getAbsolutePath();
+    }
+
+    @Test
+    public void testAnonymous() throws IOException {
+        // we disable this test for JDK 16+ as the current hacks in setupEnvironment() don't work with JDK 16+
+        Assume.assumeThat(Double.valueOf(System.getProperty("java.specification.version")), lessThan(16.0));
+
+        Map<String, String> props = new HashMap<String, String>();
+
+        props.put("endpoint", mockGitHub.apiServer().baseUrl());
+        setupEnvironment(props);
+
+        // No values present except endpoint
+        GitHubBuilder builder = GitHubBuilder
+                .fromEnvironment("customLogin", "customPassword", "customOauth", "endpoint");
+
+        assertThat(builder.endpoint, equalTo(mockGitHub.apiServer().baseUrl()));
+        assertThat(builder.authorizationProvider, sameInstance(AuthorizationProvider.ANONYMOUS));
+    }
+
     @Test
     public void testGithubBuilderWithAppInstallationToken() throws Exception {