-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Disallow schema queries when an user has not logged in. #4107
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ A review job has been created and sent to the PullRequest network.
@martinmr you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation changes were easy to follow, thank you. No further comments and no significant defects identified.
Reviewed with ❤️ by PullRequest
@@ -678,6 +678,8 @@ func authorizeQuery(ctx context.Context, parsedReq *gql.Result) error { | |||
var userId string | |||
var groupIds []string | |||
preds := parsePredsFromQuery(parsedReq.Query) | |||
isSchemaQuery := parsedReq != nil && parsedReq.Schema != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like you are already doing a parsedReq.Query
above which should fail with a nil pointer dereference before this. So you may be able to either remove the parsedReq != nil
check if it is guaranteed to not be nil
or add a check earlier on and return an error possibly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to inactivity, PullRequest has cancelled this review job. You can reactivate the code review job from the PullRequest dashboard
@mangalaman93 @animesh2049 This design has been approved, can you take a look at this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 8 files at r1, 4 of 4 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @martinmr)
This change is