feat(storage): require temporary creds for storage browser interfaces (…

…#13664)
aws-amplify · Aug 1, 2024 · 30404f4 · 30404f4
1 parent ed29226
commit 30404f4
Show file tree

Hide file tree

Showing 9 changed files with 47 additions and 40 deletions.
diff --git a/packages/storage/__tests__/storageBrowser/apis/getDataAccess.test.ts b/packages/storage/__tests__/storageBrowser/apis/getDataAccess.test.ts
@@ -97,7 +97,7 @@ describe('getDataAccess', () => {
 		});
 
 		expect(getDataAccess(sharedGetDataAccessParams)).rejects.toThrow(
-			'Service did not return credentials.',
+			'Service did not return valid temporary credentials.',
 		);
 	});
 

diff --git a/packages/storage/__tests__/storageBrowser/locationCredentialsStore/create.test.ts b/packages/storage/__tests__/storageBrowser/locationCredentialsStore/create.test.ts
@@ -1,6 +1,5 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import { AWSCredentials } from '@aws-amplify/core/internals/utils';
 
 import { createLocationCredentialsStore } from '../../../src/storageBrowser/locationCredentialsStore/create';
 import {
@@ -13,10 +12,11 @@ import {
 	StorageValidationErrorCode,
 	validationErrorMap,
 } from '../../../src/errors/types/validation';
+import { AWSTemporaryCredentials } from '../../../src/providers/s3/types/options';
 
 jest.mock('../../../src/storageBrowser/locationCredentialsStore/registry');
 
-const mockedCredentials = 'MOCK_CREDS' as any as AWSCredentials;
+const mockedCredentials = 'MOCK_CREDS' as any as AWSTemporaryCredentials;
 
 describe('createLocationCredentialsStore', () => {
 	it('should create a store', () => {

diff --git a/packages/storage/__tests__/storageBrowser/locationCredentialsStore/registry.test.ts b/packages/storage/__tests__/storageBrowser/locationCredentialsStore/registry.test.ts
@@ -1,12 +1,11 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { AWSCredentials } from '@aws-amplify/core/internals/utils';
-
 import {
 	StorageValidationErrorCode,
 	validationErrorMap,
 } from '../../../src/errors/types/validation';
+import { AWSTemporaryCredentials } from '../../../src/providers/s3/types/options';
 import {
 	createStore,
 	getValue,
@@ -47,7 +46,7 @@ describe('createStore', () => {
 });
 
 describe('getValue', () => {
-	const mockCachedValue = 'CACHED_VALUE' as any as AWSCredentials;
+	const mockCachedValue = 'CACHED_VALUE' as any as AWSTemporaryCredentials;
 	let storeSymbol: { value: symbol };
 	beforeEach(() => {
 		storeSymbol = createStore(jest.fn(), 20);

diff --git a/packages/storage/__tests__/storageBrowser/locationCredentialsStore/store.test.ts b/packages/storage/__tests__/storageBrowser/locationCredentialsStore/store.test.ts
@@ -12,6 +12,10 @@ import {
 } from '../../../src/storageBrowser/locationCredentialsStore/store';
 import { CredentialsLocation } from '../../../src/storageBrowser/types';
 
+const mockCredentials = {
+	expiration: new Date(Date.now() + 60 * 60_1000),
+};
+
 describe('initStore', () => {
 	it('should create a store with given capacity, refresh Handler and values', () => {
 		const refreshHandler = jest.fn();
@@ -42,7 +46,7 @@ describe('initStore', () => {
 describe('getCacheValue', () => {
 	it('should return a cache value for given location and permission', () => {
 		const cachedValue = {
-			credentials: 'MOCK_CREDS',
+			credentials: mockCredentials,
 			scope: 'abc',
 			permission: 'READ',
 		} as any;
@@ -114,7 +118,6 @@ describe('fetchNewValue', () => {
 
 	it('should fetch new value from remote source', async () => {
 		expect.assertions(2);
-		const mockCredentials = 'MOCK_CREDS';
 		const refreshHandler = jest.fn().mockResolvedValue({
 			credentials: mockCredentials,
 		});
@@ -143,7 +146,6 @@ describe('fetchNewValue', () => {
 
 	it('should update cache with new value', async () => {
 		expect.assertions(1);
-		const mockCredentials = 'MOCK_CREDS';
 		const refreshHandler = jest.fn().mockResolvedValue({
 			credentials: mockCredentials,
 		});
@@ -159,7 +161,6 @@ describe('fetchNewValue', () => {
 
 	it('should invoke refresh handler only once if multiple fetches for same location is called', async () => {
 		expect.assertions(1);
-		const mockCredentials = 'MOCK_CREDS';
 		const refreshHandler = jest.fn().mockResolvedValue({
 			credentials: mockCredentials,
 		});
@@ -174,7 +175,6 @@ describe('fetchNewValue', () => {
 
 	it('should invoke the refresh handler if the refresh handler previously fails', async () => {
 		expect.assertions(4);
-		const mockCredentials = 'MOCK_CREDS';
 		const refreshHandler = jest
 			.fn()
 			.mockRejectedValueOnce(new Error('Network error'))
@@ -199,7 +199,6 @@ describe('fetchNewValue', () => {
 
 	it('should call refresh handler for new cache entry if the cache is full', async () => {
 		expect.assertions(4);
-		const mockCredentials = 'MOCK_CREDS';
 		const refreshHandler = jest.fn().mockResolvedValue({
 			credentials: mockCredentials,
 		});

diff --git a/packages/storage/src/providers/s3/types/options.ts b/packages/storage/src/providers/s3/types/options.ts
@@ -15,12 +15,22 @@ import {
 	StorageSubpathStrategy,
 } from '../../../types/options';
 
+/**
+ * @internal
+ */
+export type AWSTemporaryCredentials = Required<
+	Pick<
+		AWSCredentials,
+		'accessKeyId' | 'secretAccessKey' | 'sessionToken' | 'expiration'
+	>
+>;
+
 /**
  * @internal
  */
 export type LocationCredentialsProvider = (
-	input?: CredentialsProviderOptions,
-) => Promise<{ credentials: AWSCredentials }>;
+	options?: CredentialsProviderOptions,
+) => Promise<{ credentials: AWSTemporaryCredentials }>;
 
 export interface BucketInfo {
 	bucketName: string;

diff --git a/packages/storage/src/storageBrowser/apis/getDataAccess.ts b/packages/storage/src/storageBrowser/apis/getDataAccess.ts
@@ -45,10 +45,16 @@ export const getDataAccess = async (
 	const grantCredentials = result.Credentials;
 
 	// Ensure that S3 returned credentials (this shouldn't happen)
-	if (!grantCredentials) {
+	if (
+		!grantCredentials ||
+		!grantCredentials.AccessKeyId ||
+		!grantCredentials.SecretAccessKey ||
+		!grantCredentials.SessionToken ||
+		!grantCredentials.Expiration
+	) {
 		throw new StorageError({
 			name: AmplifyErrorCode.Unknown,
-			message: 'Service did not return credentials.',
+			message: 'Service did not return valid temporary credentials.',
 		});
 	} else {
 		logger.debug(`Retrieved credentials for: ${result.MatchedGrantTarget}`);
@@ -63,8 +69,8 @@ export const getDataAccess = async (
 
 	return {
 		credentials: {
-			accessKeyId: accessKeyId!,
-			secretAccessKey: secretAccessKey!,
+			accessKeyId,
+			secretAccessKey,
 			sessionToken,
 			expiration,
 		},

diff --git a/packages/storage/src/storageBrowser/locationCredentialsStore/registry.ts b/packages/storage/src/storageBrowser/locationCredentialsStore/registry.ts
@@ -2,8 +2,7 @@
 
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import { AWSCredentials } from '@aws-amplify/core/internals/utils';
-
+import { AWSTemporaryCredentials } from '../../providers/s3/types/options';
 import { CredentialsLocation, GetLocationCredentials } from '../types';
 import { assertValidationError } from '../../errors/utils/assertValidationError';
 import { StorageValidationErrorCode } from '../../errors/types/validation';
@@ -68,7 +67,7 @@ export const getValue = async (input: {
 	storeSymbol: StoreRegistrySymbol;
 	location: CredentialsLocation;
 	forceRefresh: boolean;
-}): Promise<{ credentials: AWSCredentials }> => {
+}): Promise<{ credentials: AWSTemporaryCredentials }> => {
 	const { storeSymbol: storeReference, location, forceRefresh } = input;
 	const store = getCredentialsStore(storeReference);
 	if (!forceRefresh) {

diff --git a/packages/storage/src/storageBrowser/locationCredentialsStore/store.ts b/packages/storage/src/storageBrowser/locationCredentialsStore/store.ts
@@ -3,13 +3,12 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { AWSCredentials } from '@aws-amplify/core/internals/utils';
-
 import {
 	CredentialsLocation,
 	GetLocationCredentials,
 	Permission,
 } from '../types';
+import { AWSTemporaryCredentials } from '../../providers/s3/types/options';
 import { assertValidationError } from '../../errors/utils/assertValidationError';
 import { StorageValidationErrorCode } from '../../errors/types/validation';
 
@@ -19,8 +18,8 @@ import {
 } from './constants';
 
 interface StoreValue extends CredentialsLocation {
-	credentials?: AWSCredentials;
-	inflightCredentials?: Promise<{ credentials: AWSCredentials }>;
+	credentials?: AWSTemporaryCredentials;
+	inflightCredentials?: Promise<{ credentials: AWSTemporaryCredentials }>;
 }
 
 type S3Uri = string;
@@ -64,7 +63,7 @@ export const initStore = (
 export const getCacheValue = (
 	store: LruLocationCredentialsStore,
 	location: CredentialsLocation,
-): AWSCredentials | null => {
+): AWSTemporaryCredentials | null => {
 	const cacheKey = createCacheKey(location);
 	const cachedValue = store.values.get(cacheKey);
 	const cachedCredentials = cachedValue?.credentials;
@@ -85,13 +84,10 @@ export const getCacheValue = (
 	return null;
 };
 
-const pastTTL = (credentials: AWSCredentials) => {
+const pastTTL = (credentials: AWSTemporaryCredentials) => {
 	const { expiration } = credentials;
 
-	return (
-		expiration &&
-		expiration.getTime() - CREDENTIALS_REFRESH_WINDOW_MS <= Date.now()
-	);
+	return expiration.getTime() - CREDENTIALS_REFRESH_WINDOW_MS <= Date.now();
 };
 
 /**
@@ -102,7 +98,7 @@ const pastTTL = (credentials: AWSCredentials) => {
 export const fetchNewValue = async (
 	store: LruLocationCredentialsStore,
 	location: CredentialsLocation,
-): Promise<{ credentials: AWSCredentials }> => {
+): Promise<{ credentials: AWSTemporaryCredentials }> => {
 	const storeValues = store.values;
 	const key = createCacheKey(location);
 	if (!storeValues.has(key)) {

diff --git a/packages/storage/src/storageBrowser/types.ts b/packages/storage/src/storageBrowser/types.ts
@@ -1,10 +1,10 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-import { AWSCredentials } from '@aws-amplify/core/internals/utils';
-import { CredentialsProviderOptions } from '@aws-amplify/core/internals/aws-client-utils';
-
-import { LocationCredentialsProvider } from '../providers/s3/types/options';
+import {
+	AWSTemporaryCredentials,
+	LocationCredentialsProvider,
+} from '../providers/s3/types/options';
 
 /**
  * @internal
@@ -14,9 +14,7 @@ export type Permission = 'READ' | 'READWRITE' | 'WRITE';
 /**
  * @internal
  */
-export type CredentialsProvider = (
-	options?: CredentialsProviderOptions,
-) => Promise<{ credentials: AWSCredentials }>;
+export type CredentialsProvider = LocationCredentialsProvider;
 
 /**
  * @internal
@@ -70,7 +68,7 @@ export interface LocationCredentials extends Partial<LocationScope> {
 	/**
 	 * AWS credentials which can be used to access the specified location.
 	 */
-	readonly credentials: AWSCredentials;
+	readonly credentials: AWSTemporaryCredentials;
 }
 
 export interface AccessGrant extends LocationAccess {
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,7 +97,7 @@ describe('getDataAccess', () => { @@
     		});
     		expect(getDataAccess(sharedGetDataAccessParams)).rejects.toThrow(
-    			'Service did not return credentials.',
+    			'Service did not return valid temporary credentials.',
     		);
     	});
@@ Expand Down @@