nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
27.42k stars 4.07k forks source link

Protected namespace in localstorage #15943

Open sualko opened 5 years ago

sualko commented 5 years ago

Is your feature request related to a problem? Please describe. Starting from version 16, Nextcloud deletes the complete local/sessionstorage. This can cause issues with different apps like ojsxc. I understand the intend to protect users privacy, but it would be nice if this could be configured by the app developer.

https://github.com/nextcloud/server/blob/6f941a73a21ba926cca5b12ecab8242e34169d91/core/src/login.js#L32-L33

Describe the solution you'd like Black or white list of prefixes which should be deleted or preserved. For example:

   let prefixRegex = new RegExp('^' + prefix);
   let keys = Object.keys(localStorage);

   for (let key of keys) {
      if (prefixRegex.test(key)) {
         localStorage.removeItem(key);
      }
   }
kesselb commented 5 years ago

cc @rullzer @ChristophWurst

As user i would expect that logout clears all data.

ChristophWurst commented 5 years ago

What data would the app keep?

sualko commented 5 years ago

For example we cache the complete contact list (we have users with hundreds of contacts), conversations and encryption keys. I know the discussion about encryption on the client side, but under the assumption that Nextcloud is secure (same assumption Nextcloud does for every security feature) it's a effective protection against malicious third-party servers (XMPP supports server-to-server). Sure we could store all this data in Nextcloud, but this would require a lot of requests and a smart conflict management in the case someone is using Nextcloud in two browsers at the same time.

I agree that all data should be deleted on public computers, but I see no need for this on private machines.

A notice about this change had also been helpful in #15339.

sualko commented 5 years ago

Btw. if Nextcloud is installed in a subdirectory this deletes all data for the whole domain. This means if someone uses one domain for multiple applications, this results in side effects for all other applications.

I would volunteer to write a pull request to delete only Nextcloud specific data.

e-alfred commented 5 years ago

I agree that all data should be deleted on public computers, but I see no need for this on private machines.

Maybe adding a "this is a public computer" selection menu on the login page could solve this?

sualko commented 5 years ago

Maybe adding a "this is a public computer" selection menu on the login page could solve this?

Yes, but it could still impact other applications on the same domain. In my opinion the only correct solution would be to use a namespace for all NC related entries and to delete only those.

ChristophWurst commented 5 years ago

Sure we could store all this data in Nextcloud, but this would require a lot of requests and a smart conflict management in the case someone is using Nextcloud in two browsers at the same time.

Can't you just prefix the cached data wit the session id?

sualko commented 5 years ago

Can't you just prefix the cached data wit the session id?

If I store my cache, encryption keys and so on on the server, the idea would be that the user has the same data in every browser session. So I have to merge them. The use of a session key would only make sense, if I would bind a session to a browser, but without localstorage I can't detect if it's a new browser or not.

Nonetheless I think the Nextcloud should not mess with the domain wide storage is more important.

kesselb commented 4 years ago

cc @ChristophWurst @rullzer :ping_pong:

I guess that's still an issue for ojsxc. It's definitely a tricky one. Probably we need something like https://www.npmjs.com/package/store2 for a namespace aware localstorage.

ChristophWurst commented 4 years ago

Looks good! We could make this available as a npm wrapper package @nextcloud/local-storage so it's used in a unified way.

sualko commented 4 years ago

That would be great. Please let me know if I can help.

ChristophWurst commented 4 years ago

Voilá: https://github.com/nextcloud/nextcloud-browser-storage/issues/1. Let's discuss the API details there.

szaimen commented 3 years ago

IIRC, this was fixed in https://github.com/nextcloud/nextcloud-browser-storage. Please reopen if this shouldn't be the case. Thanks! :)

sualko commented 3 years ago

I think this issue should still be open, because the storage is still be cleared after every logout. See https://github.com/nextcloud/server/blob/master/core/src/login.js