-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add Signed URL Support #200
Comments
If we want to help out upstream, a report came in about the "edit" API endpoint not working with signed URLs. Maybe we try to reproduce the error: |
Unless I have the wrong thing, it looks like this call requires PUT not POST: https://github.com/IQSS/dataverse/blob/52e969e1fc69be19356c955f31551d605a276b8e/src/main/java/edu/harvard/iq/dataverse/api/EditDDI.java#L92-L96 |
Yep, I tried a PUT request and it worked! Thanks @qqmyers! |
Tests to validate an implementation for gdcc#200
Tests to validate an implementation for gdcc#200
While trying this out, I found out a couple of things:
So far I only wrote unit test cases and haven't done any actual implementation yet (so the tests might still change a little bit here and there, especially the test_signed_url_flow_tracks_data_payload), I also want to add an integration test against the localhost server to see that the flow actually works. shoeffner@6386001 The branch relies on #201 , so it might become rebased or modified depending on that branch. |
How is pyDataverse supposed to be using SignedURLs?
Am I missing something? So far, I focused on 2 (but this kind of entails 1), but I haven't made up any thoughts about 3, although that might also work, in particular if we implement "caching", then we could simply fill the cache with pre-signed URLs. Another interesting thing I read is (emphasis mine):
Right now, pyDataverse mostly ignores the API version, so pyDataverse can either modify the URL before requesting a signature for it, or all routes should be changed to include the API Version (which relates to #197 and #189). Ironically, the canonical example uses |
To shed a little bit more light on my question on what should be supported, here's a little overview over the three variants. Variant 1This would be variant 1, and from what I understand, the "common" use case for signed URLs (at least from Dataverse's perspective): sequenceDiagram
participant User
participant Admin/App
participant pyDataverse
participant Dataverse
User->>Admin/App: Asks to perform action
Admin/App->>pyDataverse: Requests signed URL
pyDataverse->>Dataverse: Requests signed URL
Dataverse->>pyDataverse: Issues signed URL
pyDataverse->>Admin/App: Passes signed URL
Admin/App->>User: Forwards signed URL
User->>Dataverse: Uses signed URL to perform action
Dataverse->>User: Responds with result
For this, pyDataverse would need to be able to returned signed URLs instead of actually performing the actions. Variant 2This is, as far as I understood, more or less the idea proposed with "we should support signed URLs" in #192. sequenceDiagram
participant User
participant Admin/App
participant pyDataverse
participant Dataverse
Admin/App->>pyDataverse: Asks to perform action
pyDataverse->>Dataverse: Requests signed URL
Dataverse->>pyDataverse: Issues signed URL
pyDataverse->>Dataverse: Uses signed URL to perform action
Dataverse->>pyDataverse: Responds with result
pyDataverse->>Admin/App: Delivers result
Thinking more about signed URLs, I am not sure this is what is intended, as it eventually only increases the number of API calls. Variant 3In this scenario, the user wants to perform an action and some other App/Admin generates the signed URL which the user can then use with pyDataverse. sequenceDiagram
participant User
participant Admin/App
participant pyDataverse
participant Dataverse
User->>Admin/App: Asks to perform action
Admin/App->>Dataverse: Requests signed URL
Dataverse->>Admin/App: Issues signed URL
Admin/App->>User: Forwards signed URL
User->>pyDataverse: Provides signed URL
pyDataverse->>Dataverse: Uses signed URL to perform action
Dataverse->>pyDataverse: Responds
pyDataverse->>User: Delivers result
SummaryIn general I think the idea of using signed URLs for auth does not make too much sense on a per-request basis, at least not "transparently," that is when the result is to perform the action.
The other variant, getting a signed URL and performing the action based on it, could then be naturally combined from Variants 1 and 3, but I do not really see special use cases for that. What is your take on this? |
I started implementing version 2 and also the tests on my branch kind of assume that flow, but I realized that this is not really the way it should work. For the other two variants, pyDataverse as "requester" and "consumer", I am not sure how good implementations would look like and what @JR-1991 is planning in #189 , but I feel this should be conceptually integrated when restructuring the API. |
I checked the boto3 docs and they seem to handle this in a very direct fashion, essentially instead of performing an action they change the function call to
They also only seem to implement variant 1, i.e., generating presigned URLs ; for variant 3 they basically say: use it with curl. |
I think event hooks might be a good idea: https://www.python-httpx.org/advanced/event-hooks/ # explicit args
api = DataAccessApi(URL, auth=..., middlewares=[SignedURLHook])
# wrapper
api = SignedURLs(DataAccessApi(URL, auth=...))
# alternative per method
api = DataAccessApi(URL, auth=...)
SignedURL(api.get_datafile(...))
# inheritance
class SignedURLDataAccessApi(DataAccessApi, SignedURLApi):
pass
api = SignedURLDataAccessApi(URL, auth=...)
# extra method/property
api = DataAccessApi(URL, auth=...)
api.signed_url.get_datafile(...) I am not sure how to do it the other way around, but similar ways might work, maybe with a One option which might entail quite some refactoring would be to have a template per URL and match the template for signed URLs or realize them for non-signed URLs, depending on the use case. |
In the group meeting on August 21st, we decided to put this on hold until there is a need for it. If you are in need of this feature, please re-open the issue. |
As discussed in #192, it could be a good idea to include Signed URL support as described in https://guides.dataverse.org/en/latest/api/external-tools.html#signed-urls.
This issue is there to discuss, track, etc. that proposal.
The text was updated successfully, but these errors were encountered: