SNOW-715510: MFA Token cache for libsnowflakeclient #773
Conversation
sfc-gh-jszczerbinski
requested review from
sfc-gh-ext-simba-hx and
sfc-gh-dprzybysz
|
|
||
| if (errno == EEXIST) | ||
| { | ||
| CXX_LOG_TRACE("Directory already exists %s directory.", dir); |
There was a problem hiding this comment.
nit: second directory is probably obsolete in this sentence
| "TMP" | ||
| }; | ||
|
|
||
| const std::vector<std::string> CACHE_DIR_NAMES = |
There was a problem hiding this comment.
Effectively we want to create or find .cache/snowflake directory so it's one path right? The fact that CACHE_ROOT_ENV_VARS provides alternatives while CACHE_DIR_NAMES contains parts of the same path confused me a bit. Maybe we can rename to CACHE_DIR_PATH?
|
|
||
| std::string credItemStr(const CredentialKey &key) | ||
| { | ||
| return key.host + ":" + key.user + ":SNOWFLAKE-ODBC-DRIVER:" + credTypeToString(key.type); |
There was a problem hiding this comment.
should it be hardcoded to ODBC-DRIVER? I wonder if there's some way to determine whether libsfclient is executed within ODBC or PHP driver or possibly even other drivers in the future.
| ensureObject(cache); | ||
| picojson::object &obj = cache.get<picojson::object>(); | ||
| auto pair = obj.emplace(key.account, picojson::value(picojson::object())); | ||
| auto accountCacheIt = pair.first; |
There was a problem hiding this comment.
nit: Here I also wasn't sure why we're calling pair.first then accountCacheIt->second gets the credential value. I admit it's a skill issue but would appreciate getting verbose type instead of auto either here or for pair
| /** | ||
| * storeToken | ||
| * | ||
| * API to secure store credential in Apple Keychain |
There was a problem hiding this comment.
or Windows Credential Manager?
| #include <sstream> | ||
|
|
||
| #define MAX_TOKEN_LEN 1024 | ||
| #define DRIVER_NAME "SNOWFLAKE_ODBC_DRIVER" |
There was a problem hiding this comment.
Same concern as above
No description provided.