graphile / crystal

🔮 Graphile's Crystal Monorepo; home to Grafast, PostGraphile, pg-introspection, pg-sql2 and much more!
https://graphile.org/
Other
12.63k stars 572 forks source link

Auth0 JWT gives a jwt audience invalid. expected: postgraphql #292

Closed FGRibreau closed 7 years ago

FGRibreau commented 7 years ago

Hello!

I'm experimenting around PostgraphQL for a new startup and it seems at the time being it requires clams.sub to be "postgraphql" while Auth0 gives "sub": "auth0|70522e86-1fcf-415f-aeda-9325f3413adf" (auth0|user_id), @calebmer I can't find a CLI parameter to change this, any idea to make PostgraphQL work with Auth0? :)

{
  "errors": [
    {
      "message": "jwt audience invalid. expected: postgraphql"
    }
  ]
}
calebmer commented 7 years ago

We could just remove the sub requirement, or make it optional with a command line flag. I was thinking about this use case when I made that requirement in effect to defer the decision :blush:

In my experience of integrating Firebase auth with another authentication system, I would always have two tokens. One for Firebase, and one for the app’s auth system. However I’m sure we can come to a more efficient pattern here.

Do you know if Auth0 will work out of the box with PostGraphQL if this requirement is removed?

FGRibreau commented 7 years ago

@calebmer it should work yes, because it's the last thing that is blocking me from using postgraphql on my project, everything else works well on postgrest (I will just have to change my stored function to handle both way to access to .sub (postgrest use postgrest.claims.sub while postgraphql is jwt.claims.sub) and thus give an REST api and a GraphQL api to my project which would be purely awesome)

ugiacoman commented 7 years ago

@calebmer Going to be implementing Auth0 + Postgraphql + Express very soon. I will be doing authentication as middleware and just passing the token to my Postgraphql endpoint. Would really appreciate the sub requirement to be removed 👍

FGRibreau commented 7 years ago

I think we just need to make the { audience: 'postgraphql' } optional in https://github.com/calebmer/postgraphql/blob/e72696de3d67f5d478c009f8be4c9b25fdb1e2ed/src/postgraphql/http/setupRequestPgClientTransaction.js#L39

ugiacoman commented 7 years ago

As long the secret is verified, JWT should be pretty safe. May I ask, why the audience is being verified? Also, Auth0 uses your unique clientID as the aud. According to some JWT docs, the aud should identify the Resource Server, which technically is Postgraphql, but it seems that Auth0 would like to assign it's own unique id for the client.

http://stackoverflow.com/a/32014280/4615629

angelosarto commented 7 years ago

I have been playing around with this as well and I have some observations/ideas. I am not quite seeing the same issue with sub, rather I saw it with aud(ience). I was able to work around this in auth0.

Auth 0 steps:

  1. enable the APIs beta feature of Auth0
  2. "create API"
    1. the Identifier field must be set to 'postgraphql' (because of the audience check currently in postgraphql.)
    2. the Signing Algorithm should be set to HS256 ( I haven't gotten the RS256 to verify correctly, probably not entering the cert correctly encoded)
    3. edit the API's "Non Interactive Clients" you need to authorize a client - you can either use the test client or create a client and then authorize it.

Getting the Token

Getting the access token (this may be different depending on how you want to support auth, but if you are doing a Proof of concept using a username password database in auth 0 like I am this is how I set it up )

  1. Set the Token Endpoint Authentication Method to None on your client

  2. In account settings under API Authorization Settings set the Default Directory to the name of the password database.

  3. you can now post to the endpoint https://{{auth0_domain}}/oauth/token with parameters:

    1. grant_type: password
    2. client_id: id of the client (not the api)
    3. audience: postgraphql (this has to be the identity field in the API)
    4. username: valid username
    5. password: valid password
    6. scope: can be empty
  4. The response from this post if successful will contain an access_token field.

Setting up postgraphql

  1. Specify the --secret parameter on startup. The secret can be found in the API (not the client)
  2. Add the --default-role parameter on startup.
    1. It wasn't obvious (at least to me) that not setting this parameter basically results in full access because it defaults to the user in the connection string.
    2. I set this to a role called unauth that basically has no access by default.

Ideas/Suggestions

I can probably find time to do a PR for any of these, but would appreciate your thought on if they are valid before I do the work (they aren't huge but I have to make sure I read up on your guidelines and do the tests and commits right).

  1. Add a new parameter for JWT options and let the user pass in the json config object directly to jsonwebtoken's verify. The default could be { audience: 'postgraphql' } if you are concerned about backwards compatibility. This allows the user to set additional restrictions if needed without postgraphql trying to figure out what options are needed.

  2. Make it an error to specify --secret without --default-role. I can't really see a reason why this should work. Its like the user is requiring auth but not setting a token gives then unlimited access? (This is a low priority, maybe just a doc update?)

  3. Role setting. I think setting the role using set is a fantastic solution, but I think maybe it should come from somewhere other than a 'role' attribute.

    1. My initial thought is that the best place would be to come from 'iss'+'sub'. The sub(ject) field is supposed to represent the user and the iss(uer) provides the name spacing on this identifier. iss and sub are going to present in almost any token and so it wouldn't require modification and would give granularity down to the user level. Note since currently postgraphql only accepts one secret its unlikely to have a collision on sub, however if at some point you allow multiple secrets and issuers to provide identity you could run into an issue in the future.
    2. For auth0 specifically, I haven't yet been able to set root level properties on specific users and retrieve them reliably across all of the different possible apis/integrations. Hence the recommendation as well to use sub or iss+sub.

Conclusion

Sorry for the rambling, and hijacking of the thread... been working on this a day or two and wanted to share somewhere. I have it basically working, except I need to rewrite the token to add 'role' otherwise I got everything to work with auth0.

ugiacoman commented 7 years ago

@angelosarto Thank you for this all this great information! Could you expand on the role? Conceptually, I was thinking that I would run into issues, but I thought that would be resolved via role level security policies. Not sure this is the appropriate place to discuss, but I'm on the gitter too.

calebmer commented 7 years ago

Wow @angelosarto thanks so much. Would you be willing to write a documentation article once your thoughts are more clear and some solutions have been implemented.

Here's what I'm thinking right now. We allow users to specify the verification config here as suggested by @angelosarto to give users maximum control. We can also let users specify the config to sign. This should provide maximum control over the JWT process. This would only be available in the JS API and not the CLI. So far we've been able to maintain feature parity, and I'd like to maintain as much parity as possible, but this seems like a good exception. I agree with @ugiacoman though. Post secret, most use cases can disable the aud requirement if need be. I think it's a nice default to prevent slip ups though.

We should also log an error when --secret is used without --default-role, it really doesn't make sense to not do that. I don't want an error so that we can maintain backwards compatibility, but a warning to the effects of: "Warning: running in INSECURE mode" should do the trick.

Im also open to see what role setting would look like, @angelosarto. My thought is allow users to define a function which gets the role off of a JWT object. I'm initially hesitant/opposed because that means it's a little harder to reach the one PostGraphQL/PostgREST dream for auth by adding another level of indirection. Before implementing this I want to see a really clear use case. It sounds currently like there are workarounds on the Auth0 side. Are these workarounds ideal, or hacky?

@FGRibreau as to the PostGraphQL/PostgREST dream, I think the PostgREST team is moving to a naming scheme we could be compatible with. Can't recall the status atm. The architecture of PostGraphQL is setup so that hypothetically we could serve a REST API alongside the GraphQL API. Or a Falcor API, or a SOAP API if that's your jam. Check out https://github.com/calebmer/postgraphql/issues/87 for more info. It still requires a lot of work, but if you're interested in funding we could make it happen.

ugiacoman commented 7 years ago

@calebmer Currently, if you use one of Postgraphql default generated queries, does it look for the role inside of the JWT or will it always use the role the connection was created with? Or is this what @angelosarto was referring to with the secret and default-role. To define the role you wish to use do you need to define your own functions? I'm thinking about use cases with the JS API.

ugiacoman commented 7 years ago

@FGRibreau @calebmer Small update, just implemented this with the JS API and got the same error with aud. I can try my luck at a PR if you guys would like.

EDIT:

So I got it working locally with auth0. Quick note, Auth0 has this thing called rules by which you can add fields like role to your JWT. Their default rule for adding role returns an array of roles instead of a singular role. I can't think of a good use case for multiple roles... If there is an array of roles, do we want to just take the first role? Let me know... I'm more than willing to make a PR for this, just unsure how we would support this from the JS API, but I can figure it out though :)

UPDATE 2:

Got the fix working with the JS API! Let me know how we want to handle the role or roles. I'll also update the library docs for the new optional param jwtAudience.

angelosarto commented 7 years ago

I can write up so example docs of the setup in Auth0. I got it mostly working but not adding the role. (Maybe @ugiacoman can look below to see if we are doing the same thing)

The use case I was trying for was to allow users to post to auth0 to get a token and then turn around and use that token to call graphql api directly; hopefully without building any services myself.

When calling the auth0 resource owner password grant endpoint I haven't yet been able to get a response that includes the role either from app_metadata or on the root object. I opened a topic on the auth0 forums and am hoping to see some response over the next couple days.

Locally, I made it work by modifying this line in postgraphql and changing it to this:

      if (jwtClaims.role != null)
        role = jwtClaims.role
      }
      else if (jwtClaims.iss != null && jwtClaims.sub != null) {
        role = jwtClaims.iss + jwtClaims.sub
      }

I can work around the aud check as long as I name the "api" object in auth0 to be postgraphql, I haven't (yet) found a workaround for setting the role unless I do one of these:

  1. add a custom service to modify and resign the token adding the role
  2. forking/modifying postgraphql
  3. ?maybe this could be changed if I use postgraphql as an api instead of command-line without modifying but I haven't gone down this path yet? I also created a thin dockerfile for postgraphql using the command-line and I can share that in a PR too. (Its more of a minimal dockerfile rather than the example i saw in one of the other issues)

(edit - fixed the code formatting)

ugiacoman commented 7 years ago

@angelosarto Similar, but a bit different use case. My client has authentication. When a user logs in, the response JWT is stored on the client. When a user makes a request to my API, it is passing the JWT in each request. All my API endpoints are secured by JWTs. My JWT comes out with the following structure:

{
   ...
  "roles": [
    "user_role"
  ],
  "clientID": "exampleClientID",
  "created_at": "2016-12-27T17:50:30.013Z",
  "iss": "exampleISS",
  "sub": "exampleSUB",
  "aud": "exampleAUD",
  "exp": 1483016195,
  "iat": 1482980195
}

I am defining my /graphql endpoint as follows.

app.use('/', postgraphql(config.db_string, config.db_schema, {
  jwtSecret: new Buffer(config.secret, 'base64'),
  jwtAudience: config.audience,
  graphiql: true,
}));

Inside of the JWT check, I have modified it to check for an optional jwtAudience parameter when defining it through the JS API (which I have added inside of the createPostGraphQLHttpRequestHandler). You'll also notice that Auth0 produces a roles field instead of role. Therefore I am checking if that array exists, if so use the first role found.

if (jwtToken) {
    // Try to run `jwt.verify`. If it fails, capture the error and re-throw it
    // as a 403 error because the token is not trustworthy.
    try {
        // Check optional JWT audience. if none provided, default to postgraphql.
        if (jwtAudience == undefined)
            jwtAudience = "postgraphql";

        jwtClaims = jwt.verify(jwtToken, jwtSecret, { audience: jwtAudience });

        // If there is a `role` property in the claims, use that instead of our
        // default role.
        if (jwtClaims.role != null)
            role = jwtClaims.role;
        else if (jwtClaims.roles != null)
            role = jwtClaims.roles[0];
    }
    catch (error) {
        error.statusCode = 403;
        throw error;
    }
}

The roles solution successfully works with Auth0, but also allows for a role field. I don't check for the issuer or subject fields. Not completely satisfied with this solution, but please let me know what you all think.

ugiacoman commented 7 years ago

Made a PR but had some issues, will look back into when I get a chance.

levino commented 7 years ago

So I read this issue with great interest. Any updates on the matter?

ugiacoman commented 7 years ago

Haven't gotten a chance to makes the changes necessary. The PR fixes the issue, but doesn't pass the tests. I'll take another look this weekend, see if I can fix it.

ugiacoman commented 7 years ago

Not going to get a chance this weekend but soon :)

ugiacoman commented 7 years ago

@Levino @angelosarto @FGRibreau Created a new PR without the roles object support. Passes all tests! #365

mlegenhausen commented 7 years ago

Here is a (ugly) workaround to change the audience until the pull request lands in the repository.

import jwt = require('jsonwebtoken');

const verify = jwt.verify;
jwt.verify = function (token, secret, options?) {
  if (options && options.audience === 'postgraphql') {
    options.audience = 'my_custom_audience';
  }
  return verify.apply(jwt, arguments);
};
AlineFrancois commented 7 years ago

Hey guys, any update about the roles object support ? Or any other way to make it work with Auth0 ?

mlegenhausen commented 7 years ago

@AlineFrancois you can now use the jwtAudiences parameter of the postgraphql middleware. This is also matched to the jwt-audiences command line parameter. Just use the auth0 clientID as your audience.

levino commented 7 years ago

@mlegenhausen Great stuff! Is there a complete example somewhere?

mlegenhausen commented 7 years ago

@Levino Only in one of my internal projects ;) but if you have further questions don't hesitate to ask. Just one note to get the role parameter working you need to add a auth0 hook that exposes a role parameter from the app_metadata attribute on the root level of your JWT. You can use:

function addMetadataToUserRoot(user, context, callback) {
  var scope = context.request && context.request.query && context.request.query.scope || '';
  if (scope.indexOf('app_metadata') > -1) {
    Object.assign(user, user.app_metadata);
    delete user.app_metadata;
  }

  callback(null, user, context);
}

And then add app_metadata to your auth0 scope variable. This hook simply copies all attributes defined under app_metadata to the main object which then get combined in your jwt.

benjie commented 7 years ago

If someone could write a wiki page or submit a pull request to our documetation detailing this, that would be excellent 😁

angelosarto commented 7 years ago

@benjie @calebmer I should be able to write up a small example and doc for this. perhaps a new sub folder or markdown document in in the examples directory for running with Auth-0.

Any chance there might be a release to npm that includes: https://github.com/postgraphql/postgraphql/pull/434 then I can write the doc to just use npm install as the simplest example.

fluffybonkers commented 7 years ago

@calebmer I think it would be great for the user to be able to define a function for extracting the roles, as really roles should be namespaced with a domain so that JWTs are OIDC compliant.

With the solutions proposed above, I think role is just in the JWT e.g. "role": "test" instead of "https://admin.example.com/role": [ "test" ]. If a function could be defined, the namespace could be any old thing.

Am I just imagining a problem here/creating unnecessary work?

fluffybonkers commented 7 years ago

Sorry - never mind. This #480 appears to have fixed the issue - just have to use PostgraphQL as a library rather than from the CLI.

benjie commented 7 years ago
    --jwt-role <string>              a comma seperated list of strings that create a path in the jwt from which to extract the postgres role. if none is provided it will use the key `role` on the root of the jwt.

I think you can just use the CLI option?

fluffybonkers commented 7 years ago

I get error: unknown option --jwt-role at the CLI and it is not listed as an option in the help for the CLI.

benjie commented 7 years ago

Ah; maybe we haven't shipped that yet. Try the v4 alpha:

npm i postgraphql@next

fluffybonkers commented 7 years ago

Hm. Still doesn't seem to work - same error.

I don't think I need to do anything else to get the correct version of PostgraphQL, right? I still just have to run postgraphql to start the latest v4 alpha?

benjie commented 7 years ago

Nope, should work. Try --jwtRole but failing that I'll have to look into it later.

fluffybonkers commented 7 years ago

Still doesn't work.

No worries re. getting to it later, but it would be great for it to work at some point, though.

benjie commented 7 years ago

I've just released v3.3.0 which I believe should fix this issue; if it does not please re-open 👍

benjie commented 7 years ago

Also I think you might have been using the wrong CLI flag - you want --jwt-audiences for overriding the audience.

benjie commented 7 years ago

Ah, no, I think this issue got a little muddled.

natejenkins commented 6 years ago

small correction, the command line flag is --jwt-audience not jwt-audiences, at least on version 4.0.1