From c9fdff8d16f9a43f605e2250dc0285b39b2d1bd2 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Fri, 24 Jan 2025 16:07:49 +0100 Subject: [PATCH 01/21] SNOW-1895884: Add new token cache keys --- .../snowflake/client/core/SFTrustManager.java | 2 +- .../client/core/SecureStorageManager.java | 15 ++++++++++----- .../client/core/SecureStorageManagerTest.java | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SFTrustManager.java b/src/main/java/net/snowflake/client/core/SFTrustManager.java index 275037eb0..542109193 100644 --- a/src/main/java/net/snowflake/client/core/SFTrustManager.java +++ b/src/main/java/net/snowflake/client/core/SFTrustManager.java @@ -543,7 +543,7 @@ private static void verifySignature( * @param bytes a byte array * @return a string in hexadecimal code */ - private static String byteToHexString(byte[] bytes) { + static String byteToHexString(byte[] bytes) { final char[] hexArray = "0123456789ABCDEF".toCharArray(); char[] hexChars = new char[bytes.length * 2]; for (int j = 0; j < bytes.length; j++) { diff --git a/src/main/java/net/snowflake/client/core/SecureStorageManager.java b/src/main/java/net/snowflake/client/core/SecureStorageManager.java index d64c26c38..e98496d87 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageManager.java @@ -4,12 +4,14 @@ package net.snowflake.client.core; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; + /** * Interface for accessing Platform specific Local Secure Storage E.g. keychain on Mac credential * manager on Windows */ interface SecureStorageManager { - String DRIVER_NAME = "SNOWFLAKE-JDBC-DRIVER"; int COLON_CHAR_LENGTH = 1; SecureStorageStatus setCredential(String host, String user, String type, String token); @@ -23,7 +25,6 @@ static String convertTarget(String host, String user, String type) { new StringBuilder( host.length() + user.length() - + DRIVER_NAME.length() + type.length() + 3 * COLON_CHAR_LENGTH); @@ -31,11 +32,15 @@ static String convertTarget(String host, String user, String type) { target.append(":"); target.append(user.toUpperCase()); target.append(":"); - target.append(DRIVER_NAME); - target.append(":"); target.append(type.toUpperCase()); - return target.toString(); + try { + MessageDigest md = MessageDigest.getInstance("SHA-256"); + byte[] hash = md.digest(target.toString().getBytes()); + return SFTrustManager.byteToHexString(hash); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException(e); + } } enum SecureStorageStatus { diff --git a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java index b79875038..156a520fc 100644 --- a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java +++ b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java @@ -20,6 +20,7 @@ import net.snowflake.client.annotations.RunOnMac; import net.snowflake.client.annotations.RunOnWindows; import net.snowflake.client.annotations.RunOnWindowsOrMac; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; class MockAdvapi32Lib implements SecureStorageWindowsManager.Advapi32Lib { @@ -224,6 +225,22 @@ public class SecureStorageManagerTest { private static final String ID_TOKEN = "ID_TOKEN"; private static final String MFA_TOKEN = "MFATOKEN"; + @Test + public void testConvertTarget() { + // hex values obtained using https://emn178.github.io/online-tools/sha256.html + String hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.OAUTH_ACCESS_TOKEN.getValue()); + Assertions.assertEquals("A7C7EBB89312E88552CD00664A0E20929801FACFBD682BF7C2363FB6EC8F914E", hashedKey); + + hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.OAUTH_REFRESH_TOKEN.getValue()); + Assertions.assertEquals("DB37028833FA02B125FBD6DE8CE679C7E62E7D38FAC585E98060E00987F96772", hashedKey); + + hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.ID_TOKEN.getValue()); + Assertions.assertEquals("6AA3F783E07D1D2182DAB59442806E2433C55C2BD4D9240790FD5B4B91FD4FDB", hashedKey); + + hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.MFA_TOKEN.getValue()); + Assertions.assertEquals("9D10D4EFE45605D85993C6AC95334F1B63D36611B83615656EC7F277A947BF4B", hashedKey); + } + @Test @RunOnWindowsOrMac public void testLoadNativeLibrary() { From 1d0b235619466addd247468e5292ad875fc1ad56 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Wed, 29 Jan 2025 15:50:00 +0100 Subject: [PATCH 02/21] Change cache json file structure --- .../core/SecureStorageLinuxManager.java | 66 +++++++++---------- .../client/core/SecureStorageManager.java | 6 +- .../client/core/auth/oauth/OAuthUtil.java | 6 +- .../client/core/SecureStorageManagerTest.java | 32 +++++---- 4 files changed, 58 insertions(+), 52 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index d8c44dda3..6dfaa2a2b 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -26,6 +26,7 @@ public class SecureStorageLinuxManager implements SecureStorageManager { private static final String CACHE_FILE_NAME = "temporary_credential.json"; private static final String CACHE_DIR_PROP = "net.snowflake.jdbc.temporaryCredentialCacheDir"; private static final String CACHE_DIR_ENV = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR"; + private static final String CACHE_FILE_TOKENS_OBJECT_NAME = "tokens"; private static final long CACHE_EXPIRATION_IN_SECONDS = 86400L; private static final long CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS = 60L; private FileCacheManager fileCacheManager; @@ -43,6 +44,7 @@ private SecureStorageLinuxManager() { .build(); logger.debug( "Using temporary file: {} as a token cache storage", fileCacheManager.getCacheFilePath()); + populateLocalCredCache(); } private static class SecureStorageLinuxManagerHolder { @@ -53,62 +55,60 @@ public static SecureStorageLinuxManager getInstance() { return SecureStorageLinuxManagerHolder.INSTANCE; } - private ObjectNode localCacheToJson() { - ObjectNode res = mapper.createObjectNode(); - for (Map.Entry> elem : localCredCache.entrySet()) { - String elemHost = elem.getKey(); - Map hostMap = elem.getValue(); - ObjectNode hostNode = mapper.createObjectNode(); - for (Map.Entry elem0 : hostMap.entrySet()) { - hostNode.put(elem0.getKey(), elem0.getValue()); - } - res.set(elemHost, hostNode); - } - return res; - } - public synchronized SecureStorageStatus setCredential( String host, String user, String type, String token) { if (Strings.isNullOrEmpty(token)) { logger.warn("No token provided", false); return SecureStorageStatus.SUCCESS; } - - localCredCache.computeIfAbsent(host.toUpperCase(), newMap -> new HashMap<>()); - - Map hostMap = localCredCache.get(host.toUpperCase()); - hostMap.put(SecureStorageManager.convertTarget(host, user, type), token); - + localCredCache.computeIfAbsent(CACHE_FILE_TOKENS_OBJECT_NAME, tokensMap -> new HashMap<>()); + Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + tokensMap.put(SecureStorageManager.convertTarget(host, user, type), token); fileCacheManager.writeCacheFile(localCacheToJson()); return SecureStorageStatus.SUCCESS; } public synchronized String getCredential(String host, String user, String type) { - JsonNode res = fileCacheManager.readCacheFile(); - readJsonStoreCache(res); - - Map hostMap = localCredCache.get(host.toUpperCase()); - - if (hostMap == null) { + populateLocalCredCache(); + Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + if (tokensMap == null) { return null; } - - return hostMap.get(SecureStorageManager.convertTarget(host, user, type)); + return tokensMap.get(SecureStorageManager.convertTarget(host, user, type)); } /** May delete credentials which doesn't belong to this process */ public synchronized SecureStorageStatus deleteCredential(String host, String user, String type) { - Map hostMap = localCredCache.get(host.toUpperCase()); - if (hostMap != null) { - hostMap.remove(SecureStorageManager.convertTarget(host, user, type)); - if (hostMap.isEmpty()) { - localCredCache.remove(host.toUpperCase()); + Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + if (tokensMap != null) { + tokensMap.remove(SecureStorageManager.convertTarget(host, user, type)); + if (tokensMap.isEmpty()) { + localCredCache.remove(CACHE_FILE_TOKENS_OBJECT_NAME); } } fileCacheManager.writeCacheFile(localCacheToJson()); return SecureStorageStatus.SUCCESS; } + private ObjectNode localCacheToJson() { + ObjectNode res = mapper.createObjectNode(); + for (Map.Entry> elem : localCredCache.entrySet()) { + String elemHost = elem.getKey(); + Map hostMap = elem.getValue(); + ObjectNode hostNode = mapper.createObjectNode(); + for (Map.Entry elem0 : hostMap.entrySet()) { + hostNode.put(elem0.getKey(), elem0.getValue()); + } + res.set(elemHost, hostNode); + } + return res; + } + + private void populateLocalCredCache() { + JsonNode res = fileCacheManager.readCacheFile(); + readJsonStoreCache(res); + } + private void readJsonStoreCache(JsonNode m) { if (m == null || !m.getNodeType().equals(JsonNodeType.OBJECT)) { logger.debug("Invalid cache file format."); diff --git a/src/main/java/net/snowflake/client/core/SecureStorageManager.java b/src/main/java/net/snowflake/client/core/SecureStorageManager.java index e98496d87..64a9bdb3c 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageManager.java @@ -22,11 +22,7 @@ interface SecureStorageManager { static String convertTarget(String host, String user, String type) { StringBuilder target = - new StringBuilder( - host.length() - + user.length() - + type.length() - + 3 * COLON_CHAR_LENGTH); + new StringBuilder(host.length() + user.length() + type.length() + 3 * COLON_CHAR_LENGTH); target.append(host.toUpperCase()); target.append(":"); diff --git a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java index 3ec4c78ba..968a49a00 100644 --- a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java +++ b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java @@ -26,9 +26,9 @@ public class OAuthUtil { @SnowflakeJdbcInternalApi public static URI getTokenRequestUrl(SFOauthLoginInput oauthLoginInput, String serverUrl) { URI uri = - !StringUtils.isNullOrEmpty(oauthLoginInput.getExternalTokenRequestUrl()) - ? URI.create(oauthLoginInput.getExternalTokenRequestUrl()) - : URI.create(serverUrl + SNOWFLAKE_TOKEN_REQUEST_ENDPOINT); + !StringUtils.isNullOrEmpty(oauthLoginInput.getExternalTokenRequestUrl()) + ? URI.create(oauthLoginInput.getExternalTokenRequestUrl()) + : URI.create(serverUrl + SNOWFLAKE_TOKEN_REQUEST_ENDPOINT); return uri.normalize(); } diff --git a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java index 156a520fc..ac6a68937 100644 --- a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java +++ b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java @@ -228,17 +228,27 @@ public class SecureStorageManagerTest { @Test public void testConvertTarget() { // hex values obtained using https://emn178.github.io/online-tools/sha256.html - String hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.OAUTH_ACCESS_TOKEN.getValue()); - Assertions.assertEquals("A7C7EBB89312E88552CD00664A0E20929801FACFBD682BF7C2363FB6EC8F914E", hashedKey); - - hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.OAUTH_REFRESH_TOKEN.getValue()); - Assertions.assertEquals("DB37028833FA02B125FBD6DE8CE679C7E62E7D38FAC585E98060E00987F96772", hashedKey); - - hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.ID_TOKEN.getValue()); - Assertions.assertEquals("6AA3F783E07D1D2182DAB59442806E2433C55C2BD4D9240790FD5B4B91FD4FDB", hashedKey); - - hashedKey = SecureStorageManager.convertTarget(host, user, CachedCredentialType.MFA_TOKEN.getValue()); - Assertions.assertEquals("9D10D4EFE45605D85993C6AC95334F1B63D36611B83615656EC7F277A947BF4B", hashedKey); + String hashedKey = + SecureStorageManager.convertTarget( + host, user, CachedCredentialType.OAUTH_ACCESS_TOKEN.getValue()); + Assertions.assertEquals( + "A7C7EBB89312E88552CD00664A0E20929801FACFBD682BF7C2363FB6EC8F914E", hashedKey); + + hashedKey = + SecureStorageManager.convertTarget( + host, user, CachedCredentialType.OAUTH_REFRESH_TOKEN.getValue()); + Assertions.assertEquals( + "DB37028833FA02B125FBD6DE8CE679C7E62E7D38FAC585E98060E00987F96772", hashedKey); + + hashedKey = + SecureStorageManager.convertTarget(host, user, CachedCredentialType.ID_TOKEN.getValue()); + Assertions.assertEquals( + "6AA3F783E07D1D2182DAB59442806E2433C55C2BD4D9240790FD5B4B91FD4FDB", hashedKey); + + hashedKey = + SecureStorageManager.convertTarget(host, user, CachedCredentialType.MFA_TOKEN.getValue()); + Assertions.assertEquals( + "9D10D4EFE45605D85993C6AC95334F1B63D36611B83615656EC7F277A947BF4B", hashedKey); } @Test From 8b7aa4007a90fa5acc75c132a27b4780fbeb45c3 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 30 Jan 2025 10:54:43 +0100 Subject: [PATCH 03/21] refactor convertTarget name --- .../client/core/SecureStorageAppleManager.java | 6 +++--- .../client/core/SecureStorageLinuxManager.java | 6 +++--- .../snowflake/client/core/SecureStorageManager.java | 2 +- .../client/core/SecureStorageWindowsManager.java | 6 +++--- .../client/core/SecureStorageManagerTest.java | 10 +++++----- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageAppleManager.java b/src/main/java/net/snowflake/client/core/SecureStorageAppleManager.java index fb467edcc..bfe7540bf 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageAppleManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageAppleManager.java @@ -32,7 +32,7 @@ public SecureStorageStatus setCredential(String host, String user, String type, return SecureStorageStatus.SUCCESS; } - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); byte[] targetBytes = target.getBytes(StandardCharsets.UTF_8); byte[] userBytes = user.toUpperCase().getBytes(StandardCharsets.UTF_8); byte[] credBytes = cred.getBytes(StandardCharsets.UTF_8); @@ -92,7 +92,7 @@ public SecureStorageStatus setCredential(String host, String user, String type, } public String getCredential(String host, String user, String type) { - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); byte[] targetBytes = target.getBytes(StandardCharsets.UTF_8); byte[] userBytes = user.toUpperCase().getBytes(StandardCharsets.UTF_8); @@ -141,7 +141,7 @@ public String getCredential(String host, String user, String type) { } public SecureStorageStatus deleteCredential(String host, String user, String type) { - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); byte[] targetBytes = target.getBytes(StandardCharsets.UTF_8); byte[] userBytes = user.toUpperCase().getBytes(StandardCharsets.UTF_8); diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index 6dfaa2a2b..6c6397ad9 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -63,7 +63,7 @@ public synchronized SecureStorageStatus setCredential( } localCredCache.computeIfAbsent(CACHE_FILE_TOKENS_OBJECT_NAME, tokensMap -> new HashMap<>()); Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); - tokensMap.put(SecureStorageManager.convertTarget(host, user, type), token); + tokensMap.put(SecureStorageManager.buildCredentialsKey(host, user, type), token); fileCacheManager.writeCacheFile(localCacheToJson()); return SecureStorageStatus.SUCCESS; } @@ -74,14 +74,14 @@ public synchronized String getCredential(String host, String user, String type) if (tokensMap == null) { return null; } - return tokensMap.get(SecureStorageManager.convertTarget(host, user, type)); + return tokensMap.get(SecureStorageManager.buildCredentialsKey(host, user, type)); } /** May delete credentials which doesn't belong to this process */ public synchronized SecureStorageStatus deleteCredential(String host, String user, String type) { Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); if (tokensMap != null) { - tokensMap.remove(SecureStorageManager.convertTarget(host, user, type)); + tokensMap.remove(SecureStorageManager.buildCredentialsKey(host, user, type)); if (tokensMap.isEmpty()) { localCredCache.remove(CACHE_FILE_TOKENS_OBJECT_NAME); } diff --git a/src/main/java/net/snowflake/client/core/SecureStorageManager.java b/src/main/java/net/snowflake/client/core/SecureStorageManager.java index 64a9bdb3c..485e7e316 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageManager.java @@ -20,7 +20,7 @@ interface SecureStorageManager { SecureStorageStatus deleteCredential(String host, String user, String type); - static String convertTarget(String host, String user, String type) { + static String buildCredentialsKey(String host, String user, String type) { StringBuilder target = new StringBuilder(host.length() + user.length() + type.length() + 3 * COLON_CHAR_LENGTH); diff --git a/src/main/java/net/snowflake/client/core/SecureStorageWindowsManager.java b/src/main/java/net/snowflake/client/core/SecureStorageWindowsManager.java index e47a6d88b..0cf6e5ff5 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageWindowsManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageWindowsManager.java @@ -47,7 +47,7 @@ public SecureStorageStatus setCredential(String host, String user, String type, Memory credBlobMem = new Memory(credBlob.length); credBlobMem.write(0, credBlob, 0, credBlob.length); - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); SecureStorageWindowsCredential cred = new SecureStorageWindowsCredential(); cred.Type = SecureStorageWindowsCredentialType.CRED_TYPE_GENERIC.getType(); @@ -76,7 +76,7 @@ public SecureStorageStatus setCredential(String host, String user, String type, public String getCredential(String host, String user, String type) { PointerByReference pCredential = new PointerByReference(); - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); try { boolean ret = false; @@ -127,7 +127,7 @@ public String getCredential(String host, String user, String type) { } public SecureStorageStatus deleteCredential(String host, String user, String type) { - String target = SecureStorageManager.convertTarget(host, user, type); + String target = SecureStorageManager.buildCredentialsKey(host, user, type); boolean ret = false; synchronized (advapi32Lib) { diff --git a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java index ac6a68937..710f81aec 100644 --- a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java +++ b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java @@ -226,27 +226,27 @@ public class SecureStorageManagerTest { private static final String MFA_TOKEN = "MFATOKEN"; @Test - public void testConvertTarget() { + public void testBuildCredentialsKey() { // hex values obtained using https://emn178.github.io/online-tools/sha256.html String hashedKey = - SecureStorageManager.convertTarget( + SecureStorageManager.buildCredentialsKey( host, user, CachedCredentialType.OAUTH_ACCESS_TOKEN.getValue()); Assertions.assertEquals( "A7C7EBB89312E88552CD00664A0E20929801FACFBD682BF7C2363FB6EC8F914E", hashedKey); hashedKey = - SecureStorageManager.convertTarget( + SecureStorageManager.buildCredentialsKey( host, user, CachedCredentialType.OAUTH_REFRESH_TOKEN.getValue()); Assertions.assertEquals( "DB37028833FA02B125FBD6DE8CE679C7E62E7D38FAC585E98060E00987F96772", hashedKey); hashedKey = - SecureStorageManager.convertTarget(host, user, CachedCredentialType.ID_TOKEN.getValue()); + SecureStorageManager.buildCredentialsKey(host, user, CachedCredentialType.ID_TOKEN.getValue()); Assertions.assertEquals( "6AA3F783E07D1D2182DAB59442806E2433C55C2BD4D9240790FD5B4B91FD4FDB", hashedKey); hashedKey = - SecureStorageManager.convertTarget(host, user, CachedCredentialType.MFA_TOKEN.getValue()); + SecureStorageManager.buildCredentialsKey(host, user, CachedCredentialType.MFA_TOKEN.getValue()); Assertions.assertEquals( "9D10D4EFE45605D85993C6AC95334F1B63D36611B83615656EC7F277A947BF4B", hashedKey); } From 6575f5dd89f718e3a9cb8377801a847c3476e17f Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 30 Jan 2025 16:40:31 +0100 Subject: [PATCH 04/21] Refactor cache locks --- .../client/core/FileCacheManager.java | 123 +++++++++--------- .../net/snowflake/client/core/FileUtil.java | 10 +- .../snowflake/client/core/SFTrustManager.java | 1 - .../core/SecureStorageLinuxManager.java | 90 +++++++------ .../snowflake/client/core/SessionUtil.java | 5 - .../core/auth/ClientAuthnParameter.java | 3 +- .../client/core/auth/oauth/OAuthUtil.java | 7 +- .../client/core/FileCacheManagerTest.java | 1 - .../client/core/SecureStorageManagerTest.java | 6 +- 9 files changed, 122 insertions(+), 124 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 2d00d6d0a..80ac82754 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -27,6 +27,7 @@ import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; import java.util.Date; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import net.snowflake.client.log.SFLogger; @@ -43,7 +44,6 @@ class FileCacheManager { private String cacheDirectorySystemProperty; private String cacheDirectoryEnvironmentVariable; private String baseCacheFileName; - private long cacheExpirationInMilliseconds; private long cacheFileLockExpirationInMilliseconds; private File cacheFile; @@ -74,12 +74,6 @@ FileCacheManager setBaseCacheFileName(String baseCacheFileName) { return this; } - FileCacheManager setCacheExpirationInSeconds(long cacheExpirationInSeconds) { - // converting from seconds to milliseconds - this.cacheExpirationInMilliseconds = cacheExpirationInSeconds * 1000; - return this; - } - FileCacheManager setCacheFileLockExpirationInSeconds(long cacheFileLockExpirationInSeconds) { this.cacheFileLockExpirationInMilliseconds = cacheFileLockExpirationInSeconds * 1000; return this; @@ -90,6 +84,10 @@ FileCacheManager setOnlyOwnerPermissions(boolean onlyOwnerPermissions) { return this; } + String getCacheFilePath() { + return cacheFile.getAbsolutePath(); + } + /** * Override the cache file. * @@ -100,7 +98,7 @@ void overrideCacheFile(File newCacheFile) { logger.debug("Cache file doesn't exists. File: {}", newCacheFile); } if (onlyOwnerPermissions) { - FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner( + FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner( newCacheFile, "Override cache file"); } else { FileUtil.logFileUsage(cacheFile, "Override cache file", false); @@ -199,12 +197,33 @@ FileCacheManager build() { return this; } - /** Reads the cache file. */ - JsonNode readCacheFile() { - if (cacheFile == null || !this.checkCacheLockFile()) { - // no cache or the cache is not valid. + T withLock(Supplier supplier) { + if (cacheFile == null) { + logger.error("No cache file assigned", false); + return null; + } + if (cacheLockFile == null) { + logger.error("No cache lock file assigned", false); return null; + } else if (cacheLockFile.exists()) { + deleteCacheLockIfExpired(); + } + + if (!tryToLockCacheFile()) { + logger.debug("Failed to lock the file. Skipping cache operation", false); + return null; + } + try { + return supplier.get(); + } finally { + if (!unlockCacheFile()) { + logger.debug("Failed to unlock cache file", false); + } } + } + + /** Reads the cache file. */ + JsonNode readCacheFile() { try { if (!cacheFile.exists()) { logger.debug("Cache file doesn't exists. File: {}", cacheFile); @@ -215,7 +234,7 @@ JsonNode readCacheFile() { new InputStreamReader(new FileInputStream(cacheFile), DEFAULT_FILE_ENCODING)) { if (onlyOwnerPermissions) { - FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Read cache"); + FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner(cacheFile, "Read cache"); FileUtil.throwWhenOwnerDifferentThanCurrentUser(cacheFile, "Read cache"); } else { FileUtil.logFileUsage(cacheFile, "Read cache", false); @@ -230,13 +249,6 @@ JsonNode readCacheFile() { void writeCacheFile(JsonNode input) { logger.debug("Writing cache file. File: {}", cacheFile); - if (cacheFile == null || !tryLockCacheFile()) { - // no cache file or it failed to lock file - logger.debug( - "No cache file exists or failed to lock the file. Skipping writing the cache", false); - return; - } - // NOTE: must unlock cache file try { if (input == null) { return; @@ -244,7 +256,7 @@ void writeCacheFile(JsonNode input) { try (Writer writer = new OutputStreamWriter(new FileOutputStream(cacheFile), DEFAULT_FILE_ENCODING)) { if (onlyOwnerPermissions) { - FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Write to cache"); + FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner(cacheFile, "Write to cache"); } else { FileUtil.logFileUsage(cacheFile, "Write to cache", false); } @@ -252,10 +264,6 @@ void writeCacheFile(JsonNode input) { } } catch (IOException ex) { logger.debug("Failed to write the cache file. File: {}", cacheFile); - } finally { - if (!unlockCacheFile()) { - logger.debug("Failed to unlock cache file", false); - } } } @@ -277,10 +285,10 @@ void deleteCacheFile() { * * @return true if success or false */ - private boolean tryLockCacheFile() { + private boolean tryToLockCacheFile() { int cnt = 0; boolean locked = false; - while (cnt < 100 && !(locked = lockCacheFile())) { + while (cnt < 10 && !(locked = lockCacheFile())) { try { Thread.sleep(100); } catch (InterruptedException ex) { @@ -289,56 +297,27 @@ private boolean tryLockCacheFile() { ++cnt; } if (!locked) { - logger.debug("Failed to lock the cache file.", false); + deleteCacheLockIfExpired(); + if (!lockCacheFile()) { + logger.debug("Failed to lock the cache file.", false); + } } return locked; } - /** - * Lock cache file by creating a lock directory - * - * @return true if success or false - */ - private boolean lockCacheFile() { - return cacheLockFile.mkdirs(); - } - - /** - * Unlock cache file by deleting a lock directory - * - * @return true if success or false - */ - private boolean unlockCacheFile() { - return cacheLockFile.delete(); - } - - private boolean checkCacheLockFile() { + private void deleteCacheLockIfExpired() { long currentTime = new Date().getTime(); - long cacheFileTs = fileCreationTime(cacheFile); - - if (!cacheLockFile.exists() - && cacheFileTs > 0 - && currentTime - this.cacheExpirationInMilliseconds <= cacheFileTs) { - logger.debug("No cache file lock directory exists and cache file is up to date.", false); - return true; - } - long lockFileTs = fileCreationTime(cacheLockFile); if (lockFileTs < 0) { - // failed to get the timestamp of lock directory - return false; + logger.debug("Failed to get the timestamp of lock directory"); } if (lockFileTs < currentTime - this.cacheFileLockExpirationInMilliseconds) { // old lock file if (!cacheLockFile.delete()) { logger.debug("Failed to delete the directory. Dir: {}", cacheLockFile); - return false; } - logger.debug("Deleted the cache lock directory, because it was old.", false); - return currentTime - this.cacheExpirationInMilliseconds <= cacheFileTs; + logger.debug("Deleted expired cache lock directory.", false); } - logger.debug("Failed to lock the file. Ignored.", false); - return false; } /** @@ -361,7 +340,21 @@ private static long fileCreationTime(File targetFile) { return -1; } - String getCacheFilePath() { - return cacheFile.getAbsolutePath(); + /** + * Lock cache file by creating a lock directory + * + * @return true if success or false + */ + private boolean lockCacheFile() { + return cacheLockFile.mkdirs(); + } + + /** + * Unlock cache file by deleting a lock directory + * + * @return true if success or false + */ + private boolean unlockCacheFile() { + return cacheLockFile.delete(); } } diff --git a/src/main/java/net/snowflake/client/core/FileUtil.java b/src/main/java/net/snowflake/client/core/FileUtil.java index 005b9ac18..5acb38b9b 100644 --- a/src/main/java/net/snowflake/client/core/FileUtil.java +++ b/src/main/java/net/snowflake/client/core/FileUtil.java @@ -41,11 +41,11 @@ public static void logFileUsage(String stringPath, String context, boolean logRe logFileUsage(path, context, logReadAccess); } - public static void throwWhenPermiossionDifferentThanReadWriteForOwner(File file, String context) { - throwWhenPermiossionDifferentThanReadWriteForOwner(file.toPath(), context); + public static void throwWhenPermissionsDifferentThanReadWriteForOwner(File file, String context) { + throwWhenPermissionsDifferentThanReadWriteForOwner(file.toPath(), context); } - public static void throwWhenPermiossionDifferentThanReadWriteForOwner( + public static void throwWhenPermissionsDifferentThanReadWriteForOwner( Path filePath, String context) { // we do not check the permissions for Windows if (isWindows()) { @@ -62,7 +62,9 @@ public static void throwWhenPermiossionDifferentThanReadWriteForOwner( logger.debug( "{}File {} access rights: {}", getContextStr(context), filePath, filePermissions); throw new SecurityException( - String.format("Access to file %s is wider than allowed only to the owner", filePath)); + String.format( + "Access to file %s is wider than allowed only to the owner. Remove cached files and re-run the driver.", + filePath)); } } catch (IOException e) { throw new SecurityException( diff --git a/src/main/java/net/snowflake/client/core/SFTrustManager.java b/src/main/java/net/snowflake/client/core/SFTrustManager.java index fb734a71f..f0ce0c53b 100644 --- a/src/main/java/net/snowflake/client/core/SFTrustManager.java +++ b/src/main/java/net/snowflake/client/core/SFTrustManager.java @@ -225,7 +225,6 @@ public class SFTrustManager extends X509ExtendedTrustManager { .setCacheDirectorySystemProperty(CACHE_DIR_PROP) .setCacheDirectoryEnvironmentVariable(CACHE_DIR_ENV) .setBaseCacheFileName(CACHE_FILE_NAME) - .setCacheExpirationInSeconds(CACHE_EXPIRATION_IN_SECONDS) .setCacheFileLockExpirationInSeconds(CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS) .setOnlyOwnerPermissions(false) .build(); diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index 6c6397ad9..fda5f20d6 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -27,11 +27,8 @@ public class SecureStorageLinuxManager implements SecureStorageManager { private static final String CACHE_DIR_PROP = "net.snowflake.jdbc.temporaryCredentialCacheDir"; private static final String CACHE_DIR_ENV = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR"; private static final String CACHE_FILE_TOKENS_OBJECT_NAME = "tokens"; - private static final long CACHE_EXPIRATION_IN_SECONDS = 86400L; private static final long CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS = 60L; - private FileCacheManager fileCacheManager; - - private final Map> localCredCache = new HashMap<>(); + private final FileCacheManager fileCacheManager; private SecureStorageLinuxManager() { fileCacheManager = @@ -39,12 +36,10 @@ private SecureStorageLinuxManager() { .setCacheDirectorySystemProperty(CACHE_DIR_PROP) .setCacheDirectoryEnvironmentVariable(CACHE_DIR_ENV) .setBaseCacheFileName(CACHE_FILE_NAME) - .setCacheExpirationInSeconds(CACHE_EXPIRATION_IN_SECONDS) .setCacheFileLockExpirationInSeconds(CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS) .build(); logger.debug( "Using temporary file: {} as a token cache storage", fileCacheManager.getCacheFilePath()); - populateLocalCredCache(); } private static class SecureStorageLinuxManagerHolder { @@ -55,44 +50,64 @@ public static SecureStorageLinuxManager getInstance() { return SecureStorageLinuxManagerHolder.INSTANCE; } + @Override public synchronized SecureStorageStatus setCredential( String host, String user, String type, String token) { if (Strings.isNullOrEmpty(token)) { logger.warn("No token provided", false); return SecureStorageStatus.SUCCESS; } - localCredCache.computeIfAbsent(CACHE_FILE_TOKENS_OBJECT_NAME, tokensMap -> new HashMap<>()); - Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); - tokensMap.put(SecureStorageManager.buildCredentialsKey(host, user, type), token); - fileCacheManager.writeCacheFile(localCacheToJson()); + fileCacheManager.withLock( + () -> { + Map> cachedCredentials = + readJsonStoreCache(fileCacheManager.readCacheFile()); + cachedCredentials.computeIfAbsent( + CACHE_FILE_TOKENS_OBJECT_NAME, tokensMap -> new HashMap<>()); + Map credentialsMap = cachedCredentials.get(CACHE_FILE_TOKENS_OBJECT_NAME); + credentialsMap.put(SecureStorageManager.buildCredentialsKey(host, user, type), token); + fileCacheManager.writeCacheFile( + SecureStorageLinuxManager.this.localCacheToJson(cachedCredentials)); + return null; + }); return SecureStorageStatus.SUCCESS; } + @Override public synchronized String getCredential(String host, String user, String type) { - populateLocalCredCache(); - Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); - if (tokensMap == null) { - return null; - } - return tokensMap.get(SecureStorageManager.buildCredentialsKey(host, user, type)); + return fileCacheManager.withLock( + () -> { + JsonNode res = fileCacheManager.readCacheFile(); + Map> cache = readJsonStoreCache(res); + Map credentialsMap = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + if (credentialsMap == null) { + return null; + } + return credentialsMap.get(SecureStorageManager.buildCredentialsKey(host, user, type)); + }); } - /** May delete credentials which doesn't belong to this process */ + @Override public synchronized SecureStorageStatus deleteCredential(String host, String user, String type) { - Map tokensMap = localCredCache.get(CACHE_FILE_TOKENS_OBJECT_NAME); - if (tokensMap != null) { - tokensMap.remove(SecureStorageManager.buildCredentialsKey(host, user, type)); - if (tokensMap.isEmpty()) { - localCredCache.remove(CACHE_FILE_TOKENS_OBJECT_NAME); - } - } - fileCacheManager.writeCacheFile(localCacheToJson()); + fileCacheManager.withLock( + () -> { + JsonNode res = fileCacheManager.readCacheFile(); + Map> cache = readJsonStoreCache(res); + Map credentialsMap = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + if (credentialsMap != null) { + credentialsMap.remove(SecureStorageManager.buildCredentialsKey(host, user, type)); + if (credentialsMap.isEmpty()) { + cache.remove(CACHE_FILE_TOKENS_OBJECT_NAME); + } + } + fileCacheManager.writeCacheFile(localCacheToJson(cache)); + return null; + }); return SecureStorageStatus.SUCCESS; } - private ObjectNode localCacheToJson() { + private ObjectNode localCacheToJson(Map> cache) { ObjectNode res = mapper.createObjectNode(); - for (Map.Entry> elem : localCredCache.entrySet()) { + for (Map.Entry> elem : cache.entrySet()) { String elemHost = elem.getKey(); Map hostMap = elem.getValue(); ObjectNode hostNode = mapper.createObjectNode(); @@ -104,27 +119,24 @@ private ObjectNode localCacheToJson() { return res; } - private void populateLocalCredCache() { - JsonNode res = fileCacheManager.readCacheFile(); - readJsonStoreCache(res); - } - - private void readJsonStoreCache(JsonNode m) { - if (m == null || !m.getNodeType().equals(JsonNodeType.OBJECT)) { + private Map> readJsonStoreCache(JsonNode node) { + Map> credentialsCache = new HashMap<>(); + if (node == null || !node.getNodeType().equals(JsonNodeType.OBJECT)) { logger.debug("Invalid cache file format."); - return; + return credentialsCache; } - for (Iterator> itr = m.fields(); itr.hasNext(); ) { + for (Iterator> itr = node.fields(); itr.hasNext(); ) { Map.Entry hostMap = itr.next(); String host = hostMap.getKey(); - if (!localCredCache.containsKey(host)) { - localCredCache.put(host, new HashMap<>()); + if (!credentialsCache.containsKey(host)) { + credentialsCache.put(host, new HashMap<>()); } JsonNode userJsonNode = hostMap.getValue(); for (Iterator> itr0 = userJsonNode.fields(); itr0.hasNext(); ) { Map.Entry userMap = itr0.next(); - localCredCache.get(host).put(userMap.getKey(), userMap.getValue().asText()); + credentialsCache.get(host).put(userMap.getKey(), userMap.getValue().asText()); } } + return credentialsCache; } } diff --git a/src/main/java/net/snowflake/client/core/SessionUtil.java b/src/main/java/net/snowflake/client/core/SessionUtil.java index 7711539f9..32fbc6e0e 100644 --- a/src/main/java/net/snowflake/client/core/SessionUtil.java +++ b/src/main/java/net/snowflake/client/core/SessionUtil.java @@ -31,7 +31,6 @@ import net.snowflake.client.core.auth.oauth.AccessTokenProvider; import net.snowflake.client.core.auth.oauth.OAuthAccessTokenForRefreshTokenProvider; import net.snowflake.client.core.auth.oauth.OAuthAccessTokenProviderFactory; -import net.snowflake.client.core.auth.oauth.OAuthUtil; import net.snowflake.client.core.auth.oauth.TokenResponseDTO; import net.snowflake.client.jdbc.ErrorCode; import net.snowflake.client.jdbc.SnowflakeDriver; @@ -606,10 +605,6 @@ static SFLoginOutput newSession( if (authenticatorType == AuthenticatorType.OAUTH && loginInput.getOriginalAuthenticator() != null) { data.put(ClientAuthnParameter.OAUTH_TYPE.name(), loginInput.getOriginalAuthenticator()); - URI idpUri = - OAuthUtil.getTokenRequestUrl( - loginInput.getOauthLoginInput(), loginInput.getServerUrl()); - data.put(ClientAuthnParameter.IDP_HOST.name(), idpUri.getHost()); } // map of client environment parameters, including connection parameters diff --git a/src/main/java/net/snowflake/client/core/auth/ClientAuthnParameter.java b/src/main/java/net/snowflake/client/core/auth/ClientAuthnParameter.java index 9a6f89057..95517fd45 100644 --- a/src/main/java/net/snowflake/client/core/auth/ClientAuthnParameter.java +++ b/src/main/java/net/snowflake/client/core/auth/ClientAuthnParameter.java @@ -21,6 +21,5 @@ public enum ClientAuthnParameter { SESSION_PARAMETERS, PROOF_KEY, TOKEN, - OAUTH_TYPE, - IDP_HOST + OAUTH_TYPE } diff --git a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java index 968a49a00..46906dce7 100644 --- a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java +++ b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthUtil.java @@ -10,21 +10,18 @@ import java.net.URI; import java.nio.charset.StandardCharsets; import net.snowflake.client.core.SFOauthLoginInput; -import net.snowflake.client.core.SnowflakeJdbcInternalApi; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpRequestBase; import org.apache.http.entity.StringEntity; -@SnowflakeJdbcInternalApi -public class OAuthUtil { +class OAuthUtil { private static final String SNOWFLAKE_AUTHORIZE_ENDPOINT = "/oauth/authorize"; private static final String SNOWFLAKE_TOKEN_REQUEST_ENDPOINT = "/oauth/token-request"; private static final String DEFAULT_SESSION_ROLE_SCOPE_PREFIX = "session:role:"; - @SnowflakeJdbcInternalApi - public static URI getTokenRequestUrl(SFOauthLoginInput oauthLoginInput, String serverUrl) { + static URI getTokenRequestUrl(SFOauthLoginInput oauthLoginInput, String serverUrl) { URI uri = !StringUtils.isNullOrEmpty(oauthLoginInput.getExternalTokenRequestUrl()) ? URI.create(oauthLoginInput.getExternalTokenRequestUrl()) diff --git a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java index 023c20d40..521e48702 100644 --- a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java +++ b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java @@ -54,7 +54,6 @@ public void setup() throws IOException { .setCacheDirectorySystemProperty(CACHE_DIR_PROP) .setCacheDirectoryEnvironmentVariable(CACHE_DIR_ENV) .setBaseCacheFileName(CACHE_FILE_NAME) - .setCacheExpirationInSeconds(CACHE_EXPIRATION_IN_SECONDS) .setCacheFileLockExpirationInSeconds(CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS) .build(); cacheFile = createCacheFile(); diff --git a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java index 710f81aec..5e6181fe5 100644 --- a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java +++ b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java @@ -241,12 +241,14 @@ public void testBuildCredentialsKey() { "DB37028833FA02B125FBD6DE8CE679C7E62E7D38FAC585E98060E00987F96772", hashedKey); hashedKey = - SecureStorageManager.buildCredentialsKey(host, user, CachedCredentialType.ID_TOKEN.getValue()); + SecureStorageManager.buildCredentialsKey( + host, user, CachedCredentialType.ID_TOKEN.getValue()); Assertions.assertEquals( "6AA3F783E07D1D2182DAB59442806E2433C55C2BD4D9240790FD5B4B91FD4FDB", hashedKey); hashedKey = - SecureStorageManager.buildCredentialsKey(host, user, CachedCredentialType.MFA_TOKEN.getValue()); + SecureStorageManager.buildCredentialsKey( + host, user, CachedCredentialType.MFA_TOKEN.getValue()); Assertions.assertEquals( "9D10D4EFE45605D85993C6AC95334F1B63D36611B83615656EC7F277A947BF4B", hashedKey); } From 3ae9ed7ff2f9fa02fd921db2723b04661a1dfd51 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Mon, 3 Feb 2025 12:57:21 +0100 Subject: [PATCH 05/21] Refactor readJsonStoreCache --- .../core/SecureStorageLinuxManager.java | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index fda5f20d6..68782b3a0 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -120,23 +120,20 @@ private ObjectNode localCacheToJson(Map> cache) { } private Map> readJsonStoreCache(JsonNode node) { - Map> credentialsCache = new HashMap<>(); + Map> cache = new HashMap<>(); if (node == null || !node.getNodeType().equals(JsonNodeType.OBJECT)) { logger.debug("Invalid cache file format."); - return credentialsCache; + return cache; } - for (Iterator> itr = node.fields(); itr.hasNext(); ) { - Map.Entry hostMap = itr.next(); - String host = hostMap.getKey(); - if (!credentialsCache.containsKey(host)) { - credentialsCache.put(host, new HashMap<>()); - } - JsonNode userJsonNode = hostMap.getValue(); - for (Iterator> itr0 = userJsonNode.fields(); itr0.hasNext(); ) { - Map.Entry userMap = itr0.next(); - credentialsCache.get(host).put(userMap.getKey(), userMap.getValue().asText()); - } + cache.put(CACHE_FILE_TOKENS_OBJECT_NAME, new HashMap<>()); + JsonNode credentialsNode = node.get(CACHE_FILE_TOKENS_OBJECT_NAME); + Map credentialsCache = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + if (credentialsNode != null && node.getNodeType().equals(JsonNodeType.OBJECT)) { + for (Iterator> itr = credentialsNode.fields(); itr.hasNext(); ) { + Map.Entry credential = itr.next(); + credentialsCache.put(credential.getKey(), credential.getValue().asText()); + } } - return credentialsCache; + return cache; } } From 572adfdd72508ee2e999d77aa7f197f58ea2c771 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 6 Feb 2025 11:27:23 +0100 Subject: [PATCH 06/21] CR suggestions --- .../client/core/FileCacheManager.java | 96 +++++++++---- .../net/snowflake/client/core/FileUtil.java | 39 ++++-- .../net/snowflake/client/core/HexUtil.java | 25 ++++ .../snowflake/client/core/SFTrustManager.java | 23 +-- .../core/SecureStorageLinuxManager.java | 2 +- .../client/core/SecureStorageManager.java | 2 +- .../client/core/FileCacheManagerTest.java | 131 +++++++++++++++--- 7 files changed, 236 insertions(+), 82 deletions(-) create mode 100644 src/main/java/net/snowflake/client/core/HexUtil.java diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 80ac82754..6e7de0f18 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -4,6 +4,7 @@ package net.snowflake.client.core; +import static net.snowflake.client.core.FileUtil.isWritable; import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows; import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetEnv; import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty; @@ -98,8 +99,9 @@ void overrideCacheFile(File newCacheFile) { logger.debug("Cache file doesn't exists. File: {}", newCacheFile); } if (onlyOwnerPermissions) { - FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner( + FileUtil.throwWhenFilePermissionsWiderThanUserOnly( newCacheFile, "Override cache file"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Override cache file"); } else { FileUtil.logFileUsage(cacheFile, "Override cache file", false); } @@ -132,34 +134,27 @@ FileCacheManager build() { if (cacheDirPath != null) { this.cacheDir = new File(cacheDirPath); } else { - // use user home directory to store the cache file - String homeDir = systemGetProperty("user.home"); - if (homeDir != null) { - // Checking if home directory is writable. - File homeFile = new File(homeDir); - if (!homeFile.canWrite()) { - logger.debug("Home directory not writeable, skip using cache", false); - homeDir = null; + this.cacheDir = getDefaultCacheDir(); + } + if (cacheDir == null) { + return this; + } + if (!cacheDir.exists()) { + try { + Files.createDirectories(cacheDir.toPath(), + PosixFilePermissions.asFileAttribute( + Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE) + .collect(Collectors.toSet()))); + } catch (IOException e) { + logger.info( + "Failed to create the cache directory: {}. Ignored. {}", + e.getMessage(), + cacheDir.getAbsoluteFile()); + return this; } - } - if (homeDir == null) { - // if still home directory is null, no cache dir is set. - return this; - } - if (Constants.getOS() == Constants.OS.WINDOWS) { - this.cacheDir = - new File( - new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches"); - } else if (Constants.getOS() == Constants.OS.MAC) { - this.cacheDir = new File(new File(new File(homeDir, "Library"), "Caches"), "Snowflake"); - } else { - this.cacheDir = new File(new File(homeDir, ".cache"), "snowflake"); - } } - - if (!this.cacheDir.mkdirs() && !this.cacheDir.exists()) { - logger.debug( - "Cannot create the cache directory {}. Giving up.", this.cacheDir.getAbsolutePath()); + if (!this.cacheDir.exists()) { + logger.debug("Cannot create the cache directory {}. Giving up.", this.cacheDir.getAbsolutePath()); return this; } logger.debug("Verified Directory {}", this.cacheDir.getAbsolutePath()); @@ -197,6 +192,45 @@ FileCacheManager build() { return this; } + static File getDefaultCacheDir() { + if (Constants.getOS() == Constants.OS.LINUX) { + String xdgCacheHome = getXdgCacheHome(); + if (xdgCacheHome != null) { + return new File(xdgCacheHome, "snowflake"); + } + } + + String homeDir = getHomeDirProperty(); + if (homeDir == null) { + // if still home directory is null, no cache dir is set. + return null; + } + if (Constants.getOS() == Constants.OS.WINDOWS) { + return new File( + new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches"); + } else if (Constants.getOS() == Constants.OS.MAC) { + return new File(new File(new File(homeDir, "Library"), "Caches"), "Snowflake"); + } else { + return new File(new File(homeDir, ".cache"), "snowflake"); + } + } + + private static String getXdgCacheHome() { + String xdgCacheHome = systemGetEnv("XDG_CACHE_HOME"); + if (xdgCacheHome != null && isWritable(xdgCacheHome)) { + return xdgCacheHome; + } + return null; + } + + private static String getHomeDirProperty() { + String homeDir = systemGetProperty("user.home"); + if (homeDir != null && isWritable(homeDir)) { + return homeDir; + } + return null; + } + T withLock(Supplier supplier) { if (cacheFile == null) { logger.error("No cache file assigned", false); @@ -234,7 +268,8 @@ JsonNode readCacheFile() { new InputStreamReader(new FileInputStream(cacheFile), DEFAULT_FILE_ENCODING)) { if (onlyOwnerPermissions) { - FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner(cacheFile, "Read cache"); + FileUtil.throwWhenFilePermissionsWiderThanUserOnly(cacheFile, "Read cache"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Read cache"); FileUtil.throwWhenOwnerDifferentThanCurrentUser(cacheFile, "Read cache"); } else { FileUtil.logFileUsage(cacheFile, "Read cache", false); @@ -256,7 +291,8 @@ void writeCacheFile(JsonNode input) { try (Writer writer = new OutputStreamWriter(new FileOutputStream(cacheFile), DEFAULT_FILE_ENCODING)) { if (onlyOwnerPermissions) { - FileUtil.throwWhenPermissionsDifferentThanReadWriteForOwner(cacheFile, "Write to cache"); + FileUtil.throwWhenFilePermissionsWiderThanUserOnly(cacheFile, "Write to cache"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Write to cache"); } else { FileUtil.logFileUsage(cacheFile, "Write to cache", false); } @@ -288,7 +324,7 @@ void deleteCacheFile() { private boolean tryToLockCacheFile() { int cnt = 0; boolean locked = false; - while (cnt < 10 && !(locked = lockCacheFile())) { + while (cnt < 5 && !(locked = lockCacheFile())) { try { Thread.sleep(100); } catch (InterruptedException ex) { diff --git a/src/main/java/net/snowflake/client/core/FileUtil.java b/src/main/java/net/snowflake/client/core/FileUtil.java index 5acb38b9b..4a775c24d 100644 --- a/src/main/java/net/snowflake/client/core/FileUtil.java +++ b/src/main/java/net/snowflake/client/core/FileUtil.java @@ -41,12 +41,29 @@ public static void logFileUsage(String stringPath, String context, boolean logRe logFileUsage(path, context, logReadAccess); } - public static void throwWhenPermissionsDifferentThanReadWriteForOwner(File file, String context) { - throwWhenPermissionsDifferentThanReadWriteForOwner(file.toPath(), context); + public static boolean isWritable(String path) { + File file = new File(path); + if (!file.canWrite()) { + logger.debug("File/directory not writeable: {}", path); + return false; + } + return true; + } + + public static void throwWhenParentDirectoryPermissionsWiderThanUserOnly(File file, String context) { + throwWhenDirectoryPermissionsWiderThanUserOnly(file.getParentFile(), context); + } + + public static void throwWhenFilePermissionsWiderThanUserOnly(File file, String context) { + throwWhenPermissionsWiderThanUserOnly(file.toPath(), context, false); + } + + public static void throwWhenDirectoryPermissionsWiderThanUserOnly(File file, String context) { + throwWhenPermissionsWiderThanUserOnly(file.toPath(), context, true); } - public static void throwWhenPermissionsDifferentThanReadWriteForOwner( - Path filePath, String context) { + public static void throwWhenPermissionsWiderThanUserOnly( + Path filePath, String context, boolean isDirectory) { // we do not check the permissions for Windows if (isWindows()) { return; @@ -58,18 +75,24 @@ public static void throwWhenPermissionsDifferentThanReadWriteForOwner( boolean isReadableByOthers = isPermPresent(filePermissions, READ_BY_OTHERS); boolean isExecutable = isPermPresent(filePermissions, EXECUTABLE); - if (isWritableByOthers || isReadableByOthers || isExecutable) { + boolean permissionsTooOpen; + if (isDirectory) { + permissionsTooOpen = isWritableByOthers || isReadableByOthers; + } else { + permissionsTooOpen = isWritableByOthers || isReadableByOthers || isExecutable; + } + if (permissionsTooOpen) { logger.debug( - "{}File {} access rights: {}", getContextStr(context), filePath, filePermissions); + "{}File/directory {} access rights: {}", getContextStr(context), filePath, filePermissions); throw new SecurityException( String.format( - "Access to file %s is wider than allowed only to the owner. Remove cached files and re-run the driver.", + "Access to file or directory %s is wider than allowed only to the owner. Remove cached file/directory and re-run the driver.", filePath)); } } catch (IOException e) { throw new SecurityException( String.format( - "%s Unable to access the file to check the permissions. Error: %s", filePath, e)); + "%s Unable to access the file/directory to check the permissions. Error: %s", filePath, e)); } } diff --git a/src/main/java/net/snowflake/client/core/HexUtil.java b/src/main/java/net/snowflake/client/core/HexUtil.java new file mode 100644 index 000000000..49948700f --- /dev/null +++ b/src/main/java/net/snowflake/client/core/HexUtil.java @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2025 Snowflake Computing Inc. All rights reserved. + */ + +package net.snowflake.client.core; + +class HexUtil { + + /** + * Converts Byte array to hex string + * + * @param bytes a byte array + * @return a string in hexadecimal code + */ + static String byteToHexString(byte[] bytes) { + final char[] hexArray = "0123456789ABCDEF".toCharArray(); + char[] hexChars = new char[bytes.length * 2]; + for (int j = 0; j < bytes.length; j++) { + int v = bytes[j] & 0xFF; + hexChars[j * 2] = hexArray[v >>> 4]; + hexChars[j * 2 + 1] = hexArray[v & 0x0F]; + } + return new String(hexChars); + } +} diff --git a/src/main/java/net/snowflake/client/core/SFTrustManager.java b/src/main/java/net/snowflake/client/core/SFTrustManager.java index f0ce0c53b..777b842a2 100644 --- a/src/main/java/net/snowflake/client/core/SFTrustManager.java +++ b/src/main/java/net/snowflake/client/core/SFTrustManager.java @@ -406,8 +406,8 @@ private static String encodeCacheKey(OcspResponseCacheKey ocsp_cache_key) { private static String CertificateIDToString(CertificateID certificateID) { return String.format( "CertID. NameHash: %s, KeyHash: %s, Serial Number: %s", - byteToHexString(certificateID.getIssuerNameHash()), - byteToHexString(certificateID.getIssuerKeyHash()), + HexUtil.byteToHexString(certificateID.getIssuerNameHash()), + HexUtil.byteToHexString(certificateID.getIssuerKeyHash()), MessageFormat.format("{0,number,#}", certificateID.getSerialNumber())); } @@ -537,23 +537,6 @@ private static void verifySignature( } } - /** - * Converts Byte array to hex string - * - * @param bytes a byte array - * @return a string in hexadecimal code - */ - static String byteToHexString(byte[] bytes) { - final char[] hexArray = "0123456789ABCDEF".toCharArray(); - char[] hexChars = new char[bytes.length * 2]; - for (int j = 0; j < bytes.length; j++) { - int v = bytes[j] & 0xFF; - hexChars[j * 2] = hexArray[v >>> 4]; - hexChars[j * 2 + 1] = hexArray[v & 0x0F]; - } - return new String(hexChars); - } - /** * Gets HttpClient object * @@ -1609,7 +1592,7 @@ public boolean equals(Object obj) { public String toString() { return String.format( "OcspResponseCacheKey: NameHash: %s, KeyHash: %s, SerialNumber: %s", - byteToHexString(nameHash), byteToHexString(keyHash), serialNumber.toString()); + HexUtil.byteToHexString(nameHash), HexUtil.byteToHexString(keyHash), serialNumber.toString()); } } diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index 68782b3a0..d39b29f99 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -23,7 +23,7 @@ */ public class SecureStorageLinuxManager implements SecureStorageManager { private static final SFLogger logger = SFLoggerFactory.getLogger(SecureStorageLinuxManager.class); - private static final String CACHE_FILE_NAME = "temporary_credential.json"; + private static final String CACHE_FILE_NAME = "credential_cache_v1.json"; private static final String CACHE_DIR_PROP = "net.snowflake.jdbc.temporaryCredentialCacheDir"; private static final String CACHE_DIR_ENV = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR"; private static final String CACHE_FILE_TOKENS_OBJECT_NAME = "tokens"; diff --git a/src/main/java/net/snowflake/client/core/SecureStorageManager.java b/src/main/java/net/snowflake/client/core/SecureStorageManager.java index 485e7e316..c7b45f30c 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageManager.java @@ -33,7 +33,7 @@ static String buildCredentialsKey(String host, String user, String type) { try { MessageDigest md = MessageDigest.getInstance("SHA-256"); byte[] hash = md.digest(target.toString().getBytes()); - return SFTrustManager.byteToHexString(hash); + return HexUtil.byteToHexString(hash); } catch (NoSuchAlgorithmException e) { throw new RuntimeException(e); } diff --git a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java index 521e48702..b6fcccaf7 100644 --- a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java +++ b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java @@ -24,7 +24,9 @@ import net.snowflake.client.annotations.RunOnLinuxOrMac; import net.snowflake.client.category.TestTags; import net.snowflake.client.jdbc.BaseJDBCTest; +import net.snowflake.client.jdbc.SnowflakeUtil; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Tag; @@ -41,7 +43,6 @@ class FileCacheManagerTest extends BaseJDBCTest { private static final String CACHE_FILE_NAME = "temporary_credential.json"; private static final String CACHE_DIR_PROP = "net.snowflake.jdbc.temporaryCredentialCacheDir"; private static final String CACHE_DIR_ENV = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR"; - private static final long CACHE_EXPIRATION_IN_SECONDS = 86400L; private static final long CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS = 60L; private FileCacheManager fileCacheManager; @@ -64,34 +65,39 @@ public void clean() throws IOException { if (Files.exists(cacheFile.toPath())) { Files.delete(cacheFile.toPath()); } + if (Files.exists(cacheFile.getParentFile().toPath())) { + Files.delete(cacheFile.getParentFile().toPath()); + } } @ParameterizedTest @CsvSource({ - "rwx------,false", - "rw-------,true", - "r-x------,false", - "r--------,true", - "rwxrwx---,false", - "rwxrw----,false", - "rwxr-x---,false", - "rwxr-----,false", - "rwx-wx---,false", - "rwx-w----,false", - "rwx--x---,false", - "rwx---rwx,false", - "rwx---rw-,false", - "rwx---r-x,false", - "rwx---r--,false", - "rwx----wx,false", - "rwx----w-,false", - "rwx-----x,false" + "rwx------,rwx------,false", + "rw-------,rwx------,true", + "rw-------,rwx--xrwx,false", + "r-x------,rwx------,false", + "r--------,rwx------,true", + "rwxrwx---,rwx------,false", + "rwxrw----,rwx------,false", + "rwxr-x---,rwx------,false", + "rwxr-----,rwx------,false", + "rwx-wx---,rwx------,false", + "rwx-w----,rwx------,false", + "rwx--x---,rwx------,false", + "rwx---rwx,rwx------,false", + "rwx---rw-,rwx------,false", + "rwx---r-x,rwx------,false", + "rwx---r--,rwx------,false", + "rwx----wx,rwx------,false", + "rwx----w-,rwx------,false", + "rwx-----x,rwx------,false" }) @RunOnLinuxOrMac public void throwWhenReadCacheFileWithPermissionDifferentThanReadWriteForUserTest( - String permission, boolean isSucceed) throws IOException { + String permission, String parentDirectoryPermissions, boolean isSucceed) throws IOException { fileCacheManager.overrideCacheFile(cacheFile); Files.setPosixFilePermissions(cacheFile.toPath(), PosixFilePermissions.fromString(permission)); + Files.setPosixFilePermissions(cacheFile.getParentFile().toPath(), PosixFilePermissions.fromString(parentDirectoryPermissions)); if (isSucceed) { assertDoesNotThrow(() -> fileCacheManager.readCacheFile()); } else { @@ -133,7 +139,85 @@ public void throwWhenOverrideCacheFileNotFound() { assertTrue( ex.getMessage() .contains( - "Unable to access the file to check the permissions. Error: java.nio.file.NoSuchFileException:")); + "Unable to access the file/directory to check the permissions. Error: java.nio.file.NoSuchFileException:")); + } + + @Test + public void shouldCreateCacheDirForLinuxXDG() { + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); + try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn("/XDG/Cache/"); + try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { + fileUtilMockedStatic.when(() -> FileUtil.isWritable("/XDG/Cache/")).thenReturn(true); + File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); + Assertions.assertNotNull(defaultCacheDir); + Assertions.assertEquals("/XDG/Cache/snowflake", defaultCacheDir.getAbsolutePath()); + } + } + } + } + + @Test + public void shouldCreateCacheDirForLinuxWithoutXDG() { + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); + try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn(null); + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { + fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); + File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); + Assertions.assertNotNull(defaultCacheDir); + Assertions.assertEquals("/User/Home/.cache/snowflake", defaultCacheDir.getAbsolutePath()); + } + } + } + } + + @Test + public void shouldCreateCacheDirForWindows() { + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.WINDOWS); + try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { + fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); + File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); + Assertions.assertNotNull(defaultCacheDir); + Assertions.assertEquals("/User/Home/AppData/Local/Snowflake/Caches", defaultCacheDir.getAbsolutePath()); + } + } + } + } + + @Test + public void shouldCreateCacheDirForMacOS() { + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.MAC); + try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { + fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); + File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); + Assertions.assertNotNull(defaultCacheDir); + Assertions.assertEquals("/User/Home/Library/Caches/Snowflake", defaultCacheDir.getAbsolutePath()); + } + } + } + } + + @Test + public void shouldReturnNullWhenNoHomeDirSet() { + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); + try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn(null); + snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn(null); + File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); + Assertions.assertNull(defaultCacheDir); + } + } } private File createCacheFile() { @@ -143,7 +227,10 @@ private File createCacheFile() { if (Files.exists(cacheFile)) { Files.delete(cacheFile); } - Files.createDirectories(cacheFile.getParent()); + Files.createDirectories(cacheFile.getParent(), + PosixFilePermissions.asFileAttribute( + Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE) + .collect(Collectors.toSet()))); Files.createFile( cacheFile, PosixFilePermissions.asFileAttribute( From c19b17752210a31a94a79a46123b71c4ded0df80 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Fri, 7 Feb 2025 16:55:56 +0100 Subject: [PATCH 07/21] Fix CR suggestions --- .../snowflake/client/core/FileCacheManager.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 6e7de0f18..bd5faa144 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -231,7 +231,7 @@ private static String getHomeDirProperty() { return null; } - T withLock(Supplier supplier) { + synchronized T withLock(Supplier supplier) { if (cacheFile == null) { logger.error("No cache file assigned", false); return null; @@ -346,13 +346,17 @@ private void deleteCacheLockIfExpired() { long lockFileTs = fileCreationTime(cacheLockFile); if (lockFileTs < 0) { logger.debug("Failed to get the timestamp of lock directory"); - } - if (lockFileTs < currentTime - this.cacheFileLockExpirationInMilliseconds) { + } else if (lockFileTs < currentTime - this.cacheFileLockExpirationInMilliseconds) { // old lock file - if (!cacheLockFile.delete()) { - logger.debug("Failed to delete the directory. Dir: {}", cacheLockFile); + try { + if (!cacheLockFile.delete()) { + logger.debug("Failed to delete the directory. Dir: {}", cacheLockFile); + } else { + logger.debug("Deleted expired cache lock directory.", false); + } + } catch (Exception e) { + logger.debug("Failed to delete the directory. Dir: {}, Error: {}", cacheLockFile, e.getMessage()); } - logger.debug("Deleted expired cache lock directory.", false); } } From e1fe4330674be0a7b2d7ef987678335dbf1d4cce Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Mon, 10 Feb 2025 16:04:10 +0100 Subject: [PATCH 08/21] Synchronized FileCacheManager class members --- .../client/core/FileCacheManager.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index bd5faa144..4f0793ea7 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -85,7 +85,7 @@ FileCacheManager setOnlyOwnerPermissions(boolean onlyOwnerPermissions) { return this; } - String getCacheFilePath() { + synchronized String getCacheFilePath() { return cacheFile.getAbsolutePath(); } @@ -94,7 +94,7 @@ String getCacheFilePath() { * * @param newCacheFile a file object to override the default one. */ - void overrideCacheFile(File newCacheFile) { + synchronized void overrideCacheFile(File newCacheFile) { if (!newCacheFile.exists()) { logger.debug("Cache file doesn't exists. File: {}", newCacheFile); } @@ -110,7 +110,7 @@ void overrideCacheFile(File newCacheFile) { this.baseCacheFileName = newCacheFile.getName(); } - FileCacheManager build() { + synchronized FileCacheManager build() { // try to get cacheDir from system property or environment variable String cacheDirPath = this.cacheDirectorySystemProperty != null @@ -257,7 +257,7 @@ synchronized T withLock(Supplier supplier) { } /** Reads the cache file. */ - JsonNode readCacheFile() { + synchronized JsonNode readCacheFile() { try { if (!cacheFile.exists()) { logger.debug("Cache file doesn't exists. File: {}", cacheFile); @@ -282,7 +282,7 @@ JsonNode readCacheFile() { return null; } - void writeCacheFile(JsonNode input) { + synchronized void writeCacheFile(JsonNode input) { logger.debug("Writing cache file. File: {}", cacheFile); try { if (input == null) { @@ -303,7 +303,7 @@ void writeCacheFile(JsonNode input) { } } - void deleteCacheFile() { + synchronized void deleteCacheFile() { logger.debug("Deleting cache file. File: {}, lock file: {}", cacheFile, cacheLockFile); if (cacheFile == null) { @@ -321,12 +321,12 @@ void deleteCacheFile() { * * @return true if success or false */ - private boolean tryToLockCacheFile() { + private synchronized boolean tryToLockCacheFile() { int cnt = 0; boolean locked = false; while (cnt < 5 && !(locked = lockCacheFile())) { try { - Thread.sleep(100); + Thread.sleep(10); } catch (InterruptedException ex) { // doesn't matter } @@ -341,7 +341,7 @@ private boolean tryToLockCacheFile() { return locked; } - private void deleteCacheLockIfExpired() { + private synchronized void deleteCacheLockIfExpired() { long currentTime = new Date().getTime(); long lockFileTs = fileCreationTime(cacheLockFile); if (lockFileTs < 0) { @@ -365,7 +365,7 @@ private void deleteCacheLockIfExpired() { * * @return epoch time in ms */ - private static long fileCreationTime(File targetFile) { + private synchronized static long fileCreationTime(File targetFile) { if (!targetFile.exists()) { logger.debug("File not exists. File: {}", targetFile); return -1; @@ -385,7 +385,7 @@ private static long fileCreationTime(File targetFile) { * * @return true if success or false */ - private boolean lockCacheFile() { + private synchronized boolean lockCacheFile() { return cacheLockFile.mkdirs(); } @@ -394,7 +394,7 @@ private boolean lockCacheFile() { * * @return true if success or false */ - private boolean unlockCacheFile() { + private synchronized boolean unlockCacheFile() { return cacheLockFile.delete(); } } From f86f4e07b0c4c480d35bf7d9796e82d5dd21947e Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 13:41:36 +0100 Subject: [PATCH 09/21] Changed test cache directory --- .../java/net/snowflake/client/core/FileCacheManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java index b6fcccaf7..dd82db988 100644 --- a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java +++ b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java @@ -222,7 +222,7 @@ public void shouldReturnNullWhenNoHomeDirSet() { private File createCacheFile() { Path cacheFile = - Paths.get(systemGetProperty("user.home"), ".cache", "snowflake2", CACHE_FILE_NAME); + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake3", CACHE_FILE_NAME); try { if (Files.exists(cacheFile)) { Files.delete(cacheFile); From 646b2a813908ddbe8577e2bf74276d4d71bd321e Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 13:46:15 +0100 Subject: [PATCH 10/21] Remove dir if exists in tests --- .../java/net/snowflake/client/core/FileCacheManagerTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java index dd82db988..b34203039 100644 --- a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java +++ b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java @@ -227,6 +227,9 @@ private File createCacheFile() { if (Files.exists(cacheFile)) { Files.delete(cacheFile); } + if (Files.exists(cacheFile.getParent())) { + Files.delete(cacheFile.getParent()); + } Files.createDirectories(cacheFile.getParent(), PosixFilePermissions.asFileAttribute( Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE) From 65efc99adfa95c2280df8ae2db26363e18f57b4f Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 13:56:55 +0100 Subject: [PATCH 11/21] Fixed parent dir check when overriding cache dir --- src/main/java/net/snowflake/client/core/FileCacheManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 4f0793ea7..567157bfa 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -101,7 +101,7 @@ synchronized void overrideCacheFile(File newCacheFile) { if (onlyOwnerPermissions) { FileUtil.throwWhenFilePermissionsWiderThanUserOnly( newCacheFile, "Override cache file"); - FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Override cache file"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(newCacheFile, "Override cache file"); } else { FileUtil.logFileUsage(cacheFile, "Override cache file", false); } From 9c5a1cd0c7f48c84bc9b53d4a8f9255f1418cd9a Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 15:09:03 +0100 Subject: [PATCH 12/21] Fix tests --- .../client/core/FileCacheManager.java | 51 ++-- .../net/snowflake/client/core/FileUtil.java | 13 +- .../net/snowflake/client/core/HexUtil.java | 30 +- .../snowflake/client/core/SFTrustManager.java | 4 +- .../core/SecureStorageLinuxManager.java | 8 +- .../OAuthAccessTokenProviderFactory.java | 3 +- .../client/core/FileCacheManagerTest.java | 69 +++-- .../client/core/SecureStorageManagerTest.java | 25 +- .../client/core/SnowflakeMFACacheTest.java | 259 ++++++++++-------- .../OAuthAccessTokenProviderFactoryTest.java | 6 +- .../client/jdbc/SSOConnectionTest.java | 77 +++--- 11 files changed, 319 insertions(+), 226 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 567157bfa..c8cc36089 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -99,9 +99,9 @@ synchronized void overrideCacheFile(File newCacheFile) { logger.debug("Cache file doesn't exists. File: {}", newCacheFile); } if (onlyOwnerPermissions) { - FileUtil.throwWhenFilePermissionsWiderThanUserOnly( + FileUtil.throwWhenFilePermissionsWiderThanUserOnly(newCacheFile, "Override cache file"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly( newCacheFile, "Override cache file"); - FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(newCacheFile, "Override cache file"); } else { FileUtil.logFileUsage(cacheFile, "Override cache file", false); } @@ -140,21 +140,26 @@ synchronized FileCacheManager build() { return this; } if (!cacheDir.exists()) { - try { - Files.createDirectories(cacheDir.toPath(), - PosixFilePermissions.asFileAttribute( - Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE) - .collect(Collectors.toSet()))); - } catch (IOException e) { - logger.info( - "Failed to create the cache directory: {}. Ignored. {}", - e.getMessage(), - cacheDir.getAbsoluteFile()); - return this; - } + try { + Files.createDirectories( + cacheDir.toPath(), + PosixFilePermissions.asFileAttribute( + Stream.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE) + .collect(Collectors.toSet()))); + } catch (IOException e) { + logger.info( + "Failed to create the cache directory: {}. Ignored. {}", + e.getMessage(), + cacheDir.getAbsoluteFile()); + return this; + } } if (!this.cacheDir.exists()) { - logger.debug("Cannot create the cache directory {}. Giving up.", this.cacheDir.getAbsolutePath()); + logger.debug( + "Cannot create the cache directory {}. Giving up.", this.cacheDir.getAbsolutePath()); return this; } logger.debug("Verified Directory {}", this.cacheDir.getAbsolutePath()); @@ -207,7 +212,7 @@ static File getDefaultCacheDir() { } if (Constants.getOS() == Constants.OS.WINDOWS) { return new File( - new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches"); + new File(new File(new File(homeDir, "AppData"), "Local"), "Snowflake"), "Caches"); } else if (Constants.getOS() == Constants.OS.MAC) { return new File(new File(new File(homeDir, "Library"), "Caches"), "Snowflake"); } else { @@ -225,9 +230,9 @@ private static String getXdgCacheHome() { private static String getHomeDirProperty() { String homeDir = systemGetProperty("user.home"); - if (homeDir != null && isWritable(homeDir)) { - return homeDir; - } + if (homeDir != null && isWritable(homeDir)) { + return homeDir; + } return null; } @@ -292,7 +297,8 @@ synchronized void writeCacheFile(JsonNode input) { new OutputStreamWriter(new FileOutputStream(cacheFile), DEFAULT_FILE_ENCODING)) { if (onlyOwnerPermissions) { FileUtil.throwWhenFilePermissionsWiderThanUserOnly(cacheFile, "Write to cache"); - FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly(cacheFile, "Write to cache"); + FileUtil.throwWhenParentDirectoryPermissionsWiderThanUserOnly( + cacheFile, "Write to cache"); } else { FileUtil.logFileUsage(cacheFile, "Write to cache", false); } @@ -355,7 +361,8 @@ private synchronized void deleteCacheLockIfExpired() { logger.debug("Deleted expired cache lock directory.", false); } } catch (Exception e) { - logger.debug("Failed to delete the directory. Dir: {}, Error: {}", cacheLockFile, e.getMessage()); + logger.debug( + "Failed to delete the directory. Dir: {}, Error: {}", cacheLockFile, e.getMessage()); } } } @@ -365,7 +372,7 @@ private synchronized void deleteCacheLockIfExpired() { * * @return epoch time in ms */ - private synchronized static long fileCreationTime(File targetFile) { + private static synchronized long fileCreationTime(File targetFile) { if (!targetFile.exists()) { logger.debug("File not exists. File: {}", targetFile); return -1; diff --git a/src/main/java/net/snowflake/client/core/FileUtil.java b/src/main/java/net/snowflake/client/core/FileUtil.java index 4a775c24d..a383b69ce 100644 --- a/src/main/java/net/snowflake/client/core/FileUtil.java +++ b/src/main/java/net/snowflake/client/core/FileUtil.java @@ -50,7 +50,8 @@ public static boolean isWritable(String path) { return true; } - public static void throwWhenParentDirectoryPermissionsWiderThanUserOnly(File file, String context) { + public static void throwWhenParentDirectoryPermissionsWiderThanUserOnly( + File file, String context) { throwWhenDirectoryPermissionsWiderThanUserOnly(file.getParentFile(), context); } @@ -77,13 +78,16 @@ public static void throwWhenPermissionsWiderThanUserOnly( boolean permissionsTooOpen; if (isDirectory) { - permissionsTooOpen = isWritableByOthers || isReadableByOthers; + permissionsTooOpen = isWritableByOthers || isReadableByOthers; } else { permissionsTooOpen = isWritableByOthers || isReadableByOthers || isExecutable; } if (permissionsTooOpen) { logger.debug( - "{}File/directory {} access rights: {}", getContextStr(context), filePath, filePermissions); + "{}File/directory {} access rights: {}", + getContextStr(context), + filePath, + filePermissions); throw new SecurityException( String.format( "Access to file or directory %s is wider than allowed only to the owner. Remove cached file/directory and re-run the driver.", @@ -92,7 +96,8 @@ public static void throwWhenPermissionsWiderThanUserOnly( } catch (IOException e) { throw new SecurityException( String.format( - "%s Unable to access the file/directory to check the permissions. Error: %s", filePath, e)); + "%s Unable to access the file/directory to check the permissions. Error: %s", + filePath, e)); } } diff --git a/src/main/java/net/snowflake/client/core/HexUtil.java b/src/main/java/net/snowflake/client/core/HexUtil.java index 49948700f..1200af0e6 100644 --- a/src/main/java/net/snowflake/client/core/HexUtil.java +++ b/src/main/java/net/snowflake/client/core/HexUtil.java @@ -6,20 +6,20 @@ class HexUtil { - /** - * Converts Byte array to hex string - * - * @param bytes a byte array - * @return a string in hexadecimal code - */ - static String byteToHexString(byte[] bytes) { - final char[] hexArray = "0123456789ABCDEF".toCharArray(); - char[] hexChars = new char[bytes.length * 2]; - for (int j = 0; j < bytes.length; j++) { - int v = bytes[j] & 0xFF; - hexChars[j * 2] = hexArray[v >>> 4]; - hexChars[j * 2 + 1] = hexArray[v & 0x0F]; - } - return new String(hexChars); + /** + * Converts Byte array to hex string + * + * @param bytes a byte array + * @return a string in hexadecimal code + */ + static String byteToHexString(byte[] bytes) { + final char[] hexArray = "0123456789ABCDEF".toCharArray(); + char[] hexChars = new char[bytes.length * 2]; + for (int j = 0; j < bytes.length; j++) { + int v = bytes[j] & 0xFF; + hexChars[j * 2] = hexArray[v >>> 4]; + hexChars[j * 2 + 1] = hexArray[v & 0x0F]; } + return new String(hexChars); + } } diff --git a/src/main/java/net/snowflake/client/core/SFTrustManager.java b/src/main/java/net/snowflake/client/core/SFTrustManager.java index 777b842a2..ae7e82f2d 100644 --- a/src/main/java/net/snowflake/client/core/SFTrustManager.java +++ b/src/main/java/net/snowflake/client/core/SFTrustManager.java @@ -1592,7 +1592,9 @@ public boolean equals(Object obj) { public String toString() { return String.format( "OcspResponseCacheKey: NameHash: %s, KeyHash: %s, SerialNumber: %s", - HexUtil.byteToHexString(nameHash), HexUtil.byteToHexString(keyHash), serialNumber.toString()); + HexUtil.byteToHexString(nameHash), + HexUtil.byteToHexString(keyHash), + serialNumber.toString()); } } diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index d39b29f99..aa2cad669 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -129,10 +129,10 @@ private Map> readJsonStoreCache(JsonNode node) { JsonNode credentialsNode = node.get(CACHE_FILE_TOKENS_OBJECT_NAME); Map credentialsCache = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); if (credentialsNode != null && node.getNodeType().equals(JsonNodeType.OBJECT)) { - for (Iterator> itr = credentialsNode.fields(); itr.hasNext(); ) { - Map.Entry credential = itr.next(); - credentialsCache.put(credential.getKey(), credential.getValue().asText()); - } + for (Iterator> itr = credentialsNode.fields(); itr.hasNext(); ) { + Map.Entry credential = itr.next(); + credentialsCache.put(credential.getKey(), credential.getValue().asText()); + } } return cache; } diff --git a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactory.java b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactory.java index 0872fcd74..c232717be 100644 --- a/src/main/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactory.java +++ b/src/main/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactory.java @@ -119,6 +119,7 @@ private void assertContainsClientCredentials( AssertUtil.assertTrue( loginInput.getOauthLoginInput().getClientSecret() != null, String.format( - "passing oauthClientSecret is required for %s authentication", authenticatorType.name())); + "passing oauthClientSecret is required for %s authentication", + authenticatorType.name())); } } diff --git a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java index b34203039..ab1fb676c 100644 --- a/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java +++ b/src/test/java/net/snowflake/client/core/FileCacheManagerTest.java @@ -40,7 +40,7 @@ @Tag(TestTags.CORE) class FileCacheManagerTest extends BaseJDBCTest { - private static final String CACHE_FILE_NAME = "temporary_credential.json"; + private static final String CACHE_FILE_NAME = "credential_cache_v1.json.json"; private static final String CACHE_DIR_PROP = "net.snowflake.jdbc.temporaryCredentialCacheDir"; private static final String CACHE_DIR_ENV = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR"; private static final long CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS = 60L; @@ -97,7 +97,9 @@ public void throwWhenReadCacheFileWithPermissionDifferentThanReadWriteForUserTes String permission, String parentDirectoryPermissions, boolean isSucceed) throws IOException { fileCacheManager.overrideCacheFile(cacheFile); Files.setPosixFilePermissions(cacheFile.toPath(), PosixFilePermissions.fromString(permission)); - Files.setPosixFilePermissions(cacheFile.getParentFile().toPath(), PosixFilePermissions.fromString(parentDirectoryPermissions)); + Files.setPosixFilePermissions( + cacheFile.getParentFile().toPath(), + PosixFilePermissions.fromString(parentDirectoryPermissions)); if (isSucceed) { assertDoesNotThrow(() -> fileCacheManager.readCacheFile()); } else { @@ -146,8 +148,11 @@ public void throwWhenOverrideCacheFileNotFound() { public void shouldCreateCacheDirForLinuxXDG() { try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); - try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn("/XDG/Cache/"); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")) + .thenReturn("/XDG/Cache/"); try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { fileUtilMockedStatic.when(() -> FileUtil.isWritable("/XDG/Cache/")).thenReturn(true); File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); @@ -162,9 +167,14 @@ public void shouldCreateCacheDirForLinuxXDG() { public void shouldCreateCacheDirForLinuxWithoutXDG() { try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); - try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn(null); - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")) + .thenReturn(null); + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetProperty("user.home")) + .thenReturn("/User/Home"); try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); @@ -179,13 +189,17 @@ public void shouldCreateCacheDirForLinuxWithoutXDG() { public void shouldCreateCacheDirForWindows() { try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.WINDOWS); - try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetProperty("user.home")) + .thenReturn("/User/Home"); try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); Assertions.assertNotNull(defaultCacheDir); - Assertions.assertEquals("/User/Home/AppData/Local/Snowflake/Caches", defaultCacheDir.getAbsolutePath()); + Assertions.assertEquals( + "/User/Home/AppData/Local/Snowflake/Caches", defaultCacheDir.getAbsolutePath()); } } } @@ -195,13 +209,17 @@ public void shouldCreateCacheDirForWindows() { public void shouldCreateCacheDirForMacOS() { try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.MAC); - try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn("/User/Home"); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetProperty("user.home")) + .thenReturn("/User/Home"); try (MockedStatic fileUtilMockedStatic = Mockito.mockStatic(FileUtil.class)) { fileUtilMockedStatic.when(() -> FileUtil.isWritable("/User/Home")).thenReturn(true); File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); Assertions.assertNotNull(defaultCacheDir); - Assertions.assertEquals("/User/Home/Library/Caches/Snowflake", defaultCacheDir.getAbsolutePath()); + Assertions.assertEquals( + "/User/Home/Library/Caches/Snowflake", defaultCacheDir.getAbsolutePath()); } } } @@ -211,9 +229,14 @@ public void shouldCreateCacheDirForMacOS() { public void shouldReturnNullWhenNoHomeDirSet() { try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); - try (MockedStatic snowflakeUtilMockedStatic = Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")).thenReturn(null); - snowflakeUtilMockedStatic.when(() -> SnowflakeUtil.systemGetProperty("user.home")).thenReturn(null); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetEnv("XDG_CACHE_HOME")) + .thenReturn(null); + snowflakeUtilMockedStatic + .when(() -> SnowflakeUtil.systemGetProperty("user.home")) + .thenReturn(null); File defaultCacheDir = FileCacheManager.getDefaultCacheDir(); Assertions.assertNull(defaultCacheDir); } @@ -222,7 +245,7 @@ public void shouldReturnNullWhenNoHomeDirSet() { private File createCacheFile() { Path cacheFile = - Paths.get(systemGetProperty("user.home"), ".cache", "snowflake3", CACHE_FILE_NAME); + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_cache", CACHE_FILE_NAME); try { if (Files.exists(cacheFile)) { Files.delete(cacheFile); @@ -230,10 +253,14 @@ private File createCacheFile() { if (Files.exists(cacheFile.getParent())) { Files.delete(cacheFile.getParent()); } - Files.createDirectories(cacheFile.getParent(), - PosixFilePermissions.asFileAttribute( - Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.OWNER_EXECUTE) - .collect(Collectors.toSet()))); + Files.createDirectories( + cacheFile.getParent(), + PosixFilePermissions.asFileAttribute( + Stream.of( + PosixFilePermission.OWNER_READ, + PosixFilePermission.OWNER_WRITE, + PosixFilePermission.OWNER_EXECUTE) + .collect(Collectors.toSet()))); Files.createFile( cacheFile, PosixFilePermissions.asFileAttribute( diff --git a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java index 5e6181fe5..c135cb6a7 100644 --- a/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java +++ b/src/test/java/net/snowflake/client/core/SecureStorageManagerTest.java @@ -4,6 +4,7 @@ package net.snowflake.client.core; +import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.notNullValue; @@ -13,6 +14,7 @@ import com.sun.jna.Memory; import com.sun.jna.Pointer; import com.sun.jna.ptr.PointerByReference; +import java.nio.file.Paths; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -20,8 +22,11 @@ import net.snowflake.client.annotations.RunOnMac; import net.snowflake.client.annotations.RunOnWindows; import net.snowflake.client.annotations.RunOnWindowsOrMac; +import net.snowflake.client.jdbc.SnowflakeUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; class MockAdvapi32Lib implements SecureStorageWindowsManager.Advapi32Lib { @Override @@ -289,10 +294,22 @@ public void testMacManager() { @Test @RunOnLinux public void testLinuxManager() { - SecureStorageManager manager = SecureStorageLinuxManager.getInstance(); - - testBody(manager); - testDeleteLinux(manager); + String cacheDirectory = + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") + .toAbsolutePath() + .toString(); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when( + () -> + SnowflakeUtil.systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) + .thenReturn(cacheDirectory); + SecureStorageManager manager = SecureStorageLinuxManager.getInstance(); + + testBody(manager); + testDeleteLinux(manager); + } } private void testBody(SecureStorageManager manager) { diff --git a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java index 88451aa0e..5bc04c30e 100644 --- a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java +++ b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java @@ -4,6 +4,7 @@ package net.snowflake.client.core; +import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -19,6 +20,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.StringWriter; +import java.nio.file.Paths; import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -28,6 +30,7 @@ import net.snowflake.client.AbstractDriverIT; import net.snowflake.client.jdbc.SnowflakeBasicDataSource; import net.snowflake.client.jdbc.SnowflakeSQLException; +import net.snowflake.client.jdbc.SnowflakeUtil; import org.apache.commons.io.IOUtils; import org.apache.http.client.methods.HttpPost; import org.junit.jupiter.api.Disabled; @@ -100,131 +103,147 @@ private Properties getBaseProp() { @Test public void testMFAFunctionality() throws SQLException { - SessionUtil.deleteMfaTokenCache(host, user); - try (MockedStatic mockedHttpUtil = Mockito.mockStatic(HttpUtil.class)) { - mockedHttpUtil + String cacheDirectory = + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") + .toAbsolutePath() + .toString(); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic .when( () -> - HttpUtil.executeGeneralRequest( - any(HttpPost.class), - anyInt(), - anyInt(), - anyInt(), - anyInt(), - any(HttpClientSettingsKey.class))) - .thenAnswer( - new Answer() { - int callCount = 0; - - @Override - public String answer(InvocationOnMock invocation) throws Throwable { - String res; - JsonNode jsonNode; - final Object[] args = invocation.getArguments(); - - if (callCount == 0) { - // First connection request - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - // return first mfa token - res = getNormalMockedHttpResponse(true, 0).toString(); - } else if (callCount == 1) { - // First close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 2) { - // Second connection request - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - assertEquals(mockedMfaToken[0], jsonNode.path("data").path("TOKEN").asText()); - // Normally backend won't send a new mfa token in this case. For testing - // purpose, we issue a new token to test whether the mfa token can be refreshed - // when receiving a new one from server. - res = getNormalMockedHttpResponse(true, 1).toString(); - } else if (callCount == 3) { - // Second close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 4) { - // Third connection request - // Check for the new mfa token - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - assertEquals(mockedMfaToken[1], jsonNode.path("data").path("TOKEN").asText()); - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 5) { - // Third close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 6) { - // test if failed log in response can delete the cached mfa token - res = getNormalMockedHttpResponse(false, -1).toString(); - } else if (callCount == 7) { - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - // no token should be included this time. - assertEquals("", jsonNode.path("data").path("TOKEN").asText()); - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 8) { - // final close() - res = getNormalMockedHttpResponse(true, -1).toString(); - } else { - // unexpected request - res = getNormalMockedHttpResponse(false, -1).toString(); + SnowflakeUtil.systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) + .thenReturn(cacheDirectory); + snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); + SessionUtil.deleteMfaTokenCache(host, user); + try (MockedStatic mockedHttpUtil = Mockito.mockStatic(HttpUtil.class)) { + mockedHttpUtil + .when( + () -> + HttpUtil.executeGeneralRequest( + any(HttpPost.class), + anyInt(), + anyInt(), + anyInt(), + anyInt(), + any(HttpClientSettingsKey.class))) + .thenAnswer( + new Answer() { + int callCount = 0; + + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + String res; + JsonNode jsonNode; + final Object[] args = invocation.getArguments(); + + if (callCount == 0) { + // First connection request + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + // return first mfa token + res = getNormalMockedHttpResponse(true, 0).toString(); + } else if (callCount == 1) { + // First close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 2) { + // Second connection request + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + assertEquals(mockedMfaToken[0], jsonNode.path("data").path("TOKEN").asText()); + // Normally backend won't send a new mfa token in this case. For testing + // purpose, we issue a new token to test whether the mfa token can be + // refreshed + // when receiving a new one from server. + res = getNormalMockedHttpResponse(true, 1).toString(); + } else if (callCount == 3) { + // Second close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 4) { + // Third connection request + // Check for the new mfa token + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + assertEquals(mockedMfaToken[1], jsonNode.path("data").path("TOKEN").asText()); + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 5) { + // Third close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 6) { + // test if failed log in response can delete the cached mfa token + res = getNormalMockedHttpResponse(false, -1).toString(); + } else if (callCount == 7) { + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + // no token should be included this time. + assertEquals("", jsonNode.path("data").path("TOKEN").asText()); + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 8) { + // final close() + res = getNormalMockedHttpResponse(true, -1).toString(); + } else { + // unexpected request + res = getNormalMockedHttpResponse(false, -1).toString(); + } + + callCount += 1; // this will be incremented on both connecting and closing + return res; } - - callCount += 1; // this will be incremented on both connecting and closing - return res; - } - }); - - Properties prop = getBaseProp(); - - // connect url - String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; - - // The first connection contains no mfa token. After the connection, a mfa token will be saved - Connection con = DriverManager.getConnection(url, prop); - con.close(); - - // The second connection is expected to include the mfa token issued for the first connection - // and a new mfa token is issued - Connection con1 = DriverManager.getConnection(url, prop); - con1.close(); - - // The third connection is expected to include the new mfa token. - Connection con2 = DriverManager.getConnection(url, prop); - con2.close(); - - // This connection would receive an exception and then should clean up the mfa cache - try { - Connection con3 = DriverManager.getConnection(url, prop); - fail(); - } catch (SnowflakeSQLException ex) { - // An exception is forced to happen by mocking. Do nothing. + }); + + Properties prop = getBaseProp(); + + // connect url + String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; + + // The first connection contains no mfa token. After the connection, a mfa token will be + // saved + Connection con = DriverManager.getConnection(url, prop); + con.close(); + + // The second connection is expected to include the mfa token issued for the first + // connection + // and a new mfa token is issued + Connection con1 = DriverManager.getConnection(url, prop); + con1.close(); + + // The third connection is expected to include the new mfa token. + Connection con2 = DriverManager.getConnection(url, prop); + con2.close(); + + // This connection would receive an exception and then should clean up the mfa cache + try { + Connection con3 = DriverManager.getConnection(url, prop); + fail(); + } catch (SnowflakeSQLException ex) { + // An exception is forced to happen by mocking. Do nothing. + } + // This connect request should not contain mfa cached token + Connection con4 = DriverManager.getConnection(url, prop); + con4.close(); } - // This connect request should not contain mfa cached token - Connection con4 = DriverManager.getConnection(url, prop); - con4.close(); + SessionUtil.deleteMfaTokenCache(host, user); } - SessionUtil.deleteMfaTokenCache(host, user); } class MockUnavailableAdvapi32Lib implements SecureStorageWindowsManager.Advapi32Lib { diff --git a/src/test/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactoryTest.java b/src/test/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactoryTest.java index 0a1c74d9a..840f0e3d1 100644 --- a/src/test/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactoryTest.java +++ b/src/test/java/net/snowflake/client/core/auth/oauth/OAuthAccessTokenProviderFactoryTest.java @@ -84,7 +84,8 @@ public void shouldFailToCreateClientCredentialsAccessTokenProviderWithoutClientI AuthenticatorType.OAUTH_CLIENT_CREDENTIALS, loginInput)); Assertions.assertTrue( e.getMessage() - .contains("passing oauthClientId is required for OAUTH_CLIENT_CREDENTIALS authentication.")); + .contains( + "passing oauthClientId is required for OAUTH_CLIENT_CREDENTIALS authentication.")); } @Test @@ -138,7 +139,8 @@ public void shouldFailToCreateAuthzCodeAccessTokenProviderWithoutClientId() { AuthenticatorType.OAUTH_AUTHORIZATION_CODE, loginInput)); Assertions.assertTrue( e.getMessage() - .contains("passing oauthClientId is required for OAUTH_AUTHORIZATION_CODE authentication.")); + .contains( + "passing oauthClientId is required for OAUTH_AUTHORIZATION_CODE authentication.")); } @Test diff --git a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java index 71618c9e9..14efce6a7 100644 --- a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java +++ b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java @@ -4,6 +4,7 @@ package net.snowflake.client.jdbc; +import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -25,6 +26,7 @@ import java.net.Socket; import java.net.URI; import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; import java.sql.Connection; import java.sql.DriverManager; import java.util.ArrayList; @@ -306,38 +308,49 @@ private SFLoginInput initMockLoginInput() { @Test public void testIdTokenInSSO() throws Throwable { - try (MockedStatic mockedHttpUtil = mockStatic(HttpUtil.class); - MockedStatic mockedSessionUtilExternalBrowser = - mockStatic(SessionUtilExternalBrowser.class)) { - - initMock(mockedHttpUtil, mockedSessionUtilExternalBrowser); - SessionUtil.deleteIdTokenCache("testaccount.snowflakecomputing.com", "testuser"); - - Properties properties = new Properties(); - properties.put("user", "testuser"); - properties.put("password", "testpassword"); - properties.put("account", "testaccount"); - properties.put("insecureMode", true); - properties.put("authenticator", "externalbrowser"); - properties.put("CLIENT_STORE_TEMPORARY_CREDENTIAL", true); - - // connect url - String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; - - // initial connection getting id token and storing in the cache file. - Connection con = DriverManager.getConnection(url, properties); - SnowflakeConnectionV1 sfcon = (SnowflakeConnectionV1) con; - assertThat("token", sfcon.getSfSession().getSessionToken(), equalTo(MOCK_SESSION_TOKEN)); - assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); - - // second connection reads the cache and use the id token to get the - // session token. - Connection conSecond = DriverManager.getConnection(url, properties); - SnowflakeConnectionV1 sfconSecond = (SnowflakeConnectionV1) conSecond; - assertThat( - "token", sfconSecond.getSfSession().getSessionToken(), equalTo(MOCK_NEW_SESSION_TOKEN)); - // we won't get a new id_token here - assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + String cacheDirectory = + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") + .toAbsolutePath() + .toString(); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) + .thenReturn(cacheDirectory); + snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); + try (MockedStatic mockedHttpUtil = mockStatic(HttpUtil.class); + MockedStatic mockedSessionUtilExternalBrowser = + mockStatic(SessionUtilExternalBrowser.class)) { + + initMock(mockedHttpUtil, mockedSessionUtilExternalBrowser); + SessionUtil.deleteIdTokenCache("testaccount.snowflakecomputing.com", "testuser"); + + Properties properties = new Properties(); + properties.put("user", "testuser"); + properties.put("password", "testpassword"); + properties.put("account", "testaccount"); + properties.put("insecureMode", true); + properties.put("authenticator", "externalbrowser"); + properties.put("CLIENT_STORE_TEMPORARY_CREDENTIAL", true); + + // connect url + String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; + + // initial connection getting id token and storing in the cache file. + Connection con = DriverManager.getConnection(url, properties); + SnowflakeConnectionV1 sfcon = (SnowflakeConnectionV1) con; + assertThat("token", sfcon.getSfSession().getSessionToken(), equalTo(MOCK_SESSION_TOKEN)); + assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + + // second connection reads the cache and use the id token to get the + // session token. + Connection conSecond = DriverManager.getConnection(url, properties); + SnowflakeConnectionV1 sfconSecond = (SnowflakeConnectionV1) conSecond; + assertThat( + "token", sfconSecond.getSfSession().getSessionToken(), equalTo(MOCK_NEW_SESSION_TOKEN)); + // we won't get a new id_token here + assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + } } } } From ec0c86360874183a930491cbd3fcf0dbe3028e5e Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 16:22:57 +0100 Subject: [PATCH 13/21] Fix sso tests --- .../client/core/SnowflakeMFACacheTest.java | 276 +++++++++--------- .../client/jdbc/SSOConnectionTest.java | 90 +++--- 2 files changed, 189 insertions(+), 177 deletions(-) diff --git a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java index 5bc04c30e..8a2a10aaa 100644 --- a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java +++ b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java @@ -103,146 +103,152 @@ private Properties getBaseProp() { @Test public void testMFAFunctionality() throws SQLException { - String cacheDirectory = - Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") - .toAbsolutePath() - .toString(); - try (MockedStatic snowflakeUtilMockedStatic = - Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic - .when( - () -> - SnowflakeUtil.systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) - .thenReturn(cacheDirectory); - snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); - SessionUtil.deleteMfaTokenCache(host, user); - try (MockedStatic mockedHttpUtil = Mockito.mockStatic(HttpUtil.class)) { - mockedHttpUtil + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); + String cacheDirectory = + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") + .toAbsolutePath() + .toString(); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic .when( () -> - HttpUtil.executeGeneralRequest( - any(HttpPost.class), - anyInt(), - anyInt(), - anyInt(), - anyInt(), - any(HttpClientSettingsKey.class))) - .thenAnswer( - new Answer() { - int callCount = 0; - - @Override - public String answer(InvocationOnMock invocation) throws Throwable { - String res; - JsonNode jsonNode; - final Object[] args = invocation.getArguments(); - - if (callCount == 0) { - // First connection request - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - // return first mfa token - res = getNormalMockedHttpResponse(true, 0).toString(); - } else if (callCount == 1) { - // First close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 2) { - // Second connection request - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - assertEquals(mockedMfaToken[0], jsonNode.path("data").path("TOKEN").asText()); - // Normally backend won't send a new mfa token in this case. For testing - // purpose, we issue a new token to test whether the mfa token can be - // refreshed - // when receiving a new one from server. - res = getNormalMockedHttpResponse(true, 1).toString(); - } else if (callCount == 3) { - // Second close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 4) { - // Third connection request - // Check for the new mfa token - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - assertEquals(mockedMfaToken[1], jsonNode.path("data").path("TOKEN").asText()); - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 5) { - // Third close() request - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 6) { - // test if failed log in response can delete the cached mfa token - res = getNormalMockedHttpResponse(false, -1).toString(); - } else if (callCount == 7) { - jsonNode = parseRequest((HttpPost) args[0]); - assertTrue( - jsonNode - .path("data") - .path("SESSION_PARAMETERS") - .path("CLIENT_REQUEST_MFA_TOKEN") - .asBoolean()); - // no token should be included this time. - assertEquals("", jsonNode.path("data").path("TOKEN").asText()); - res = getNormalMockedHttpResponse(true, -1).toString(); - } else if (callCount == 8) { - // final close() - res = getNormalMockedHttpResponse(true, -1).toString(); - } else { - // unexpected request - res = getNormalMockedHttpResponse(false, -1).toString(); + SnowflakeUtil.systemGetProperty( + "net.snowflake.jdbc.temporaryCredentialCacheDir")) + .thenReturn(cacheDirectory); + snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); + SessionUtil.deleteMfaTokenCache(host, user); + try (MockedStatic mockedHttpUtil = Mockito.mockStatic(HttpUtil.class)) { + mockedHttpUtil + .when( + () -> + HttpUtil.executeGeneralRequest( + any(HttpPost.class), + anyInt(), + anyInt(), + anyInt(), + anyInt(), + any(HttpClientSettingsKey.class))) + .thenAnswer( + new Answer() { + int callCount = 0; + + @Override + public String answer(InvocationOnMock invocation) throws Throwable { + String res; + JsonNode jsonNode; + final Object[] args = invocation.getArguments(); + + if (callCount == 0) { + // First connection request + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + // return first mfa token + res = getNormalMockedHttpResponse(true, 0).toString(); + } else if (callCount == 1) { + // First close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 2) { + // Second connection request + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + assertEquals( + mockedMfaToken[0], jsonNode.path("data").path("TOKEN").asText()); + // Normally backend won't send a new mfa token in this case. For testing + // purpose, we issue a new token to test whether the mfa token can be + // refreshed + // when receiving a new one from server. + res = getNormalMockedHttpResponse(true, 1).toString(); + } else if (callCount == 3) { + // Second close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 4) { + // Third connection request + // Check for the new mfa token + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + assertEquals( + mockedMfaToken[1], jsonNode.path("data").path("TOKEN").asText()); + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 5) { + // Third close() request + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 6) { + // test if failed log in response can delete the cached mfa token + res = getNormalMockedHttpResponse(false, -1).toString(); + } else if (callCount == 7) { + jsonNode = parseRequest((HttpPost) args[0]); + assertTrue( + jsonNode + .path("data") + .path("SESSION_PARAMETERS") + .path("CLIENT_REQUEST_MFA_TOKEN") + .asBoolean()); + // no token should be included this time. + assertEquals("", jsonNode.path("data").path("TOKEN").asText()); + res = getNormalMockedHttpResponse(true, -1).toString(); + } else if (callCount == 8) { + // final close() + res = getNormalMockedHttpResponse(true, -1).toString(); + } else { + // unexpected request + res = getNormalMockedHttpResponse(false, -1).toString(); + } + + callCount += 1; // this will be incremented on both connecting and closing + return res; } - - callCount += 1; // this will be incremented on both connecting and closing - return res; - } - }); - - Properties prop = getBaseProp(); - - // connect url - String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; - - // The first connection contains no mfa token. After the connection, a mfa token will be - // saved - Connection con = DriverManager.getConnection(url, prop); - con.close(); - - // The second connection is expected to include the mfa token issued for the first - // connection - // and a new mfa token is issued - Connection con1 = DriverManager.getConnection(url, prop); - con1.close(); - - // The third connection is expected to include the new mfa token. - Connection con2 = DriverManager.getConnection(url, prop); - con2.close(); - - // This connection would receive an exception and then should clean up the mfa cache - try { - Connection con3 = DriverManager.getConnection(url, prop); - fail(); - } catch (SnowflakeSQLException ex) { - // An exception is forced to happen by mocking. Do nothing. + }); + + Properties prop = getBaseProp(); + + // connect url + String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; + + // The first connection contains no mfa token. After the connection, a mfa token will be + // saved + Connection con = DriverManager.getConnection(url, prop); + con.close(); + + // The second connection is expected to include the mfa token issued for the first + // connection + // and a new mfa token is issued + Connection con1 = DriverManager.getConnection(url, prop); + con1.close(); + + // The third connection is expected to include the new mfa token. + Connection con2 = DriverManager.getConnection(url, prop); + con2.close(); + + // This connection would receive an exception and then should clean up the mfa cache + try { + Connection con3 = DriverManager.getConnection(url, prop); + fail(); + } catch (SnowflakeSQLException ex) { + // An exception is forced to happen by mocking. Do nothing. + } + // This connect request should not contain mfa cached token + Connection con4 = DriverManager.getConnection(url, prop); + con4.close(); } - // This connect request should not contain mfa cached token - Connection con4 = DriverManager.getConnection(url, prop); - con4.close(); + SessionUtil.deleteMfaTokenCache(host, user); } - SessionUtil.deleteMfaTokenCache(host, user); } } diff --git a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java index 14efce6a7..524bc445f 100644 --- a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java +++ b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java @@ -31,6 +31,7 @@ import java.sql.DriverManager; import java.util.ArrayList; import java.util.Properties; +import net.snowflake.client.core.Constants; import net.snowflake.client.core.HttpClientSettingsKey; import net.snowflake.client.core.HttpUtil; import net.snowflake.client.core.SFException; @@ -308,48 +309,53 @@ private SFLoginInput initMockLoginInput() { @Test public void testIdTokenInSSO() throws Throwable { - String cacheDirectory = - Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") - .toAbsolutePath() - .toString(); - try (MockedStatic snowflakeUtilMockedStatic = - Mockito.mockStatic(SnowflakeUtil.class)) { - snowflakeUtilMockedStatic - .when(() -> systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) - .thenReturn(cacheDirectory); - snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); - try (MockedStatic mockedHttpUtil = mockStatic(HttpUtil.class); - MockedStatic mockedSessionUtilExternalBrowser = - mockStatic(SessionUtilExternalBrowser.class)) { - - initMock(mockedHttpUtil, mockedSessionUtilExternalBrowser); - SessionUtil.deleteIdTokenCache("testaccount.snowflakecomputing.com", "testuser"); - - Properties properties = new Properties(); - properties.put("user", "testuser"); - properties.put("password", "testpassword"); - properties.put("account", "testaccount"); - properties.put("insecureMode", true); - properties.put("authenticator", "externalbrowser"); - properties.put("CLIENT_STORE_TEMPORARY_CREDENTIAL", true); - - // connect url - String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; - - // initial connection getting id token and storing in the cache file. - Connection con = DriverManager.getConnection(url, properties); - SnowflakeConnectionV1 sfcon = (SnowflakeConnectionV1) con; - assertThat("token", sfcon.getSfSession().getSessionToken(), equalTo(MOCK_SESSION_TOKEN)); - assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); - - // second connection reads the cache and use the id token to get the - // session token. - Connection conSecond = DriverManager.getConnection(url, properties); - SnowflakeConnectionV1 sfconSecond = (SnowflakeConnectionV1) conSecond; - assertThat( - "token", sfconSecond.getSfSession().getSessionToken(), equalTo(MOCK_NEW_SESSION_TOKEN)); - // we won't get a new id_token here - assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + try (MockedStatic constantsMockedStatic = Mockito.mockStatic(Constants.class)) { + constantsMockedStatic.when(Constants::getOS).thenReturn(Constants.OS.LINUX); + String cacheDirectory = + Paths.get(systemGetProperty("user.home"), ".cache", "snowflake_test_cache") + .toAbsolutePath() + .toString(); + try (MockedStatic snowflakeUtilMockedStatic = + Mockito.mockStatic(SnowflakeUtil.class)) { + snowflakeUtilMockedStatic + .when(() -> systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) + .thenReturn(cacheDirectory); + snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); + try (MockedStatic mockedHttpUtil = mockStatic(HttpUtil.class); + MockedStatic mockedSessionUtilExternalBrowser = + mockStatic(SessionUtilExternalBrowser.class)) { + + initMock(mockedHttpUtil, mockedSessionUtilExternalBrowser); + SessionUtil.deleteIdTokenCache("testaccount.snowflakecomputing.com", "testuser"); + + Properties properties = new Properties(); + properties.put("user", "testuser"); + properties.put("password", "testpassword"); + properties.put("account", "testaccount"); + properties.put("insecureMode", true); + properties.put("authenticator", "externalbrowser"); + properties.put("CLIENT_STORE_TEMPORARY_CREDENTIAL", true); + + // connect url + String url = "jdbc:snowflake://testaccount.snowflakecomputing.com"; + + // initial connection getting id token and storing in the cache file. + Connection con = DriverManager.getConnection(url, properties); + SnowflakeConnectionV1 sfcon = (SnowflakeConnectionV1) con; + assertThat("token", sfcon.getSfSession().getSessionToken(), equalTo(MOCK_SESSION_TOKEN)); + assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + + // second connection reads the cache and use the id token to get the + // session token. + Connection conSecond = DriverManager.getConnection(url, properties); + SnowflakeConnectionV1 sfconSecond = (SnowflakeConnectionV1) conSecond; + assertThat( + "token", + sfconSecond.getSfSession().getSessionToken(), + equalTo(MOCK_NEW_SESSION_TOKEN)); + // we won't get a new id_token here + assertThat("idToken", sfcon.getSfSession().getIdToken(), equalTo(MOCK_ID_TOKEN)); + } } } } From b8feaa8076a0a65edaf9e6f33bdd07fd0ca2e87b Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 16:23:40 +0100 Subject: [PATCH 14/21] Fix sso tests --- src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java index 524bc445f..86a77eee0 100644 --- a/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java +++ b/src/test/java/net/snowflake/client/jdbc/SSOConnectionTest.java @@ -320,7 +320,6 @@ public void testIdTokenInSSO() throws Throwable { snowflakeUtilMockedStatic .when(() -> systemGetProperty("net.snowflake.jdbc.temporaryCredentialCacheDir")) .thenReturn(cacheDirectory); - snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); try (MockedStatic mockedHttpUtil = mockStatic(HttpUtil.class); MockedStatic mockedSessionUtilExternalBrowser = mockStatic(SessionUtilExternalBrowser.class)) { From 15cca904d31e8de6b37599eb828aab29de4af358 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Tue, 11 Feb 2025 18:31:40 +0100 Subject: [PATCH 15/21] Fix config file home directory getter --- .../client/config/SFClientConfigParser.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/snowflake/client/config/SFClientConfigParser.java b/src/main/java/net/snowflake/client/config/SFClientConfigParser.java index 9d93aed0a..7b05150d9 100644 --- a/src/main/java/net/snowflake/client/config/SFClientConfigParser.java +++ b/src/main/java/net/snowflake/client/config/SFClientConfigParser.java @@ -58,11 +58,13 @@ public static SFClientConfig loadSFClientConfig(String configFilePath) throws IO derivedConfigFilePath = driverLocation; } else { // 4. Read SF_CLIENT_CONFIG_FILE_NAME if it is present in user home directory. - String userHomeFilePath = - Paths.get(systemGetProperty("user.home"), SF_CLIENT_CONFIG_FILE_NAME).toString(); - if (Files.exists(Paths.get(userHomeFilePath))) { - logger.info("Using config file specified from home directory: {}", userHomeFilePath); - derivedConfigFilePath = userHomeFilePath; + String homeDirectory = systemGetProperty("user.home"); + if (homeDirectory != null) { + String userHomeFilePath = Paths.get(homeDirectory, SF_CLIENT_CONFIG_FILE_NAME).toString(); + if (Files.exists(Paths.get(userHomeFilePath))) { + logger.info("Using config file specified from home directory: {}", userHomeFilePath); + derivedConfigFilePath = userHomeFilePath; + } } } } From 457257f1754418016eb901c97fa69c7996efdd43 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Wed, 12 Feb 2025 14:46:41 +0100 Subject: [PATCH 16/21] Refactor localCacheTJson --- .../client/core/SecureStorageLinuxManager.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index aa2cad669..ed21ee8e6 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -108,13 +108,13 @@ public synchronized SecureStorageStatus deleteCredential(String host, String use private ObjectNode localCacheToJson(Map> cache) { ObjectNode res = mapper.createObjectNode(); for (Map.Entry> elem : cache.entrySet()) { - String elemHost = elem.getKey(); - Map hostMap = elem.getValue(); - ObjectNode hostNode = mapper.createObjectNode(); - for (Map.Entry elem0 : hostMap.entrySet()) { - hostNode.put(elem0.getKey(), elem0.getValue()); + String rootObject = elem.getKey(); + Map subMap = elem.getValue(); + ObjectNode subNode = mapper.createObjectNode(); + for (Map.Entry elem0 : subMap.entrySet()) { + subNode.put(elem0.getKey(), elem0.getValue()); } - res.set(elemHost, hostNode); + res.set(rootObject, subNode); } return res; } From 5ac2f92684453b90527636f585ccc7c50a6e3899 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Wed, 12 Feb 2025 14:59:41 +0100 Subject: [PATCH 17/21] Fix localCacheToJson --- .../client/core/SecureStorageLinuxManager.java | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index ed21ee8e6..f8ca963b8 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -106,17 +106,14 @@ public synchronized SecureStorageStatus deleteCredential(String host, String use } private ObjectNode localCacheToJson(Map> cache) { - ObjectNode res = mapper.createObjectNode(); - for (Map.Entry> elem : cache.entrySet()) { - String rootObject = elem.getKey(); - Map subMap = elem.getValue(); - ObjectNode subNode = mapper.createObjectNode(); - for (Map.Entry elem0 : subMap.entrySet()) { - subNode.put(elem0.getKey(), elem0.getValue()); - } - res.set(rootObject, subNode); + ObjectNode jsonNode = mapper.createObjectNode(); + Map tokensMap = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); + ObjectNode tokensNode = mapper.createObjectNode(); + for (Map.Entry credential : tokensMap.entrySet()) { + tokensNode.put(credential.getKey(), credential.getValue()); } - return res; + jsonNode.set(CACHE_FILE_TOKENS_OBJECT_NAME, tokensNode); + return jsonNode; } private Map> readJsonStoreCache(JsonNode node) { From 1cfcad5a5121d7b9798068c68d5ddcbfa8cf6d93 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 13 Feb 2025 09:53:31 +0100 Subject: [PATCH 18/21] Fix NPE --- .../snowflake/client/core/SecureStorageLinuxManager.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java index f8ca963b8..00a67f372 100644 --- a/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java +++ b/src/main/java/net/snowflake/client/core/SecureStorageLinuxManager.java @@ -108,11 +108,13 @@ public synchronized SecureStorageStatus deleteCredential(String host, String use private ObjectNode localCacheToJson(Map> cache) { ObjectNode jsonNode = mapper.createObjectNode(); Map tokensMap = cache.get(CACHE_FILE_TOKENS_OBJECT_NAME); - ObjectNode tokensNode = mapper.createObjectNode(); - for (Map.Entry credential : tokensMap.entrySet()) { + if (tokensMap != null) { + ObjectNode tokensNode = mapper.createObjectNode(); + for (Map.Entry credential : tokensMap.entrySet()) { tokensNode.put(credential.getKey(), credential.getValue()); + } + jsonNode.set(CACHE_FILE_TOKENS_OBJECT_NAME, tokensNode); } - jsonNode.set(CACHE_FILE_TOKENS_OBJECT_NAME, tokensNode); return jsonNode; } From e548ba2d4b3f2f84838d7b84a990eccbedcc5206 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 13 Feb 2025 12:13:54 +0100 Subject: [PATCH 19/21] Add logs --- .../java/net/snowflake/client/core/CredentialManager.java | 2 +- src/main/java/net/snowflake/client/core/FileCacheManager.java | 4 ++++ .../java/net/snowflake/client/core/SnowflakeMFACacheTest.java | 1 - 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/CredentialManager.java b/src/main/java/net/snowflake/client/core/CredentialManager.java index 3ca5069c0..bb5cc0588 100644 --- a/src/main/java/net/snowflake/client/core/CredentialManager.java +++ b/src/main/java/net/snowflake/client/core/CredentialManager.java @@ -23,7 +23,7 @@ private CredentialManager() { private void initSecureStorageManager() { try { if (Constants.getOS() == Constants.OS.MAC) { - secureStorageManager = SecureStorageAppleManager.builder(); + secureStorageManager = SecureStorageLinuxManager.getInstance(); } else if (Constants.getOS() == Constants.OS.WINDOWS) { secureStorageManager = SecureStorageWindowsManager.builder(); } else if (Constants.getOS() == Constants.OS.LINUX) { diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index c8cc36089..693ed8fdd 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -112,6 +112,8 @@ synchronized void overrideCacheFile(File newCacheFile) { synchronized FileCacheManager build() { // try to get cacheDir from system property or environment variable + logger.info("Cache file from property: ", systemGetProperty(this.cacheDirectorySystemProperty)); + logger.info("Cache file from env: ", systemGetEnv(this.cacheDirectoryEnvironmentVariable)); String cacheDirPath = this.cacheDirectorySystemProperty != null ? systemGetProperty(this.cacheDirectorySystemProperty) @@ -133,8 +135,10 @@ synchronized FileCacheManager build() { if (cacheDirPath != null) { this.cacheDir = new File(cacheDirPath); + logger.info("Using cache dir: ", cacheDirPath); } else { this.cacheDir = getDefaultCacheDir(); + logger.info("Using default cache dir : ", cacheDir.getAbsoluteFile()); } if (cacheDir == null) { return this; diff --git a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java index 8a2a10aaa..97d8d8258 100644 --- a/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java +++ b/src/test/java/net/snowflake/client/core/SnowflakeMFACacheTest.java @@ -117,7 +117,6 @@ public void testMFAFunctionality() throws SQLException { SnowflakeUtil.systemGetProperty( "net.snowflake.jdbc.temporaryCredentialCacheDir")) .thenReturn(cacheDirectory); - snowflakeUtilMockedStatic.when(() -> systemGetProperty("os.name")).thenReturn("linux"); SessionUtil.deleteMfaTokenCache(host, user); try (MockedStatic mockedHttpUtil = Mockito.mockStatic(HttpUtil.class)) { mockedHttpUtil From 8aa7b0bc8623eba3d63b04f5aa4beec590d6c3ff Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 13 Feb 2025 12:14:15 +0100 Subject: [PATCH 20/21] Add Logs --- src/main/java/net/snowflake/client/core/CredentialManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/snowflake/client/core/CredentialManager.java b/src/main/java/net/snowflake/client/core/CredentialManager.java index bb5cc0588..3ca5069c0 100644 --- a/src/main/java/net/snowflake/client/core/CredentialManager.java +++ b/src/main/java/net/snowflake/client/core/CredentialManager.java @@ -23,7 +23,7 @@ private CredentialManager() { private void initSecureStorageManager() { try { if (Constants.getOS() == Constants.OS.MAC) { - secureStorageManager = SecureStorageLinuxManager.getInstance(); + secureStorageManager = SecureStorageAppleManager.builder(); } else if (Constants.getOS() == Constants.OS.WINDOWS) { secureStorageManager = SecureStorageWindowsManager.builder(); } else if (Constants.getOS() == Constants.OS.LINUX) { From 10307637c83fa400fc01c43c2d84b59f960aabf3 Mon Sep 17 00:00:00 2001 From: Dawid Heyman Date: Thu, 13 Feb 2025 12:44:51 +0100 Subject: [PATCH 21/21] Remove logs --- src/main/java/net/snowflake/client/core/FileCacheManager.java | 4 ---- .../snowflake/client/core/auth/oauth/TokenResponseDTO.java | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main/java/net/snowflake/client/core/FileCacheManager.java b/src/main/java/net/snowflake/client/core/FileCacheManager.java index 693ed8fdd..c8cc36089 100644 --- a/src/main/java/net/snowflake/client/core/FileCacheManager.java +++ b/src/main/java/net/snowflake/client/core/FileCacheManager.java @@ -112,8 +112,6 @@ synchronized void overrideCacheFile(File newCacheFile) { synchronized FileCacheManager build() { // try to get cacheDir from system property or environment variable - logger.info("Cache file from property: ", systemGetProperty(this.cacheDirectorySystemProperty)); - logger.info("Cache file from env: ", systemGetEnv(this.cacheDirectoryEnvironmentVariable)); String cacheDirPath = this.cacheDirectorySystemProperty != null ? systemGetProperty(this.cacheDirectorySystemProperty) @@ -135,10 +133,8 @@ synchronized FileCacheManager build() { if (cacheDirPath != null) { this.cacheDir = new File(cacheDirPath); - logger.info("Using cache dir: ", cacheDirPath); } else { this.cacheDir = getDefaultCacheDir(); - logger.info("Using default cache dir : ", cacheDir.getAbsoluteFile()); } if (cacheDir == null) { return this; diff --git a/src/main/java/net/snowflake/client/core/auth/oauth/TokenResponseDTO.java b/src/main/java/net/snowflake/client/core/auth/oauth/TokenResponseDTO.java index fa9d3baf0..e0cd3eb1c 100644 --- a/src/main/java/net/snowflake/client/core/auth/oauth/TokenResponseDTO.java +++ b/src/main/java/net/snowflake/client/core/auth/oauth/TokenResponseDTO.java @@ -24,7 +24,7 @@ public class TokenResponseDTO { @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) public TokenResponseDTO( - @JsonProperty("access_token") String accessToken, + @JsonProperty(value = "access_token", required = true) String accessToken, @JsonProperty("refresh_token") String refreshToken, @JsonProperty("token_type") String tokenType, @JsonProperty("scope") String scope,