diff --git a/src/main/java/org/zalando/stups/tokens/AccessTokensBuilder.java b/src/main/java/org/zalando/stups/tokens/AccessTokensBuilder.java index 4c4c3e2..3f68253 100644 --- a/src/main/java/org/zalando/stups/tokens/AccessTokensBuilder.java +++ b/src/main/java/org/zalando/stups/tokens/AccessTokensBuilder.java @@ -15,10 +15,11 @@ */ package org.zalando.stups.tokens; -import static org.zalando.stups.tokens.EndsWithFilenameFilter.forSuffix; -import static org.zalando.stups.tokens.FileSupplier.getCredentialsDir; -import static org.zalando.stups.tokens.util.Objects.notNull; +import org.zalando.stups.tokens.fs.FilesystemSecretRefresher; +import org.zalando.stups.tokens.fs.FilesystemSecretsRefresherConfiguration; +import org.zalando.stups.tokens.mcb.MCBConfig; +import java.io.File; import java.net.URI; import java.util.Collections; import java.util.HashSet; @@ -27,9 +28,8 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import org.zalando.stups.tokens.fs.FilesystemSecretRefresher; -import org.zalando.stups.tokens.fs.FilesystemSecretsRefresherConfiguration; -import org.zalando.stups.tokens.mcb.MCBConfig; +import static org.zalando.stups.tokens.EndsWithFilenameFilter.forSuffix; +import static org.zalando.stups.tokens.util.Objects.notNull; /** * Use the AccessTokensBuilder obtained via @@ -464,7 +464,7 @@ public AccessTokensBuilder metricsListener(MetricsListener metricsListener) { return this; } - public FilesystemSecretsRefresherConfiguration whenUsingFilesystemSecrets(){ + public FilesystemSecretsRefresherConfiguration whenUsingFilesystemSecrets() { return this.filesystemSecretsRefresherConfiguration; } @@ -557,8 +557,8 @@ public Set getAccessTokenConfigurations() { * for any of your configured tokenIds. */ public AccessTokens start() { - if (accessTokenConfigurations.size() == 0) { - throw new IllegalArgumentException("no scopes defined"); + if (accessTokenConfigurations.isEmpty()) { + throw new IllegalArgumentException("No tokens configured"); } locked = true; @@ -596,11 +596,13 @@ protected AbstractAccessTokenRefresher getAccessTokenRefresher() { } private boolean isFilesystemSecretsLayout() { - try { - return getCredentialsDir().list(forSuffix("-token-secret")).length > 0; - } catch (Exception e) { - return false; - } + return FileSupplier + .credentialsDir() + .map(dir -> { + String[] files = new File(dir).list(forSuffix("-token-secret")); + return files != null && files.length > 0; + }) + .orElse(Boolean.FALSE); } @Override diff --git a/src/main/java/org/zalando/stups/tokens/FileSupplier.java b/src/main/java/org/zalando/stups/tokens/FileSupplier.java index 20e6097..6c2b178 100644 --- a/src/main/java/org/zalando/stups/tokens/FileSupplier.java +++ b/src/main/java/org/zalando/stups/tokens/FileSupplier.java @@ -15,10 +15,11 @@ */ package org.zalando.stups.tokens; -import java.io.File; - import org.apache.http.util.Args; +import java.io.File; +import java.util.Optional; + /** * Replacement to make it compatible with Java7. * @@ -48,17 +49,16 @@ public synchronized File get() { } public static File getCredentialsDir() { - String dir = System.getenv("CREDENTIALS_DIR"); - if (dir == null) { - - // this for testing - dir = System.getProperty("CREDENTIALS_DIR"); - if (dir == null) { - throw new IllegalStateException("environment variable CREDENTIALS_DIR not set"); - } - } + return credentialsDir() + .map(File::new) + .orElseThrow(() -> new IllegalStateException( + "environment variable or application property CREDENTIALS_DIR not set" + )); + } - return new File(dir); + public static Optional credentialsDir() { + Optional optionalDir = Optional.ofNullable(System.getenv("CREDENTIALS_DIR")); + return optionalDir.isPresent() ? optionalDir : Optional.ofNullable(System.getProperty("CREDENTIALS_DIR")); } } diff --git a/src/test/java/org/zalando/stups/tokens/AccessTokenBuilderTest.java b/src/test/java/org/zalando/stups/tokens/AccessTokenBuilderTest.java index af1bec8..27d451e 100644 --- a/src/test/java/org/zalando/stups/tokens/AccessTokenBuilderTest.java +++ b/src/test/java/org/zalando/stups/tokens/AccessTokenBuilderTest.java @@ -15,13 +15,6 @@ */ package org.zalando.stups.tokens; -import static org.assertj.core.api.Assertions.assertThat; - -import java.io.File; -import java.io.IOException; -import java.net.URI; -import java.util.concurrent.ScheduledExecutorService; - import org.assertj.core.api.Assertions; import org.junit.After; import org.junit.Before; @@ -31,6 +24,14 @@ import org.junit.rules.TemporaryFolder; import org.mockito.Mockito; import org.mockito.internal.util.io.IOUtil; +import org.zalando.stups.tokens.fs.FilesystemSecretRefresher; + +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.util.concurrent.ScheduledExecutorService; + +import static org.assertj.core.api.Assertions.assertThat; /** * @@ -119,7 +120,7 @@ public void httpConfigShouldBeNotNull() { } @Test(expected = IllegalArgumentException.class) - public void accessTokenConfigurationWithouScopesShouldFail() { + public void accessTokenConfigurationWithoutScopesShouldFail() { Tokens.createAccessTokensWithUri(uri).start(); } @@ -167,4 +168,14 @@ public void usinEnvCreatesBuilder() { Assertions.assertThat(builder).isNotNull(); } + @Test + public void checkCorrectRefresherUsed() throws Exception { + environmentVariables.set("OAUTH2_ACCESS_TOKEN_URL", "https://somwhere.test/tokens"); + AccessTokensBuilder builder = Tokens.createAccessTokens(); + AbstractAccessTokenRefresher refresher = builder.getAccessTokenRefresher(); + assertThat(refresher instanceof FilesystemSecretRefresher); + + System.getProperties().remove(CREDENTIALS_DIR); + assertThat(refresher instanceof AccessTokenRefresher); + } } diff --git a/src/test/java/org/zalando/stups/tokens/FileSupplierTest.java b/src/test/java/org/zalando/stups/tokens/FileSupplierTest.java index 84fb7bb..7ea35e5 100644 --- a/src/test/java/org/zalando/stups/tokens/FileSupplierTest.java +++ b/src/test/java/org/zalando/stups/tokens/FileSupplierTest.java @@ -15,14 +15,14 @@ */ package org.zalando.stups.tokens; -import java.io.File; - import org.assertj.core.api.Assertions; import org.junit.Assert; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.io.File; + /** * @author jbellmann */ @@ -70,19 +70,16 @@ public void testFilenameParameterIsNotNull() { supplier.get(); } - @Test + @Test(expected = IllegalStateException.class) public void testNoCredentialsDirSet() { if (System.getenv("CREDENTIALS_DIR") != null) { LOG.warn("ENV 'CREDENTIALS_DIR' was set so we skip"); return; } - try { - FileSupplier supplier = new FileSupplier("notExistent"); - Assert.assertNotNull(supplier); - supplier.get(); - Assertions.fail("expect an exception, when 'CREDENTIALS_DIR' not set in environment"); - } catch (IllegalStateException t) { - } + FileSupplier supplier = new FileSupplier("notExistent"); + Assert.assertNotNull(supplier); + supplier.get(); + Assertions.fail("expect an exception, when 'CREDENTIALS_DIR' not set in environment"); } }