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

feat(GraphQL): Allow standard claims into auth variables #7381

Merged
merged 11 commits into from
Feb 3, 2021

Conversation

minhaj-shakeel
Copy link
Contributor

@minhaj-shakeel minhaj-shakeel commented Jan 29, 2021

Fixes GRAPHQL-945.
This PR adds support for adding standard claims of a jwt token in the Auth Variables.
For eg, if the token contains claims given below and the namespace given in the authorization header is https://xyz.io/jwt/claims:

{
   "https://xyz.io/jwt/claims": [
      ....
   ],
  "ROLE": "ADMIN",
  "USERROLE": "user1",
  "email": "random@example.com",
  "email_verified": true,
  "sub": "1234567890",
  "aud": "63do0q16n6ebjgkumu05kkeian",
  "iat": 1611694692,
  "exp": 2611730692
}

Then the auth variables will also include the rest of the given claims along with the claims provided under https://xyz.io/jwt/claims.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Jan 29, 2021
@minhaj-shakeel minhaj-shakeel changed the title feat feat(GraphQL): Allow standard claims into auth variables Feb 1, 2021
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

Can we add an e2e test showing that a standard claim is accessible and can be used within an auth rule?

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @minhaj-shakeel)


graphql/resolve/auth_test.go, line 182 at r1 (raw file):

		"ROLE":             "ADMIN",
		"email_verified":   true,
		"iss":              "https://cognito-idp.ap-southeast-2.amazonaws.com/ap-southeast-2_GfmeHdFz4",

change to a dummy value


graphql/resolve/auth_test.go, line 189 at r1 (raw file):

		"event_id":         "31c9d684-1d45-46f7-8c2b-cc27b1f6f01b",
		"token_use":        "id",
		"name":             "David Peek",

change the name


graphql/resolve/auth_test.go, line 190 at r1 (raw file):

		"token_use":        "id",
		"name":             "David Peek",
		"email":            "david@typejoin.com",

change this to a dummy value


graphql/resolve/auth_test.go, line 258 at r1 (raw file):

				"ROLE":             "ADMIN",
				"email_verified":   true,
				"iss":              "https://cognito-idp.ap-southeast-2.amazonaws.com/ap-southeast-2_GfmeHdFz4",

same as above, obfuscate the values


graphql/resolve/auth_test.go, line 386 at r1 (raw file):

				"token_use":        "id",
				"name":             "David Peek",
				"email":            "david@typejoin.com",

add a case where there is a collision between default and user-defined namespace

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r2.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @minhaj-shakeel)

@minhaj-shakeel minhaj-shakeel merged commit 23c8796 into master Feb 3, 2021
@minhaj-shakeel minhaj-shakeel deleted the minhaj/standard-claims branch February 3, 2021 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants