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

FEEDBACK on REQUEST ACCESS EXTERNAL TOOL #7219

Closed
wants to merge 45 commits into from

Conversation

mdmADA
Copy link
Contributor

@mdmADA mdmADA commented Aug 24, 2020

I am creating this pull request to ask for feedback on the experimenting I have done with the Dataverse code to see how ADA can launch a Request Access tool as an 'External Tool'. The idea is to replace the usual 'Request Access' buttons with 'Request Access' buttons that will call an external tool that will then manage the request access details, etc.

Danny has also indicated that the dataset and file pages are being redesigned so perhaps it is good timing in that it might be possible to incorporate this ExternalTool solution for Request Access in with the new code.

Note that my implemented code is not necessarily the 'ideal' code as a few of the methods are repeated between the Dataset-level code and the Page-level code just to get it working for demo purposes. These repeated methods would be better implemented in a single class (FileDownloadHelper? other?) but in the interest of time for demo purposes, I added them where they are now.

I have implemented and deployed this code on our test Dataverse installation: archive-deve.ada.edu.au. You can test it out with the following datasets although you will have to create an account to do so:

all restricted:
https://archive-dev.ada.edu.au/dataset.xhtml?persistentId=doi:10.80408/5Z2BAF

1/2 restricted, 1/2 unrestricted, with guestbook (guestbook at download works as expected):
https://archive-dev.ada.edu.au/dataset.xhtml?persistentId=doi:10.80408/R5LSJX

all unrestricted and with guestbook (guestbook at download works as expected):
https://archive-dev.ada.edu.au/dataset.xhtml?persistentId=doi:10.80408/DWUBPM

The code allows the following (assumption: user is logged in and does not have access to the restricted files and has not already requested access):

  1. In the dataset.xhtml page (request multiple files):
  • if there is a DatasetFileRequestAccessTool enabled in Dataverse, the Multi-select RequestAccess button (rendered by the filesFragment.xhtml component) is replaced by a 'Request Access' button that looks the same as the Regular 'Request Access' button but launches the external tool (currently represented by the ADA website with parameters passed as we haven't developed the actual external request access tool) when clicked

  • the external tool is passed the dataset_id, the user's APIkey (assumes they have enabled APIKey in their account), etc.

  • the external tool will use the Native API (or perhaps a straight database query) 'list dataset files' to pull all of the files associated with that dataset and that the user does not have access to, and will request the user to enter all the required details to request access

  • the external tool will have a 'Request Access' or 'Submit Request Details' button - when clicked this will call the Dataverse DataAccess API 'Request Access' endpoint to request access to the file(s) on behalf of the user with the API key passed to the tool by Dataverse.

[ADA will also develop functionality within our data management process that ADA request managers will be able to click buttons to call the Dataverse DataAccess API to grant/access/revoke access to a specific user].

  1. In dataset.xhtml page (request single file):
  • for each individual file in the list of files (filesFragment.xhtml) that the user has not requested access to, nor already has access to, the 'Request Access' click will launch the file-level request access tool.

  • in the filesFragment.xhtml component, the page is identified as NOT a filePg

  • if there is a fileRequestAccessTool enabled for the dataverse installation, the 'Request Access' command link that is rendered launches the external request access tool (the ADA site) and passes the dataset_id, the file_id, the user's API key, etc.

  • how the request of a single file will be managed by the external tool will be determined later - most likely, the external ADA request access tool will see if the user has already requested access to other files in this dataset. If yes, add functionality to add the file to that request.

  1. In the file.xhtml page (request single file):
  • in the filesFragment.xhtml component, the page is identified as a filePg

  • as in 2. above, if there is a fileRequestAccessTool enabled for the dataverse installation, the 'Request Access' command link rendered launches the external request access tool (the ADA site) and passes the dataset_id, the file_id, the user's API key, etc.

To implement 1, 2, and 3 above, the following is required:

  1. Creating a file-level FileRequestAccessTool ExternalTool type in the database
  • this is used in 2. and 3.
  1. Creating a dataset-level DatasetFileRequestAccessTool ExternalTool type in the database
  • this is used for the multi-select button in 1 and is implemented to prevent having to append all the (potentially many) selected file IDs to the external url tool - just call the dataset-level external tool and let the user select multiple files there.

Note: 1, and only 1, of each of the file- and dataset-level buttons must exist. I didn't code anything to enforce these conditions and this enforcing may or may not be required and instead may depend on people configuring both levels of tools properly within the database.

Java source code modified:

  1. ExternalTool.java to accommodate 'RequestAccess' type for both file and dataset level
  2. DatasetPage.java
  3. FilePage.java

.xhtml modified:

  1. dataset.xhtml
  2. file.xhtml
  3. filesFragment.xhtml
  4. file-download-button-fragment.xhtml

Additional Notes:

  1. The guestbook-at-download can still be used without any changes to the code base
  2. In the event that the request-access-popup (With terms of access/use) would be triggered in a 'regular' Dataverese workflow, this popup would be bypassed and the external ADA tool would show the terms of access/use that the user would have to agree to from within the tool.

ISSUE:

  1. how to deal with individual people requesting access via the API and bypassing the external tool.
  • want them to request access through the external tool only.
  • potential solution: write a script to see who has requested access in DV but aren't in our external tool and send an email to ADA (in our case) indicating who has done this or send an standard email to those people stating they must request access through the external tool
  1. When the external tool is used to request access, the Dataverse UI won't refresh automatically to show the user that they have requested access. The user would have to refresh the Dataverse Dataset and/or File page manually.

  2. The multi-select 'Request Access' button is enabled all the time even if access has been requested for all the restricted data files. This would have to be fixed.

mdmADA and others added 30 commits August 4, 2020 13:52
merge iqss develop into mdm develop
merge IQSS develop into mdmADA
…instance variable for the local variable returned by the method. : (
…et.xhtml (rendered as part of the filesFragment.xhtml UI component).
…er 'Request Access' if 'Request Access' has been turned off and there is a fileRequestAccessTool deployed.
…ng to turn off request access to achieve this
also use the bullhorn glyphicon for the single request access button instead of text.
merge IQSS develop into mdm develop
merge dmd develop into mdm external_tool_request_access_type
…t_single_file_RA_tool

External request access type attempt single file ra tool
@coveralls
Copy link

coveralls commented Aug 24, 2020

Coverage Status

Coverage decreased (-0.01%) to 19.467% when pulling 6da8c77 on mdmADA:external_tool_request_access_type into 25cb2eb on IQSS:develop.

if (downloadButtonAllEnabled == null) {
boolean downloadButtonAllEnabled = true;

if (downloadButtonAllEnabled) { //check to see if it should be false
Copy link
Member

@qqmyers qqmyers Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bug in current code? Add it's what you meant by "
The multi-select 'Request Access' button is enabled all the time even if access has been requested for all the restricted data files. This would have to be fixed." in your comments?

ExternalToolHandler externalToolHandler = new ExternalToolHandler(externalTool, dataset, apiToken, session.getLocaleCode());
String toolUrl = externalToolHandler.getToolUrlWithQueryParams();
logger.fine("Exploring with " + toolUrl);
PrimeFaces.current().executeScript("window.open('"+toolUrl + "', target='_blank');");
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than the original method being named 'explore', is there a reason multiple methods are needed? Unless I missed something, they all do the same thing. Perhaps explore() could be 'launchTool' or similar?

@@ -347,10 +347,28 @@

<!-- Request Access Button -->
<ui:fragment rendered="#{!fileMetadata.datasetVersion.deaccessioned and !fileDownloadHelper.canDownloadFile(fileMetadata)
and fileMetadata.dataFile.owner.fileAccessRequest}">
<!-- FILE PG -->
and (fileMetadata.dataFile.owner.fileAccessRequest or not empty fileRequestAccessTools)}">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why or? Is the internal selection of whether file access requests can be made being overridden? If so, should some/all of the UI for managing terms be removed if external RA is enabled?

return fileRequestAccessTools;
}

public ExternalTool getFileRequestAccessTool(){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the singular version is only used in the .xhtml to be able to do a null check for rendering? If so, some of the xhtml files already just check whether the list is empty (versus the first element being null). Could do that everywhere and drop these methods?

@qqmyers
Copy link
Member

qqmyers commented Sep 21, 2020

Made some comments in the code - also have some about the notes above:

re:

the external tool is passed the dataset_id, the user's APIkey (assumes they have enabled APIKey in their account), etc.

the external tool will have a 'Request Access' or 'Submit Request Details' button - when clicked this will call the Dataverse DataAccess API 'Request Access' endpoint to request access to the file(s) on behalf of the user with the API key passed to the tool by Dataverse.

  • If the tool isn't doing something with the requesting user's authority, we should probably add a way to send the user's id instead. The list of params that external tools can access via the manifest used when you register them is extensible, so this should be relatively simple to do.

re:

the external tool will use the Native API (or perhaps a straight database query) 'list dataset files' to pull all of the files associated with that dataset and that the user does not have access to, and will request the user to enter all the required details to request access

Creating a dataset-level DatasetFileRequestAccessTool ExternalTool type in the database
this is used for the multi-select button in 1 and is implemented to prevent having to append all the (potentially many) selected file IDs to the external url tool - just call the dataset-level external tool and let the user select multiple files there.
  • With the file selection being visible, it seems like people might expect it to be honored in the remote tool. That said, we know that the full list of files on some datasets is too long for the URL, so this a POST would be required to send the list of files. That can be done - I did it for the file download when you select many/all files, but it hasn't been supported for external tools yet and would require the external tool to handle POST. It's probably a good idea to let people update the selection in the remote tool, make no selection in Dataverse, so this is mostly an enhancement idea.

@qqmyers
Copy link
Member

qqmyers commented Sep 21, 2020

re: having dataset and file-level tools:

  • is there a need from the remote tool side to register as two tools? Is it required for some reason in Dataverse? Or is it just to be able to reuse the existing find Tools methods? If it is just the last, we probably could/should just implement a findRequestAccessTool() method (which could return one tool instead of a list too).

re:

In the event that the request-access-popup (With terms of access/use) would be triggered in a 'regular' Dataverese workflow, this popup would be bypassed and the external ADA tool would show the terms of access/use that the user would have to agree to from within the tool.

  • how? Or more broadly - what's the concept for managing terms and conditions? Does the external tool completely override that part of Dataverse? Does that mean those parts of the UI should be removed? Or will the tool keep the terms updated somehow? If the latter, is the idea still that the terms become read-only in Dataverse and this PR (or another) needs to turn off the editing somehow? (This is related to https://github.com/IQSS/dataverse/pull/7219/files#r492239855 as well - where it looks like the tool is overriding the selection of whether file access requests are allowed being set in the Dataverse UI. )

re: considering how to grant access to new files:
With blue/green/yellow/orange/red, etc. coming, is the expectation that external request access tools could/will have to be sensitivity-level aware and keep track of managing files in those different levels? I don't think that will affect the PR directly, but the tool might not want to assume all files are just restricted/open in its design, and if the tool is overriding some/all of Dataverse handling the terms, that part will have to keep up with whatever changes come with supporting sensitivity levels.

@qqmyers
Copy link
Member

qqmyers commented Sep 21, 2020

re:
how to deal with individual people requesting access via the API and bypassing the external tool.

  • although it wouldn't fit the external tool model well, it could be possible to make a call to the external RA tool when the API is called. That would avoid having to poll Dataverse. Would it generally be possible for an external tool to work from an API request, e.g. with no interaction with the user? Presumably if the API called the RA tool, the tool would have to send the user email to follow up in the tool UI or similar?
  • with an API call - would Dataverse still manage the same state as when the request is from the UI? I.e. in the UI, it sounds like the request is 'hijacked' by the tool and Dataverse wouldn't be setting any 'access requested' flag in the db. Will the tool be responsible for that? or will it ignore that and just set the access controls directly? If so, the API probably should be consistent.

re:
When the external tool is used to request access, the Dataverse UI won't refresh automatically to show the user that they have requested access. The user would have to refresh the Dataverse Dataset and/or File page manually.

  • the more issues that appear, the less an RA tool looks like 'one more external tool type'. If there's only one allowed, and it applies to datasets and files, etc., and all RA tools need the same params, are we just reusing the page launch mechanism? The tie here is that, while external Tools don't result in page updates - they just launch to a separate page - I think its possible to update the page state, e.g. to send a message that a request was made, etc. Is that all that has to happen? Or would you mark access-requested/update the state of the dataset/file?

@pdurbin
Copy link
Member

pdurbin commented Jan 3, 2023

@mdmADA @qqmyers I know we had a call in early October 2022 where we talked about this PR but the notes are a bit vague. What's the status, please?

Related:

@mreekie mreekie removed the bk2211 label Jan 11, 2023
@pdurbin
Copy link
Member

pdurbin commented Jul 20, 2023

Closing in favor of this new PR:

@pdurbin pdurbin closed this Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants