这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@karthikvt26
Copy link
Contributor

Fix #1369

Description

JWT header as below can be analyzed
screenshot 775

Affected components

  • Console

Related Issues

close #1369

Solution and Design

Steps to test and verify

Create a jwt token and add a header Authorization: Bearer <token in the explorer and click on the + icon to analyze.

Limitations, known bugs & workarounds

@karthikvt26 karthikvt26 requested a review from rikinsk-zz as a code owner March 14, 2019 06:42
@karthikvt26 karthikvt26 changed the title Add jwt bearer tokens analyzer in Explorer Add jwt bearer token analyzer in Explorer Mar 14, 2019
@hasura-bot
Copy link
Contributor

Review app for commit da8687c deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-da8687c

@hasura-bot
Copy link
Contributor

Review app for commit 3975dda deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-3975dda

@hasura-bot
Copy link
Contributor

Review app for commit 0401074 deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-0401074

@shahidhk shahidhk added the c/console Related to console label Mar 15, 2019
@shahidhk shahidhk changed the title Add jwt bearer token analyzer in Explorer add jwt analyzer on console (close #1369) Mar 15, 2019
@karthikvt26 karthikvt26 requested a review from wawhal March 15, 2019 11:01

import jwt from 'jsonwebtoken';

import Modal from 'react-bootstrap/lib/Modal';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us implement a common modal at the root at the app that subscribes to title, content and isOpen in redux. Then we can change these properties from anywhere and the modal becomes reusable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey I am not able to wrap my head around this pattern where CommonModal can be used as

this.props.dispatch({ type: COMMON_MODAL_OPEN, title: <react-element>, content: <react-element>})

instead of

<CommonModal title={ title }>
    <Body>
</CommonModal>

The body can be as complicated as it can. @wawhal Let me know what you think.

Copy link
Contributor

@wawhal wawhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine.
We can make the reusable moddal and we're good to go.

@hasura-bot
Copy link
Contributor

Review app for commit 0aa6b52 deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-0aa6b52

@coco98
Copy link
Contributor

coco98 commented Mar 19, 2019

I think it would be important to highlight a few things in the JWT analyser to make this useful:

  1. Show me if Hasura has been setup to verify this signature. If hasura has not been setup then prompt me to setup Hasura and link to the docs so that I can.
  2. Highlight the Hasura claims (once hasura jwt config is setup you'll know which claims are the right claims to look at). Further tell me if Hasura specific claims are missing. allowed-roles and default-role are mandatory.

Without these 2 things this is not useful. Also, the + icon should be an i(information) or ?(help) icon right?
@rikinsk

@coco98
Copy link
Contributor

coco98 commented Mar 19, 2019

Also, set tabwidth to 2 spaces? It looks like it's 8 right now.
Screenshot 2019-03-18 at 11 28 37 PM

@netlify
Copy link

netlify bot commented Mar 20, 2019

Deploy preview for hasura-docs ready!

Built with commit af93aea

https://deploy-preview-1770--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

Review app for commit 7a98d0a deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-7a98d0a

@karthikvt26
Copy link
Contributor Author

It is possible for me to achieve both. But not in a straight forward way. I have mentioned them below.

Show me if Hasura has been setup to verify this signature. If hasura has not been setup then prompt me to setup Hasura and link to the docs so that I can.

I will make introspection query without the Authorization header and parse the response to come to the conclusion.

Highlight the Hasura claims (once hasura jwt config is setup you'll know which claims are the right claims to look at). Further tell me if Hasura specific claims are missing. allowed-roles and default-role are mandatory.

I can parse the payload to find out x-hasura-* inside the object to get the relevant header. I can highlight and show it to the user.

The above issues will become straight forward if for 1, the server hydrates a flag isJWTSet it will be useful. For the second scenario, if it can provide the claims_namespace then it will become straight forward to implement it.

@coco98 @rikinsk
Let me know what you think?

@coco98
Copy link
Contributor

coco98 commented Mar 20, 2019

Why can’t the server template the entire jwt config for the console?

That way you have everything.

@karthikvt26
Copy link
Contributor Author

karthikvt26 commented Mar 20, 2019 via email

@ecthiender
Copy link
Contributor

ecthiender commented Mar 20, 2019 via email

@coco98
Copy link
Contributor

coco98 commented Mar 20, 2019 via email

@ecthiender
Copy link
Contributor

But having key is good so that console can tell you if the jwt is valid per Hasura. Otherwise capturing feedback about invalid jwt key only happens via logs :(

For invalid jwt, the response from server has invalid-jwt as a code with a reason message.

But in any case, the key can't be templated on the console html, because then anyone can grab the jwt secret. Server can provide a separate admin only endpoint to dump the config. Does that make sense @coco98 ?

@shahidhk shahidhk added the s/do-not-merge Do not merge this pull request to master label Mar 27, 2019
@shahidhk shahidhk added the s/wip Status: This issue is a work in progress label Mar 27, 2019
@ecthiender
Copy link
Contributor

According to the latest discussion with @dsandip and @rikinsk we are doing the following:
cc @karthikvt26

Server

  1. Server will not dump the entire jwt config to the console. It will have an endpoint which will return is_jwt_set, claims_namespace and claims_format. (should this endpoint be available only on dev-mode?)

  2. Server will need to improve current error messages when JWT is invalid. And also return more details in the reponse, when the verbose-api response flag is enabled.

Console

  1. Console need not replicate the entire logic for verifying the JWT (because the jwt config can also contain jwk url not just the key, the verification becomes more complex). It can hit the server to verify the JWT and display the error messages returned by the server.

  2. Console needs to use the claims_namespace and claims_format to highlight the hasura claims correctly.

Please add if I missed anything.

@hasura-bot
Copy link
Contributor

Review app for commit 731ef1b deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-731ef1b

@hasura-bot
Copy link
Contributor

Review app for commit bdf228c deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-bdf228c

@hasura-bot
Copy link
Contributor

Review app for commit 952003c deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-952003c

@hasura-bot
Copy link
Contributor

Review app for commit af93aea deployed to Heroku: https://hge-ci-pull-1770.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1770-af93aea

@dsandip dsandip added this to the release-candidates milestone Apr 15, 2019
@shahidhk
Copy link
Member

shahidhk commented Jun 3, 2019

closed in favour of #1925

@shahidhk shahidhk closed this Jun 3, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1770.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Mar 25, 2025
… expression types (#1770)

<!-- The PR description should answer 2 important questions: -->

### What

Introduces stricter validation for scalar boolean expression operators
during the build process. This change helps catch invalid operator usage
early by validating:
- List type requirements for `_in` operator
- Type matching for comparison operators (`_eq`, `_lt`, `_lte`, `_gt`,
`_gte`)
- Operator existence in data connector
- Argument type compatibility with NDC operator types
- Operators like `contains`, `icontains`, `starts_with` and `ends_with`
etc. must be applicable to string scalar and argument type must be a
string.

These validations are controlled by a compatibility flag, issuing
warnings for existing projects while enforcing strict errors for new
projects created after or with a compatibility date set beyond the
release date of this fix.

### How

- Added new compatibility flag
`ValidateScalarBooleanExpressionOperators`
- Implemented validation checks in the build process to verify operator
usage
- Updated changelog with the new validations
- Fix existing tests' metadata to avoid new warnings

<!-- How is it trying to accomplish it (what are the implementation
steps)? -->

V3_GIT_ORIGIN_REV_ID: d468a6794af68d06a4a8fae3f8f1023c7824bdfb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c/console Related to console s/do-not-merge Do not merge this pull request to master s/wip Status: This issue is a work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add jwt inspector to hasura console

7 participants