Skip to content
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

Store the last namespace used in the browser storage #2165

Merged
merged 3 commits into from
Nov 17, 2020

Conversation

andresmgot
Copy link
Contributor

Description of the change

Another item for #2018, store the latest namespace used in the browser so it's easier to resume previous work.

When logging out of the system (or when the session expires), this information is cleaned up.

Applicable issues

Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that sessions can be quite short, I wonder if it's worthwhile leaving the data there for a session expiry and only clearing it on an intentional logout?


export function setStoredNamespace(cluster: string, namespace: string) {
const ns: string = localStorage.getItem(namespaceKey) || "{}";
const nsObject = JSON.parse(ns);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we be handling exceptions (SyntaxError) both here and 5 lines up? (just defaulting to {} if there is a syntax error, which is perhaps what you intended?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, this info is not something the user will modify on their own so a SyntaxError won't happen but it's true that if someone tamper this data, it may cause a DoS.

@andresmgot
Copy link
Contributor Author

andresmgot commented Nov 17, 2020

Given that sessions can be quite short, I wonder if it's worthwhile leaving the data there for a session expiry and only clearing it on an intentional logout?

I thought about this but since a session may have expired a minute ago or a month ago, there is no way of knowing that. Since there is nothing stored in the browser (and you need to re-login), and you may choose a different account, I think it's "safer" to just clean up everything (even though trying to use a namespace by default is harmless). This is to follow the same approach for all the info).

@andresmgot andresmgot merged commit 83166b9 into master Nov 17, 2020
@andresmgot andresmgot deleted the storedNamespace branch November 26, 2020 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants