qri-io / qri

you're invited to a data party!
https://qri.io
GNU General Public License v3.0
1.1k stars 66 forks source link

bug(collection): list always forces the current user #1985

Closed Arqu closed 2 years ago

Arqu commented 2 years ago

If you go to https://qri.cloud/chriswhong/ and either view the dataset list logged in as a non chriswhong user or logged out, you will get your own list of datasets or an empty list respectively.

This is due to us simply passing the scope.ActiveProfile().ID on the call which should be fine for the CLI version, but doesn't account for multiple profiles or the case for cloud. This was not apparent as we used the old previews endpoints prior to this on cloud.

Arqu commented 2 years ago

A similar thing happens on /collection/get if you view another users dataset. This worked because we simply didn't utilize the profileID before on cloud.

For the 1st case we should resolve the users profile ID and supply that to get the correct collection list. For the 2nd case with get, we should simply ignore and pass an empty profileID if the requested item doesn't belong to the active profileID. Alternatively I can simply ignore the profileID on cloud but it's not ideal as we need a way to also check on that in some cases.

ramfox commented 2 years ago

WOW, missed this COMPLETELY. Thank you for investigating.

So there are two cardinatities we should be paying attentions to:

what information are you requesting

In the list endpoint, this is determined by the username param

In the get endpoint, we have a ref string or initID

Are you allowed to view that information?

Are we allowed to view a user's profile page, that includes their datasets, if we are not logged in. I think yes. Which means that right now everyone has permission to view anything. But we should still understand where in the sequence any future permissions might want to live.

Potential places:

Starting with collection first. Do our lower subsystems each need to keep track of permissions? This means that we need the same security in automation, base, logbook, etc.

It feels like the "natural" place for this to exist is in scope or dispatch, since scope is supposed to limit what the request has access to. Maybe in the future we have a kind of interface for validating permissions, and that gets run when the method is dispatched. The interface method does what it needs with the ActiveProfile & the command params to determine whether the user can request the information it is trying to request.

Or maybe this NEEDS to happen lower, because only collection can understand what should and shouldn't be shared based on the parameters and the user id.


Anyway, those are just some thoughts.

I'm working on a short term fix right now (basically just implemented what you suggested)