fix(AWS Nodes): Handle query string and body properly for AWS related…

… requests (n8n-io#4039)
valya · Sep 6, 2022 · cde6b7f · cde6b7f
1 parent 1cdb940
commit cde6b7f
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 35 deletions.
diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts
@@ -1461,19 +1461,19 @@ export async function requestWithAuthentication(
 					// make the updated property in the credentials
 					// available to the authenticate method
 					Object.assign(credentialsDecrypted, data);
+					requestOptions = await additionalData.credentialsHelper.authenticate(
+						credentialsDecrypted,
+						credentialsType,
+						requestOptions as IHttpRequestOptions,
+						workflow,
+						node,
+						additionalData.timezone,
+					);
+					// retry the request
+					return await proxyRequestToAxios(requestOptions as IDataObject);
 				}
-
-				requestOptions = await additionalData.credentialsHelper.authenticate(
-					credentialsDecrypted,
-					credentialsType,
-					requestOptions as IHttpRequestOptions,
-					workflow,
-					node,
-					additionalData.timezone,
-				);
 			}
-			// retry the request
-			return await proxyRequestToAxios(requestOptions as IDataObject);
+			throw error;
 		} catch (error) {
 			throw new NodeApiError(this.getNode(), error);
 		}

diff --git a/packages/nodes-base/credentials/Aws.credentials.ts b/packages/nodes-base/credentials/Aws.credentials.ts
@@ -272,35 +272,36 @@ export class Aws implements ICredentialType {
 		credentials: ICredentialDataDecryptedObject,
 		requestOptions: IHttpRequestOptions,
 	): Promise<IHttpRequestOptions> {
-		let endpoint;
+		let endpoint: URL;
 		let service = requestOptions.qs?.service as string;
 		let path = requestOptions.qs?.path;
 		const method = requestOptions.method;
 		const body = requestOptions.body;
 		let region = credentials.region;
 		const query = requestOptions.qs?.query as IDataObject;
 		if (!requestOptions.baseURL && !requestOptions.url) {
+			let endpointString: string;
 			if (service === 'lambda' && credentials.lambdaEndpoint) {
-				endpoint = credentials.lambdaEndpoint;
+				endpointString = credentials.lambdaEndpoint as string;
 			} else if (service === 'sns' && credentials.snsEndpoint) {
-				endpoint = credentials.snsEndpoint;
+				endpointString = credentials.snsEndpoint as string;
 			} else if (service === 'sqs' && credentials.sqsEndpoint) {
-				endpoint = credentials.sqsEndpoint;
+				endpointString = credentials.sqsEndpoint as string;
 			} else if (service === 's3' && credentials.s3Endpoint) {
-				endpoint = credentials.s3Endpoint;
+				endpointString = credentials.s3Endpoint as string;
 			} else if (service === 'ses' && credentials.sesEndpoint) {
-				endpoint = credentials.sesEndpoint;
+				endpointString = credentials.sesEndpoint as string;
 			} else if (service === 'rekognition' && credentials.rekognitionEndpoint) {
-				endpoint = credentials.rekognitionEndpoint;
+				endpointString = credentials.rekognitionEndpoint as string;
 			} else if (service === 'sqs' && credentials.sqsEndpoint) {
-				endpoint = credentials.sqsEndpoint;
+				endpointString = credentials.sqsEndpoint as string;
 			} else if (service) {
-				endpoint = `https://${service}.${credentials.region}.amazonaws.com`;
+				endpointString = `https://${service}.${credentials.region}.amazonaws.com`;
 			}
-			endpoint = new URL((endpoint as string).replace('{region}', credentials.region as string));
+			endpoint = new URL(endpointString!.replace('{region}', credentials.region as string) + path);
 		} else {
 			// If no endpoint is set, we try to decompose the path and use the default endpoint
-			const customUrl = new URL(requestOptions.baseURL! + requestOptions.url!);
+			const customUrl = new URL(requestOptions.baseURL! + requestOptions.url! + path);
 			service = customUrl.hostname.split('.')[0] as string;
 			region = customUrl.hostname.split('.')[1] as string;
 			if (service === 'sts') {
@@ -311,19 +312,24 @@ export class Aws implements ICredentialType {
 					console.log(err);
 				}
 			}
-			path = customUrl.pathname as string;
 			endpoint = customUrl;
 		}
-		if (service.includes('.s3')) {
-			path = `${endpoint.pathname}?${queryToString(query).replace(/\+/g, '%2B')}`;
+
+		if (query && Object.keys(query).length !== 0) {
+			Object.keys(query).forEach((key) => {
+				endpoint.searchParams.append(key, query[key] as string);
+			});
 		}
 
+		path = endpoint.pathname + endpoint.search;
+
 		const signOpts = {
-			headers: requestOptions.headers,
+			...requestOptions,
+			headers: requestOptions.headers ?? {},
 			host: endpoint.host,
 			method,
 			path,
-			body,
+			body: body !== '' ? body : undefined,
 			region,
 		} as Request;
 
@@ -340,10 +346,12 @@ export class Aws implements ICredentialType {
 			console.log(err);
 		}
 		const options: IHttpRequestOptions = {
+			...requestOptions,
 			headers: signOpts.headers,
 			method,
 			url: endpoint.origin + path,
 			body: signOpts.body,
+			qs: undefined, // override since it's already in the url
 		};
 
 		return options;

diff --git a/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts b/packages/nodes-base/nodes/Aws/S3/GenericFunctions.ts
@@ -1,11 +1,5 @@
-import { URL } from 'url';
-
-import { Request, sign } from 'aws4';
-
 import { get } from 'lodash';
 
-import { OptionsWithUri } from 'request';
-
 import { parseString } from 'xml2js';
 
 import {
@@ -20,7 +14,6 @@ import {
 	IHttpRequestOptions,
 	JsonObject,
 	NodeApiError,
-	NodeOperationError,
 } from 'n8n-workflow';
 
 export async function awsApiRequest(
@@ -44,7 +37,7 @@ export async function awsApiRequest(
 			query,
 		},
 		method,
-		body: JSON.stringify(body),
+		body,
 		url: '',
 		headers,
 		//region: credentials?.region as string,