[s3-persistent-adapter] Throw Error if item does not exists

hideokamoto commented 4 years ago

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Example code

export const handler = SkillBuilders.custom()
        .addRequestHandlers(
            {
            canHandle(){
                return true
            },
            async handle(handlerInput) {
                const data = await handlerInput.attributesManager.getPersistentAttributes()
                console.log(data)
                return handlerInput.responseBuilder.getResponse()
            }
        })
        .withPersistenceAdapter(
            new S3PersistenceAdapter({
                bucketName: 'ANY_BUCKET',
                pathPrefix: 'ANY_DIRECTORY_NAME'
            })
        )

Expected Behavior

The console.log(data) will show undefined or {} or null if the persistent attributes do not exist for the user.

Current Behavior

handlerInput.attributesManager.getPersistentAttributes() will throw Exception.

 {
        "message": "Could not read item (ANY_DIRECTORY_NAME/amzn1.ask.account.testUser1) from bucket (ANY_BUCKET): Access Denied",
        "name": "AskSdk.S3PersistenceAdapter Error",
        "stack": "AskSdk.S3PersistenceAdapter Error: Could not read item (CatAgeConverter/amzn1.ask.account.testUser1) from bucket (wp-kyoto-ask-db): Access Denied\n    at Object.createAskSdkError (/var/task/build/index.js:6763:17)\n    at S3PersistenceAdapter.<anonymous> (/var/task/build/index.js:6944:46)\n    at step (/var/task/build/index.js:6896:23)\n    at Object.throw (/var/task/build/index.js:6877:53)\n    at rejected (/var/task/build/index.js:6869:65)\n    at processTicksAndRejections (internal/process/task_queues.js:97:5)"
    }

Possible Solution

We need to handle the Access Denied exception from AWS SDK(S3).

Steps to Reproduce (for bugs)

Context

Your Environment

    "ask-sdk-core": "^2.6.0",
    "ask-sdk-model": "^1.18.0",
    "ask-sdk-runtime": "^2.8.0",
    "ask-sdk-s3-persistence-adapter": "^2.8.0",

Lambda version using Nodejs12.x

Node.js and NPM Info

Node.js version used for development:
NPM version used for development:

hideokamoto commented 4 years ago

FIY.

The Skill's IAM policy for S3 is this.


  AlexaSkillIAMRole:
      Type: AWS::IAM::Role
      Properties:
        AssumeRolePolicyDocument:
          Version: 2012-10-17
          Statement:
            - Effect: Allow
              Principal:
                Service:
                  - lambda.amazonaws.com
              Action:
                - sts:AssumeRole
        Path: /
        Policies:
          - PolicyName: alexaSkillExecutionPolicy
            PolicyDocument:
              Version: 2012-10-17
              Statement:
                - Effect: Allow
                  Action:
                    - logs:*
                  Resource: arn:aws:logs:*:*:*
                - Action:
                    - s3:PutObject
                    - s3:GetObject
                    - s3:DeleteObject
                  Resource:
                    - arn:aws:s3:::ANY_BUCKET/*
                  Effect: Allow

And the example code will work well before creating a new item like this.

'''Failed'''

                const data = await handlerInput.attributesManager.getPersistentAttributes()
                console.log(data)

'''Works'''

                handlerInput.attributesManager.setPersistentAttributes({dummy: true})
                await handlerInput.attributesManager.savePersistentAttributes()
                const data = await handlerInput.attributesManager.getPersistentAttributes(false)
                console.log(data)

hideokamoto commented 4 years ago

Is the IAM policy missing any actions for the s3-persistent-adapter? Or should we create a patch for the issue?

Thanks.

ShenChen93 commented 4 years ago

@hideokamoto

Thanks for reporting this issue. I could reproduce this issue.

In summary, our S3 persistent adapter based on aws sdk, and the getAttributes function is based s3Client.getObject() function. For this issue, when there's no object in S3 bucket, getObject function return an error with error code: 'AccessDenied' instead of 'NoSuchKey'.

I will reach out to aws sdk team for the help, and get back to you. Please keep tuned :)

Thanks, Shen

ShenChen93 commented 4 years ago

Hi @hideokamoto ,

After doing more research, found the root cause. Here is the statement in aws sdk doc: If the object you request does not exist, the error Amazon S3 returns depends on whether you also have the s3:ListBucket permission.

If you have the s3:ListBucket permission on the bucket, Amazon S3 will return an HTTP status code 404 ("no such key") error.

If you don’t have the s3:ListBucket permission, Amazon S3 will return an HTTP status code 403 ("access denied") error.

Thus, you would need to have ListBucket permission as well.

Thanks, Shen

alexa / alexa-skills-kit-sdk-for-nodejs