feat(GraphQL): Adds auth for subscriptions #5984

JatinDev543 · 2020-07-14T13:08:15Z

This PR adds authorization for subscriptions.

This change is

Docs Preview:

pawanrawal

Initial comments. Will have another look tomorrow after you have some e2e tests.

Reviewed 7 of 7 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @jatindevdg and @MichaelJCompton)

graphql/authorization/auth.go, line 220 at r1 (raw file):

	}

	// The JWT library supports comparison of `aud` in JWT against a single string. Hence, we

Don't remove this.

graphql/authorization/auth.go, line 240 at r1 (raw file):

			}
			return nil, errors.Errorf("couldn't parse signing method from token header: %s", algo)
		}, jwt.WithoutClaimsValidation())

this should not be removed.

graphql/authorization/auth.go, line 248 at r1 (raw file):

	}

	// by default, the MapClaims.Valid will return true if the exp field is not set

remove this comment, basicaly validateToken has undergone some changes in master. Keep those instead of losing them.

graphql/authorization/auth.go, line 251 at r1 (raw file):

	}

	if err := claims.validateAudience(); err != nil {

Don't remove, keep this function.

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

	customClaims, err := authorization.ExtractCustomClaims(ctx)
	authVar := customClaims.AuthVariables

First check error then assign authVar

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

			customClaims, err := authorization.ExtractCustomClaims(ctx)
			authVar := customClaims.AuthVariables

Same as above.

graphql/resolve/mutation.go, line 379 at r1 (raw file):

	customClaims, err := authorization.ExtractCustomClaims(ctx)
	authVariables := customClaims.AuthVariables

Check error and return if non-nil first. Then assign authVariables.

graphql/resolve/mutation_rewriter.go, line 361 at r1 (raw file):

	customClaims, err := authorization.ExtractCustomClaims(ctx)
	authVariables := customClaims.AuthVariables

Same as above everywhere.

graphql/resolve/query_rewriter.go, line 83 at r1 (raw file):

	if authVariables == nil {
		customClaims, err := authorization.ExtractCustomClaims(ctx)
		authVariables1 := customClaims.AuthVariables

Why do all this and why not just do

customClaims, err := authorization.ExtractCustomClaims(ctx)
if err != nil {
  return nil, err
}
authVariables = customClaim.AuthVariables

graphql/subscription/poller.go, line 86 at r1 (raw file):

	if customClaims.AuthVariables != nil {

		//ToDo-Add custom marshal function that marhsal's the json in sorted order.

marshal's

arijitAD

Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @jatindevdg and @MichaelJCompton)

graphql/subscription/poller.go, line 70 at r1 (raw file):

// AddSubscriber tries to add subscription into the existing polling goroutine if it exists.
// If it doesn't exist, then it creates a new polling goroutine for the given request.
func (p *Poller) AddSubscriber(req *schema.Request, customClaims *authorization.CustomClaims) (*SubscriberResponse, error) {

Fix line width.

graphql/subscription/poller.go, line 116 at r1 (raw file):

	glog.Infof("Subscription polling is started for the ID %d", subscriptionID)

	subscriptions[subscriptionID] = subscriber{(customClaims.StandardClaims.ExpiresAt.Unix()), updateCh, subscriptionID}

Fix line width.

graphql/subscription/poller.go, line 172 at r1 (raw file):

		}

		ctx := context.WithValue(context.Background(), "authVariables", req.authVariables)

The context key should be constant. https://stackoverflow.com/questions/40891345/fix-should-not-use-basic-type-string-as-key-in-context-withvalue-golint
Fix at other places as well.

JatinDev543

Reviewable status: 0 of 7 files reviewed, 13 unresolved discussions (waiting on @arijitAD, @jatindevdg, @MichaelJCompton, and @pawanrawal)

graphql/authorization/auth.go, line 248 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

remove this comment, basicaly validateToken has undergone some changes in master. Keep those instead of losing them.

removed comments.

graphql/authorization/auth.go, line 251 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Don't remove, keep this function.

which function ? in the above comments also i am not getting what not to remove. Can you clarify a bit ?

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

Previously, pawanrawal (Pawan Rawal) wrote…

First check error then assign authVar

done.

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

Previously, pawanrawal (Pawan Rawal) wrote…

Same as above.

done.

graphql/resolve/mutation.go, line 379 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Check error and return if non-nil first. Then assign authVariables.

done.

graphql/resolve/mutation_rewriter.go, line 361 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Same as above everywhere.

done.

graphql/resolve/query_rewriter.go, line 83 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Why do all this and why not just do

customClaims, err := authorization.ExtractCustomClaims(ctx)
if err != nil {
  return nil, err
}
authVariables = customClaim.AuthVariables

done.

graphql/subscription/poller.go, line 70 at r1 (raw file):

Previously, arijitAD (Arijit Das) wrote…

Fix line width.

done

graphql/subscription/poller.go, line 86 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

marshal's

done.

graphql/subscription/poller.go, line 116 at r1 (raw file):

Previously, arijitAD (Arijit Das) wrote…

Fix line width.

done.

graphql/subscription/poller.go, line 172 at r1 (raw file):

Previously, arijitAD (Arijit Das) wrote…

The context key should be constant. https://stackoverflow.com/questions/40891345/fix-should-not-use-basic-type-string-as-key-in-context-withvalue-golint
Fix at other places as well.

done.

…y-pick-auth+subscriptions

…t-ws

pawanrawal

Reviewed 2 of 10 files at r2, 4 of 7 files at r3, 1 of 1 files at r4, 6 of 22 files at r5.
Reviewable status: 13 of 29 files reviewed, 30 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

graphql/e2e/subscription/sample_private_key.pem, line 1 at r5 (raw file):

-----BEGIN RSA PRIVATE KEY-----

Do you need a private key or could you just use a shared secret here?

graphql/e2e/subscription/subscription_test.go, line 67 at r5 (raw file):

   ){
     	id: ID!
    	text: String! @search(by: [term])

fix indentation please

graphql/e2e/subscription/subscription_test.go, line 194 at r5 (raw file):

	}
	addResult := add.ExecuteAsPost(t, adminEndpoint)
	require.Nil(t, addResult.Errors)

Add a newline after this.

graphql/e2e/subscription/subscription_test.go, line 195 at r5 (raw file):

	addResult := add.ExecuteAsPost(t, adminEndpoint)
	require.Nil(t, addResult.Errors)
	metaInfo.AuthVars = map[string]interface{}{}

Why is metaInfo a global in this file? It should be defined right here.

Also fill the map values line.

metaInfo.AuthVars = map[string]interface{}{
  "USER": "jatin",
  ...
  ...
}

graphql/e2e/subscription/subscription_test.go, line 199 at r5 (raw file):

	metaInfo.AuthVars["ROLE"] = "USER"
	metaInfo.AuthVars["exp"] = time.Now().Unix() + 600
	jwtToken, err := metaInfo.GetSignedToken("./sample_private_key.pem")

You need to use secret here that you defined as the VerificationKey in the schema above.
Also check your errors.
Newline after checking errors. Separate code into logical blocks.

graphql/resolve/mutation_rewriter.go, line 367 at r5 (raw file):

	authRw := &authRewriter{
		authVariables: authVariables,

Just use customClaims.AuthVariables here directly instead of defining it above. General rule is that if something is just being used once don't define a variable for it.

graphql/resolve/mutation_rewriter.go, line 419 at r5 (raw file):

		return nil, err
	}
	authVariables := customClaims.AuthVariables

Same as above and everywhere.

graphql/resolve/query_rewriter.go, line 81 at r5 (raw file):

	authVariables, _ := ctx.Value(authorization.AuthVariables).(map[string]interface{})

	if authVariables == nil {

We need to fix this and ideally do this parsing for queries early on. Otherwise ExtractCustomClaims would be called twice for subscription queries without Auth.

graphql/subscription/poller.go, line 74 at r5 (raw file):

	localEpoch := atomic.LoadUint64(p.globalEpoch)
	if customClaims == nil {

Instead of doing this check here, just initialize this in the caller function with this value.

graphql/subscription/poller.go, line 75 at r5 (raw file):

	localEpoch := atomic.LoadUint64(p.globalEpoch)
	if customClaims == nil {
		customClaims = &authorization.CustomClaims{}

Initialiaze ExpiresAt inline along with the customClaims declaration above.

graphql/subscription/poller.go, line 76 at r5 (raw file):

	if customClaims == nil {
		customClaims = &authorization.CustomClaims{}
		customClaims.ExpiresAt = jwt.NewTime(float64(time.Time{}.Unix()))

Why not just initialize as jwt.Time{}?

graphql/subscription/poller.go, line 88 at r5 (raw file):

	if customClaims.AuthVariables != nil {

		//ToDo-Add custom marshal function that marshal's the json in sorted order.

Also format it like

// TODO -  Add

Not the spaces and the casing.

graphql/subscription/poller.go, line 90 at r5 (raw file):

		//ToDo-Add custom marshal function that marshal's the json in sorted order.
		authvariables, err := json.Marshal(customClaims.AuthVariables)
		x.Check(err)

Nope bad idea, don't crash the server, return the error.

graphql/subscription/poller.go, line 193 at r5 (raw file):

				return
			}
			for _, subscriber := range subscribers {

The key here is subscriptionID, do we need it to be in the value of the map as well?

graphql/subscription/poller.go, line 195 at r5 (raw file):

			for _, subscriber := range subscribers {
				expiry, err := jwt.ParseTime(subscriber.expiry)
				if err != nil {

Shouldn't this be an error? Why are we only checking for expiry when err != nil?

graphql/subscription/poller.go, line 218 at r5 (raw file):

		for _, subscriber := range subscribers {
			expiry, err := jwt.ParseTime(subscriber.expiry)
			if err != nil {

Same as above.

graphql/web/http.go, line 126 at r5 (raw file):

	err error) {

	header, _ := ctx.Value("Header").(json.RawMessage)

Add a comment saying that the library (graphql-transport-ws) passes the headers which are part of the INIT payload to us in the context. And we are extracting the Auth JWT from those and passing them along.

graphql/web/http.go, line 128 at r5 (raw file):

	header, _ := ctx.Value("Header").(json.RawMessage)
	var customClaim *authorization.CustomClaims
	if header != nil {

Since json.RawMessage is just a []byte check for if len(header) > 0

graphql/web/http.go, line 135 at r5 (raw file):

		name := authorization.GetHeader()
		val, ok := payload[name]

Do this

val, ok := payload[name].(string)

graphql/web/http.go, line 137 at r5 (raw file):

		val, ok := payload[name]
		if ok {
			md, ok := metadata.FromIncomingContext(ctx)

You don't need all of this as the ctx won't have any metadata. Just initialize md like

md := metadata.New("authorizationJwt", val)

JatinDev543

Reviewable status: 7 of 13 files reviewed, 30 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

graphql/e2e/subscription/sample_private_key.pem, line 1 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Do you need a private key or could you just use a shared secret here?

deleted.

graphql/e2e/subscription/subscription_test.go, line 67 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

fix indentation please

done.

graphql/e2e/subscription/subscription_test.go, line 194 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a newline after this.

added.

graphql/e2e/subscription/subscription_test.go, line 195 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Why is metaInfo a global in this file? It should be defined right here.

Also fill the map values line.
metaInfo.AuthVars = map[string]interface{}{
  "USER": "jatin",
  ...
  ...
}

Done.We also need to initialize metainfo it is giving panic.

graphql/e2e/subscription/subscription_test.go, line 199 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

You need to use secret here that you defined as the VerificationKey in the schema above.

Also check your errors.

Newline after checking errors. Separate code into logical blocks.

done.

graphql/resolve/mutation_rewriter.go, line 367 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Just use customClaims.AuthVariables here directly instead of defining it above. General rule is that if something is just being used once don't define a variable for it.

Changed.

graphql/resolve/mutation_rewriter.go, line 419 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Same as above and everywhere.

done.

graphql/resolve/query_rewriter.go, line 81 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

We need to fix this and ideally do this parsing for queries early on. Otherwise ExtractCustomClaims would be called twice for subscription queries without Auth.

yeah , but ExtractCustomClaims won't be called twice because in http.go we are calling it only for the subscriptions which have some header info.

graphql/subscription/poller.go, line 74 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Instead of doing this check here, just initialize this in the caller function with this value.

done.

graphql/subscription/poller.go, line 75 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Initialiaze ExpiresAt inline along with the customClaims declaration above.

done.

graphql/subscription/poller.go, line 76 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Why not just initialize as jwt.Time{}?

because customClaims.ExpiresAt is of type "*Time".

graphql/subscription/poller.go, line 88 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Also format it like
// TODO -  Add
Not the spaces and the casing.

yeah, done.

graphql/subscription/poller.go, line 90 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Nope bad idea, don't crash the server, return the error.

done.

graphql/subscription/poller.go, line 195 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Shouldn't this be an error? Why are we only checking for expiry when err != nil?

returned error. but i thing there is some other way of handling errors for goroutines.

graphql/subscription/poller.go, line 218 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Same as above.

returned error.

graphql/web/http.go, line 126 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment saying that the library (graphql-transport-ws) passes the headers which are part of the INIT payload to us in the context. And we are extracting the Auth JWT from those and passing them along.

done.

graphql/web/http.go, line 128 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Since json.RawMessage is just a []byte check for if len(header) > 0

done.

graphql/web/http.go, line 135 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Do this

val, ok := payload[name].(string)

done

graphql/web/http.go, line 137 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

You don't need all of this as the ctx won't have any metadata. Just initialize md like
md := metadata.New("authorizationJwt", val)

done, paased a map because it doesn't take two aruments directly.

pawanrawal

Fix the deepsource warnings.

Reviewed 1 of 7 files at r6, 1 of 4 files at r10, 1 of 1 files at r12.
Reviewable status: 9 of 13 files reviewed, 18 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

graphql/e2e/subscription/subscription_test.go, line 430 at r12 (raw file):

}

func TestTwoSubscriptionAuthDifferentExpirysamejWt(t *testing.T) {

JWT can't be same if expiry is different as expirt is part of JWT.

Call it something like below.

TestSubscriptionAuth_SameQueryAndClaimsButDifferentExpiry_ShouldExpireIndependently

graphql/e2e/subscription/subscription_test.go, line 481 at r12 (raw file):

	require.NoError(t, err)

	//first subscription

// first

graphql/e2e/subscription/subscription_test.go, line 499 at r12 (raw file):

	err = json.Unmarshal(res, &resp)
	require.NoError(t, err)

This space isn't required

graphql/e2e/subscription/subscription_test.go, line 551 at r12 (raw file):

	res, err = subscriptionClient.RecvMsg()
	require.NoError(t, err)
	require.Nil(t, res) //1st subscription should get the empty response as subscriptio has expired

correct the typo here. Also, always add a space after //.

graphql/e2e/subscription/subscription_test.go, line 568 at r12 (raw file):

	}]}`, string(resp.Data))

	//add extra delay for 2nd subscription to timeout

// add

graphql/e2e/subscription/subscription_test.go, line 570 at r12 (raw file):

	//add extra delay for 2nd subscription to timeout
	time.Sleep(10 * time.Second)
	// Add another TODO for jatin which 2nd subscription shouldn't get.

shouldn't get updates

graphql/e2e/subscription/subscription_test.go, line 588 at r12 (raw file):

	res, err = subscriptionClient1.RecvMsg()
	require.NoError(t, err)
	require.Nil(t, res) //2nd subscription should get the empty response as subscriptio has expired

subscription has

graphql/e2e/subscription/subscription_test.go, line 591 at r12 (raw file):

}

func TestTwoSubscriptionAuthDifferentExpirydiffjWt(t *testing.T) {

TestSubscriptionAuth_SameQueryDifferentClaimsAndExpiry_ShouldExpireIndependently

graphql/e2e/subscription/subscription_test.go, line 686 at r12 (raw file):

	//2nd subscription
	metaInfo.AuthVars["USER"] = "pawan"
	jwtToken, err = metaInfo.GetSignedToken("secret", 20*time.Second)

This doesn't need t

graphql/e2e/subscription/subscription_test.go, line 731 at r12 (raw file):

	// 1st subscription should get the empty response as subscription has expired
	res, err = subscriptionClient.RecvMsg()

Check the error here as well that it should be nil

graphql/e2e/subscription/subscription_test.go, line 774 at r12 (raw file):

		Query: `mutation{
	         addTodo(input: [
	            {text : "Graph Database is the future!!",

nice tests

graphql/subscription/poller.go, line 193 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

The key here is subscriptionID, do we need it to be in the value of the map as well?

What happened to this one?

graphql/subscription/poller.go, line 195 at r5 (raw file):

Previously, JatinDevDG (Jatin Dev) wrote…

returned error. but i thing there is some other way of handling errors for goroutines.

You don't need to return error now if its always nil.

graphql/subscription/poller.go, line 218 at r5 (raw file):

Previously, JatinDevDG (Jatin Dev) wrote…

returned error.

Time to revert that

graphql/web/http.go, line 128 at r12 (raw file):

	err error) {

	//library (graphql-transport-ws) passes the headers which are part of the INIT payload to us in the context.

Space after // please

JatinDev543

Reviewable status: 8 of 13 files reviewed, 18 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

graphql/e2e/subscription/subscription_test.go, line 430 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

JWT can't be same if expiry is different as expirt is part of JWT.

Call it something like below.
TestSubscriptionAuth_SameQueryAndClaimsButDifferentExpiry_ShouldExpireIndependently

done.

graphql/e2e/subscription/subscription_test.go, line 481 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

// first

done.

graphql/e2e/subscription/subscription_test.go, line 499 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

This space isn't required

removed space.

graphql/e2e/subscription/subscription_test.go, line 551 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

correct the typo here. Also, always add a space after //.

done.

graphql/e2e/subscription/subscription_test.go, line 568 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

// add

done.

graphql/e2e/subscription/subscription_test.go, line 570 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

shouldn't get updates

done.

graphql/e2e/subscription/subscription_test.go, line 588 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

subscription has

done.

graphql/e2e/subscription/subscription_test.go, line 591 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

TestSubscriptionAuth_SameQueryDifferentClaimsAndExpiry_ShouldExpireIndependently

done.

graphql/e2e/subscription/subscription_test.go, line 731 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Check the error here as well that it should be nil

done.

graphql/e2e/subscription/subscription_test.go, line 774 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

nice tests

Thanks!!

graphql/subscription/poller.go, line 193 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

What happened to this one?

i didn't fully get then point, but We need subscription ID in the value of struct subscriber because now we are terminating individual subscriptions. is there any other way to do the same without adding Subscription ID in the struct.

graphql/subscription/poller.go, line 195 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

You don't need to return error now if its always nil.

changed.

graphql/subscription/poller.go, line 218 at r5 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Time to revert that

already removed.

graphql/web/http.go, line 128 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Space after // please

done.

JatinDev543

Reviewable status: 8 of 13 files reviewed, 18 unresolved discussions (waiting on @arijitAD, @manishrjain, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

graphql/e2e/subscription/subscription_test.go, line 686 at r12 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

This doesn't need t

didn't get this. How to check test without t.

pawanrawal

Reviewed 1 of 7 files at r6, 1 of 2 files at r9, 3 of 3 files at r13.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, and @vvbalaji-dgraph)

graphql/e2e/subscription/subscription_test.go, line 686 at r12 (raw file):

Previously, JatinDevDG (Jatin Dev) wrote…

didn't get this. How to check test without t.

sorry, seems like the comment was a mistake

graphql/subscription/poller.go, line 193 at r5 (raw file):

Previously, JatinDevDG (Jatin Dev) wrote…

i didn't fully get then point, but We need subscription ID in the value of struct subscriber because now we are terminating individual subscriptions. is there any other way to do the same without adding Subscription ID in the struct.

I pushed a commit with this change, they for this map over which you are ranging is the subscriptionID, so you can just use that.

This reverts commit 3922649.

pawanrawal

Reviewed 3 of 3 files at r14.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @arijitAD, @jatindevdg, @manishrjain, @MichaelJCompton, and @vvbalaji-dgraph)

…y-pick-auth+subscriptions

…aph-io/dgraph into jatin/cherry-pick-auth+subscriptions

This reverts commit 5f81aac.

Co-authored-by: Pawan Rawal <[email protected]> Co-authored-by: JatinDevDG <[email protected]> (cherry picked from commit 5f81aac)

* feat(GraphQL): Adds auth for subscriptions (#5984) Co-authored-by: Pawan Rawal <[email protected]> Co-authored-by: JatinDevDG <[email protected]> (cherry picked from commit 5f81aac) * skip fleaky tests * Changed function name in auth_test * fixed test

JatinDev543 added 4 commits July 14, 2020 15:11

removed log info

d743a6e

added missing functions in auth.go

845319c

resolved merge issues

7475833

added authvariable code for subscriptions

988f7fd

JatinDev543 requested review from MichaelJCompton and pawanrawal as code owners July 14, 2020 13:08

github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Jul 14, 2020

JatinDev543 added 2 commits July 14, 2020 19:14

removed a print statement

5f87d11

removed authvariable function and changed it's occurences

db28ea5

pawanrawal suggested changes Jul 14, 2020

View reviewed changes

arijitAD suggested changes Jul 14, 2020

View reviewed changes

resolved review comments

ccdb986

JatinDev543 commented Jul 15, 2020

View reviewed changes

JatinDev543 added 2 commits July 15, 2020 11:25

Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/cherr…

c329081

…y-pick-auth+subscriptions

replaced gophers library to our own

4be137c

JatinDev543 requested review from manishrjain and vvbalaji-dgraph as code owners July 15, 2020 07:57

added test

6ddd15b

pawanrawal mentioned this pull request Jul 15, 2020

fix(GraphQL): Error if both @auth and @withSubscription are used on same type. #5791

Closed

pawanrawal and others added 6 commits July 15, 2020 19:00

Fix build error by using latest version of dgraph-io/graphql-transpor…

ccdda66

…t-ws

Fix the audience unit tests

19556cd

Fix formatting

bd93fe8

fix for subscription withou auth

f39b836

fixed subscription error

8865a2e

added auth test

5febebb

pawanrawal suggested changes Jul 16, 2020

View reviewed changes

JatinDevDG and others added 4 commits July 17, 2020 11:32

resolved pawan's comments.

65a20ec

latest code.

3f3d819

Delete p,w and zw from checked in code

a385b11

Use jwt.At function so that time.IsZero returns correctly

01e2f2a

pawanrawal and others added 7 commits July 17, 2020 16:56

Complete the test

1546f14

Add another test cases to check subscription ends after JWT expires

16f1bd3

added test for multiple subscriptions

e17cefb

added test for multiple subscriptions with different jw data

ceeb83d

clean code

bf3367a

modified subscriptions test

4f03b0b

Fix the test

28386c8

JatinDev543 commented Jul 20, 2020

View reviewed changes

pawanrawal suggested changes Jul 20, 2020

View reviewed changes

addresses review comments.

18523ce

JatinDev543 commented Jul 20, 2020

View reviewed changes

JatinDev543 and others added 2 commits July 20, 2020 15:04

edited few comments

cd1424a

Remove subscriptionID from subscriber

c32512f

pawanrawal suggested changes Jul 20, 2020

View reviewed changes

pawanrawal added 3 commits July 20, 2020 15:46

Run go mod tidy to fix the go.mod and go.sum files

09da5b9

Clean up some code

3922649

Revert "Clean up some code"

cd8c31e

This reverts commit 3922649.

pawanrawal approved these changes Jul 20, 2020

View reviewed changes

JatinDev543 and others added 5 commits July 20, 2020 17:05

Merge branch 'master' of github.com:dgraph-io/dgraph into jatin/cherr…

cdaceb7

…y-pick-auth+subscriptions

Fix failures in custom logic test

0df5f69

Merge branch 'jatin/cherry-pick-auth+subscriptions' of github.com:dgr…

b2aef8c

…aph-io/dgraph into jatin/cherry-pick-auth+subscriptions

fixed deepsource error

a202293

fixed auth test

ddff473

JatinDev543 merged commit 5f81aac into master Jul 20, 2020

JatinDev543 added a commit that referenced this pull request Jul 20, 2020

Revert "feat(GraphQL): Adds auth for subscriptions (#5984)"

4a1acd1

This reverts commit 5f81aac.

pawanrawal deleted the jatin/cherry-pick-auth+subscriptions branch July 31, 2020 08:34

pawanrawal pushed a commit that referenced this pull request Jul 31, 2020

feat(GraphQL): Adds auth for subscriptions (#5984)

f5be5a5

Co-authored-by: Pawan Rawal <[email protected]> Co-authored-by: JatinDevDG <[email protected]> (cherry picked from commit 5f81aac)

JatinDev543 added a commit that referenced this pull request Aug 12, 2020

feat(GraphQL): Adds auth for subscriptions (#5984)

065f20d

Co-authored-by: Pawan Rawal <[email protected]> Co-authored-by: JatinDevDG <[email protected]> (cherry picked from commit 5f81aac)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(GraphQL): Adds auth for subscriptions #5984

feat(GraphQL): Adds auth for subscriptions #5984

JatinDev543 commented Jul 14, 2020 •

edited by dgraph-bot

Loading

pawanrawal left a comment

arijitAD left a comment

JatinDev543 left a comment

pawanrawal left a comment

JatinDev543 left a comment

pawanrawal left a comment

JatinDev543 left a comment

JatinDev543 left a comment

pawanrawal left a comment

pawanrawal left a comment

feat(GraphQL): Adds auth for subscriptions #5984

feat(GraphQL): Adds auth for subscriptions #5984

Conversation

JatinDev543 commented Jul 14, 2020 • edited by dgraph-bot Loading

pawanrawal left a comment

Choose a reason for hiding this comment

arijitAD left a comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

JatinDev543 left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

JatinDev543 commented Jul 14, 2020 •

edited by dgraph-bot

Loading