specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
62 stars 36 forks source link

Guest users can edit statistics #4044

Open grantfitzsimmons opened 11 months ago

grantfitzsimmons commented 11 months ago
image

https://discourse.specifysoftware.org/t/enable-anonymous-guest-access-in-specify-7/992

Should be disabled for guest users.

http://ichthyology.specify.ku.edu/specify/stats

maxpatiiuk commented 11 months ago

Hmm, why? If user has access to creating/running queries, then they effectively have what is needed for the stats page

Or is your objection against anonymous users being able to change the layout of the stats page?

realVinayak commented 11 months ago

Stats page is hidden if users don't have permissions to run a query. In theory, you can open multiple tabs for query builder and do the same "attack" you would do with stats page. So, I don't think disabling stats page would prevent someone overloading the server.

Anonymous users can't change the collection layout, only their own layout.

I'd vote for not fixing this issue, unless there is deeper reason.

grantfitzsimmons commented 11 months ago

Disagree. For people with anonymous/guest users configured, they want them to be able to query and use statistics, but not create their own.

grantfitzsimmons commented 11 months ago

@maxpatiiuk Consider this– anonymous users can enter whatever text they want in the category name and create whatever queries they want, then those changes will be shown to the next guest user. It's not something we'd want to encourage...

maxpatiiuk commented 11 months ago

I see. And the same problem could exist for any other part of specify - anonymous users creating weirdly named record sets or queries? (assuming they have those permissions)

So the only reason this case is different is that anonymous user is more likely to have stats page access than they are to have access to creating record sets/queries?

grantfitzsimmons commented 11 months ago

Here are the permissions given to the guest user on the KU Ichthyology deployment:

image Screenshot 2023-09-27 at 8 33 42 PM

They can create queries, but they cannot save or modify any other data. That is, with one notable exception– the stats page.

maxpatiiuk commented 11 months ago

The stats page is stored as a user app resource Just like user preferences are stored as a user app resource There is no feature to restrict access to user resources since you are the owner of those resources Of course, anonymous user is stretching the definition of "owner", so I can see why you would want a different behavior here

Side note: do you see issue with anonymous users changing background color or font family in weird ways? also, if we lock this down, that would also lock it down for the demo instance? or do you want this to be a new permission, that would be not given by default, but that demo database will have?

grantfitzsimmons commented 11 months ago

Side note: do you see issue with anonymous users changing background color or font family in weird ways? also, if we lock this down, that would also lock it down for the demo instance? or do you want this to be a new permission, that would be not given by default, but that demo database will have?

I guess this gets to the root of the issue– we do not manage guests differently than any other specify user. Ideally, guest preferences would be reset for each user and not permanent, but I understand why we don't want to use our time building that.

I think even preferences should be a policy given to a user explicitly, so I do see this as something we should resolve.

realVinayak commented 11 months ago

@maxpatiiuk can we skip saving preferences to backend if guest user? this would fix this.

That way, users who want to test preferences would still be able to do it. Stats page would work too. It would even persist if the users refresh the page or move from one tab to another. It would not persist if they close the entire page or if some other user goes to specify. exactly what we want?

maxpatiiuk commented 11 months ago

great idea!

as long as admins can still set default user preferences though app resources

might also be worth adding a institution pref for this in the future once we have new institution prefs (as I could see people who are setting up a new database wanting to do a lot of changes in bulk to the anonymous user to set up defaults nicely)

emenslin commented 1 month ago

Can recreate in edge (7.9.6)