Skip to content
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

Feat: Add pagination to Amplify Default Auth storage Browser #13897

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/aws-amplify/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@
"name": "[Storage] copy (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ copy }",
"limit": "16.05 kB"
"limit": "16.08 kB"
},
{
"name": "[Storage] downloadData (S3)",
Expand Down Expand Up @@ -491,7 +491,7 @@
"name": "[Storage] remove (S3)",
"path": "./dist/esm/storage/index.mjs",
"import": "{ remove }",
"limit": "15.50 kB"
"limit": "15.52 kB"
},
{
"name": "[Storage] uploadData (S3)",
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/parseAmplifyOutputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
/* eslint-disable camelcase */

/* Does not like exhaustive checks */
/* eslint-disable no-case-declarations */

import {
APIConfig,
Expand Down Expand Up @@ -87,12 +86,14 @@ function parseAuth(
oauth,
username_attributes,
standard_required_attributes,
groups,
} = amplifyOutputsAuthProperties;

const authConfig = {
Cognito: {
userPoolId: user_pool_id,
userPoolClientId: user_pool_client_id,
groups,
},
} as AuthConfig;

Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/singleton/AmplifyOutputs/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export type AmplifyOutputsAuthMFAConfiguration =
| 'NONE';

export type AmplifyOutputsAuthMFAMethod = 'SMS' | 'TOTP';

type UserGroupName = string;
type UserGroupPrecedence = Record<string, number>;
export interface AmplifyOutputsAuthProperties {
aws_region: string;
authentication_flow_type?: 'USER_SRP_AUTH' | 'CUSTOM_AUTH';
Expand Down Expand Up @@ -41,6 +42,7 @@ export interface AmplifyOutputsAuthProperties {
unauthenticated_identities_enabled?: boolean;
mfa_configuration?: string;
mfa_methods?: string[];
groups?: Record<UserGroupName, UserGroupPrecedence>[];
}

export interface AmplifyOutputsStorageBucketProperties {
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/singleton/Auth/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ export type LegacyUserAttributeKey = Uppercase<AuthStandardAttributeKey>;

export type AuthVerifiableAttributeKey = 'email' | 'phone_number';

type UserGroupName = string;
type UserGroupPrecedence = Record<string, number>;

export type AuthConfigUserAttributes = Partial<
Record<AuthStandardAttributeKey, { required: boolean }>
>;
Expand All @@ -130,6 +133,7 @@ export interface AuthIdentityPoolConfig {
userAttributes?: never;
mfa?: never;
passwordFormat?: never;
groups?: never;
};
}

Expand Down Expand Up @@ -171,6 +175,7 @@ export interface CognitoUserPoolConfig {
requireNumbers?: boolean;
requireSpecialCharacters?: boolean;
};
groups?: Record<UserGroupName, UserGroupPrecedence>[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanity check, a user pool can have multiple records each containing multiple user group names? What's the case for multiple user group names in the record?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is auth config. Basically possible user group a user could belong to.
Screenshot 2024-10-24 at 2 58 52 PM

}

export interface OAuthConfig {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,24 @@ const mockFetchAuthSession = fetchAuthSession as jest.Mock;
const mockResolveLocationsFromCurrentSession =
resolveLocationsForCurrentSession as jest.Mock;

const mockAuthConfig = {
Auth: {
Cognito: {
userPoolClientId: 'userPoolClientId',
userPoolId: 'userPoolId',
identityPoolId: 'identityPoolId',
groups: [{ admin: { precedence: 0 } }],
},
},
};

describe('createAmplifyAuthConfigAdapter', () => {
beforeEach(() => {
jest.clearAllMocks();
});

mockGetConfig.mockReturnValue({
...mockAuthConfig,
Storage: {
S3: {
bucket: 'bucket1',
Expand Down Expand Up @@ -70,7 +82,10 @@ describe('createAmplifyAuthConfigAdapter', () => {
});

it('should return empty locations when buckets are not defined', async () => {
mockGetConfig.mockReturnValue({ Storage: { S3: { buckets: undefined } } });
mockGetConfig.mockReturnValue({
...mockAuthConfig,
Storage: { S3: { buckets: undefined } },
});

const adapter = createAmplifyAuthConfigAdapter();
const result = await adapter.listLocations();
Expand All @@ -93,16 +108,15 @@ describe('createAmplifyAuthConfigAdapter', () => {
};

mockGetConfig.mockReturnValue({
...mockAuthConfig,
Storage: { S3: { buckets: mockBuckets } },
});
mockResolveLocationsFromCurrentSession.mockReturnValue([
{
type: 'PREFIX',
permission: ['read', 'write'],
scope: {
bucketName: 'bucket1',
path: '/path1',
},
bucket: 'bucket1',
prefix: '/path1',
},
]);

Expand All @@ -114,10 +128,8 @@ describe('createAmplifyAuthConfigAdapter', () => {
{
type: 'PREFIX',
permission: ['read', 'write'],
scope: {
bucketName: 'bucket1',
path: '/path1',
},
bucket: 'bucket1',
prefix: '/path1',
},
],
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0

import {
UserGroupConfig,
getHighestPrecedenceUserGroup,
} from '../../../src/internals/amplifyAuthConfigAdapter/getHighestPrecedenceUserGroup';

const userGroupsFromConfig: UserGroupConfig = [
{
editor: {
precedence: 0,
},
},
{
admin: {
precedence: 1,
},
},
{
auditor: {
precedence: 2,
},
},
];
const currentUserGroups = ['guest', 'user', 'admin'];

describe('getHighestPrecedenceUserGroup', () => {
it('should return the user group with the highest precedence', () => {
const result = getHighestPrecedenceUserGroup(
userGroupsFromConfig,
currentUserGroups,
);
expect(result).toBe('admin');
});

it('should return undefined if userGroupsFromConfig is undefined', () => {
const result = getHighestPrecedenceUserGroup(undefined, currentUserGroups);
expect(result).toBeUndefined();
});

it('should return undefined if currentUserGroups is undefined', () => {
const result = getHighestPrecedenceUserGroup(
userGroupsFromConfig,
undefined,
);
expect(result).toBeUndefined();
});

it('should handle currentUserGroups containing groups not present in userGroupsFromConfig', () => {
const result = getHighestPrecedenceUserGroup(userGroupsFromConfig, [
'unknown',
'user',
]);
expect(result).toBe(undefined);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { getPaginatedLocations } from '../../../src/internals/amplifyAuthConfigAdapter/getPaginatedLocations';
import { PathAccess } from '../../../src/internals/types/credentials';

describe('getPaginatedLocations', () => {
const mockLocations: PathAccess[] = [
{
type: 'PREFIX',
permission: ['read'],
bucket: 'bucket1',
prefix: 'path1/',
},
{
type: 'PREFIX',
permission: ['write'],
bucket: 'bucket2',
prefix: 'path2/',
},
{
type: 'PREFIX',
permission: ['read', 'write'],
bucket: 'bucket3',
prefix: 'path3/',
},
];

it('should return all locations when no pagination is specified', () => {
const result = getPaginatedLocations({ locations: mockLocations });
expect(result).toEqual({ locations: mockLocations });
});

it('should return paginated locations when pageSize is specified', () => {
const result = getPaginatedLocations({
locations: mockLocations,
pageSize: 2,
});
expect(result).toEqual({
locations: mockLocations.slice(0, 2),
nextToken: '1',
});
});

it('should return paginated locations when pageSize and nextToken are specified', () => {
const result = getPaginatedLocations({
locations: mockLocations,
pageSize: 1,
nextToken: '2',
});
expect(result).toEqual({
locations: mockLocations.slice(1, 2),
nextToken: '1',
});
});

it('should return empty locations when locations array is empty', () => {
const result = getPaginatedLocations({ locations: [], pageSize: 2 });
expect(result).toEqual({ locations: [] });
});

it('should return empty location when nextToken is beyond array length', () => {
const result = getPaginatedLocations({
locations: mockLocations,
pageSize: 2,
nextToken: '5',
});
expect(result).toEqual({ locations: [], nextToken: undefined });
});

it('should return all remaining location when page size is greater than remaining locations length', () => {
const result = getPaginatedLocations({
locations: mockLocations,
pageSize: 5,
nextToken: '2',
});
expect(result).toEqual({
locations: mockLocations.slice(-2),
nextToken: undefined,
});
});

it('should return undefined nextToken when end of array is reached', () => {
const result = getPaginatedLocations({
locations: mockLocations,
pageSize: 5,
});
expect(result).toEqual({
locations: mockLocations.slice(0, 3),
nextToken: undefined,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ describe('resolveLocationsForCurrentSession', () => {
buckets: mockBuckets,
isAuthenticated: true,
identityId: '12345',
userGroup: 'admin',
});

expect(result).toEqual([
Expand All @@ -47,12 +46,6 @@ describe('resolveLocationsForCurrentSession', () => {
bucket: 'bucket1',
prefix: 'path1/*',
},
{
type: 'PREFIX',
permission: ['get', 'list', 'write', 'delete'],
bucket: 'bucket1',
prefix: 'path2/*',
},
{
type: 'PREFIX',
permission: ['get', 'list', 'write', 'delete'],
Expand All @@ -62,30 +55,35 @@ describe('resolveLocationsForCurrentSession', () => {
]);
});

it('should generate locations correctly when tokens are true & bad userGroup', () => {
it('should generate locations correctly when tokens are true & userGroup', () => {
const result = resolveLocationsForCurrentSession({
buckets: mockBuckets,
isAuthenticated: true,
identityId: '12345',
userGroup: 'editor',
userGroup: 'admin',
});

expect(result).toEqual([
{
type: 'PREFIX',
permission: ['get', 'list', 'write'],
bucket: 'bucket1',
prefix: 'path1/*',
},
{
type: 'PREFIX',
permission: ['get', 'list', 'write', 'delete'],
bucket: 'bucket1',
prefix: 'profile-pictures/12345/*',
prefix: 'path2/*',
},
]);
});

it('should return empty locations when tokens are true & bad userGroup', () => {
const result = resolveLocationsForCurrentSession({
buckets: mockBuckets,
isAuthenticated: true,
identityId: '12345',
userGroup: 'editor',
});

expect(result).toEqual([]);
});

it('should continue to next bucket when paths are not defined', () => {
const result = resolveLocationsForCurrentSession({
buckets: {
Expand All @@ -107,7 +105,6 @@ describe('resolveLocationsForCurrentSession', () => {
},
isAuthenticated: true,
identityId: '12345',
userGroup: 'admin',
});

expect(result).toEqual([
Expand Down
Loading
Loading