-
Notifications
You must be signed in to change notification settings - Fork 1
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
Support alternative method of passing Presigned params #178
Conversation
If there is a presign param, we need to decode it and add it to the args. This is to provide a secondary way to pass pre-sign params, as using them in a Lambda function URL invocation will trigger a Lambda error.
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.
Minor things, but the debug/test stuff should be removed before merge.
tests/test-private-upload.ts
Outdated
{ | ||
region: process.env.S3_REGION, | ||
bucket: process.env.S3_BUCKET, | ||
} as S3ClientConfig |
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.
Why is this required? Is there a type error here?
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 this was copied from the lib.ts. It seems the ts types don't account for this style of passing these times for some reason. Mostly just copied it though
/** | ||
* Middleware to remove the x-id query string param form the GetObject call. | ||
*/ | ||
const middleware: MiddlewareType<any, any> = ( next: any, context: any ) => async ( args: any ) => { |
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.
Can these be more specifically typed?
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.
They could but it's a test so I'm not that worried
tests/test-private-upload.ts
Outdated
let queryStringParameters: Args = {}; | ||
presignedUrl.searchParams.forEach( ( value, key ) => { | ||
queryStringParameters[ key as keyof Args ] = value; | ||
} ); | ||
|
||
return queryStringParameters; |
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.
Maybe: Object.fromEntries( presignedUrl.searchParams )
?
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.
Ah that's the function I was looking for I think!
Allow passing
?presign={signature params}
to work around lambda not allowing the pre-soign params directly.