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: Connect to API Gateway with IAM Auth #113

Closed
jayair opened this issue Jul 19, 2017 · 38 comments
Closed

Comments: Connect to API Gateway with IAM Auth #113

jayair opened this issue Jul 19, 2017 · 38 comments

Comments

@jayair
Copy link
Contributor

jayair commented Jul 19, 2017

Link to chapter - http://serverless-stack.com/chapters/connect-to-api-gateway-with-iam-auth.html

@p31d3ng
Copy link

p31d3ng commented Aug 3, 2017

if (AWS.config.credentials && Date.now() < AWS.config.credentials.expireTime - 60000) {
    return;
  }

This block of code in function getAwsCredentials(userToken) will create some issues if a user login to a different account before AWS.config.credentials expires.

Imagine the following scenario on a certain machine:

  1. User A login
  2. User A creates a new note
  3. User A logs out
  4. User B login
  5. User B sees all notes of User A. Because AWS.config.credentials exists and not yet expires, the userId of the outgoing requests are actually from user A.

@jayair
Copy link
Contributor Author

jayair commented Aug 3, 2017

@p31d3ng It's not in this chapter but we clear the credentials when a user logs out.

I should also add that there is a bit of an issue with the current logout code that I'll be putting out a fix for soon - #50 (comment).

@yashg5
Copy link

yashg5 commented Aug 4, 2017

is there any easy to trigger API calls by using AWS SDK without manually signing the request so that AWS SDK takes care of signing process. is it possible like this from mobile front end using AWS IOS SDK?

@jayair
Copy link
Contributor Author

jayair commented Aug 6, 2017

@yashg5 We can disable IAM temporarily and test our APIs but we would need to mock where we get our user id from.

You should be able to do a similar setup from iOS as well.

@quantuminformation
Copy link

Anyone know why AWS passes over to the user all this complexity (signing etc) instead of handling it with the sdk, kinda like what firebase does? I'm sure there are good reasons.

@jayair
Copy link
Contributor Author

jayair commented Oct 6, 2017

@quantuminformation I think the Firebase equivalent would be if you used the Cognito User Pool on it's own and instead of IAM as the authorizer use the User Pool as the authorizer. Unfortunately you still need to work with IAM roles and such when you need to work with S3 or other AWS resources.

One of our older versions covers this setup - https://59caadbd424ef20abdc342b4--serverless-stack.netlify.com/chapters/call-the-create-api.html.

We might add a chapter on why use IAM as an authorizer vs User Pool.

@quantuminformation
Copy link

lol I thought we used the User Pool as the authorizer

@jayair
Copy link
Contributor Author

jayair commented Oct 6, 2017

@quantuminformation AWS is pretty confusing but I should have been more clear, we are using IAM as an authorizer for API Gateway. But to talk to API Gateway we generate a set of temporary credentials through Cognito Federated Identities where the User Pool is the authentication provider.

@zacacollier
Copy link

@jayair first of all thank you for writing this section, it is super helpful. I was surprised at how hard it was to find a straightforward, pluggable solution for signing requests on the client (most of what I came across was intended for serverside use)

just wanted to point out that the rawgit link to the sigV4Client.js is out of date, I encountered an issue while trying to sign a request, but when I went to open a PR I discovered you'd fixed it on master

@jayair
Copy link
Contributor Author

jayair commented Oct 11, 2017

@zacacollier Thank!

For the sigV4Client.js, I'm not sure I see the issue you are talking about because that file has never been edited (if you check the history, https://github.com/AnomalyInnovations/serverless-stack-demo-client/commits/master/src/libs/sigV4Client.js). I wonder what didn't work for you.

@tommedema
Copy link

Hi all. I'm confused as to why we need to deal with IAM role creation and Policy Documents like these:

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Action": [
        "mobileanalytics:PutEvents",
        "cognito-sync:*",
        "cognito-identity:*"
      ],
      "Resource": [
        "*"
      ]
    },
    {
      "Effect": "Allow",
      "Action": [
        "s3:*"
      ],
      "Resource": [
        "arn:aws:s3:::YOUR_S3_UPLOADS_BUCKET_NAME/${cognito-identity.amazonaws.com:sub}*"
      ]
    },
    {
      "Effect": "Allow",
      "Action": [
        "execute-api:Invoke"
      ],
      "Resource": [
        "arn:aws:execute-api:YOUR_API_GATEWAY_REGION:*:YOUR_API_GATEWAY_ID/*"
      ]
    }
  ]
}

Isn't serverless.com supposed to take care of this complexity, and automatically create and assign roles when necessary?

My biggest pain in following the guide is how specific it is to AWS and how much knowledge is required on the specifics on AWS services. I had expected these to all be abstracted away into the serverless framework.

@quantuminformation
Copy link

You'll never be able to completely abstract away AWS otherwise you will end up with more complexity. My next video will cover this policy file as it isn't that hard once you get it.

@jayair
Copy link
Contributor Author

jayair commented Oct 19, 2017

@tommedema It's an interesting thought for sure. One of the unwritten aspects of the serverless approach is that you are investing more in the specific cloud provider's services and methods.

Serverless Framework (or potentially any other framework) could abstract these out but then you'll be relying heavily on which features of the cloud provider they expose to you. Serverless Framework has decided that they want to be a layer on top of all these providers instead of being a specific solution for AWS. This is why a lot of this has not been abstracted out.

@dailenspencer
Copy link

Still receiving the following error if anyone has any thoughts :)

TypeError: __WEBPACK_IMPORTED_MODULE_4__sigV4Client__.a.newClient(...).signRequest is not a function

@jayair
Copy link
Contributor Author

jayair commented Oct 30, 2017

@dailenspencer Can I try your project somehow? I want to see what is going.

@dailenspencer
Copy link

@jayair yes, if you click on this link you should be able to have access
https://github.com/dailenspencer/Element/invitations

@jayair
Copy link
Contributor Author

jayair commented Oct 30, 2017

@dailenspencer Got it. How to I reproduce the error again?

@dailenspencer
Copy link

dailenspencer commented Oct 30, 2017

@jayair Steps to reproduce

  1. Run yarn install
  2. Run yarn start and wait for app to load at localhost:3000
  3. Head to localhost:3000/signin and login with the following credentials

username: dailenspencer@gmail.com
password: Sodacan123!

@dailenspencer
Copy link

I've just pushed an update so you may want to re-pull if you've already cloned the project

@jayair
Copy link
Contributor Author

jayair commented Oct 31, 2017

@dailenspencer This is element-app-client correct?

I logged in an everything seems fine. I'm using NPM but it seems curious that your package.json does not have the aws-sdk like the one we have - https://github.com/AnomalyInnovations/serverless-stack-demo-client/blob/master/package.json#L7

Any idea why it's not in there?

screen shot 2017-10-31 at 7 44 31 pm

@dailenspencer
Copy link

@jayair Apologies. The correct folder is isomorphic/

@jayair
Copy link
Contributor Author

jayair commented Nov 1, 2017

@dailenspencer I'm not sure how much I can help you with your Isomorphic app. This isn't a project based on create-react-app so my experience is a little limited here. But I noticed the aws-sdk is not in your package.json in this project either. Any idea why?

@fcostarodrigo
Copy link
Contributor

@dailenspencer I am having the same issue as you.

Basically when sigV4Config.secretKey is undefined, sigV4Client.newClient(sigV4Config) returns and empty object {} that doesn't have the signRequest function, causing the error you see.

This happens when we call authUser twice. While the first call waits for the promise in getAwsCredentials to resolve, the second call overwrites the config in AWS.config.credentials = new AWS.CognitoIdentityCredentials. Then, when the promise of the first call resolves, the config won't be the original config with the secret key you were waiting for.
This bug doesn't happen always, if you keep refreshing the page, you can see that sometimes the code does work.

This is my workaround:

let loadingCredentials = null;

function getAwsCredentials(userToken) {
  if (loadingCredentials) return loadingCredentials;

  const authenticator = `cognito-idp.${config.cognito
    .REGION}.amazonaws.com/${config.cognito.USER_POOL_ID}`;

  AWS.config.update({ region: config.cognito.REGION });

  AWS.config.credentials = new AWS.CognitoIdentityCredentials({
    IdentityPoolId: config.cognito.IDENTITY_POOL_ID,
    Logins: {
      [authenticator]: userToken
    }
  });

  loadingCredentials = AWS.config.credentials.getPromise();

  await loadingCredentials;

  loadingCredentials = null;
}

@jayair
Copy link
Contributor Author

jayair commented Nov 22, 2017

@fcostarodrigo Thanks for tracking this down This one has been a tough one to reproduce. I'm going to take a better look and figure it out.

@jayair
Copy link
Contributor Author

jayair commented Nov 29, 2017

@fcostarodrigo I've been trying to reproduce this and I haven't been able to. Are you just refreshing the home page constantly trying to get the error? Also, if you could share your repo and the browser you are using, that would help greatly.

@fcostarodrigo
Copy link
Contributor

@jayair I tried to write a test to reproduce the error, but it is really hard. It depends on the order the promises are resolved.

I will create a new project in my personal account and share it with you soon. Maybe our changes introduced the bug too, who knows.

Thanks.

@fcostarodrigo
Copy link
Contributor

@jayair
I reproduced the bug here: https://github.com/fcostarodrigo/notes-app-client
I had to make a few changes to the original code to trigger the bug.

  1. In Home.js you have to check if the user is logged with await authUser() instead of this.props.isAuthenticated.
  2. In App.js you have to render even when this.state.isAuthenticating is true.

If App.js is the only thing checking if the user is authenticated and all other components wait for this check to come from App.js, then things work fine. But if another component tries to see if the user is authenticated concurrently, then the code can fail.

I guess this is not really a bug, it just happens that authUser should not be called concurrently.

@jayair
Copy link
Contributor Author

jayair commented Dec 4, 2017

@fcostarodrigo Thanks for putting this together.

Yeah I see what you mean about calling authUser concurrently. I'm not sure how with the original code it can happen.

@fcostarodrigo
Copy link
Contributor

@jayair Yes, this won't happen in the original code. It only happens if you modify it enough.

@appernetic
Copy link

A similar error also happens in the original code. I have followed the guide to this step and just got:

TypeError: WEBPACK_IMPORTED_MODULE_0_aws_sdk_global.util.crypto.lib.randomBytes is not a function

@jayair
Copy link
Contributor Author

jayair commented Jan 13, 2018

@appernetic Can you share your repo? I need to try this out. It really should not happen.

@xpresslanej
Copy link

@appernetic - see this thread: amazon-archives/amazon-cognito-identity-js#646

@qwtel
Copy link

qwtel commented Jan 31, 2018

For anybody who'd rather not depend on a JS crypto implementation, I've created a modified version of sigV4Client.js that uses the Web Cryptography API. It only uses crypto-js as a fallback (Browser support looks pretty good though).

Since web crypto uses promises, signRequest now needs to return a promise as well.
You'll have to add an extra await when using it:

const signedRequest = await sigV4Client // added `await`
  .newClient({...})
  .signRequest({...})

Unrelated: This tutorial series is amazing. Thanks so much for making this available!

@sanbeaman
Copy link

Great info on this topic!
I modified your project for what I thought would be a simpler task, but I'm having a fetch / CORs issue.

So I know the following issue is NOT an issue with your code, but I'm hoping someone can point me in the right direction. Because I've made the following changes:

  1. DynamoDb has a single primary key, no sort key
  2. I'm still using Congnito & Federated Identities, but not using the userid as part of the sort key in dynamodb. Basically, I just want an authorized user to be able to view / modify any item in the database. (ISsue probably related to this, maybe I sholdn't use FEderated Ids...?)
  3. I removed the file upload to s3 portion (I made sure to keep the api gateway policy info when I removed the s3 part)

ISSUE: When I login, I'm placed on the home page and a list of items is fetched and displayed. Great!. However, clicking on one of the items. I'll get:

TypeError: Failed to Fetch

Failed to load https://xxxxx.execute-api.us-east-1.amazonaws.com/prod/contestents/+13135551212: 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. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

API works using CLI and your test cli tool,
have same issue if client is running locally or published from s3...

Why would the scan call work on Home.js (I changed the query call to a scan call since I no longer have a sort key) but subsequent GET calls get denied access?
I'm thinking it must be on client side and possibly with sigV4Client.js... but I've been troubleshooting without success for awhile now..
thank you!

@jayair
Copy link
Contributor Author

jayair commented Jan 31, 2018

@sanbeaman If you are having some trouble debugging this and you think you've looked at all the usual suspects, I'd suggest trying to look through the logs and seeing what is going on - https://serverless-stack.com/chapters/api-gateway-and-lambda-logs.html.

@sanbeaman
Copy link

@jayair thanks for the response. I've since removed the serverless API's and have started to research IAM , Cognito, etc... in more detail. The guides helped me thru a number of things, and I hope to revist the cloudformation and s3 upload info at a later date! thank you

@clarkgrg
Copy link

clarkgrg commented Feb 20, 2018

@sanbeaman - I ran into that same problem today TypeError: Failed to Fetch. I still don't know why I was receiving it but will describe what I saw in the hope it helps someone else.

I added another entity names tasks with the path /tasks. This task entity is basically just a note with a different name. Since Home.js (route "/") displays a list of notes when I'm logged in I wanted to display a list of tasks when I'm logged in using TaskList.js (route "/tasks") file. So I added a link to the Navbar

\<Nav> <NavItem eventKey={20} href="/tasks">Tasks</NavItem> \</Nav>

Now the curious part. When I manually typed "/tasks" at the end of my URL I had no issues. When I clicked the link Tasks in the menubar I got the Failed to fetch error. In both cases I still displayed my list of tasks. One difference I noted was the application refreshes when I click the link.

So I got location from this.props and changed NavItem to be

<NavItem eventKey={20} componentClass={Link} href="/tasks" to="/tasks" active={location.pathname === '/tasks'}>Tasks</NavItem>

This removed the application refresh and the problem.

@jayair
Copy link
Contributor Author

jayair commented May 9, 2018

@jayair jayair closed this as completed May 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests