Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-1895884: Token cache refactor #2044

Merged
merged 26 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c9fdff8
SNOW-1895884: Add new token cache keys
sfc-gh-dheyman Jan 24, 2025
1d0b235
Change cache json file structure
sfc-gh-dheyman Jan 29, 2025
197242f
Merge branch 'oauth-code-flow' into SNOW-1895884-token-cache-keys
sfc-gh-dheyman Jan 29, 2025
0e1f28f
Merge branch 'oauth-code-flow' of github.com:snowflakedb/snowflake-jd…
sfc-gh-dheyman Jan 29, 2025
4bee3f1
Merge branch 'SNOW-1895884-token-cache-keys' of github.com:snowflaked…
sfc-gh-dheyman Jan 29, 2025
8b7aa40
refactor convertTarget name
sfc-gh-dheyman Jan 30, 2025
6575f5d
Refactor cache locks
sfc-gh-dheyman Jan 30, 2025
3ae9ed7
Refactor readJsonStoreCache
sfc-gh-dheyman Feb 3, 2025
daa9a5a
Merge branch 'oauth-code-flow' of github.com:snowflakedb/snowflake-jd…
sfc-gh-dheyman Feb 4, 2025
572adfd
CR suggestions
sfc-gh-dheyman Feb 6, 2025
0853e53
Merge branch 'oauth-code-flow' of github.com:snowflakedb/snowflake-jd…
sfc-gh-dheyman Feb 6, 2025
c19b177
Fix CR suggestions
sfc-gh-dheyman Feb 7, 2025
e1fe433
Synchronized FileCacheManager class members
sfc-gh-dheyman Feb 10, 2025
f86f4e0
Changed test cache directory
sfc-gh-dheyman Feb 11, 2025
646b2a8
Remove dir if exists in tests
sfc-gh-dheyman Feb 11, 2025
65efc99
Fixed parent dir check when overriding cache dir
sfc-gh-dheyman Feb 11, 2025
9c5a1cd
Fix tests
sfc-gh-dheyman Feb 11, 2025
ec0c863
Fix sso tests
sfc-gh-dheyman Feb 11, 2025
b8feaa8
Fix sso tests
sfc-gh-dheyman Feb 11, 2025
15cca90
Fix config file home directory getter
sfc-gh-dheyman Feb 11, 2025
457257f
Refactor localCacheTJson
sfc-gh-dheyman Feb 12, 2025
5ac2f92
Fix localCacheToJson
sfc-gh-dheyman Feb 12, 2025
1cfcad5
Fix NPE
sfc-gh-dheyman Feb 13, 2025
e548ba2
Add logs
sfc-gh-dheyman Feb 13, 2025
8aa7b0b
Add Logs
sfc-gh-dheyman Feb 13, 2025
1030763
Remove logs
sfc-gh-dheyman Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 58 additions & 65 deletions src/main/java/net/snowflake/client/core/FileCacheManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,7 +44,6 @@ class FileCacheManager {
private String cacheDirectorySystemProperty;
private String cacheDirectoryEnvironmentVariable;
private String baseCacheFileName;
private long cacheExpirationInMilliseconds;
private long cacheFileLockExpirationInMilliseconds;

private File cacheFile;
Expand Down Expand Up @@ -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;
Expand All @@ -90,6 +84,10 @@ FileCacheManager setOnlyOwnerPermissions(boolean onlyOwnerPermissions) {
return this;
}

String getCacheFilePath() {
return cacheFile.getAbsolutePath();
}

/**
* Override the cache file.
*
Expand All @@ -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);
Expand Down Expand Up @@ -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> T withLock(Supplier<T> 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);
Expand All @@ -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);
Expand All @@ -230,32 +249,21 @@ 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;
}
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);
}
writer.write(input.toString());
}
} catch (IOException ex) {
logger.debug("Failed to write the cache file. File: {}", cacheFile);
} finally {
if (!unlockCacheFile()) {
logger.debug("Failed to unlock cache file", false);
}
}
}

Expand All @@ -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) {
Expand All @@ -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;
}

/**
Expand All @@ -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();
}
}
10 changes: 6 additions & 4 deletions src/main/java/net/snowflake/client/core/FileUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/net/snowflake/client/core/SFTrustManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -544,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++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
Loading