-
Notifications
You must be signed in to change notification settings - Fork 11
perf: Implement mechanism for caching token on login #146
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
b395177
Implement mechanism for caching token on login
yuryfirebolt fec5e17
Merge branch 'main' into FIR-11540-reuse-token
yuryfirebolt 84390f1
Merge branch 'main' into FIR-11540-reuse-token
yuryfirebolt cc9a72b
Add missing dependency
yuryfirebolt 718df13
Merge branch 'FIR-11540-reuse-token' of github.com:firebolt-db/firebo…
yuryfirebolt 12d2152
Add missing cryptography
yuryfirebolt 5bbe341
Add appdirs stubs
yuryfirebolt 9eefe8a
Add missing pyfakefs for testing
yuryfirebolt f5920a4
Merge branch 'main' into FIR-11540-reuse-token
yuryfirebolt 168fd27
remove redundant code check
stepansergeevitch 9a3edb8
return UNAUTHORIZED error handling
stepansergeevitch cf41384
minor imports refactoring
stepansergeevitch e5c8a96
minor refactoring
stepansergeevitch 5272e58
allow multiple tokens, add global fs mock for tests
stepansergeevitch 1a85c75
add token expiration handlin
stepansergeevitch 73a4cfc
fix code checks
stepansergeevitch b9dada1
patch fs for each test
stepansergeevitch 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
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
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 |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| from base64 import b64decode, b64encode, urlsafe_b64encode | ||
| from hashlib import sha256 | ||
| from json import JSONDecodeError | ||
| from json import dump as json_dump | ||
| from json import load as json_load | ||
| from os import makedirs, path, urandom | ||
| from time import time | ||
| from typing import Optional | ||
|
|
||
| from appdirs import user_data_dir | ||
| from cryptography.fernet import Fernet, InvalidToken | ||
| from cryptography.hazmat.primitives import hashes | ||
| from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC | ||
|
|
||
| APPNAME = "firebolt" | ||
|
|
||
|
|
||
| def generate_salt() -> str: | ||
| return b64encode(urandom(16)).decode("ascii") | ||
|
|
||
|
|
||
| def generate_file_name(username: str, password: str) -> str: | ||
| username_hash = sha256(username.encode("utf-8")).hexdigest()[:32] | ||
| password_hash = sha256(password.encode("utf-8")).hexdigest()[:32] | ||
|
|
||
| return f"{username_hash}{password_hash}.json" | ||
|
|
||
|
|
||
| class TokenSecureStorage: | ||
| def __init__(self, username: str, password: str): | ||
| """ | ||
| Class for permanent storage of token in the filesystem in encrypted way | ||
|
|
||
| :param username: username used for toke encryption | ||
| :param password: password used for toke encryption | ||
| """ | ||
| self._data_dir = user_data_dir(appname=APPNAME) | ||
| makedirs(self._data_dir, exist_ok=True) | ||
|
|
||
| self._token_file = path.join( | ||
| self._data_dir, generate_file_name(username, password) | ||
| ) | ||
|
|
||
| self.salt = self._get_salt() | ||
| self.encrypter = FernetEncrypter(self.salt, username, password) | ||
|
|
||
| def _get_salt(self) -> str: | ||
| """ | ||
| Get salt from the file if exists, or generate a new one | ||
|
|
||
| :return: salt | ||
| """ | ||
| res = self._read_data_json() | ||
| return res.get("salt", generate_salt()) | ||
|
|
||
| def _read_data_json(self) -> dict: | ||
| """ | ||
| Read json token file | ||
|
|
||
| :return: json object as dict | ||
| """ | ||
| if not path.exists(self._token_file): | ||
| return {} | ||
|
|
||
| with open(self._token_file) as f: | ||
| try: | ||
| return json_load(f) | ||
| except JSONDecodeError: | ||
| return {} | ||
|
|
||
| def get_cached_token(self) -> Optional[str]: | ||
| """ | ||
| Get decrypted token using username and password | ||
| If the token not found or token cannot be decrypted using username, password | ||
| None will be returned | ||
|
|
||
| :return: token or None | ||
| """ | ||
| res = self._read_data_json() | ||
| if "token" not in res: | ||
| return None | ||
|
|
||
| # Ignore expired tokens | ||
| if "expiration" in res and res["expiration"] <= int(time()): | ||
| return None | ||
|
|
||
| return self.encrypter.decrypt(res["token"]) | ||
|
|
||
| def cache_token(self, token: str, expiration_ts: int) -> None: | ||
| """ | ||
|
|
||
| :param token: | ||
| :return: | ||
| """ | ||
| token = self.encrypter.encrypt(token) | ||
|
|
||
| with open(self._token_file, "w") as f: | ||
| json_dump( | ||
| {"token": token, "salt": self.salt, "expiration": expiration_ts}, f | ||
| ) | ||
|
|
||
|
|
||
| class FernetEncrypter: | ||
| def __init__(self, salt: str, username: str, password: str): | ||
| """ | ||
|
|
||
| :param salt: | ||
| :param username: | ||
| :param password: | ||
| """ | ||
|
|
||
| kdf = PBKDF2HMAC( | ||
| algorithm=hashes.SHA256(), | ||
| salt=b64decode(salt), | ||
| length=32, | ||
| iterations=39000, | ||
| ) | ||
| self.fernet = Fernet( | ||
| urlsafe_b64encode( | ||
| kdf.derive(bytes(f"{username}{password}", encoding="utf-8")) | ||
| ) | ||
| ) | ||
|
|
||
| def encrypt(self, data: str) -> str: | ||
| return self.fernet.encrypt(bytes(data, encoding="utf-8")).decode("utf-8") | ||
|
|
||
| def decrypt(self, data: str) -> Optional[str]: | ||
| try: | ||
| return self.fernet.decrypt(bytes(data, encoding="utf-8")).decode("utf-8") | ||
| except InvalidToken: | ||
| return None |
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 |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| from pyfakefs.fake_filesystem_unittest import Patcher | ||
| from pytest import fixture | ||
|
|
||
|
|
||
| @fixture(autouse=True) | ||
| def global_fake_fs() -> None: | ||
| with Patcher(): | ||
| yield | ||
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
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.
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.
To be honest, I preferred explicitly adding fs to the relevant tests. That way there's no hidden functionality, you know which tests are hitting a fake file system and which do not.
But up to you, I'm not too worried about it.
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.
This is required because now any authentication leaves token cache artifact file, which also causes tests to fail
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.
We would need to add fake fs to almost every integration test and to a lot of unit tests