Added memory mode support#243
Open
DhananjayNazare wants to merge 4 commits into
Open
Conversation
Owner
|
You will need to elaborate on why you consider local-/sessionstorage as a vulnerability, and how storing the token in memory remedies that. Also, please run the formatter included in the project via pre-commit. |
Author
|
Also added few more inputs on in issue section as an justification for the change. Please let me know if u have any further review comments. |
sebastianvitterso
left a comment
sebastianvitterso
left a comment
Collaborator
There was a problem hiding this comment.
@soofstad will make the final review, to consider whether this is something we need/want to support.
Code-wise, this looks good and proper. Fits our code style well. The inMemoryStorage definition is simple and elegant. Nice to have tests properly covering it. All-in-all very good.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What does this pull request change?
Add support for memory storage along with local and session storage
Why is this pull request needed?
In case local storage and session storage is considered as the vulnerability, need a safe option for storage
Issues related to this change
Token stored in local storage is considered as sensitive data which is getting exposed. With mythos release, organisations are scanning all the applications leading to these kind local storage usages being triggered a vulnerabilties and auto suggestion from model is either have a inmemroy storage or build server driven solution. In most scenarios inmemory solution is a quick solution rather than build server driven solutions.