-
Notifications
You must be signed in to change notification settings - Fork 50
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
Feature - Replacing Bcrypt with NodeJS built in PBKDF2 #315
Open
elliot-sabitov
wants to merge
7
commits into
rc-1.6.0
Choose a base branch
from
feature/60-replace-bcrypt
base: rc-1.6.0
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.
Conversation
This file contains 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
Contributor
elliot-sabitov
commented
Dec 6, 2024
- feat: replacing bcrypt with nodes built in pbkdf2Sync to persist and check for matching hashes of passwordString VariableAttribute dataType during BasicAuthentication handling
…check for matching hashes of passwordString VariableAttribute dataType during BasicAuthentication handling
elliot-sabitov
changed the title
Feature - Replacing Bcrypt with NodeJS built in pbkdf2Sync
Feature - Replacing Bcrypt with NodeJS built in PBKDF2
Dec 6, 2024
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.
Can we fix what is happening with the jest tests as well?
…lper method to getHashFromStringWithSalt test: fixed failing test
* rc-1.6.0: Updating localstack volumes Changing healthcheck settings for graphql-engine on docker compose. Managing execution order on docker compose Moving hasura into core
* rc-1.6.0: chore: removing missed unnecessary protected access modifiers chore: removing deasync from dockerfiles feat: removing deasync dependency by instead awaiting promises outside of constructors refactor: adjusted logic such that the requests/responses are defined in the AbstractModule and creating an initHandlers helper method that can be triggered on the module to await the existing initHandler promise that was previously wrapped in deasyncPromise refactor: adjusted logging behavior such that the AbstractModule can log the initialization and initialization duration messages refactor: adjusted the CitrineOSServer to trigger initSystem() during the async initialize() that is triggered post constructor initialization via run(), made initSystem() async and made, created helper methods to initialize the individual modules, and fixed initModule helper to invoke one of the individual module initialization helpers. Individual module initialization methods are now also async and will await initHandlersAndAddModule helper method which will trigger and await the new initHandlers method in the AbstractModule, thereby allowing the promises to be awaited outside of the constructors chore: removing Timer class entirely and simplifying how we computed the initialization duration captured solely within AbstractModule class via single startTime variable chore: removing getSync and setSync along with todo as these methods are no longer used and are only other place where deasyncPromise was used # Conflicts: # 02_Util/package.json # Server/deploy.Dockerfile # Server/local.Dockerfile # Server/package.json
…on and persisting keyLen and digest as part of the hashed PW
Signed-off-by: thanaParis <thana.paris@s44.team>
Tests were fixed 👍 |
* rc-1.6.0: updating dockerfiles to use node 22 increasing node version for all github action pipelines to match new requirement of >=22.x satifying linter, no hanging promises feat: updating typescript to newer version feat: moving more shared dependencies to Base so that they can be picked up transitively feat: updating eslint dependencies fix: linting issues that came up after updating eslint feat: upgraded fastify to latest version feat: updated NodeJS version to latest LTS 22.11.0 fix: fixed vulnerabilities by updated other dependencies chore: renamed npm run fresh-and-install-all to npm run fi feature/handle-meter-value-cost-update # Conflicts: # 01_Data/package.json # Server/package.json
FYI working so far on windows 11 as well 👀🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.