aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
83 stars 71 forks source link

Add User id automatically #2639

Open jewells07 opened 1 month ago

jewells07 commented 1 month ago

Describe the feature you'd like to request

Most of the time, we have the user ID (e.g., blog post's author, etc.). Currently, we have to add it manually, or we need to create mutation handlers for this. It will have many handlers for the same work in the schema.

Describe the solution you'd like

We can have something like eventRequestUser, meaning who is creating the post, the userId will set it automatically.


Post: a
      .model({
        userId: a
          .id()
          .required().eventRequestUser(),
        user: a.belongsTo('User', 'userId'),
        content: a.string().required(),
      })
      .authorization((allow) => [
        allow.authenticated()
      ])```

### Describe alternatives you've considered

we have to add it manually, or we need to create mutation handlers for this.

### Additional context

_No response_

### Is this something that you'd be interested in working on?

- [X] 👋 I may be able to implement this feature request

### Would this feature include a breaking change?

- [ ] ⚠️ This feature might incur a breaking change
AnilMaktala commented 1 month ago

Hey @jewells07, thanks for raising this. Can you please try below schema and let us know if meets your usecase?

Post: a
      .model({
        id: a.id().required(),
        userId: a.id(),
        user: a.belongsTo('User', 'userId'),
        content: a.string().required(),
      })
      .authorization(allow => [allow.ownerDefinedIn('userId')])
jewells07 commented 1 month ago

Hey @jewells07, thanks for raising this. Can you please try below schema and let us know if meets your usecase?

Post: a
      .model({
        id: a.id().required(),
        userId: a.id(),
        user: a.belongsTo('User', 'userId'),
        content: a.string().required(),
      })
      .authorization(allow => [allow.ownerDefinedIn('userId')])

But this can change if I pass a different user ID, right? It is not good for security purposes.

jewells07 commented 1 month ago

Can relationships work? I have read that the owner field was made like this: ${event.request.userAttributes.sub}::${event.userName} (https://docs.amplify.aws/react/build-a-backend/functions/examples/create-user-profile-record/). So in this case, it would not work.

userId: a.id(),
user: a.belongsTo('User', 'userId')
biller-aivy commented 1 month ago

I don't understand why amplify is still using subs from cognito? At some point this was a "new feature for security reasons" but I can't understand why? This is horrible! WARNING: if you ever have to migrate your user to a new cognito instance, the users will get a NEW sub, because the subs are unique in the whole cognito service. So this is a nogo!!!

There is a flag to just use username - I recommend it to use it.

jewells07 commented 1 month ago

Nope, it's not working; it saved as the string ${event.request.userAttributes.sub}::${event.userName}. and for relationships, we need an id to connect.

biller-aivy commented 1 month ago

Nope, it's not working; it saved as the string ${event.request.userAttributes.sub}::${event.userName}. and for relationships, we need an id to connect.

in Gen1:

{
          allow: owner
          identityClaim: "username" # explicit use of username
          operations: [delete, read, create] #
        }
jewells07 commented 1 month ago

There is a similar thing in Gen 2 (https://docs.amplify.aws/react/build-a-backend/data/customize-authz/configure-custom-identity-and-group-claim/). But I still get confused about how to use it and whether it will work in relationships or not.

jewells07 commented 1 month ago

I got the solution, and it's working in the relationship as well.

Post: a
.model({
  userId: a
    .string()
    .required()
    .authorization((allow) => [
      allow.authenticated().to(['read']),
      allow.ownerDefinedIn('userId').identityClaim('user_id').to(['read'])
    ]),
  user: a.belongsTo('UserProfile', 'userId'),
  content: a
    .string()
    .required()
})
.authorization((allow) => [
  allow.authenticated().to(['read']),
  allow.ownerDefinedIn('userId').identityClaim('user_id').to(['read', 'update'])
])

or in mutation handler

import { util } from '@aws-appsync/utils';
import * as ddb from '@aws-appsync/utils/dynamodb';

export function request(ctx) {
  const { sub } = ctx.identity;
  const now = util.time.nowISO8601();
  const item = {
    ...ctx.args,
    createdAt: now,
    updatedAt: now,
    userId: sub,
    __typename: 'Post',
  };
  const key = { id: util.autoId() };
  return ddb.put({ key, item });
}

export function response(ctx) {
  return ctx.result;
}

What should we use, sub or username? if anyone can tell us the pros and cons. or we need to stick with the default `sub::username. Which will be advantageous in the future?


Sometimes I get this error and resolve it when I restart Sandbox. it's weird image

The CloudFormation deployment has failed.
Caused By: ❌ Deployment failed: MissingRequiredParameter: Missing required key 'functionId' in params

Resolution: Find more information in the CloudFormation AWS Console for this stack.