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

Comments: Delete a Note #56

Closed
jayair opened this Issue Apr 10, 2017 · 21 comments

Comments

Projects
None yet
7 participants
@jayair
Copy link
Member

jayair commented Apr 10, 2017

@jayair jayair added the Discussion label Apr 10, 2017

@geirman

This comment has been minimized.

Copy link

geirman commented Apr 16, 2017

shouldn't we also be deleting the attachment as well?

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Apr 16, 2017

Yeah we should. But just for the sake of the tutorial we are keeping it simple.

@geirman

This comment has been minimized.

Copy link

geirman commented Apr 16, 2017

I get that, but it's such a complete tutorial in all other ways... I'd love to see this included too. Does it make things that much more complex? Really REALLY loving this tutorial BTW! It's just what I needed to get me started... my requirements are slightly more complicated with regard to authorization and I'd love to integrate graphql, tests, and cli-based-deployment... but this is a really great start! THANK YOU!

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Apr 16, 2017

It adds an extra step, I might work on it when I get a chance.

@nerdguru

This comment has been minimized.

Copy link

nerdguru commented Apr 27, 2017

I fear I missed another config somewhere as I'm 502'ing on my delete. Checked everything manually, but still stuck. Any suggestions?

image

@geirman

This comment has been minimized.

Copy link

geirman commented Apr 27, 2017

@nerdguru

I think you're probably missing the cors header in the response-lib.js

function buildResponse(statusCode, body) {
  return {
    statusCode: statusCode,
    headers: {
      'Access-Control-Allow-Origin': '*',
      'Access-Control-Allow-Credentials': true,
    },
    body: JSON.stringify(body),
  };
}
@nerdguru

This comment has been minimized.

Copy link

nerdguru commented Apr 27, 2017

Nope, good suggestion @geirman but I have the exact same buildResponse in response-lib.js

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Apr 28, 2017

@nerdguru the other area to check would be your serverless.yml. Just make sure that cors: true is set under your delete block.

https://github.com/AnomalyInnovations/serverless-stack-demo-api/blob/master/serverless.yml#L95

@nerdguru

This comment has been minimized.

Copy link

nerdguru commented Apr 30, 2017

@jayair It was a serverless.yml issue, just not that one. My line below the authorizer for my ARN wasn't indented correctly. Thanks for the spark to find this one!

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented May 1, 2017

@nerdguru quite a few people run into the issue with the YAML indentation!

@djheru

This comment has been minimized.

Copy link

djheru commented Aug 22, 2017

HI @jayair thanks for the awesome guide. I was wondering if you could help me out with deleting the attachment. I created a function in awsLib.js:

export async function deleteS3Object(filename, userToken) {
  await getAwsCredentials(userToken);
  const {Bucket} = config.s3;
  const s3 = new AWS.S3({
    params: { Bucket }
  });
  return s3.deleteObject({
    Bucket, Key: filename
  }).promise();
}

and then in Notes.js, I made a function:

deleteAttachment() {
    if (this.state.note && this.state.note.attachment) {
      return deleteS3Object(this.state.note.attachment, this.props.userToken);
    }
  }

Then, in handleDelete, I call the function:

  handleDelete = async (event) => {
    event.preventDefault();

    const confirmed = window.confirm('Are you sure you want to delete this note?');

    if ( ! confirmed) {
      return;
    }

    this.setState({ isDeleting: true });

    try {
      await this.deleteNote();
      await this.deleteAttachment();
      this.props.history.push('/');
    }
    catch(e) {
      alert(e);
      this.setState({ isDeleting: false });
    }
  };

And for update, I call it if we're uploading a new attachment:

handleSubmit = async (event) => {
    let uploadedFilename;

    event.preventDefault();

    if (this.file && this.file.size > config.MAX_ATTACHMENT_SIZE) {
      alert('Please pick a file smaller than 5MB');
      return;
    }

    this.setState({ isLoading: true });

    try {

      if (this.file) {
        await this.deleteAttachment();
        uploadedFilename = (await s3Upload(this.file, this.props.userToken)).Location;
      }

      await this.saveNote({
        ...this.state.note,
        content: this.state.content,
        attachment: uploadedFilename || this.state.note.attachment,
      });
      this.props.history.push('/');
    }
    catch(e) {
      alert(e);
      this.setState({ isLoading: false });
    }
  };

Am I doing something wrong in awsLib with the deleteS3Object function? I am getting a 403 forbidden error. Everything else (including adding attachments to new notes) is working fine.

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Aug 22, 2017

@djheru The only difference while deleting is that the filename that we need to pass in should not include the full url. We just need the file name.

Here is a PR that I didn't get a chance to merge yet that maybe helpful - https://github.com/AnomalyInnovations/serverless-stack-demo-client/pull/3/files

@jdeagle

This comment has been minimized.

Copy link

jdeagle commented Sep 3, 2017

@jayair I think I am getting the same error as @djheru. Using the pull request above, I get a

"NotAuthorizedException". "Access to Identity 'us-east-1:......' is forbidden."

I noticed the pull request is part of an older step so it uses getAwsCredentials in s3Delete rather than if(!await authUser) {. If I replace getAwsCredentials with the same if(!await authUser) block as s3Upload I get a different error tied to the S3 asset:

Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource.

When I look at the request, its using OPTIONS as the method. I tried adding that to the bucket cors policy but it errored saying its not allowed.

I only get the error if I try to delete the s3Object. I am able to add images to s3 but not delete. How does the bucket know what incognito user can delete an object? I have been searching through the bucket permissions. Its using the correct cors policy from the https://serverless-stack.com/chapters/create-an-s3-bucket-for-file-uploads.html chapter but I am still confused about how a cognito user (and only that user) can be allowed to delete an object they uploaded.

My delete method:

export async function s3Delete(filename) {
    if(!await authUser) {
        throw new Error("User is not logged in");
    }

    const s3 = new AWS.S3();

    return s3.deleteObject({
        Bucket: config.s3.BUCKET,
        Key: filename
    }).promise();
}

and it being called from handleDelete

try {
        if(this.state.note.attachment) {
            const s3File = this.state.note.attachment.match(/(?:.*?\/){3}(.*)/);
            await s3Delete(unescape(s3File[1]));
        }
        await this.deleteNote();
        this.props.history.push("/");
} catch (e) { ... }

Thanks

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Sep 3, 2017

@jdeagle I'm not sure if you've already done this. But in that chapter (https://serverless-stack.com/chapters/create-an-s3-bucket-for-file-uploads.html) you might have noticed this block.

<CORSConfiguration>
	<CORSRule>
		<AllowedOrigin>*</AllowedOrigin>
		<AllowedMethod>GET</AllowedMethod>
		<AllowedMethod>PUT</AllowedMethod>
		<AllowedMethod>POST</AllowedMethod>
		<AllowedMethod>HEAD</AllowedMethod>
		<MaxAgeSeconds>3000</MaxAgeSeconds>
		<AllowedHeader>*</AllowedHeader>
	</CORSRule>
</CORSConfiguration>

We need to add a row here allowing our users to be able to perform the DELETE method as well.

@jdeagle

This comment has been minimized.

Copy link

jdeagle commented Sep 4, 2017

Thanks, that did it. Also found this in the S3 CORS documentation:

The request method (for example, GET or PUT) or the Access-Control-Request-Method header in case of a preflight OPTIONS request must be one of the AllowedMethod elements.

Note
The ACLs and policies continue to apply when you enable CORS on the bucket.

So even though DELETE method is allowed because we added it to the CORS rule, the bucket policies prevent deletes from the general public. Yay.

@DavidNaMills

This comment has been minimized.

Copy link

DavidNaMills commented Mar 13, 2018

help!!!
my notes wont delete. i've literally copied and pasted the code from the tutorial but nothing works.
i hit delete, i am redirected to the homepage, and the note is still there.
there is a response code of 200, there is no error message.

my serverless.yaml code is as follows:

 delete:
    handler: delete.main
    events:
      - http:
          path: notes/{id}
          method: delete
          cors: true
          authorizer: aws_iam

the CORS is fine, I have no problem with any other api gateway
the IAM has the "amazonAPIGatewayInvokeFullAccess" policy attached.

but no errors anywhere.

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Mar 13, 2018

@DavidNaMills If you are getting a response of 200, then your best bet would be to use some console.log statements in your Lambda function to see what is going on.

Maybe after this line here - https://github.com/AnomalyInnovations/serverless-stack-demo-api/blob/master/delete.js#L17.

@DavidNaMills

This comment has been minimized.

Copy link

DavidNaMills commented Mar 13, 2018

@jayair Thanks for your help. The problem was in that file. as "get" "put" "delete" are practically the same code, i copied and pasted but forgot to change to "delete".
thanks for your time. im such an idiot.

@alex-romanov

This comment has been minimized.

Copy link

alex-romanov commented Apr 19, 2018

This might not belong here, but is in this step when I found the error.
I'm unable to do anything with attachments, when I select an attachment and click save i get the following errors on the console:

Failed to load https://arm-notes-app-uploads.s3.amazonaws.com/?max-keys=0: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. Origin 'http://localhost:3000' is therefore not allowed access. The response had HTTP status code 403.
2xhr.js:83 OPTIONS https://arm-notes-app-uploads.s3.amazonaws.com/?max-keys=0 403 (Forbidden)

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented Apr 20, 2018

@alex-romanov A couple of things to check. Check the CORS config we set in this chapter - https://serverless-stack.com/chapters/create-an-s3-bucket-for-file-uploads.html. And make sure the IAM policy in the chapter is set properly - https://serverless-stack.com/chapters/create-a-cognito-identity-pool.html. There are also some tips on debugging the IAM policy here - https://serverless-stack.com/chapters/debugging-serverless-api-issues.html#missing-iam-policy.

@jayair jayair closed this May 9, 2018

@jayair jayair reopened this May 9, 2018

@jayair

This comment has been minimized.

Copy link
Member Author

jayair commented May 9, 2018

@jayair jayair closed this May 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.