feat: Add option to generate pre-signed URL with expiration time #180

andrewalc · 2023-03-29T20:03:18Z

Addresses:

Presigned URLs #78

PR branched off of:

Add option to generate a presigned url with a expiration time #117

Updated test with refactoring request that was mentioned here: https://github.com/parse-community/parse-server-s3-adapter/pull/117/files#r676915013

Signed-off-by: Daniel San <[email protected]>

…peration in the getSignedUrl function Signed-off-by: Daniel San <[email protected]>

Signed-off-by: Daniel San <[email protected]>

penchef · 2023-03-31T16:23:56Z

can't wait for it ! 😱

codecov · 2023-04-21T15:12:30Z

Codecov Report

Patch coverage: 96.29% and project coverage change: -0.16 ⚠️

Comparison is base (73b17e4) 95.72% compared to head (fadb515) 95.56%.

❗ Current head fadb515 differs from pull request most recent head 807e51c. Consider uploading reports for the commit 807e51c to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #180      +/-   ##
==========================================
- Coverage   95.72%   95.56%   -0.16%     
==========================================
  Files           2        2              
  Lines         187      203      +16     
  Branches       41       43       +2     
==========================================
+ Hits          179      194      +15     
- Misses          8        9       +1

Impacted Files	Coverage Δ
index.js	`95.08% <95.65%> (-0.38%)`	⬇️
lib/optionsFromArguments.js	`96.29% <100.00%> (+0.19%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

parse-github-assistant · 2023-04-22T14:07:56Z

Thanks for opening this pull request!

❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

mtrezza · 2023-04-22T14:10:58Z

lib/optionsFromArguments.js

@@ -93,6 +95,8 @@ const optionsFromArguments = function optionsFromArguments(args) {
  options = fromEnvironmentOrDefault(options, 'baseUrlDirect', 'S3_BASE_URL_DIRECT', false);
  options = fromEnvironmentOrDefault(options, 'signatureVersion', 'S3_SIGNATURE_VERSION', 'v4');
  options = fromEnvironmentOrDefault(options, 'globalCacheControl', 'S3_GLOBAL_CACHE_CONTROL', null);
+  options = fromEnvironmentOrDefault(options, 'presignedUrl', 'S3_PRESIGNED_URL', false);
+  options = fromEnvironmentOrDefault(options, 'presignedUrlExpires', 'S3_PRESIGNED_URL_EXPIRES', 900);


How did you get to 900? Is this a random value you chose?

i think the previous PR owner figured 15 mins was a good default value, should this be changed?

Does the AWS API use a default value if none is supplied?

https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/S3.html#getSignedUrl-property

Oh it does look like s3's getSignedURL defaults Expires to 900 seconds

Options Hash (params):

Expires (Integer) — default: 900 — the number of seconds to expire the pre-signed URL operation in. Defaults to 15 minutes.

So we don't have to set a default value, as the S3 SDK will set that, right? If so, I'd suggest to only send a value to the S3 SDK if the Parse Server option is set, otherwise not send anything and let the default value of the SDK apply. You could add a line to the Parse Server option table in the README that if no value is set, the AWS S3 SDK default value applies.

spec/test.spec.js

jimnor0xF · 2023-05-02T07:29:11Z

Wouldl love this!

mtrezza

Looks good, let's wait for the CI to pass and I think we can merge this.

# [2.1.0](2.0.2...2.1.0) (2023-05-12) ### Features * Add option to generate pre-signed URL with expiration time ([#180](#180)) ([d92363d](d92363d))

parseplatformorg · 2023-05-12T18:29:08Z

🎉 This change has been released in version 2.1.0

dblythy · 2023-05-17T04:27:16Z

Nice work @danielsanfr and @andrewalc!

danielsanfr and others added 16 commits November 17, 2020 00:41

Add option to generate a presigned url with a expiration time

642feac

Signed-off-by: Daniel San <[email protected]>

Update README.md

6495047

Signed-off-by: Daniel San <[email protected]>

Uses AWS S3 presigned url together with baseUrl

ea46045

Signed-off-by: Daniel San <[email protected]>

Improves code to decrease complexity

1c4948c

Signed-off-by: Daniel San <[email protected]>

Simplify code complexity using "function early return" concept

1f82746

Signed-off-by: Daniel San <[email protected]>

Update README.md

e901d51

Signed-off-by: Daniel San <[email protected]>

Fix get presingedUrl and presignedUrl from class constructor args

1051f98

Signed-off-by: Daniel San <[email protected]>

Using the AWS S3 default expire time

9718934

Signed-off-by: Daniel San <[email protected]>

When using s3client.getSignedUrl(), avoid return old path-style URL

140bdf9

Signed-off-by: Daniel San <[email protected]>

Remove redundant code

7a3c5ca

Signed-off-by: Daniel San <[email protected]>

Add tests for presigned url

21e4d9c

Signed-off-by: Daniel San <[email protected]>

added new parameters to parameter table

d761881

Small test improvement

4b5dd8d

Signed-off-by: Daniel San <[email protected]>

Add test to ensure that the AWS S3 client receives getObject as the o…

2330ac6

…peration in the getSignedUrl function Signed-off-by: Daniel San <[email protected]>

Add comment to explain security concerns when using presigned URL

0873698

Signed-off-by: Daniel San <[email protected]>

refactored test according to previous PRs comment

0a4b80c

andrewalc mentioned this pull request Mar 29, 2023

Add option to generate a presigned url with a expiration time #117

Closed

mtrezza requested a review from a team April 3, 2023 23:00

Merge branch 'master' into feature/presigned-url

04bf89d

mtrezza changed the title ~~Add option to generate a presigned url with a expiration time~~ feat: Add option to generate pre-signed URL with expiration time Apr 22, 2023

mtrezza reviewed Apr 22, 2023

View reviewed changes

spec/test.spec.js Show resolved Hide resolved

added back original tests

1e66f66

andrewalc requested a review from mtrezza April 25, 2023 16:28

andrewalc added 2 commits May 9, 2023 11:01

Expires time is only set if defined. Defaults to AWS default value

e9797c3

updated readme with correct default val and s3 doc link

807e51c

mtrezza approved these changes May 12, 2023

View reviewed changes

mtrezza merged commit d92363d into parse-community:master May 12, 2023

mtrezza linked an issue May 12, 2023 that may be closed by this pull request

Presigned URLs #78

Closed

parseplatformorg pushed a commit that referenced this pull request May 12, 2023

chore(release): 2.1.0 [skip ci]

7b5a6da

# [2.1.0](2.0.2...2.1.0) (2023-05-12) ### Features * Add option to generate pre-signed URL with expiration time ([#180](#180)) ([d92363d](d92363d))

mtrezza mentioned this pull request May 12, 2023

Presigned URLs #78

Closed

parseplatformorg added the state:released label May 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add option to generate pre-signed URL with expiration time #180

feat: Add option to generate pre-signed URL with expiration time #180

andrewalc commented Mar 29, 2023

penchef commented Mar 31, 2023

codecov bot commented Apr 21, 2023 •

edited

Loading

parse-github-assistant bot commented Apr 22, 2023

mtrezza Apr 22, 2023

andrewalc Apr 25, 2023

mtrezza Apr 30, 2023 •

edited

Loading

andrewalc May 1, 2023

mtrezza May 2, 2023 •

edited

Loading

jimnor0xF commented May 2, 2023

mtrezza left a comment

parseplatformorg commented May 12, 2023

dblythy commented May 17, 2023

feat: Add option to generate pre-signed URL with expiration time #180

feat: Add option to generate pre-signed URL with expiration time #180

Conversation

andrewalc commented Mar 29, 2023

penchef commented Mar 31, 2023

codecov bot commented Apr 21, 2023 • edited Loading

Codecov Report

parse-github-assistant bot commented Apr 22, 2023

Thanks for opening this pull request!

mtrezza Apr 22, 2023

Choose a reason for hiding this comment

andrewalc Apr 25, 2023

Choose a reason for hiding this comment

mtrezza Apr 30, 2023 • edited Loading

Choose a reason for hiding this comment

andrewalc May 1, 2023

Choose a reason for hiding this comment

mtrezza May 2, 2023 • edited Loading

Choose a reason for hiding this comment

jimnor0xF commented May 2, 2023

mtrezza left a comment

Choose a reason for hiding this comment

parseplatformorg commented May 12, 2023

dblythy commented May 17, 2023

codecov bot commented Apr 21, 2023 •

edited

Loading

mtrezza Apr 30, 2023 •

edited

Loading

mtrezza May 2, 2023 •

edited

Loading