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

FaunaDB user storage #31

Merged
merged 2 commits into from
Mar 7, 2017
Merged

FaunaDB user storage #31

merged 2 commits into from
Mar 7, 2017

Conversation

jchris
Copy link
Contributor

@jchris jchris commented Feb 18, 2017

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

thanks!
Chris

@laardee
Copy link
Owner

laardee commented Feb 20, 2017

@jchris nice, I'll check this later today

@jchris
Copy link
Contributor Author

jchris commented Feb 22, 2017

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
Copy link
Contributor Author

jchris commented Feb 22, 2017

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
Copy link
Owner

laardee commented Feb 23, 2017

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
Copy link

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

@jchris
Copy link
Contributor Author

jchris commented Feb 23, 2017 via email

@jchris
Copy link
Contributor Author

jchris commented Feb 24, 2017

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
Copy link
Owner

laardee commented Feb 24, 2017

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
Copy link
Owner

laardee commented Feb 24, 2017

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
Copy link
Contributor Author

jchris commented Feb 24, 2017

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
Copy link
Owner

laardee commented Feb 24, 2017

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

Allows user storage to attach database access token or other secrets
to the Authorization Token. The authorizer converts these to API
Gateway context, so they are available to authorized functions.

In the case of FaunaDB we use this payload to deliver a FaunaDB access
secret that corresponds to the authenticated user.
@jchris
Copy link
Contributor Author

jchris commented Mar 2, 2017

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
Copy link
Owner

laardee commented Mar 7, 2017

Works perfectly 👍 I'll review the code later today.

@laardee laardee merged commit d2f40a3 into laardee:master Mar 7, 2017
@jchris
Copy link
Contributor Author

jchris commented Mar 12, 2017 via email

@laardee
Copy link
Owner

laardee commented Mar 13, 2017

That's nice 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants