-
Notifications
You must be signed in to change notification settings - Fork 138
SNOW-1825789 Secure token cache #1012
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
Open
sfc-gh-astachowski
wants to merge
60
commits into
master
Choose a base branch
from
SNOW-1825789-secure-token-cache
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+392
−93
Open
Changes from all commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
d72bac3
Secure token cache
sfc-gh-astachowski 89e3b4b
Merge branch 'master' into SNOW-1825789-secure-token-cache
sfc-gh-astachowski ada2872
Test fixes
sfc-gh-astachowski f638e60
Disable tests on windows
sfc-gh-astachowski c9c6bd6
Enable tests on windows
sfc-gh-astachowski a431cb8
Diagnostic logs
sfc-gh-astachowski 770e3b1
Potential windows fix
sfc-gh-astachowski 7e3dc4f
More logs
sfc-gh-astachowski dcc6d2e
Attempted fix
sfc-gh-astachowski d51750d
Added error log
sfc-gh-astachowski b0c0752
Windows fix
sfc-gh-astachowski 1c3dc35
Fixes
sfc-gh-astachowski f813df5
Extra logging
sfc-gh-astachowski 9cb1c16
Improved logging
sfc-gh-astachowski 03e376f
Improved logging
sfc-gh-astachowski 800a58a
Added more logging
sfc-gh-astachowski 47a684a
Test fixes
sfc-gh-astachowski 7f4afd4
Test fixes
sfc-gh-astachowski ef6e428
Further test fixes
sfc-gh-astachowski d279ca6
Added even more logs
sfc-gh-astachowski 52a088c
Change to util exists
sfc-gh-astachowski 43e1040
Improved cleanup
sfc-gh-astachowski 6d49ccf
More logs
sfc-gh-astachowski 1778e9f
More logs
sfc-gh-astachowski 7a1b31d
Fixes, cleanup
sfc-gh-astachowski c99010b
Merge branch 'master' into SNOW-1825789-secure-token-cache
sfc-gh-astachowski cdc5484
Sym link fix
sfc-gh-astachowski 3ba209a
Added key hashing
sfc-gh-astachowski 948cd27
Switched to file handles
sfc-gh-astachowski 8e9e85d
Windows fix
sfc-gh-astachowski 13e04df
Windows fix?
sfc-gh-astachowski 90f758d
Windows fix?
sfc-gh-astachowski f1e8ebf
Logging
sfc-gh-astachowski 74c6d1c
Remove logs
sfc-gh-astachowski 5f2cf3c
Close description in case of loch error
sfc-gh-astachowski d5898c9
Close descriptor before opening a new one
sfc-gh-astachowski 2d65bbf
Change flag to integer
sfc-gh-astachowski 47e6449
revert change flag to integer
sfc-gh-astachowski 3f66eb2
Some logging
sfc-gh-astachowski dc36c39
Additional cleanup
sfc-gh-astachowski 3097c53
Handle closing fix
sfc-gh-astachowski c036882
Change error handling
sfc-gh-astachowski f2ea3b3
Change flags to use NOFOLLOW
sfc-gh-astachowski a24a749
Force cleanup
sfc-gh-astachowski 50e7e39
Close file handles in tests
sfc-gh-astachowski 25f6ccb
Review improvements
sfc-gh-astachowski 86fd21b
Fix imports
sfc-gh-astachowski 3514cd7
Merge branch 'master' into SNOW-1825789-secure-token-cache
sfc-gh-astachowski 81a56df
Permission fixes
sfc-gh-astachowski 24995a4
Add directory check
sfc-gh-astachowski 3048378
Remove anti-symlink flag
sfc-gh-astachowski 0140865
Remove chmod
sfc-gh-astachowski 76462a5
Review fixes
sfc-gh-astachowski 1d15e2e
Removed unused import
sfc-gh-astachowski 380c2f0
Review feedback
sfc-gh-astachowski cf9b613
Flag changes
sfc-gh-astachowski 55a6b4b
Removed duplicated code
sfc-gh-astachowski aed35d0
Extract retry interval to a constant
sfc-gh-astachowski 29c74f2
Review improvements
sfc-gh-astachowski 669e214
Merge branch 'master' into SNOW-1825789-secure-token-cache
sfc-gh-dheyman File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
252 changes: 211 additions & 41 deletions
252
lib/authentication/secure_storage/json_credential_manager.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,87 +1,257 @@ | ||
const path = require('path'); | ||
const Logger = require('../../logger'); | ||
const fs = require('node:fs/promises'); | ||
const os = require('os'); | ||
const Util = require('../../util'); | ||
const { validateOnlyUserReadWritePermissionAndOwner } = require('../../file_util'); | ||
const os = require('os'); | ||
const crypto = require('crypto'); | ||
const { getSecureHandle, closeHandle } = require('../../file_util'); | ||
|
||
const defaultJsonTokenCachePaths = { | ||
'win32': ['AppData', 'Local', 'Snowflake', 'Caches'], | ||
'linux': ['.cache', 'snowflake'], | ||
'darwin': ['Library', 'Caches', 'Snowflake'] | ||
}; | ||
|
||
function JsonCredentialManager(credentialCacheDir, timeoutMs = 60000) { | ||
const tokenMapKey = 'tokens'; | ||
const retryInterval = 100; | ||
|
||
this.hashKey = function (key) { | ||
return crypto.createHash('sha256').update(key).digest('hex'); | ||
}; | ||
|
||
this.getTokenDirCandidates = function () { | ||
const candidates = []; | ||
candidates.push({ folder: credentialCacheDir, subfolders: [] }); | ||
|
||
candidates.push({ folder: process.env.SF_TEMPORARY_CREDENTIAL_CACHE_DIR, subfolders: [] }); | ||
|
||
switch (process.platform) { | ||
case 'win32': | ||
candidates.push({ folder: os.homedir(), subfolders: defaultJsonTokenCachePaths['win32'] }); | ||
break; | ||
case 'linux': | ||
candidates.push({ folder: process.env.XDG_CACHE_HOME, subfolders: ['snowflake'] }); | ||
candidates.push({ folder: process.env.HOME, subfolders: defaultJsonTokenCachePaths['linux'] }); | ||
break; | ||
case 'darwin': | ||
candidates.push({ folder: process.env.HOME, subfolders: defaultJsonTokenCachePaths['darwin'] }); | ||
} | ||
return candidates; | ||
}; | ||
|
||
this.createCacheDir = async function (cacheDir) { | ||
const options = { recursive: true }; | ||
if (process.platform !== 'win32') { | ||
options.mode = 0o755; | ||
} | ||
await fs.mkdir(cacheDir, options); | ||
if (process.platform !== 'win32') { | ||
await fs.chmod(cacheDir, 0o700); | ||
} | ||
}; | ||
|
||
this.tryTokenDir = async function (dir, subDirs) { | ||
if (!Util.exists(dir)) { | ||
return false; | ||
} | ||
const cacheDir = path.join(dir, ...subDirs); | ||
try { | ||
const stat = await fs.stat(dir); | ||
if (!stat.isDirectory()) { | ||
Logger.getInstance().info(`Path ${dir} is not a directory`); | ||
return false; | ||
} | ||
const cacheStat = await fs.stat(cacheDir).catch(async (err) => { | ||
if (err.code !== 'ENOENT') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe create dir here when ENOENT? Then stat it again and recover from error? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, changed that |
||
throw err; | ||
} | ||
await this.createCacheDir(cacheDir); | ||
return await fs.stat(cacheDir); | ||
}); | ||
if (!cacheStat.isDirectory()) { | ||
return false; | ||
} | ||
if (process.platform === 'win32') { | ||
return true; | ||
} | ||
if ((cacheStat.mode & 0o777) !== 0o700 && cacheStat.uid === os.userInfo().uid) { | ||
Logger.getInstance().warn(`Token cache directory ${cacheDir} has unsecure permissions.`); | ||
} | ||
if (cacheStat.uid !== os.userInfo().uid) { | ||
Logger.getInstance().warn(`Token cache directory ${cacheDir} has unsecure owner.`); | ||
} | ||
return true; | ||
} catch (err) { | ||
Logger.getInstance().warn(`The path location ${cacheDir} is invalid. Please check this location is accessible or existing`); | ||
return false; | ||
} | ||
}; | ||
|
||
function JsonCredentialManager(credentialCacheDir) { | ||
|
||
this.getTokenDir = async function () { | ||
let tokenDir = credentialCacheDir; | ||
if (!Util.exists(tokenDir)) { | ||
tokenDir = os.homedir(); | ||
} else { | ||
Logger.getInstance().info(`The credential cache directory is configured by the user. The token will be saved at ${tokenDir}`); | ||
const candidates = this.getTokenDirCandidates(); | ||
for (const candidate of candidates) { | ||
const { folder: dir, subfolders: subDirs } = candidate; | ||
if (await this.tryTokenDir(dir, subDirs)) { | ||
return path.join(dir, ...subDirs); | ||
} | ||
} | ||
return null; | ||
}; | ||
|
||
this.getTokenFilePath = async function () { | ||
const tokenDir = await this.getTokenDir(); | ||
|
||
if (!Util.exists(tokenDir)) { | ||
throw new Error(`Temporary credential cache directory is invalid, and the driver is unable to use the default location(home). | ||
throw new Error(`Temporary credential cache directory is invalid, and the driver is unable to use the default location. | ||
Please set 'credentialCacheDir' connection configuration option to enable the default credential manager.`); | ||
} | ||
|
||
const tokenCacheFile = path.join(tokenDir, 'temporary_credential.json'); | ||
await validateOnlyUserReadWritePermissionAndOwner(tokenCacheFile); | ||
return tokenCacheFile; | ||
return path.join(tokenDir, 'credential_cache_v1.json'); | ||
}; | ||
|
||
this.readJsonCredentialFile = async function () { | ||
this.readJsonCredentialFile = async function (fileHandle) { | ||
if (!Util.exists(fileHandle)) { | ||
return null; | ||
} | ||
try { | ||
const cred = await fs.readFile(await this.getTokenDir(), 'utf8'); | ||
const cred = await fileHandle.readFile('utf8'); | ||
return JSON.parse(cred); | ||
} catch (err) { | ||
Logger.getInstance().warn('Failed to read token data from the file. Err: %s', err.message); | ||
return null; | ||
} | ||
}; | ||
|
||
this.removeStale = async function (file) { | ||
const stat = await fs.stat(file).catch(() => { | ||
return undefined; | ||
}); | ||
if (!Util.exists(stat)) { | ||
return; | ||
} | ||
if (new Date().getTime() - stat.birthtimeMs > timeoutMs) { | ||
try { | ||
await fs.rmdir(file); | ||
} catch (err) { | ||
Logger.getInstance().warn('Failed to remove stale file. Error: %s', err.message); | ||
} | ||
} | ||
|
||
}; | ||
|
||
this.lockFile = async function (filename) { | ||
const lckFile = filename + '.lck'; | ||
await this.removeStale(lckFile); | ||
let attempts = 1; | ||
let locked = false; | ||
const options = {}; | ||
if (process.platform !== 'win32') { | ||
options.mode = 0o600; | ||
} | ||
while (attempts <= 10) { | ||
Logger.getInstance().debug('Attempting to get a lock on file %s, attempt: %d', filename, attempts); | ||
attempts++; | ||
await fs.mkdir(lckFile, options).then(() => { | ||
sfc-gh-pmotacki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
locked = true; | ||
}, () => {}); | ||
if (locked) { | ||
break; | ||
} | ||
await new Promise(resolve => setTimeout(resolve, retryInterval)); | ||
} | ||
if (!locked) { | ||
Logger.getInstance().warn('Could not acquire lock on cache file %s', filename); | ||
} | ||
return locked; | ||
}; | ||
|
||
this.unlockFile = async function (filename) { | ||
const lckFile = filename + '.lck'; | ||
await fs.rmdir(lckFile); | ||
}; | ||
|
||
this.withFileLocked = async function (fun) { | ||
const filename = await this.getTokenFilePath(); | ||
if (await this.lockFile(filename)) { | ||
const res = await fun(filename); | ||
await this.unlockFile(filename); | ||
return res; | ||
} | ||
return null; | ||
}; | ||
|
||
this.write = async function (key, token) { | ||
sfc-gh-jszczerbinski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!validateTokenCacheOption(key)) { | ||
return null; | ||
} | ||
|
||
const jsonCredential = await this.readJsonCredentialFile() || {}; | ||
jsonCredential[key] = token; | ||
|
||
try { | ||
await fs.writeFile(await this.getTokenDir(), JSON.stringify(jsonCredential), { mode: 0o600 }); | ||
} catch (err) { | ||
throw new Error(`Failed to write token data. Please check the permission or the file format of the token. ${err.message}`); | ||
} | ||
const keyHash = this.hashKey(key); | ||
|
||
await this.withFileLocked(async (filename) => { | ||
const fileHandle = await getSecureHandle(filename, fs.constants.O_RDWR | fs.constants.O_CREAT, fs); | ||
const jsonCredential = await this.readJsonCredentialFile(fileHandle) || {}; | ||
if (!Util.exists(jsonCredential[tokenMapKey])) { | ||
jsonCredential[tokenMapKey] = {}; | ||
} | ||
jsonCredential[tokenMapKey][keyHash] = token; | ||
|
||
try { | ||
await fileHandle.truncate(); | ||
await fileHandle.write(JSON.stringify(jsonCredential), 0); | ||
await closeHandle(fileHandle); | ||
} catch (err) { | ||
Logger.getInstance().warn(`Failed to write token data in ${filename}. Please check the permission or the file format of the token. ${err.message}`); | ||
} | ||
}); | ||
}; | ||
|
||
this.read = async function (key) { | ||
if (!validateTokenCacheOption(key)) { | ||
return null; | ||
} | ||
|
||
const jsonCredential = await this.readJsonCredentialFile(); | ||
if (!!jsonCredential && jsonCredential[key]){ | ||
return jsonCredential[key]; | ||
} else { | ||
return null; | ||
} | ||
const keyHash = this.hashKey(key); | ||
|
||
return await this.withFileLocked(async (filename) => { | ||
const fileHandle = await getSecureHandle(filename, fs.constants.O_RDWR, fs); | ||
sfc-gh-astachowski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const jsonCredential = await this.readJsonCredentialFile(fileHandle); | ||
await closeHandle(fileHandle); | ||
if (!!jsonCredential && jsonCredential[tokenMapKey] && jsonCredential[tokenMapKey][keyHash]) { | ||
return jsonCredential[tokenMapKey][keyHash]; | ||
} else { | ||
return null; | ||
} | ||
}); | ||
}; | ||
|
||
this.remove = async function (key) { | ||
if (!validateTokenCacheOption(key)) { | ||
return null; | ||
} | ||
const jsonCredential = await this.readJsonCredentialFile(); | ||
|
||
if (jsonCredential && jsonCredential[key]) { | ||
try { | ||
jsonCredential[key] = null; | ||
await fs.writeFile(await this.getTokenDir(), JSON.stringify(jsonCredential), { mode: 0o600 }); | ||
} catch (err) { | ||
throw new Error(`Failed to write token data from the file in ${await this.getTokenDir()}. Please check the permission or the file format of the token. ${err.message}`); | ||
} | ||
} | ||
|
||
const keyHash = this.hashKey(key); | ||
|
||
await this.withFileLocked(async (filename) => { | ||
const fileHandle = await getSecureHandle(filename, fs.constants.O_RDWR, fs); | ||
sfc-gh-astachowski marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const jsonCredential = await this.readJsonCredentialFile(fileHandle); | ||
|
||
if (jsonCredential && jsonCredential[tokenMapKey] && jsonCredential[tokenMapKey][keyHash]) { | ||
try { | ||
jsonCredential[tokenMapKey][keyHash] = null; | ||
await fileHandle.truncate(); | ||
await fileHandle.write(JSON.stringify(jsonCredential), 0); | ||
await closeHandle(fileHandle); | ||
} catch (err) { | ||
Logger.getInstance().warn(`Failed to remove token data from the file in ${filename}. Please check the permission or the file format of the token. ${err.message}`); | ||
} | ||
} | ||
}); | ||
}; | ||
|
||
function validateTokenCacheOption(key) { | ||
return Util.checkParametersDefined(key); | ||
} | ||
} | ||
|
||
module.exports = JsonCredentialManager; | ||
module.exports.defaultJsonTokenCachePaths = defaultJsonTokenCachePaths; | ||
module.exports.JsonCredentialManager = JsonCredentialManager; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: it's hard to see what's returned for each system. Can you do it like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't do exactly that, but without the null/undefined checks the function is much more readable now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good