fix(client): correct clock skew from any response (aws-amplify#11503)

ashwinkumar6 · Jun 16, 2023 · c1fa9c7 · c1fa9c7
1 parent 2e1e1f5
commit c1fa9c7
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 57 deletions.
diff --git a/.../clients/utils/isClockSkewedError-test.ts → ...ddleware/retry/isClockSkewedError-test.ts b/.../clients/utils/isClockSkewedError-test.ts → ...ddleware/retry/isClockSkewedError-test.ts
@@ -1,7 +1,7 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { isClockSkewError } from '../../../src/clients/utils/isClockSkewError';
+import { isClockSkewError } from '../../../../src/clients/middleware/retry/isClockSkewError';
 
 describe('isClockSkewError', () => {
 	test('returns true if error code is a clock skew error', () => {

diff --git a/packages/core/__tests__/clients/middleware/signing/middleware-test.ts b/packages/core/__tests__/clients/middleware/signing/middleware-test.ts
@@ -13,7 +13,6 @@ import {
 	HttpResponse,
 	MiddlewareHandler,
 } from '../../../../src/clients/types';
-import { isClockSkewError } from '../../../../src/clients/utils/isClockSkewError';
 import {
 	credentials,
 	signingDate,
@@ -23,15 +22,13 @@ import {
 } from './signer/signatureV4/testUtils/data';
 import { signingTestTable } from './signer/signatureV4/testUtils/signingTestTable';
 
-jest.mock('../../../../src/clients/utils/isClockSkewError');
 jest.mock(
 	'../../../../src/clients/middleware/signing/utils/getSkewCorrectedDate'
 );
 jest.mock(
 	'../../../../src/clients/middleware/signing/utils/getUpdatedSystemClockOffset'
 );
 
-const mockisClockSkewError = isClockSkewError as jest.Mock;
 const mockGetSkewCorrectedDate = getSkewCorrectedDate as jest.Mock;
 const mockGetUpdatedSystemClockOffset =
 	getUpdatedSystemClockOffset as jest.Mock;
@@ -96,46 +93,32 @@ describe('Signing middleware', () => {
 	});
 
 	test.each([
-		['skew error', null],
-		['error with Date header', 'Date'],
-		['error with date header', 'date'],
+		['response with Date header', 'Date'],
+		['response with date header', 'date'],
+		['response with x-amz-date header', 'x-amz-date'],
 	])('should adjust clock offset if server returns %s', async (_, key) => {
-		mockisClockSkewError.mockReturnValue(true);
 		const serverTime = signingDate.toISOString();
 		const parsedServerTime = Date.parse(serverTime);
-		const nextHandler = key
-			? jest.fn().mockRejectedValue({
-					$response: {
-						headers: {
-							[key]: serverTime,
-						},
-					},
-			  })
-			: jest.fn().mockRejectedValue({ ServerTime: serverTime });
+		const nextHandler = jest.fn().mockResolvedValue({
+			headers: {
+				[key!]: serverTime,
+			},
+		});
 
 		const middlewareFunction = signingMiddleware(defaultSigningOptions)(
 			nextHandler
 		);
 
-		try {
-			await middlewareFunction(defaultRequest);
-		} catch (error) {
-			expect(mockGetSkewCorrectedDate).toBeCalledWith(0);
-			expect(mockGetUpdatedSystemClockOffset).toBeCalledWith(
-				parsedServerTime,
-				0
-			);
-			jest.clearAllMocks();
-			try {
-				await middlewareFunction(defaultRequest);
-			} catch (error) {
-				expect(mockGetSkewCorrectedDate).toBeCalledWith(updatedOffset);
-				expect(mockGetUpdatedSystemClockOffset).toBeCalledWith(
-					parsedServerTime,
-					updatedOffset
-				);
-			}
-		}
-		expect.assertions(4);
+		await middlewareFunction(defaultRequest);
+		expect(mockGetUpdatedSystemClockOffset).toBeCalledWith(parsedServerTime, 0);
+
+		jest.clearAllMocks();
+		await middlewareFunction(defaultRequest);
+		expect(mockGetSkewCorrectedDate).toBeCalledWith(updatedOffset);
+		expect(mockGetUpdatedSystemClockOffset).toBeCalledWith(
+			parsedServerTime,
+			updatedOffset
+		);
+		expect.assertions(3);
 	});
 });
diff --git a/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts b/packages/core/src/clients/middleware/retry/defaultRetryDecider.ts
@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { HttpResponse, ErrorParser } from '../../types';
-import { isClockSkewError } from '../../utils/isClockSkewError';
+import { isClockSkewError } from './isClockSkewError';
 
 /**
  * Get retry decider function

diff --git a/...ore/src/clients/utils/isClockSkewError.ts → ...ents/middleware/retry/isClockSkewError.ts b/...ore/src/clients/utils/isClockSkewError.ts → ...ents/middleware/retry/isClockSkewError.ts
diff --git a/packages/core/src/clients/middleware/signing/middleware.ts b/packages/core/src/clients/middleware/signing/middleware.ts
@@ -7,7 +7,6 @@ import {
 	HttpResponse,
 	MiddlewareHandler,
 } from '../../types';
-import { isClockSkewError } from '../../utils/isClockSkewError';
 import { signRequest } from './signer/signatureV4';
 import { getSkewCorrectedDate } from './utils/getSkewCorrectedDate';
 import { getUpdatedSystemClockOffset } from './utils/getUpdatedSystemClockOffset';
@@ -23,6 +22,7 @@ export interface SigningOptions {
 
 /**
  * Middleware that SigV4 signs request with AWS credentials, and correct system clock offset.
+ * This middleware is expected to be placed after retry middleware.
  */
 export const signingMiddleware = ({
 	credentials,
@@ -40,26 +40,20 @@ export const signingMiddleware = ({
 				signingService: service,
 			};
 			const signedRequest = await signRequest(request, signRequestOptions);
-			try {
-				const response = await next(signedRequest);
-				return response;
-			} catch (error) {
-				const dateString = shouldUseServerTime(error)
-					? error.ServerTime
-					: getDateHeader(error.$response);
-				if (dateString) {
-					currentSystemClockOffset = getUpdatedSystemClockOffset(
-						Date.parse(dateString),
-						currentSystemClockOffset
-					);
-				}
-				throw error;
+			const response = await next(signedRequest);
+			// Update system clock offset if response contains date header, regardless of the status code.
+			// non-2xx response will still be returned from next handler instead of thrown, because it's
+			// only thrown by the retry middleware.
+			const dateString = getDateHeader(response);
+			if (dateString) {
+				currentSystemClockOffset = getUpdatedSystemClockOffset(
+					Date.parse(dateString),
+					currentSystemClockOffset
+				);
 			}
+			return response;
 		};
 };
 
-const shouldUseServerTime = ({ ServerTime, Code }: any): boolean =>
-	ServerTime && isClockSkewError(Code);
-
 const getDateHeader = ({ headers }: any = {}): string | undefined =>
-	headers?.date ?? headers?.Date;
+	headers?.date ?? headers?.Date ?? headers?.['x-amz-date'];