"File Downloader" role: should not allow users to access "unpublished" data, should not send link to user if role assigned but data unpublished

[sbarbosadataverse]

RT ticket: https://help.hmdc.harvard.edu/Ticket/Display.html?id=228447

user reported he has file downloader role, but when he clicks on dataset name gets an error page.

I gave myself the same role with a non superuser account and I get the same error.

when you login you see the dataset is : draft unpublished file downloader clearly labeled
attempting to access throws error: NOT AUTHORIZED

[mercecrosas]

Changed this to Critical - needs to be fixed in the next patch.

[sbarbosadataverse]

reproduced in Demo site:
gave myself access to a draft/unpublished dataset
went in and clicked on the link in my notification and got the same error. I didn't navigate directly to the dataset but will do that as well to see if anything changes.

@pdurbin in demo...when I am notified I have been given "file downloaded access" there is a link to the dataset---but that link also throws an error. seems like another bug. If a user gets a role, gets a link, the link should work if they are logged in. Not sure how to report this.

[pdurbin]

It looks like I can reproduce this on my laptop in the 4.3 branch (I'm on commit ea290e4).

Here's the "spruce" user giving access to a restricted file to the "finch" user:

Then "finch" logs in and clicks on either "trees.png" or "Spruce Goose" from MyData...

... and gets an error when trying to go to http://localhost:8080/dataset.xhtml?persistentId=doi:10.5072/FK2/QBQGQG

[pdurbin]

I pushed 8f0549a to a new branch as a proposed fix. Ideally it would be code reviewed. I'll see if @sekmiller or @raprasad or @scolapasta is available.

[pdurbin]

@scolapasta I'm passing this ticket to you to review the summary of my conversation with @sekmiller (and the commit itself) at 8f0549a and decide if we should merge the fix as-is or not.

[sbarbosadataverse]

@scolapasta @pdurbin @eaquigley

if "file downloaders" shoudn't access unpublished, that needs to be made clear here too as there is no reference to "published/unpublished" in these descriptions

[sbarbosadataverse]

I updated the issue title to fit the actual bug. Did I miss anything?

[eaquigley]

Updated the json file for this role description (forget it was in json not bundle)

[pdurbin]

Updated the json file for this role description (forget it was in json not bundle)

@eaquigley please check out the comment I left on that commit you just made: c8e49f2 . The tl;dr is that someone with the "File Downloader" role is able to download files even if they are not published.

[pdurbin]

@eaquigley basically, I think we should revert c8e49f2 since it misrepresents how the system actually behaves.

I updated the issue title to fit the actual bug. Did I miss anything?

@sbarbosadataverse you changed the title to '"File Downloader" role: should not allow users to access "unpublished" data, should not send link to user if role assigned but data unpublished' which more or less makes sense but to make this more clear and actionable for a developer, let me try to express it this way:

• When the file downloader role/permission is assigned at the dataset level, rather sending a notification with a link to a dataset that the user is unable to view ("Not Authorized"), the notification should... what? Not be sent? Not include a link? It sounds like you're saying a notification shouldn't be sent.
• When someone has the "File Downloader" role on a file, MyData currently displays file cards with links to datasets that can not be viewed ("Not Authorized"). Rather than this, we should... provide a link to download the file? (Both @landreev and @raprasad suggested this.) Or should something else happen?

[sbarbosadataverse]

Notifications should not be sent until dataset is published. Liz and I
agreed on this yesterday.

No file card should be displayed under my data/ no notification of such
dataset under mydata, until dataset is published.

File downloader active role is dependent on dataset being "published."
On Oct 16, 2015 9:12 AM, "Philip Durbin" notifications@github.com wrote:

@eaquigley
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_eaquigley&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=cpfTaN7DjlB7G6K6JaAEobiqY_P1DYI_0KRhu0NbjSE&e=
basically, I think we should revert c8e49f2
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_commit_c8e49f2001171ae0e9baab7e9acefeb7ff5a121b&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=ZGkfLE_3bz2EoAqDm1atkAaZfoECZEm0YDZGjc6DcpE&e=
since it misrepresents how the system actually behaves.

I updated the issue title to fit the actual bug. Did I miss anything?

@sbarbosadataverse
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sbarbosadataverse&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=Tn_a4yc5mfte9fOZntI2LSCu7QdwqdBo_S-a4qGo95s&e=
you changed the title to '"File Downloader" role: should not allow users to
access "unpublished" data, should not send link to user if role assigned
but data unpublished' which more or less makes sense but to make this more
clear and actionable for a developer, let me try to express it this way:

• When the file downloader role/permission is assigned at the dataset
  level, rather sending a notification with a link to a dataset that the
  user is unable to view
  https://urldefense.proofpoint.com/v2/url?u=https-3A__cloud.githubusercontent.com_assets_8322346_10518376_1b05ec6e-2D732f-2D11e5-2D8df1-2D421e6b24004a.png&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=GX_1J8TRG4yEVpOA_xnLpc3bKLXf7OxjclXZdRMRqVY&e=
  ("Not Authorized"), the notification should... what? Not be sent? Not
  include a link? It sounds like you're saying a notification shouldn't be
  sent.
• When someone has the "File Downloader" role on a file, MyData
  currently displays file cards with links to datasets that can not be
  viewed
  https://urldefense.proofpoint.com/v2/url?u=https-3A__cloud.githubusercontent.com_assets_8322346_10517621_84351358-2D732b-2D11e5-2D9717-2Db66b008b4c65.png&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=1rxsoLylWJZPmVt1HNwEjnSZUxHjTCSKxyBKcZ7HzKc&e=
  ("Not Authorized"). Rather than this, we should... provide a link to
  download the file? (Both @landreev
  https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_landreev&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=tkP2snoQQ3LMjDXtxnmZpFENkpd9oprfhyAlzYUsCQ0&e=
  and @raprasad
  https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_raprasad&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=Uo5wJyldNkIxrdg6VHd0m0jVVLOXjTSGwQ0W6opPO8o&e=
  suggested this.) Or should something else happen?

—
Reply to this email directly or view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_issues_2645-23issuecomment-2D148713191&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=rao0QftfoUIRijMCrLIg0siH9pyRp7CxbhAvlF3td70&s=cAa7JQBgkgx4QYkikrAEiS5h0Mxdm6pEFp8y52hAvDw&e=
.

[sekmiller]

Right now there's no mechanism by which we can delay the sending of a notification. We can add it and then have the publish command check to see if there are pending file downloader notifications waiting to be sent.

Are there any other circumstances under which it would be appropriate/necessary to delay the sending of a notification?

[sbarbosadataverse]

This case in particular. File downloader itself is for "published" data. If
admins are allowed to set this role before data are published,
notifications should not be sent. The other option is to NOT allow this
role to be set pre-publishing. I'm ok with either one.
Liz may want to chime in.

On Fri, Oct 16, 2015 at 10:36 AM, Stephen Kraffmiller <
notifications@github.com> wrote:

Right now there's no mechanism by which we can delay the sending of a
notification. We can add it and then have the publish command check to see
if there are pending file downloader notifications waiting to be sent.

Are there any other circumstances under which it would be
appropriate/necessary to delay the sending of a notification?

—
Reply to this email directly or view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_issues_2645-23issuecomment-2D148734197&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=DhMAc-yOTaNwNiqOcPKllLy43SFJ7Dfsh8wk5T3KS7c&s=7SsPZ6E4-wjs6SnMd4VG5MBdKxtAPtgi5NYTzkXT5Vg&e=
.

Sonia Barbosa
Manager of Data Curation, IQSS Dataverse Network
Manager of the Murray Research Archive, IQSS
Data Science
Harvard University

Dataverse 4.0 is now available for use!
http://dataverse.harvard.edu

All test dataverses should be created in 4.0 Demo!
http://dataverse-demo.iq.harvard.edu/

Join our Dataverse Community!
https://groups.google.com/forum/#!forum/dataverse-community

[pdurbin]

File downloader itself is for "published" data.

@sbarbosadataverse no, it's not. That's why I'm arguing that c8e49f2 should be reverted because saying that the file dowloader role only applies to published data is a misrepresentation of that role given how the code works today. I gave demos of this yesterday and explained how if you know the file id you can download unpublished files if you have the file downloader role: c8e49f2

[sbarbosadataverse]

I understand how it's working is wrong. That's the issue. The code is
working incorrectly.
On Oct 16, 2015 10:54 AM, "Philip Durbin" notifications@github.com wrote:

File downloader itself is for "published" data.

@sbarbosadataverse
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_sbarbosadataverse&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=5XquOTEcLIsyxjM36i_AKSE6QrAkH3deWjZ5FpxGI2s&s=GsaMFYBLFirGCsa_i0CL-GsLV-fEJbJFy5MJ0A2zRBI&e=
no, it's not. That's why I'm arguing that c8e49f2
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_commit_c8e49f2001171ae0e9baab7e9acefeb7ff5a121b&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=5XquOTEcLIsyxjM36i_AKSE6QrAkH3deWjZ5FpxGI2s&s=6pjGCKPT8UngRaOuj8my-RzzsZqbSCGsckvFKk2yD9c&e=
should be reverted because saying that the file dowloader role only applies
to published data is a misrepresentation of that role given how the code
works today. I gave demos of this yesterday and explained how if you know
the file id you can download unpublished files if you have the file
downloader role: c8e49f2
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_commit_c8e49f2001171ae0e9baab7e9acefeb7ff5a121b&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=5XquOTEcLIsyxjM36i_AKSE6QrAkH3deWjZ5FpxGI2s&s=6pjGCKPT8UngRaOuj8my-RzzsZqbSCGsckvFKk2yD9c&e=

—
Reply to this email directly or view it on GitHub
https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_IQSS_dataverse_issues_2645-23issuecomment-2D148738491&d=BQMCaQ&c=WO-RGvefibhHBZq3fL85hQ&r=8R6PzVqt1PEocauQgZMGXsGz29-nb19M7eqlo1d8EVs&m=5XquOTEcLIsyxjM36i_AKSE6QrAkH3deWjZ5FpxGI2s&s=2hnRWcEKWJ7gwT46XMq0jG5nqxDAuzzcaa6Sr-g3rMM&e=
.

[eaquigley]

@sekmiller yes, when we bring embargoed datasets in as a functionality, we would need a delayed notification sent to someone if they have "subscribed" to the dataset to let them know it has been released.

@pdurbin @sbarbosadataverse I think this ticket is getting a bit long winded and I should create tickets based off the email with the 5 issues in it that i sent yesterday. we also need a ticket for the file ID downloading bug @pdurbin mentioned.

[pdurbin]

I understand how it's working is wrong. That's the issue. The code is working incorrectly.

@sbarbosadataverse I wanted to confirm with @scolapasta that the code isn't working as desired, which he did, so I created #2648 about the bug that if you know the file id and have the DownloadFile permission you can download files when they haven't been published.

[pdurbin]

When someone has the "File Downloader" role on a file, MyData currently displays file cards with links to datasets that can not be viewed ("Not Authorized"). Rather than this, we should... provide a link to download the file? (Both @landreev and @raprasad suggested this.) Or should something else happen?

@scolapasta and I discussed this and we decided that what should happen is that MyData should not show the cards in the first place. I created a separate issue about this: #2649.

[pdurbin]

I think this ticket is getting a bit long winded and I should create tickets

@eaquigley yes, please create more issues. These are the three I've already created for this issue:

• Bug: unpublished files can be downloaded by users with the File Downloader role (DownloadFile permission) on that file or inherited from dataset or dataverse #2648 Downloads
• MyData: the "File Downloader" role alone should not be enough for cards of unpublished files to be displayed #2649 MyData
• File Downloader notifications #2654 Notifications ( @eaquigley please touch base with @sekmiller about what's possible in a reasonable time frame )

Some of the issues you emailed about feel like post 4.2.1 issues to me since 4.2.1 is about performance and stability. We should definitely capture the ideas, though! If an FRD is better, that's fine too. I'm giving this issue to you @eaquigley to decide how best to capture those ideas. Please pass this back to @kcondon when you're done.

[pdurbin]

@eaquigley captured many ideas in a variety of issues (thanks!):

• User Guide: Permissions + Publish Dataset/Dataverse Need More Info #2653
• File Downloader: Name of accounts with inherited permissions are not appearing on files permission page #2655
• Assign Role Pop Up: File Downloader Assignment Confirmation #2657
• Assign Role Pop Up: Carry Role Descriptions Over #2658
• File Permissions: Change Name to Reflect it is only for restricted files #2659

@kcondon I'm passing this to QA to decide what to do with this issue. In my opinion, if #2654 and #2649 pass QA, that's enough to reply to the user in https://help.hmdc.harvard.edu/Ticket/Display.html?id=228447 to say that the bugs have been fixed. I'm still planning to work on #2648 for 4.2.1 but it's not really the bug reported by the user; it's a bug I discovered while poking around in this code. Since it's security-related I think it's worth looking at.

[kcondon]

@pdurbin @eaquigley @scolapasta @sbarbosadataverse @sekmiller
It appears this was the initial ticket that subsequently was broken up into other related tickets. So, it can be difficult to test and close this one unless we say the child tickets adequately represent the issues. It sounds like from various discussions that having the file card not appear incorrectly and not sending notifications prematurely are the main issues. There are two tickets for these, the ones Phil mentioned, Liz also mentioned adding some messaging. I don't know if there is a ticket for that.
I am closing this ticket as a dupe/ parent ticket and will test the remaining notification ticket when it arrives.

[pdurbin]

I pushed 8f0549a to a new branch as a proposed fix.

I'm just noting here that since we didn't go with this fix I'm deleting the branch it was in: 2645-file-downloader-view-dataset