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

Clears the local storage after logout #10874

Merged
merged 1 commit into from
Jan 29, 2019

Conversation

weeman1337
Copy link
Member

The logout controller adds a param to the redirect url. Then the login code checks if it's there and clears the local storage. I also added a note this can be taken out again if the browsers support Clear-Site-Data properly.

closes #10859

Copy link
Member

@juliushaertl juliushaertl left a comment

Choose a reason for hiding this comment

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

Clearing all local storage will cause the setting videoDisabled from the talk app to be removed. I think we should not clear all data if we cannot be sure that it is restored during the login again.

@MorrisJobke MorrisJobke added this to the Nextcloud 15 milestone Aug 28, 2018
@georgehrke
Copy link
Member

Clearing all local storage will cause the setting videoDisabled from the talk app to be removed. I think we should not clear all data if we cannot be sure that it is restored during the login again.

@juliushaertl We want to use local storage in Calendar and Contacts, to speed up loading when you open the page, but obviously, you don't want your events and contacts to remain in local storage once you logged out.

Maybe videoDisabled should use iConfig::getUserValue?

@juliushaertl juliushaertl dismissed their stale review August 29, 2018 15:57

Nevermind. Of course settings should be persisted in the user config.

@ChristophWurst
Copy link
Member

Side note: there's also session storage. For one, this can be used in apps for data that must not remain forever but we also might want to clear that on logout.

@weeman1337 weeman1337 force-pushed the feature/10859/clear-localstorage-on-logout branch from 7bfc5a9 to ccaa812 Compare September 6, 2018 18:02
@weeman1337
Copy link
Member Author

@ChristophWurst jep - I added cleanup of the SessionStorage.

@rullzer
Copy link
Member

rullzer commented Oct 2, 2018

From a security point of view I'd want

  • Clear this aways when reaching the login page javascript
  • Have some logic to clear this when the logout button is pressed

Currently that would means to just always clear it when the login page is loaded. As this is where you are also redirected when your session expired etc.

Of course settings should be stored in the DB as well ;) to obtain them if the local storage is empty etc.

@rullzer rullzer added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Oct 2, 2018
@MorrisJobke
Copy link
Member

@weeman1337 I guess this is nothing that will be ready and merged until late Wednesday? Then we would need to move this to 16.

@MorrisJobke
Copy link
Member

Let's move it to 16.

@MorrisJobke MorrisJobke modified the milestones: Nextcloud 15, Nextcloud 16 Nov 7, 2018
@ChristophWurst
Copy link
Member

Hey @weeman1337 :) Could you rebase onto latest master? Than we can integrate this into Nextcloud 16 ✌️

@skjnldsv skjnldsv force-pushed the feature/10859/clear-localstorage-on-logout branch from ccaa812 to e083e8a Compare January 29, 2019 08:03
@skjnldsv
Copy link
Member

Hey @weeman1337 :) Could you rebase onto latest master? Than we can integrate this into Nextcloud 16

Done

@juliushaertl juliushaertl added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jan 29, 2019
@juliushaertl juliushaertl added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jan 29, 2019
@skjnldsv
Copy link
Member

sh: 1: kill: No such process

Unrelated

@skjnldsv skjnldsv merged commit 34d5001 into master Jan 29, 2019
@skjnldsv skjnldsv deleted the feature/10859/clear-localstorage-on-logout branch January 29, 2019 09:40
@skjnldsv skjnldsv mentioned this pull request Jun 3, 2019
@deusoz
Copy link

deusoz commented Jun 11, 2019

It seems to me that the clearing of local storage has broken the JSXC chat app which no longer autologs in chat user for NC16.
jsxc/jsxc#783

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Purge LocalStorage on logout
8 participants