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

embargoed datasets should say that they are embargoed #1350

Closed
bendichter opened this issue Oct 31, 2022 · 19 comments · Fixed by #2060
Closed

embargoed datasets should say that they are embargoed #1350

bendichter opened this issue Oct 31, 2022 · 19 comments · Fixed by #2060
Assignees
Labels
bug Something isn't working embargo Issues around embargo functionality released This issue/pull request has been released. UX Affects usability of the system

Comments

@bendichter
Copy link
Member

Right now an embargoed dataset looks like this to anyone that is not an admin and is not an owner:

image

It would be better if this page showed a message indicating that this dandiset is embargoed.

@bendichter bendichter added the enhancement New feature or request label Oct 31, 2022
@yarikoptic
Copy link
Member

yarikoptic commented Nov 1, 2022

I think it

  • relates a change which yet to be released -- show that smth went wrong , and in this case -- Permission denied:
    image
  • I believe it is fixed in staging but not released where it should at least show smth like
    image
  • but in the long run we do need more user friendly and specific DLP for such cases. https://github.com/dandi/dandi-archive/blob/HEAD/doc/design/embargo-full.md#non-private-embargoed-dandisets outlines an additional type of embargoed dandiset (RESTRICTED) which I believe is nohow implemented yet. So immediate fix should be only to show informative message that dandiset is embargoed and neither its metadata nor data is accessible. And when RESTRICTED implemented -- show DLP.

More of related issues for backrefs:

@waxlamp waxlamp added bug Something isn't working UX Affects usability of the system and removed enhancement New feature or request labels Mar 2, 2023
@waxlamp waxlamp self-assigned this Mar 2, 2023
@waxlamp waxlamp added this to the Web app usability milestone Mar 2, 2023
@waxlamp
Copy link
Member

waxlamp commented Mar 10, 2023

@bendichter, @yarikoptic: my initial gut reaction is that embargoed dandisets should appear the same as non-existent dandisets do:
404

This is a standard design meant to prevent the leaking of any information about the protected resource--even that it exists. But in DANDI's case, that may not be the correct thing. Since DANDI is a publicly accessible data archive, is it ok to explicitly inform people that a dandiset exists but is under embargo?

If yes, then we can create a screen that responds to a 401 with an appropriate message; if no, then we can reuse the 404 response for this case.

@satra
Copy link
Member

satra commented Mar 10, 2023

i don't have a problem with this for the moment.

however, i would also like to consider a social side to this where a group can make an embargoed dataset, but make the metadata/DLP public. this may encourage other groups to reach out to collaborate, collect similar, more data, etc.,. i think any step we can take to nudge this community out of silos we should.

@waxlamp
Copy link
Member

waxlamp commented Mar 10, 2023

i don't have a problem with this for the moment.

With what--showing as 404, or as 401?

(edited to add: I assume you're ok with exposing the existence of an embargoed dandiset, given your idea about open metadata and collaboration below--but I want to make sure.)

however, i would also like to consider a social side to this where a group can make an embargoed dataset, but make the metadata/DLP public. this may encourage other groups to reach out to collaborate, collect similar, more data, etc.,. i think any step we can take to nudge this community out of silos we should.

This is a good idea, but I'm going to file a separate issue to track it as there will be further design considerations / requirements gathering necessary to do it.

@bendichter
Copy link
Member Author

If a user tries to access an embargoed dataset but forgets to log in, I think it would be more confusing for them get a "Permission denied" error than a "Does not exist" error.

Omitting this for security reasons might have made sense if we were using uuids for identifiers, but with our current format is that really helping?

@waxlamp
Copy link
Member

waxlamp commented Mar 10, 2023

If a user tries to access an embargoed dataset but forgets to log in, I think it would be more confusing for them get a "Permission denied" error than a "Does not exist" error.

Do you mean in the case where that person is an owner of the dandiset? We can account for that simply by saying "if you are an owner of this dandiset, please log in to see it" or something like that.

Omitting this for security reasons might have made sense if we were using uuids for identifiers, but with our current format is that really helping?

I think the strict answer is "yes" (even with numeric IDs, you can't know if it's a 404 because it doesn't yet exist, it once existed but was deleted, or exists and is embargoed), but the larger issue here is simply that this is not meant to be as "secure" as the typical archive would be. That is, I just want to confirm what the "security" semantics of embargoed dandisets should be--pretend like they don't exist, or be open about the fact that they do exist but are under embargo. My gut is shifting to the latter interpretation.

@bendichter
Copy link
Member Author

Do you mean in the case where that person is an owner of the dandiset? We can account for that simply by saying "if you are an owner of this dandiset, please log in to see it" or something like that.

That would be an improvement over the current UX and would fix the problem 90%. I think I'd slightly prefer a message specifically for embargoed dandisets, but mostly I found the incorrect message confusing.

you can't know if it's a 404 because it doesn't yet exist, it once existed but was deleted, or exists and is embargoed

That is true, but my point is if you are looking for embargoed data all you would have to do is try the ~50 or so gaps in the available dandiset ids. Yes, some of those will be deleted or will have never existed, but you have a short list of IDs where a good portion of those dandisets are embargoed so any attempt to break in can be iterated over that list.

@waxlamp
Copy link
Member

waxlamp commented Mar 10, 2023

Do you mean in the case where that person is an owner of the dandiset? We can account for that simply by saying "if you are an owner of this dandiset, please log in to see it" or something like that.

That would be an improvement over the current UX and would fix the problem 90%. I think I'd slightly prefer a message specifically for embargoed dandisets, but mostly I found the incorrect message confusing.

Oh, perhaps we've been talking past each other: my intention here would be to say something like "This is an embargoed dandiset. If you are an owner, you can see this dandiset by logging in" or something to that effect.

you can't know if it's a 404 because it doesn't yet exist, it once existed but was deleted, or exists and is embargoed

That is true, but my point is if you are looking for embargoed data all you would have to do is try the ~50 or so gaps in the available dandiset ids. Yes, some of those will be deleted or will have never existed, but you have a short list of IDs where a good portion of those dandisets are embargoed so any attempt to break in can be iterated over that list.

You're right that out choice of ID type does make it easier to carry out this sort of attack. But (not to beat a dead horse) we can avert such "attacks" by deciding that embargo status is a matter of public information.

@satra
Copy link
Member

satra commented Sep 8, 2023

my intention here would be to say something like "This is an embargoed dandiset. If you are an owner, you can see this dandiset by logging in" or something to that effect.

can we have this implemented? seems fine to me. it does seem like we have:

  • embargoed
  • does not exist
  • normal DLP

@bendichter
Copy link
Member Author

@waxlamp Circling back about this. Then not logged in and going to an embargoed DLP, the current user interface displays as:

Sorry, something went wrong on our side (the developers have been notified). Please try that operation again later.

image

I am imagining a use-case where someone has a link to a dandiset that a lab-mate has posted, but is not a co-owner. They may not be familiar with the "embargo" UX, and they click the link and get this page. I think this would be confusing to that user, because it indicates that this is an internal error, when it is in fact a permissions issue that they can resolve themselves by requesting co-ownership. It would be nicer to have a page that says something like:

If no logged in: "This is an embargoed dandiset. If you are an owner, you can see this dandiset by logging in."

If logged in but not owner: "This is an embargoed dandiset. Only owners of the dandiset can view it."

@yarikoptic
Copy link
Member

This sounds like a relatively trivial development to be done. @waxlamp could you please prioritize this in the queue. Please let me know if issue actually requires some non-trivial development or you need help.

@yarikoptic yarikoptic added the embargo Issues around embargo functionality label Oct 9, 2024
@kabilar
Copy link
Member

kabilar commented Oct 10, 2024

Thanks, Yarik. We will prioritize this issue.

@kabilar
Copy link
Member

kabilar commented Oct 10, 2024

Hi @yarikoptic, this issue has been moved to the top of the queue - 20241021 Engineering Core.

@waxlamp waxlamp assigned jjnesbitt and unassigned waxlamp Oct 24, 2024
@jjnesbitt
Copy link
Member

I'm working on this now and have the following prototype:

image

This is shown in either case of the user not being logged in, or the user being logged in but not an owner on the dandiset.

Any thoughts?

@kabilar
Copy link
Member

kabilar commented Oct 25, 2024

Thank you, Jake. This looks good to me.

@kabilar
Copy link
Member

kabilar commented Oct 25, 2024

@bendichter What do you think?

@satra
Copy link
Member

satra commented Oct 25, 2024

i think the second sentence needs some clarity to indicate. if you are a member of the embargoed dandiset project, contact the owner for enabling access.

the current wording seems to suggest anyone can contact the owner, and that information is not available.

@jjnesbitt
Copy link
Member

How's this?

image

@dandibot
Copy link
Member

🚀 Issue was released in v0.3.109 🚀

@dandibot dandibot added the released This issue/pull request has been released. label Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working embargo Issues around embargo functionality released This issue/pull request has been released. UX Affects usability of the system
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants