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
88 stars 76 forks source link

API Owner Authorization from another Table #341

Open ngnathan opened 4 years ago

ngnathan commented 4 years ago

Which Category is your question related to? API, Auth

What AWS Services are you utilizing? Cognito, AppSync

Context

Problem

Is there a way to achieve this?

Provide additional details e.g. code snippets

Part of the GraphQL schema is shown below.

type Profile 
    @model 
    @key(name: "byUserId", fields: ["userId"], queryField: "profilesByUserId")
    @key(name: "byUsername", fields: ["username"], queryField: "profilesByUsername")
    @auth(rules: [
        { allow: owner, operations: [update, delete] }, 
        { allow: groups, groups: ["Admin"], operations: [update, delete] }
    ]) 
{
    id: ID!
    userId: String!
    username: String!
    email: String!
    firstName: String!
    lastName: String!
    owner: String!
    preferences: UserPreferences!
    profileImageKey: String
    status: ProfileStatus!
    termsAndConditionsAcceptance: TermsAndConditionsAcceptance!
    organizations: [OrganizationUser!]! @connection(keyName: "organizationsByUser", fields: ["id"])
}

type Organization 
    @model 
    @key(name: "byOwner", fields: ["owner", "id"], queryField: "organizationsByOwner" )
    @auth(rules: [
        { allow: owner }, 
        { allow: groups, groups: ["Administrator"] }, 
        { allow: owner, ownerField: "members" }
    ]) 
{
    id: ID!
    name: String!
    city: String!
    state: String!
    country: String!
    owner: String!
    members: [String!]!
    organizationImageKey: String
    users: [OrganizationUser!]! @connection(keyName: "profilesByOrganization", fields: ["id"])
    locations: [Location!] @connection(keyName: "locationsByOrganization", fields: ["id"])
}

type OrganizationUser 
    @model(queries: null)
    @key(name: "profilesByOrganization", fields: ["organizationId", "profileId"])
    @key(name: "organizationsByUser", fields: ["profileId", "organizationId"])
{
    id: ID!
    organizationId: ID!
    profileId: ID!
    organization: Organization! @connection(fields: ["organizationId"])
    profile: Profile! @connection(fields: ["profileId"])
    role: Role!
}

... (other enums)

type Location
    @model
    @key(name: "locationsByOrganization", fields: ["organizationId", "name"])
    @key(name: "ByStatus", fields: ["status", "name"], queryField: "locationsByStatus")
{
    id: ID!
    name: String!
    address: String!
    city: String!
    state: String!
    country: String!
    code: String!
    mapsUrl: String
    notes: String
    status: Status!
    organizationId: ID!
    organization: Organization @connection(fields: ["organizationId"])
}
elorzafe commented 4 years ago

@ngnathan I will transfer your question to CLI team, they will answer this better than me.

RossWilliams commented 4 years ago

(b) restrict it by Cognito Group (i.e. define a group per organization, but there's a limit to Cognito Groups, and I don't think its intention is to scale the number of Groups with the # of users / organizations)

When a user logs in you can provide additional user groups or custom claims using Cognito’s preToken lambda. You can write and attach a lambda that makes DynamoDB calls to lookup the user’s orgs and add these orgs to their group membership, or add them as an additional claim. Then you can use dynamic group auth to restrict access. This gets around any limits in Cognito w.r.t groups.

See aws-amplify/amplify-cli#3118 for a similar question

buggy commented 4 years ago

I've had similar discussions with various people at AWS.

My ideal solution would be an API Gateway style Lambda authorizer that allowed me to pass data into each resolver so the user profile only needed to be loaded once per request. It doesn't seem like this is going to implemented any time soon.

The next best solution seems to be a directive like @user_data that would load a record from DynamoDB into $ctx.stash based on the users ID. This would require conversion to pipeline resolvers. The @auth directive could be modified so that there was a way to say "get your groups from this variable in $ctx.stash" much like groupClaim currently does with the Cognito Access/Identity token.

My least favorite option is using a pre-token generation trigger on the Cognito User Pool to load the profile record from DynamoDB then set virtual groups in the users token. I understand that this is more "scalable" but it provides a bad user experience compared to the other two options which allow permissions to be changed immediately without requiring the user to log out/in. At least this option works today.

ngnathan commented 4 years ago

Thanks @RossWilliams for the pointer. I've started building out the pre token lambda. Ideally, I'd like to run it through GraphQL APIs, since I already have the data available in there, but I get stuck on the circular dependency ( aws-amplify/amplify-cli#1874 ). So I'm researching workarounds, or I'll have the build out a separate table in DynamoDB, which is not ideal (as I'll need to manage two tables of organization/user data and keep them in sync).

I like @buggy 's ideas for a solution, but for now I'll probably have to go through the workaround, deal with the log in / log out issue, and look at workarounds for the pre token lambda accessing GraphQL APIs.

ngnathan commented 4 years ago

As an update, I've successfully created the Pre Token Lambda that invokes a separate lambda with access to GraphQL APIs, retrieving data from the OrganizationUser table mentioned above (modified a bit to include Cognito userIds), and then including the organizationId as the Cognito group. However, the drawback with this solution is that my login process now takes 5-10 seconds, as the Pre Token Lambda needs to wait for the secondary lambda and data retrieval before passing the data back to the client. We're fine with this drawback for now, but may decide to switch to a separately managed DynamoDB table in the future.

RossWilliams commented 4 years ago

@ngnathan you can access the DynamoDB tables directly, no need to hop through another lambda and AppSync. If there is a circular dependency issue, you can break it with a custom resource lambda to attach the preToken trigger via AWS-SDK calls to Cognito.

ngnathan commented 4 years ago

@RossWilliams I'm still trying to wrap my head around using a custom resource lambda as you mentioned.

If I don't hop through another lambda + appsync, I keep getting circular dependency issues. Can you perhaps elaborate further on what you mean by: "custom resource lambda to attach the preToken trigger via AWS-SDK calls to Cognito."?

Do you mean: (a) creating a custom resource (similar to https://aws-amplify.github.io/docs/cli-toolchain/quickstart#custom-cloudformation-stacks ) with AppSync permissions and somehow granting the Pre Token Lambda access to the process.env.GraphQLAPIIdOutput and process.env.GraphQLAPIEndpointOutput variables to make calls within the Pre Token Lambda itself?

(b) creating a new lambda, and changing the cognito trigger parameters to call the new lambda (instead of the normal Pre Token Generation function)?

(c) perhaps accessing the DynamoDB tables without the GraphQL APIs?

SebSchwartz commented 4 years ago

For your (c) point, add permission to the lambda for:

{
      Effect: 'Allow',
      Action: ['appsync:ListGraphqlApis'],
      Resource: '*',
    }

We are doing this via a custom:LambdaCallout when deploying (to add the permission to the PreToken lambda via a lambda adding the permission the PreToken lambda role). I don't know it this will work if you add it manually to the lambda cloudformation with amplify (i think it will be override when updating the function via amplify function update later).

Then inside you PreToken function:

// Outside handler:
let appSyncApiId: string | null = null;
const getAppSyncApiId = async () => {
  const apis = await appSync.listGraphqlApis().promise();
  if (isNil(apis) || isNil(apis.graphqlApis) || isEmpty(apis.graphqlApis)) {
    return;
  }
  const apiId = apis.graphqlApis.find(d => d.name === `YOURAPINAME-${ENV}`);
  appSyncApiId = apiId ? apiId.apiId! : null;
};

// Inside the handler:
    if (isNil(appSyncApiId)) {
      await getAppSyncApiId();
    }

// Get Dynamo table name
const tableName = `YourTable-${appSyncApiId}-${ENV}`;

So if your lambda is warm, you have the apiId otherwise, you have to get it first but it's not so much time consuming.

Hope this helps.

ngnathan commented 4 years ago

@SebSchwartz Thanks so much for the guidance! Do you have any guidance on how to create a lambda to add a permission to the Pre Token Lambda role?

SebSchwartz commented 4 years ago

I followed this comment to get it working: https://github.com/aws-amplify/amplify-cli/issues/1204#issuecomment-521211197 (thanks @RossWilliams ;) )

You'll also need to pack cfn-response in your function so you can tell Cloudformation that it's done: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/cfn-lambda-function-code-cfnresponsemodule.html Be carefull with this and always send response back to cloudformation (big try catch for instance) otherwhise in case of mistake, you could be stuck 2 hours waiting for cloudformation: 1h for create (that fails but CF is waiting for response) and 1h for the rollback (which could have same behaviour).

Then when your lambda is working with CF, you just have to call aws sdk for policy management:

ngnathan commented 4 years ago

Thanks @SebSchwartz - I finally started understanding CFTs a little bit more. And thanks again to @RossWilliams for the guidance on multiple threads.

It's sort of diverging from the original topic now, but thought I'd provide an update.

I finally got it working, but in the way that @RossWilliams had initially recommended (I had initially misunderstood his statement) where I used a custom resource to attach a function as a pre token trigger. I didn't understand CFT's role and didn't know functions created through amplify CLI could be used to manipulate other functions during the amplify push process. As a result, I finally was able to create a custom function like #1204 (comment)

If anyone else needs guidance on this, especially those trying to work around circular dependencies, I did the following:

Function 1:

Function 2:

In CFT json:

    "Parameters": {
...
        "functionccadminPreTokenOrgCheckArn": {
            "Type": "String",
            "Default": "functionccadminPreTokenOrgCheckArn"
        }
    },
...
    "Resources": {
        "LambdaFunction": {
            "Type": "AWS::Lambda::Function",
...
                "Environment": {
                    "Variables": {
...
                        "FUNCTION_CCADMINPRETOKENORGCHECK_ARN": {
                            "Ref": "functionccadminPreTokenOrgCheckArn"
                        }
                    }
                },
...
        "AddPreTokenTriggerToLambda": {
            "Type": "Custom::AddPreTokenTriggerToLambda",
            "Properties": {
                "ServiceToken": {
                    "Fn::GetAtt": [
                        "LambdaFunction",
                        "Arn"
                    ]
                },
                "TriggerVersion": 1
            }
        }
    },

In backend-config.json:

    "function": {
...
        "ccadmin8c93026cAddPreTokenTriggerToLambda": {
...
            "dependsOn": [
...
                {
                    "category": "function",
                    "resourceName": "ccadminca9d06a8PreTokenOrgCheck",
                    "attributes": [
                        "Name",
                        "Arn"
                    ]

My lambda function (index.js) looks like the following. Note that this is super rough and not optimized, but just to show:

exports.handler = require('safe-cfn-custom-resource')(async () => {
    const aws = require('aws-sdk');
    const region = process.env.REGION;
    const authCcadminUserPoolId = process.env.AUTH_CCADMIN_USERPOOLID;
    const functionCcadminPreTokenOrgCheckArn = process.env.FUNCTION_CCADMINPRETOKENORGCHECK_ARN;
    const identity = new aws.CognitoIdentityServiceProvider({
        apiVersion: '2016-04-18',
        region: region
    });
    return {
        async create(event, context) {
            try {
                // Get User Pool
                const params = {
                    UserPoolId: authCcadminUserPoolId
                };
                const userPoolResult = await identity.describeUserPool(params).promise();

                // Update User Pool
                if (userPoolResult && userPoolResult.UserPool && userPoolResult.UserPool.LambdaConfig) {
                    const preTokenLambdaArn = userPoolResult.UserPool.LambdaConfig.PreTokenGeneration;

                    // If Pre Token Generation Trigger does not exist, add trigger
                    // Else, replace PreTokenGeneration trigger with PreTokenOrgCheck Function
                    if (!preTokenLambdaArn || preTokenLambdaArn !== functionCcadminPreTokenOrgCheckArn) {
                        const updatedPreTokenLambdaArn = functionCcadminPreTokenOrgCheckArn;
                        const updatedUserPoolSettings = {
                            UserPoolId: userPoolResult.UserPool.Id,
                            LambdaConfig: {
                                ...userPoolResult.UserPool.LambdaConfig,
                                PreTokenGeneration: updatedPreTokenLambdaArn
                            }
                        };
                        const updateUserPoolResult = await identity.updateUserPool(updatedUserPoolSettings).promise();
                    }
                }

                return {
                    id: 'ccadminAddPreTokenTriggerToLambda',
                    data: { triggerAdded: 'Success' }
                };
            } catch (error) {
                console.log('ccadminAddPreTokenTriggerToUserPool > create > error', error);
                return {
                    id: 'ccadminAddPreTokenTriggerToLambda',
                    data: { triggerAdded: 'Error' }
                };
            }
        },
        async update(event, context) {

... (same as create)

        },
        delete(event, context) {
            console.log('ccadminAddPreTokenTriggerToUserPool > delete');
        }
    };
});

The other option with @SebSchwartz 's suggestion is to create the Pre Token Generation trigger normally, and create a new function (similar to above) to add AppSync IAM permissions with AWS-SDK.

Hope this helps someone!

ngnathan commented 4 years ago

Forgot to mention in the comment above that you'll also need to add permissions (lambda:invokeFunction specifically) to the lambda function through aws-sdk. When updating a function through the console, it will add that permission, but through aws-sdk (and CLI) it doesn't add it, so you'll need:

    const aws = require('aws-sdk');
    const lambda = new aws.Lambda();
...
        try {
            const userPoolResult = await identity.describeUserPool(params).promise();
...
                const updatedPreTokenOrgCheckPermissionsParams = {
                    Action: 'lambda:invokeFunction',
                    FunctionName: functionCcadmincaPreTokenOrgCheckName,
                    Principal: 'cognito-idp.amazonaws.com',
                    SourceArn: userPoolResult.UserPool.Arn,
                    StatementId: `1`
                };
                const updatePreTokenOrgCheckPermissionsResult = await lambda
                    .addPermission(updatedPreTokenOrgCheckPermissionsParams)
                    .promise();
undefobj commented 4 years ago

@ngnathan Glad that you got this working! One thing I'd be curious on is if you believe we should add some improvements to the documentation for pretoken generation: https://aws-amplify.github.io/docs/cli-toolchain/quickstart#adding-a-lambda-trigger https://aws-amplify.github.io/docs/cli-toolchain/cognito-triggers#override-id-token-claims If so we would certainly welcome a PR to the docs which is just markdown, or any tactical suggestions we can take otherwise.

Aside from this and rewinding to your top level question there are some advantages to having your authorization metadata defined on a separate table. While @buggy comment on tokens being more scalable is true in some circumstances this is a tradeoff as there are legitimate use cases for having this metadata state defined in a table rather than the token, namely more dynamic access controls and centralization of authority across your @model types. This is especially true for "Friendship" patterns which we've written about a couple of times:

https://hackernoon.com/graphql-authorization-with-multiple-data-sources-using-aws-appsync-dfae2e350bf2 https://docs.aws.amazon.com/appsync/latest/devguide/tutorial-pipeline-resolvers.html

This authorization use case was one of the motivators for creating Pipeline Resolvers and we are looking at more holistic support for these in 2020. You'll see that we have an RFC however it is a non-trivial design and implementation so we don't have an explicit timeline. However we'll keep this issue open as a reference to that use case.

RossWilliams commented 4 years ago

@buggy regarding pre-token

provides a bad user experience compared to the other two options which allow permissions to be changed immediately without requiring the user to log out/in

you can have users request new tokens without having to log out and in again. The trick is telling users that they should update their credentials. This can be solved with subscriptions on a USER_PERMISSONS model.

MontoyaAndres commented 4 years ago

Any news?

thomasklein-winemaker commented 4 years ago

Hi @ngnathan !

I followed you example and got a bit confused with the resource names:

You created the function ccadminAddPreTokenTriggerToUserPool but then in backend-config.json it is referenced as ccadmin8c93026cAddPreTokenTriggerToLambda - so some hash inbetween was added. In the lambda's index.js you return as the physical id ccadminAddPreTokenTriggerToLambda which is, as far as I get it, a non-existent resource. In my case whatever I return at this point I always end up with an error Invalid PhysicalResourceId. Would you know which ID is meant to be referenced here?

UPDATE: Nevermind, had something wrong in my index.js.

asmiki commented 3 years ago

you can have users request new tokens without having to log out and in again. The trick is telling users that they should update their credentials. This can be solved with subscriptions on a USER_PERMISSONS model.

Thanks, @RossWilliams. Super helpful comments in multiple threads regarding dynamic group authorization. Could you please elaborate on this comment? How is this done in practice?

r0zar commented 3 years ago

I've had similar discussions with various people at AWS.

My ideal solution would be an API Gateway style Lambda authorizer that allowed me to pass data into each resolver so the user profile only needed to be loaded once per request. It doesn't seem like this is going to implemented any time soon.

The next best solution seems to be a directive like @user_data that would load a record from DynamoDB into $ctx.stash based on the users ID. This would require conversion to pipeline resolvers. The @auth directive could be modified so that there was a way to say "get your groups from this variable in $ctx.stash" much like groupClaim currently does with the Cognito Access/Identity token.

My least favorite option is using a pre-token generation trigger on the Cognito User Pool to load the profile record from DynamoDB then set virtual groups in the users token. I understand that this is more "scalable" but it provides a bad user experience compared to the other two options which allow permissions to be changed immediately without requiring the user to log out/in. At least this option works today.

I've managed to find a elegant solution to this using the provided directives, and it looks a lot like the first solution provided in the above comment. Reply to this if you'd like more details I can go over it in Amplify's discord channel.

jackbkennedy commented 3 years ago

@r0zar would love to see an implementation / solution using directives if possible.