feat: add dataset.create_items_public_url and key_value_store.create_keys_public_url#453
feat: add dataset.create_items_public_url and key_value_store.create_keys_public_url#453
Conversation
tobice
left a comment
There was a problem hiding this comment.
Nothing surprising here, so here is my approve 👍 I'll leave the Python stuff to Python experts.
| view=view, | ||
| ) | ||
|
|
||
| if dataset and 'urlSigningSecretKey' in dataset: |
There was a problem hiding this comment.
Hmmm starting here the rest of the function seems identical to the async variant. Now sure if there is a nice way to reuse the code? I guess this is how we do it here in the client? 🤔
There was a problem hiding this comment.
not sure about this, but this is how every other methods are written. WDYT @janbuchar?
There was a problem hiding this comment.
We sure do repeat stuff here, but no need to change that now. Perhaps one day we can generate the whole client lib from an OpenAPI spec. Until then, this will have to do
src/apify_client/_utils.py
Outdated
| Returns: | ||
| str: Base62 encoded signature | ||
| """ | ||
| signature = hmac.new(secret_key.encode('utf-8'), message.encode('utf-8'), hashlib.sha256).hexdigest()[:30] |
There was a problem hiding this comment.
Just curious: Why the encode all over the place? 🤔
There was a problem hiding this comment.
because hmac.new accepts bytes, not strings
tests/integration/key_value_store.py
Outdated
|
|
||
|
|
||
| class TestKeyValueStoreSync: | ||
| def test_key_value_store_should_create_public_keys_expiring_url_with_params( |
There was a problem hiding this comment.
| def test_key_value_store_should_create_public_keys_expiring_url_with_params( | |
| def test_key_value_store_should_create_expiring_keys_public_url_with_params( |
tests/integration/key_value_store.py
Outdated
| # Assert that the request is now forbidden (expired signature) | ||
| httpx_client = httpx.Client() | ||
| response_after_expiry = httpx_client.get(keys_public_url, timeout=5) | ||
| assert response_after_expiry.status_code == 403 |
There was a problem hiding this comment.
So these tests are actually executed against real APIs? No mocking? Nice.
There was a problem hiding this comment.
Yeah, i was also surprised 😅
And we can actually test it against multi-staging as well. Would love to see this also in JS client
janbuchar
left a comment
There was a problem hiding this comment.
LGTM, one publishing-related issue
When storage resources (Datasets or Key-Value Stores) are set to Restricted, accessing or sharing their data externally becomes difficult due to limited permissions. This PR introduces functionality to generate signed URLs that allow controlled external access to these resources without adding token to the request.
This PR introduces methods to generate signed URLs for Dataset items and Key-Value Store records:
Datasets
dataset(:datasetId).create_items_public_url(options, expires_in_millis)→ Returns a signed URL like:
/v2/datasets/:datasetId/items?signature=xxxKey-Value Stores
key_value_store(:storeId).create_keys_public_url(options, expires_in_millis)→ Returns a signed URL like:
/v2/key-value-stores/:storeId/keys?signature=xxx🕒 Expiration:
The
expires_in_millisparameter defines how long the signature is valid.Note 1: The signature is included only if the token has WRITE access to the storage. Otherwise, an unsigned URL is returned.
P.S. We're not yet exposing
urlSigningSecretKeyfor datasets, it will be released after PR is merged.More context here
Same PR in JS apify/apify-client-js#720