Enable public statistics gives guest users full access to private albums #71

Chiuso
aperto 2018-07-28 23:33:08 +01:00 da aheathershaw · 8 commenti
Proprietario

LNT00:

Steps:

  1. Set an album as private, only available for users
  2. Create a sub-album inside the first one.
  3. Enable statistics.
  4. While not logged in, open mydomain/statistics
  5. Albums is also available, and can be opened by guest users.

Note: Statistics is not displayed when not logged in, but can be opened with direct link. Also, seems this is an issue with permissions, only sub-albums are available, main albums that were set as private are not available, so it's probably a permission inheritance issue.

[LNT00](https://github.com/LNT00): Steps: 1. Set an album as private, only available for users 2. Create a sub-album inside the first one. 2. Enable statistics. 3. While not logged in, open mydomain/statistics 4. Albums is also available, and can be opened by guest users. Note: Statistics is not displayed when not logged in, but can be opened with direct link. Also, seems this is an issue with permissions, only sub-albums are available, main albums that were set as private are not available, so it's probably a permission inheritance issue.
aheathershaw aggiunta alle pietre miliari Version 2.2.0-beta.1 2018-07-28 23:33:08 +01:00
aheathershaw 2018-07-28 23:33:08 +01:00 auto-assegnato
aheathershaw ha aggiunto l'etichetta
enhancement
2018-07-28 23:33:08 +01:00
Autore
Proprietario

Permissions do not inherit, therefore yes, if a user has access to a child album but not the parent, as far as the system is concerned, they still have access to the child unless you’ve told it otherwise.

If we inherited permissions and you moved a child album from a parent that a user didn’t have permission to, into one they do, you could be inadvertently allowing access to a previously-disallowed album without realising it.

Permissions do not inherit, therefore yes, if a user has access to a child album but not the parent, as far as the system is concerned, they still have access to the child unless you’ve told it otherwise. If we inherited permissions and you moved a child album from a parent that a user didn’t have permission to, into one they do, you could be inadvertently allowing access to a previously-disallowed album without realising it.
Autore
Proprietario

LNT00:

Permissions should be inherited by default. If a user decides to allow access to a child album, then he assumes the action.

When you move a child album from a parent without permissions to a parent with permissions, then you accept that it will inherit the parent. If you still don't want to allow access to it, you can set it's own permission.

Issue came up from my own case, where i created parent albums without permissions, and i assume that everything i add inside will have the same restrictions. Otherwise, i will have to set permissions to each album i create, which i think is very time consuming. In my case i have about 10 parent albums, and 300+ child albums, for which i didn't set individual permissions.

As a workaround, i would suggest at least give the option to automatically set permissions to all content inside an album, or just the edited one.

[LNT00](https://github.com/LNT00): Permissions should be inherited by default. If a user decides to allow access to a child album, then he assumes the action. When you move a child album from a parent without permissions to a parent with permissions, then you accept that it will inherit the parent. If you still don't want to allow access to it, you can set it's own permission. Issue came up from my own case, where i created parent albums without permissions, and i assume that everything i add inside will have the same restrictions. Otherwise, i will have to set permissions to each album i create, which i think is very time consuming. In my case i have about 10 parent albums, and 300+ child albums, for which i didn't set individual permissions. As a workaround, i would suggest at least give the option to automatically set permissions to all content inside an album, or just the edited one.
Autore
Proprietario

I understand but I’m just saying it doesn’t work like that at the moment.

I will introduce a “inherit” option that works like Windows folder permissions for child albums but it will increase complexity of the code, particularly the statistics, so it isn’t going to be an immediate change - there are other things I would like to do first.

I understand but I’m just saying it doesn’t work like that at the moment. I will introduce a “inherit” option that works like Windows folder permissions for child albums but it will increase complexity of the code, particularly the statistics, so it isn’t going to be an immediate change - there are other things I would like to do first.
Autore
Proprietario
Migrated from https://github.com/andysh-uk/blue-twilight/issues/71
Autore
Proprietario

I've been thinking about this a bit more, and due to the complexity of the logic, it needs to be implemented in code.

This could be cached in the database in a similar way to how we cache disk usage in Blue Twilight Cloud (e.g. a flag indicating the permission cache is dirty.)

The cache will need to be dirtied when the following is true:

  • a new user is added - not needed as groups don't change until a user is edited
  • a user's group memberships are changed
  • a user is deleted (not really dirtied, just rely on a FK to delete the associated record)
  • a new album is created
  • an album's parent is changed
  • an album's permissions are changed
  • an album is deleted (not really dirtied, just rely on a FK to delete the associated record)

Also we should add a button to the Settings screen that allows an administrator to rebuild the permissions cache.

I've been thinking about this a bit more, and due to the complexity of the logic, it needs to be implemented in code. This could be cached in the database in a similar way to how we cache disk usage in Blue Twilight Cloud (e.g. a flag indicating the permission cache is dirty.) The cache will need to be dirtied when the following is true: - [x] ~~a new user is added~~ - not needed as groups don't change until a user is edited - [x] a user's group memberships are changed - [x] a user is deleted (not really dirtied, just rely on a FK to delete the associated record) - [x] a new album is created - [x] an album's parent is changed - [x] an album's permissions are changed - [x] an album is deleted (not really dirtied, just rely on a FK to delete the associated record) Also we should add a button to the Settings screen that allows an administrator to rebuild the permissions cache.
aheathershaw ha aggiunto l'etichetta
wip
2018-09-14 18:28:11 +01:00
Autore
Proprietario

Subject to testing, this is now complete.

Subject to testing, this is now complete.
aheathershaw chiuso questo probleam 2018-09-16 22:14:49 +01:00
aheathershaw rimosso l'etichetta
wip
2018-09-16 22:14:54 +01:00
Autore
Proprietario

Re-opening this as unless a user is specifically granted permissions via their groups, they cannot see albums that even an anonymous user can!

Therefore every user should inherit the same permissions as anonymous users.

This means we do need to rebuild the cache when a new user is activated or created.

Re-opening this as unless a user is specifically granted permissions via their groups, they cannot see albums that even an anonymous user can! Therefore every user should inherit the same permissions as anonymous users. This means we do need to rebuild the cache when a new user is activated or created.
aheathershaw riaperto questo problema 2018-10-10 19:17:57 +01:00
Autore
Proprietario

I've implemented this by doing an additional security check against anonymous users first - if that passes, it applies to logged-in users as well, otherwise it falls back to checking a specific user.

I've implemented this by doing an additional security check against anonymous users first - if that passes, it applies to logged-in users as well, otherwise it falls back to checking a specific user.
aheathershaw chiuso questo probleam 2018-11-18 21:38:23 +00:00
Effettua l'accesso per partecipare alla conversazione.
Nessuna etichetta
bug
enhancement
feature
invalid
task
wip
Nessuna milestone
Nessuna assegnatario
1 Partecipanti
Notifiche
Data di scadenza
La data di scadenza non è valida o fuori intervallo. Si prega di utilizzare il formato 'aaaa-mm-dd'.

Nessuna data di scadenza impostata.

Dipendenze

Nessuna dipendenza impostata.

Riferimento: aheathershaw/blue-twilight#71
No description provided.