laardee / serverless-authentication-boilerplate

Generic authentication boilerplate for Serverless framework
http://laardee.github.io/serverless-authentication-gh-pages
MIT License
569 stars 72 forks source link

FaunaDB user storage #31

Closed jchris closed 7 years ago

jchris commented 7 years ago

This PR adds support for using FaunaDB Serverless Cloud as user storage.

thanks! Chris

laardee commented 7 years ago

@jchris nice, I'll check this later today

jchris commented 7 years ago

I'm enhancing this to make it so the userStorage can return a user token that is threaded through the JWT and the API Gateway authorizer, so you can for instance have the user model return a database access token that other functions can use to connect to the database on behalf of the user. (At least that's what I'm using it for.) The main thing that can be changed here is user on the payload, I considered using the name secret.

jchris commented 7 years ago

I'm working on another finishing touch for the test-token handler so that it doesn't have share env.yml. Once it is using the pattern where the authorizer passes in the database access token, then it won't have to have any database access token in its configuration at all.

laardee commented 7 years ago

The user is saved to DB as expected, but I keep getting permission denied error with the test-token endpoint. The _secret is same what comes in (I masked it even though it is changing token), also the terms in request content matches saved user id.

{
  "name": "PermissionDenied",
  "message": "permission denied",
  "requestResult": {
    "client": {
      "_baseUrl": "https://cloud.faunadb.com:443",
      "_timeout": 60000,
      "_secret": "######",
      "_observer": null
    },
    "method": "POST",
    "path": "",
    "query": null,
    "requestContent": {
      "get": {
        "match": {
          "index": "userId"
        },
        "terms": "######"
      }
    },
    "responseContent": {
      "errors": [
        {
          "position": [
            "get"
          ],
          "code": "permission denied",
          "description": "Insufficient privileges to perform the action."
        }
      ]
    },
    "statusCode": 403,
    "responseHeaders": {
      "content-type": "application/json;charset=utf-8",
      "date": "Thu, 23 Feb 2017 22:12:53 GMT",
      "x-faunadb-build": "2.1.26-3e4e327",
      "x-faunadb-host": "ec2-52-91-30-141.compute-1.amazonaws.com",
      "content-length": "123",
      "connection": "Close"
    },
    "startTime": 1487887973143,
    "endTime": 1487887973184
  }
}
quantuminformation commented 7 years ago

I'm looking forward to playing with this repo, after I get the scope example working from youtube serverless.

jchris commented 7 years ago

Thank you for reviewing this. There is one issue that might be causing what you're seeing. The refresh token function does not maintain the user data. So I plan to work on adding that bit of the flow today or tomorrow.

Is there a security reason why the cache shouldn't store this user data and attach it to the refresh token? That is my plan. But it is not always clear which aspects of a single sign on architecture are mitigating attack vectors, and which are just feature driven.

jchris commented 7 years ago

This is still not ready to merge. I'm running into CORS stuff which I hope is just workstation related, but is making me question the sanity of my changes in /test-token/serverless.yml so I'm double checking that. I also still haven't threaded the token through the cache but that's seeming easy compared to this CORS stuff.

If you had it (almost) working before and only deploy the testToken function, that's probably the best thing to do at the moment. I've moved it to a query that should work with the FaunaDB secret. (Before it was trying to query an index that is not available to user-level keys.)

laardee commented 7 years ago

Now I got it working. The test-token service in PR is using lambda-proxy integration, but in the response, plain callbacks are used.

Here's one option how to fix it:

const createResponse = (statusCode, payload) => ({
  statusCode,
  body: JSON.stringify(payload),
});

const contentHandler = (event, context, cb) => {
  console.log('event', event);
  const authData = event.requestContext.authorizer;
  if (authData.principalId) {
    if (authData.faunadb) {
      const client = new faunadb.Client({ secret: authData.faunadb });
      client.query(q.Get(q.Ref(`classes/${userClassName}/self`)))
        .then((result) => {
          console.log('result', result);
          cb(null, createResponse(200, result));
        })
        .catch((error) => {
          console.log('error', error);
          cb(null, createResponse(400, error));
        });
    } else {
      cb(null, createResponse(200, { username: authData.principalId }));
    }
  } else {
    cb(null, createResponse(400, { error: 'Invalid request' }));
  }
};

I'm also thinking of splitting the test-token service to separate services, something like this:

test-token (maybe change the name to user-details or something)
  default
    handler.js
    serverless.yml
    package.json
    ...
  cognito
    handler.js
    serverless.yml
    package.json
    ...
  dynamodb
    handler.js
    serverless.yml
    package.json
    ...
  faunadb
    handler.js
    serverless.yml
    package.json
    ...

Then there would be fewer if-else statements.

laardee commented 7 years ago

About the refresh tokens, here is a blog post from Auth0 https://auth0.com/blog/refresh-tokens-what-are-they-and-when-to-use-them/. I think the refresh token should be just a random token and it is used to get new authorization token.

jchris commented 7 years ago

Thanks for the feedback. I'm not sure if it makes sense to have a Dynamo testToken, since the token service doesn't read from storage usually. The reason I added FaunaDB to the testToken, is to show how it can look when your database is user-aware. Maybe in a case where Cognito is being used to store the content, and Cognito users are authenticating, that would make sense, but I don't know Cognito well enough to say.

I think I will be able to keep the refresh token architecture the same, and just add some user data to the cache objects.

laardee commented 7 years ago

The test-token structure is not in the scope of this PR. I was just thinking aloud. 😁

jchris commented 7 years ago

I've cleaned this up to be as great as I can make it, and rebased it to one commit to remove a lot of the back and forth I did while figuring things out. This is working great for me. Thanks for reviewing the earlier effort. This is ready to merge if it passes review.

laardee commented 7 years ago

Works perfectly :+1: I'll review the code later today.

jchris commented 7 years ago

Btw I'm working on a blogpost about this here. https://github.com/serverless/blog/pull/106/files#diff-2292421f7c0ac2f9855ccd791a7eb132 😀 On Tue, Mar 7, 2017 at 2:13 PM Eetu Tuomala notifications@github.com wrote:

Merged #31 https://github.com/laardee/serverless-authentication-boilerplate/pull/31 .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/laardee/serverless-authentication-boilerplate/pull/31#event-990424089, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAA_VZynxMSWK3gkY55oy59w9bB_fW_ks5rjdaBgaJpZM4ME6fp .

-- 971-235-1414 http://twitter.com/jchris

laardee commented 7 years ago

That's nice 👍