-
Notifications
You must be signed in to change notification settings - Fork 14
feat: add dataset.create_items_public_url and key_value_store.create_keys_public_url #453
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about this, but this is how every other methods are written. WDYT @janbuchar?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: Why the encode
all over the place? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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( |
# 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these tests are actually executed against real APIs? No mocking? Nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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=xxx
Key-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_millis
parameter 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
urlSigningSecretKey
for datasets, it will be released after PR is merged.More context here
Same PR in JS apify/apify-client-js#720
Note 2:
create_hmac_signature
would removed from client and SDK once we move it to shared package. apify/apify-shared-python#44, this PR will be merged after shared package is updated.Note 3: integration tests are expected to fail until this PR will be realeased