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
89 stars 77 forks source link

@auth directive for custom query works only with function resolvers #1220

Open mstoyanovv opened 1 year ago

mstoyanovv commented 1 year ago

How did you install the Amplify CLI?

npm

If applicable, what version of Node.js are you using?

No response

Amplify CLI Version

10.5.1

What operating system are you using?

Mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes made

Describe the bug

Following the documentation here - if you have used 'amplify add custom' to create vtl pipeline via aws cdk template and add @auth directive to the custom query the build will fail because cdk is clashing with the auto created resolvers inside /build folder.

That happens because when @auth is used amplify adds a resolver for that query in the generated cloudformation template. Without @auth I could not find a way to attach IAM statement for that custom query to the unauth role.

Seems like @auth on custom query can be used only with functions at the moment.

Expected behavior

@auth directive should be working with custom vtls created by cdk as described in the documentation.

Reproduction steps

1.Init amplify project with sample graphql schema 2.Create custom query in schema.graphql 3.Create vtls for that query by following this doc 4.Add @auth(rules: [{ allow: public, provider: iam }]) to that query doc 5.Deploy app

Project Identifier

No response

Log output

``` # Put your logs below this line ```

Additional information

Current workaround is to not use @auth directive but instead use CustomResources.json and attach policy to the unauth role from there.

Changes that made it possible to use @auth without @model but works only with @function: https://github.com/aws-amplify/amplify-cli/pull/3590

Before submitting, please confirm:

AnilMaktala commented 1 year ago

Hey @mstoyanovv, Thank you for raising this issue. This is expected functionality, if you add a custom query to your schema then amplify will generate the required resources for you. For more info please refer to the documentation here.

mstoyanovv commented 1 year ago

Hi @AnilMaktala, thanks for your reply.

What you are saying is not exactly correct. When you add custom query without any directives like @auth or @function attached to it Amplify is not generating any resolvers for it because it leaves that to the developer. You can either create them under 'resolvers' folder or use cdk tempate as it is described in the documentation.

The issue comes when you add @auth directive to that custom query as I have stated because amplify is adding resolvers to the cloudformation file together with the IAM rule under auth/unauth role. Those resolvers are clashing with the ones from cdk and therefore you cannot use @auth if you have used cdk for that resolver.

In cases where you want to specify what data source should be used for that custom query you are limited to use either custom cloudformation or cdk because there is no way to specify 'dataSource' in the vtl itslef that is under '/backend/api/[apiName]/resolvers' . Then you need to figure out how to add the custom query field to the unAuth IAM managed policy if your graphql authorization is set to IAM.

I think this could be avoided if @auth directive was just adding the right resource to unauth/auth managed policy in the produced cloudformation without including the auto generated resolvers. Then in the cdk stack you just need to create the resolver and you won't get 401 error after that.

Currently it does that but only if you combine @auth and @function directive for that custom query which should not be the case.

AnilMaktala commented 1 year ago

Hey @mstoyanovv, I appreciate the additional information. I'll check with the engineering team, then get back to you.

AnilMaktala commented 1 year ago

Hi @mstoyanovv, Apologies for the delay!! Thanks for providing those specifics. I was able to duplicate this issue using the details you provided. This is being flagged as a feature request for the team to consider further.

mstoyanovv commented 1 year ago

Hi @AnilMaktala. I am sorry but isn't future request for something that is not implemented in the product and is wanted by clients? What I have described is something that does not work as expected therefore it goes to category 'bug'. Devs deploying from local can get into this by mistake and loose lots of time trying to figure out why their schema file is being overwritten.

sundersc commented 1 year ago

@mstoyanovv - I see that @auth directive is creating the resolvers with names Query.queryName.req.vtl and Query.queryName.res.vtl in this specific case. So when you are adding custom resources you should be able to add files with different names (say Query.queryName.req.1.vtl) and add them as PIPELINE resolver. Could you post the error you got related to the conflict? Also, please post the custom CDK code if it is okay.

sundersc commented 1 year ago

I see the issue now. Even when I'm using pipeline resolver, I get the error.

🛑 The following resources failed to deploy:
Resource Name: pipelineresolver (AWS::AppSync::Resolver)
Event Type: create
Reason: Only one resolver is allowed per field. (Service: AWSAppSync; Status Code: 400; Error Code: BadRequestException; Request ID: xyz; Proxy: null)

🛑 Resource is not in the state stackUpdateComplete
Name: pipelineresolver (AWS::AppSync::Resolver), Event Type: create, Reason: Only one resolver is allowed per field. (Service: AWSAppSync; Status Code: 400; Error Code: BadRequestException; Request ID: xyz; Proxy: null)
sundersc commented 1 year ago

There is a workaround (but not ideal). Since auth directive is creating resolvers Query.queryName.req.vtl and Query.queryName.res.vtl, you can place a custom resolver with name Query.queryName.finish.req.vtl (this uses 'finish' slot) in <proj_root>/amplify/backend/api/<api_name>/resolvers, it should get automatically assigned to the query, there is no need for custom resource. (This is not going to solve the usecase here because there is no way to specify the datasource for the resolver).

Talking about the actual issue, it is definitely a bug but the fix is not trivial. Even though we are using pipeline resolvers, from a CDK perspective it is still a single "Type": "AWS::AppSync::Resolver" resource. I can't find a way to define pipelineConfig for this resolver resource across stacks.

csmcallister commented 1 year ago

I, too, have just encountered this issue. As a workaround, couldn't we replace the custom VTL resolver with a custom Lambda Function resolver? For instance:

type Mutation {
  myCustomMutation(args: String): String
}

could be replaced with:

type Mutation {
  myCustomMutation(args: String): String @function(name: "myCustomMutation-${env}") @auth(rules: [{ allow: private, provider: iam}])
}

The Lambda function would implement the business logic encapsulated in the VTL resolver. Any gotchas I'm missing here?

hackmajoris commented 1 year ago

Another high issue which has to be served as a high priority, in my opinion.

hackmajoris commented 1 year ago

Update: Found a way to add authorisation for a custom resolver.

You can do the authorisation logic at the resolver level. It can be either VLT or JavaScript resolver. For my case, I have a JavaScript resolver, and having something like this, does the trick:

function exportResponse(ctx) {
  const isAuthorized = checkAuthorization();

  if (!isAuthorized) {
    return {
      items: [],
      error: 'Unauthorized',
      total: 0,
      nextToken: null,
    };
  }

  return {
    items: [{ "someitems" }],
    total: 1,
    nextToken: null,
  };
}

function checkAuthorization() {
  if (util.authType() === 'User Pool Authorization') {
    const staticGroupRoles = [
      { claim: 'cognito:groups', entity: 'SuperProjectManager' },
      { claim: 'cognito:groups', entity: 'ProjectManager' },
    ];

    return staticGroupRoles.some((groupRole) => {
      const groupsInToken = ctx?.identity?.claims?.[groupRole.claim];
      return groupsInToken && groupsInToken.includes(groupRole.entity);
    });
  }

  return false;
}

At the schema.graphql level, you also have to add @aws_cognito_user_pools directive for your custom resolver query/mutation.

  customUserSearchQuery(input: CustomUserSearchQueryInput): CustomSearchableUserResult @aws_cognito_user_pools

The developer guide is useful: https://docs.aws.amazon.com/pdfs/appsync/latest/devguide/appsync-dg.pdf#security-authz

I think we have to update the official docs because you can't find anything there related to this use-case