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

Provide more grant APIs for the StateMachine construct #5933

Closed
2 tasks
jindriago-scf opened this issue Jan 23, 2020 · 5 comments · Fixed by #8486
Closed
2 tasks

Provide more grant APIs for the StateMachine construct #5933

jindriago-scf opened this issue Jan 23, 2020 · 5 comments · Fixed by #8486
Assignees
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on.

Comments

@jindriago-scf
Copy link

jindriago-scf commented Jan 23, 2020

Like StateMachine#grantStartExecution, be able to have a StateMachine#grantDescribeExecution

Use Case

In order to improve readability and have a standard way of give permissions over a StateMachine
As a devops
I'll want to give permission to describe an execution to an aws resource

Proposed Solution

First create an executionArn, like the stateMachineArn, this because the arn are different when giving permissions.
for example:
arn:aws:states:us-east-1:1234567890:stateMachine:StateMachineExample
arn:aws:states:us-east-1:1234567890:execution:StateMachineExample

Then create a method grantDescribeExecution and use the executionArn.

something like this:

abstract class StateMachineBase extends Resource implements IStateMachine {
    // ...
    public abstract readonly stateMachineArn: string;
    public abstract readonly executionArn: string;
    /**
     * Grant the given identity permissions to start an execution of this state
     * machine.
     */
    public grantStartExecution(identity: iam.IGrantable): iam.Grant {
        return iam.Grant.addToPrincipal({
            grantee: identity,
            actions: ['states:StartExecution'],
            resourceArns: [this.stateMachineArn]
        });
    }
    public grantDescribeExecution(identity: iam.IGrantable): iam.Grant {
        return iam.Grant.addToPrincipal({
            grantee: identity,
            actions: ['states:DescribeExecution'],
            resourceArns: [this.executioArn, "*"] // this I'm not sure because could be a better way?     
            // the idea is to have something like this arn:aws:states:us-east-1:123456:execution:StateMachineExample:*
        });
    }
}

and

export class StateMachine extends StateMachineBase {
    /**
     * The ARN of the state machine
     */
    public readonly stateMachineArn: string;
    /**
     * The ARN of the state machine execution
     */
    public abstract readonly executionArn: string;
    constructor(scope: Construct, id: string, props: StateMachineProps) {
        // ...
        this.stateMachineName = this.getResourceNameAttribute(resource.attrName);
        this.stateMachineArn = this.getResourceArnAttribute(resource.ref, {
          service: 'states',
          resource: 'stateMachine',
          resourceName: this.physicalName,
          sep: ':',
        });

        this.executionArn = this.getResourceArnAttribute(resource.ref, {
          service: 'states',
          resource: 'execution',
          resourceName: this.physicalName,
          sep: ':',
        });
    }

On the other hand and not totally related, this could be done also:

    public grantSendTaskSuccess(identity: iam.IGrantable): iam.Grant {
        return iam.Grant.addToPrincipal({
            grantee: identity,
            actions: ['states:SendTaskSuccess'],
            resourceArns: [this.stateMachineArn]
        });
    }
    public grantSendTaskFailure(identity: iam.IGrantable): iam.Grant {
        return iam.Grant.addToPrincipal({
            grantee: identity,
            actions: ['states:SendTaskFailure'],
            resourceArns: [this.stateMachineArn]
        });
    }

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@jindriago-scf jindriago-scf added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 23, 2020
@SomayaB SomayaB added the @aws-cdk/aws-stepfunctions Related to AWS StepFunctions label Jan 23, 2020
@nija-at
Copy link
Contributor

nija-at commented Feb 10, 2020

I would re-shape these APIs to be -

  1. a grantRead() which provides Describe, List and a number of other read operations,
  2. a grantTaskResponse() (or something similar) that provides both SendTaskSuccess, SendTaskFailure and SendTaskHeartbeat that allow for the task to report its status, and
  3. a general grant() API that takes a list of string IAM action names that can be used by the user for fine-grained control.

@nija-at nija-at added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md and removed needs-triage This issue or PR still needs to be triaged. labels Feb 10, 2020
@nija-at nija-at changed the title Create a grantDescribeExecution and maybe grantSendTaskSuccess and grantSendTaskFailure Provide more grant APIs for the StateMachine construct Feb 10, 2020
@jindriago-scf
Copy link
Author

jindriago-scf commented Feb 10, 2020

Oh yeah! Good call Niranjan.

Points 1. and 2. makes all more sense.

The only thing to consider when implementing the 3rd point is that, some IAM actions goes over the states resource and some goes over the execution.
So I see 2 possibilities of implementation for point 3:

  1. 2 grants API, one for the executions and other for the state machine. grant_over_states() grant_over_execution(), or
  2. receive the actions in this grant() API and internally assign it to the correct arn.

@ospfranco
Copy link

Maybe off-topic but I'm not finding much information out there, how can one achieve this right now? manually creating an IAM role and attaching the permissions?

@jindriago-scf
Copy link
Author

Hi @ospfranco ,
We made it like the example above. First we extended the StateMachine implementation and then used instead the original stepfunctions.StateMachine with the new permissions in order to not do it every single time.

class StateMachine(stepfunctions.StateMachine):

    def __init__(
            self,
            scope: core.Construct,
            id_: str,
            definition: IChainable,
            role: Optional[iam.IRole] = None,
            state_machine_name: Optional[str] = None,
            state_machine_type: Optional[StateMachineType] = None,
            timeout: Optional[core.Duration] = None) -> None:
        super().__init__(
            scope=scope,
            id=id_,
            definition=definition,
            timeout=timeout,
            role=role,
            state_machine_name=state_machine_name,
            state_machine_type=state_machine_type
        )
        self.execution_arn = core.Arn.format(
            components=core.ArnComponents(
                resource="execution",
                service="states",
                account=self.stack.account,
                partition=self.stack.partition,
                region=self.stack.region,
                resource_name=self.state_machine_name,
                sep=":"
            ),
            stack=self.stack)

    def grant_describe_execution(
            self,
            identity: iam.IGrantable) -> iam.Grant:
        return iam.Grant.add_to_principal(
            grantee=identity,
            actions=[Action.DESCRIBE_EXECUTION.value],
            resource_arns=[f"{self.execution_arn}:*"]
        )

    def grant_send_task_success_token(
            self,
            identity: iam.IGrantable) -> iam.Grant:
        return iam.Grant.add_to_principal(
            grantee=identity,
            actions=[Action.SEND_TASK_SUCCESS.value],
            resource_arns=[self.state_machine_arn]
        )

    def grant_send_task_failure_token(
            self,
            identity: iam.IGrantable) -> iam.Grant:
        return iam.Grant.add_to_principal(
            grantee=identity,
            actions=[Action.SEND_TASK_FAILURE.value],
            resource_arns=[self.state_machine_arn]
        )

And we use it like this:

class ExampleStateMachine(StateMachine):
    ARN_ENV_VAR = "EXAMPLE_STEP_FUNCTION"

    def __init__(self,
                 scope: core.Construct,
                 example_lambda: ExampleLambda) -> None:
        example_state = LambdaTask( # this is also our extension of a Task
            scope=scope,
            task_id="ExampleState",
            lambda_function=example_state,
            timeout=core.Duration.minutes(5))

        definition = example_state

        super().__init__(
            scope=scope,
            id_="ExampleStateMachine",
            definition=definition,
            timeout=core.Duration.minutes(5))

Then to instance the state machine:

example_state_machine = ExampleStateMachine(
            scope=self,
            example_lambda=example_lambda)

And to grant the permission:

        example_state_machine.grant_start_execution(
            identity=lambda_who_call_the_state_machine.grant_principal)
        example_state_machine.grant_describe_execution(
            identity=lambda_who_call_the_state_machine.grant_principal)

You can also do it as @nija-at suggest, that I think is more elegant

@nija-at nija-at assigned shivlaks and unassigned nija-at May 26, 2020
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Jun 11, 2020
@mergify mergify bot closed this as completed in #8486 Jun 22, 2020
mergify bot pushed a commit that referenced this issue Jun 22, 2020
This PR closes #5933. It adds additional grant APIs to supplement `grantStartExecution()`. 

API additions to `state-machine.ts`:

- `grantRead()` method that grants read access to a role.
- `grantTaskResponse()` method that grants task response access to a role.
- `grantExecution()` method that allows user to specify what action to map onto all executions.
- `grant()` method that allows user to specify what action to map onto the state machine.

API additions to `activity.ts`:

- `grant()` method that allows user to specify what action to map onto the activity.


The idea behind these API methods is to mimic the convention of other `grant()` APIs in other modules. This is slightly more difficult with Step Functions because of the multiple resources that IAM actions can map onto. The rationale is that `grant()` in `state-machine.ts` will handle all custom permissions for the state machine, `grantExecution()` in `state-machine.ts` will handle all custom permissions for the executions, and `grant()` in `activity.ts` will handle all custom permissions for activities.

`grantRead()` and `grantTaskResponse()` are convenience APIs that were specified in the original issue for this feature.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@seansullivan
Copy link

Perhaps I'm missing something here, but the grantTaskResponse() does not appropriately set the permission on a Lambda function as it has an UnknownPrincipal as stated here

Upon deploying CDK project, I am met with the warning message:

Add statement to this resource's role: {"action":["states:SendTaskSuccess"],"notAction":[],"principal":{},"notPrincipal":{},"resource":[{"Ref":"myStateMachineStackMyStateMachineA67BBFB7"}],"notResource":[],"condition":{},"effect":"Allow"}

Of course, if I add the permission directly on the Lambda's role as a policy statement, everything works fine:

new PolicyStatement({
        effect: Effect.ALLOW,
        actions: ['states:SendTask*'],
        resources: [`arn:aws:states:${this.region}:${this.account}:stateMachine:*`]
})

Is the intent of these APIs to allow to use as such for a Lambda function?

const myLambdaFunction = Function.fromFunctionArn(myLambdaFunctionArn);

myStateMachine.grantTaskResponse(myLambdaFunction);

For further context, this Lambda lives outside of StepFunctions and is used for manual task response via API Gateway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-stepfunctions Related to AWS StepFunctions effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants