fix: ensure MIS model name mismatch cannot cause unexpected behavior (#403)

Author: iartemiev

.changeset/stale-apricots-turn.md

@@ -0,0 +1,5 @@
+---
+'@aws-amplify/data-schema': patch
+---
+
+fix prevent mis model mismatch

packages/data-schema/__tests__/runtime/APIClient.test.ts

@@ -809,27 +809,39 @@ describe('generateGraphQLDocument()', () => {
     };
 
     test.each([
-      ['GET', 'User', '$userId: ID!'],
+      ['GET', userSchemaModel, '$userId: ID!'],
       [
         'GET',
-        'Product',
+        productSchemaModel,
         '$sku: String!,$factoryId: String!,$warehouseId: String!',
       ],
-      ['GET', 'person', 'getPerson'],
-      ['LIST', 'person', 'listPeople'],
-      ['LIST', 'person', '$filter: ModelPersonFilterInput'],
-      ['CREATE', 'person', '$input: CreatePersonInput!'],
-      ['UPDATE', 'person', '$input: UpdatePersonInput!'],
-      ['DELETE', 'person', '$input: DeletePersonInput!'],
-      ['ONCREATE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
-      ['ONUPDATE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
-      ['ONDELETE', 'person', '$filter: ModelSubscriptionPersonFilterInput'],
+      ['GET', personSchemaModel, 'getPerson'],
+      ['LIST', personSchemaModel, 'listPeople'],
+      ['LIST', personSchemaModel, '$filter: ModelPersonFilterInput'],
+      ['CREATE', personSchemaModel, '$input: CreatePersonInput!'],
+      ['UPDATE', personSchemaModel, '$input: UpdatePersonInput!'],
+      ['DELETE', personSchemaModel, '$input: DeletePersonInput!'],
+      [
+        'ONCREATE',
+        personSchemaModel,
+        '$filter: ModelSubscriptionPersonFilterInput',
+      ],
+      [
+        'ONUPDATE',
+        personSchemaModel,
+        '$filter: ModelSubscriptionPersonFilterInput',
+      ],
+      [
+        'ONDELETE',
+        personSchemaModel,
+        '$filter: ModelSubscriptionPersonFilterInput',
+      ],
     ])(
       '%s generates operation name or arguments for model %s to contain %s',
-      (modelOperation, modelName, expectedArgs) => {
+      (modelOperation, model, expectedArgs) => {
         const document = generateGraphQLDocument(
           mockModelDefinitions,
-          modelName,
+          model,
           modelOperation as ModelOperation,
         );
 
@@ -873,31 +885,31 @@ describe('generateGraphQLDocument()', () => {
 
     test.each([
       [
-        'EnumAsIndexPartitionKey',
+        modelIntroSchema.models.EnumAsIndexPartitionKey,
         '$status: EnumAsIndexPartitionKeyStatus!',
         getIndexMeta('EnumAsIndexPartitionKey'),
         { status: 'yes' },
       ],
       [
-        'EnumAsIndexSortKey',
+        modelIntroSchema.models.EnumAsIndexSortKey,
         '$status: ModelStringKeyConditionInput',
         getIndexMeta('EnumAsIndexSortKey'),
         { title: 'test' },
       ],
       [
-        'RefEnumAsIndexPartitionKey',
+        modelIntroSchema.models.RefEnumAsIndexPartitionKey,
         '$status: EnumIndexStatus!',
         getIndexMeta('RefEnumAsIndexPartitionKey'),
         { status: 'yes' },
       ],
       [
-        'RefEnumAsIndexSortKey',
+        modelIntroSchema.models.RefEnumAsIndexSortKey,
         '$status: ModelStringKeyConditionInput',
         getIndexMeta('RefEnumAsIndexSortKey'),
         { title: 'test' },
       ],
       [
-        'SecondaryIndexModel',
+        modelIntroSchema.models.SecondaryIndexModel,
         '$timestamp: ModelIntKeyConditionInput',
         getIndexMeta(
           'SecondaryIndexModel',
@@ -907,10 +919,10 @@ describe('generateGraphQLDocument()', () => {
       ],
     ])(
       'generates document for model %p containing input type: %p',
-      (modelName, expectedSortKeyInputString, indexMeta, args) => {
+      (model, expectedSortKeyInputString, indexMeta, args) => {
         const document = generateGraphQLDocument(
           modelIntroSchema,
-          modelName,
+          model,
           modelOperation,
           args,
           indexMeta,

packages/data-schema/src/runtime/internals/APIClient.ts

@@ -1,7 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 
 // SPDX-License-Identifier: Apache-2.0
-
 import {
   AmplifyServer,
   AssociationBelongsTo,
@@ -245,7 +244,7 @@ export function initializeModel(
               if (record[curVal]) {
                 acc[curVal] = record[curVal];
               }
-              return acc; 
+              return acc;
             },
             {},
           );
@@ -843,13 +842,11 @@ export function generateSelectionSet(
 
 export function generateGraphQLDocument(
   modelIntrospection: ModelIntrospectionSchema,
-  modelName: string,
+  modelDefinition: SchemaModel,
   modelOperation: ModelOperation,
   listArgs?: ListArgs | QueryArgs,
   indexMeta?: IndexMeta,
 ): string {
-  const modelDefinition = modelIntrospection.models[modelName];
-
   const {
     name,
     pluralName,
@@ -917,7 +914,7 @@ export function generateGraphQLDocument(
       )?.properties?.name;
 
       skQueryArgs = {
-        [compositeSkArgName]: `Model${capitalize(modelName)}${capitalize(keyName)}CompositeKeyConditionInput`,
+        [compositeSkArgName]: `Model${capitalize(name)}${capitalize(keyName)}CompositeKeyConditionInput`,
       };
     }
 
@@ -941,7 +938,7 @@ export function generateGraphQLDocument(
 
   const selectionSetFields = generateSelectionSet(
     modelIntrospection,
-    modelName,
+    name,
     selectionSet as ListArgs['selectionSet'],
   );
 
@@ -1011,7 +1008,7 @@ export function generateGraphQLDocument(
         const compositeSkArgName = resolvedSkName(sortKeyFieldNames);
 
         return {
-          [compositeSkArgName]: `Model${capitalize(modelName)}PrimaryCompositeKeyConditionInput`,
+          [compositeSkArgName]: `Model${capitalize(name)}PrimaryCompositeKeyConditionInput`,
         };
       }
     }

packages/data-schema/src/runtime/internals/operations/get.ts

@@ -92,7 +92,7 @@ function _get(
 
     const query = generateGraphQLDocument(
       modelIntrospection,
-      name,
+      model,
       operation,
       options,
     );

packages/data-schema/src/runtime/internals/operations/indexQuery.ts

@@ -120,7 +120,7 @@ function _indexQuery(
 
     const query = generateGraphQLDocument(
       modelIntrospection,
-      name,
+      model,
       'INDEX_QUERY',
       args,
       indexMeta,
@@ -189,7 +189,12 @@ function _indexQuery(
       const { data, errors } = error;
 
       // `data` is not `null`, and is not an empty object:
-      if (data !== undefined && data !== null && Object.keys(data).length !== 0 && errors) {
+      if (
+        data !== undefined &&
+        data !== null &&
+        Object.keys(data).length !== 0 &&
+        errors
+      ) {
         const [key] = Object.keys(data);
 
         if (data[key]?.items) {

packages/data-schema/src/runtime/internals/operations/list.ts

@@ -60,7 +60,7 @@ export function listFactory(
       getInternals,
       args,
       undefined,
-      customUserAgentDetails
+      customUserAgentDetails,
     );
   };
 
@@ -81,7 +81,7 @@ function _list(
 
     const query = generateGraphQLDocument(
       modelIntrospection,
-      name,
+      model,
       'LIST',
       args,
     );

packages/data-schema/src/runtime/internals/operations/subscription.ts

@@ -32,7 +32,7 @@ export function subscriptionFactory(
   const subscription = (args?: Record<string, any>) => {
     const query = generateGraphQLDocument(
       modelIntrospection,
-      name,
+      model,
       operation,
       args,
     );

packages/integration-tests/__tests__/defined-behavior/3-exhaustive/mis-manipulation.ts

@@ -0,0 +1,155 @@
+import { a, ClientSchema } from '@aws-amplify/data-schema';
+import { Amplify } from 'aws-amplify';
+import {
+  buildAmplifyConfig,
+  mockedGenerateClient,
+  optionsAndHeaders,
+  expectGraphqlMatches,
+} from '../../utils';
+
+const sampleTodo = {
+  __typename: 'Todo',
+  id: 'some-id',
+  content: 'some content',
+  description: 'something something',
+  owner: 'some-body',
+  done: false,
+  updatedAt: '2024-03-01T19:05:44.536Z',
+  createdAt: '2024-03-01T18:05:44.536Z',
+};
+
+function manipulateMis(config: Awaited<ReturnType<typeof buildAmplifyConfig>>) {
+  const todoDef = { ...config.modelIntrospection.models['Todo'] };
+  const notTodoDef = { ...config.modelIntrospection.models['NotTodo'] };
+
+  // swap definitions
+  config.modelIntrospection.models['Todo'] = notTodoDef;
+  config.modelIntrospection.models['NotTodo'] = todoDef;
+
+  return config;
+}
+
+describe('Manipulated MIS cannot be used to call the wrong model', () => {
+  const schema = a
+    .schema({
+      Todo: a.model({
+        content: a.string(),
+      }),
+      NotTodo: a.model({
+        content: a.string(),
+      }),
+    })
+    .authorization((allow) => [allow.publicApiKey()]);
+  type Schema = ClientSchema<typeof schema>;
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('create performs operation against intended model', async () => {
+    // #region mocking
+    const { spy, generateClient } = mockedGenerateClient([
+      {
+        data: {
+          createTodo: sampleTodo,
+        },
+      },
+    ]);
+
+    // simulated amplifyconfiguration.json
+    const original = await buildAmplifyConfig(schema);
+    const config = manipulateMis(original);
+    // #endregion mocking
+
+    // App.tsx
+    Amplify.configure(config);
+    const client = generateClient<Schema>();
+    const { errors, data: newTodo } = await client.models.Todo.create({
+      content: 'My new todo',
+    });
+    // #endregion
+
+    // #region assertions
+    expectGraphqlMatches(
+      optionsAndHeaders(spy)[0][0].query,
+      `mutation($input: CreateTodoInput!) { 
+        createTodo(input: $input) { id content createdAt updatedAt }
+      }`,
+    );
+    expect(errors).toBeUndefined();
+    expect(newTodo).toEqual(sampleTodo);
+    // #endregion assertions
+  });
+
+  test('update performs operation against intended model', async () => {
+    // #region mocking
+    const { spy, generateClient } = mockedGenerateClient([
+      {
+        data: {
+          updateTodo: sampleTodo,
+        },
+      },
+    ]);
+
+    // simulated amplifyconfiguration.json
+    const original = await buildAmplifyConfig(schema);
+    const config = manipulateMis(original);
+    // #endregion mocking
+
+    Amplify.configure(config);
+    const client = generateClient<Schema>();
+    const { data: updatedTodo, errors } = await client.models.Todo.update({
+      id: 'some_id',
+      content: 'Updated content',
+    });
+    // #endregion
+
+    // #region assertions
+    expectGraphqlMatches(
+      optionsAndHeaders(spy)[0][0].query,
+      `mutation($input: UpdateTodoInput!) { 
+        updateTodo(input: $input) { id content createdAt updatedAt }
+      }`,
+    );
+    expect(errors).toBeUndefined();
+    expect(updatedTodo).toEqual(sampleTodo);
+    // #endregion assertions
+  });
+
+  test('delete performs operation against intended model', async () => {
+    // #region mocking
+    const { spy, generateClient } = mockedGenerateClient([
+      {
+        data: {
+          deleteTodo: sampleTodo,
+        },
+      },
+    ]);
+
+    // simulated amplifyconfiguration.json
+    const original = await buildAmplifyConfig(schema);
+    const config = manipulateMis(original);
+    // #endregion mocking
+
+    // #region
+    Amplify.configure(config);
+    const client = generateClient<Schema>();
+    const toBeDeletedTodo = {
+      id: '123123213',
+    };
+    const { data: deletedTodo, errors } =
+      await client.models.Todo.delete(toBeDeletedTodo);
+    // #endregion
+
+    // #region assertions
+    expectGraphqlMatches(
+      optionsAndHeaders(spy)[0][0].query,
+      `mutation($input: DeleteTodoInput!) { 
+        deleteTodo(input: $input) { id content createdAt updatedAt }
+      }`,
+    );
+    expect(errors).toBeUndefined();
+    expect(deletedTodo).toEqual(sampleTodo);
+    // #endregion assertions
+  });
+});