fetch NamespaceSelector namespaces from GraphQL (viewer { affiliatedNamespaces })#63592
Conversation
Check notice
Code scanning / Semgrep OSS
Semgrep Finding: security-semgrep-rules.semgrep-rules.golang.unauthorized-graphql-resolver-rule
There was a problem hiding this comment.
I think this alert might be correct - outside of dotcom we probably want to return an auth error here 🤔
There was a problem hiding this comment.
Yeah, good call. I agree, but I want to confirm we are thinking for the same reason:
Even though the GraphQL API would be completely inaccessible to any unauthenticated client outside of dotcom, for defense in depth and limiting the negative consequences of a misconfiguration, it is good to restrict this to dotcom only.
(Let me know if there is something else.)
There was a problem hiding this comment.
Oh! I actually misremembered that a subset of the GQL API was publicly accessible - that doesn't seem to be the case though:
curl -X POST https://sourcegraph.sourcegraph.com/.api/graphql -d '{}'
Private mode requires authentication.
So I think this would not actually be an issue. Maybe we can instead add a comment that the user == nil && err == nil case is implicitly dotcom-only - but not a blocker
There was a problem hiding this comment.
I think this alert might be correct - outside of dotcom we probably want to return an auth error here 🤔
Check notice
Code scanning / Semgrep OSS
Semgrep Finding: security-semgrep-rules.semgrep-rules.generic.comment-tagging-rule
Check notice
Code scanning / Semgrep OSS
Semgrep Finding: security-semgrep-rules.semgrep-rules.generic.comment-tagging-rule
Check notice
Code scanning / Semgrep OSS
Semgrep Finding: security-semgrep-rules.semgrep-rules.generic.comment-tagging-rule
15547fe to
f846bee
Compare
Previously, the Batch Changes namespace selector looked at the user's settings that were loaded on the initial page load. This bypassed the GraphQL API, which should be the source of truth for "what namespaces can a user write to" (i.e., their user account and all orgs they are a member of), and introduced a reliance on non-type-safe `jscontext` data. Also, adding any other features to this NamespaceSelector would require changing the data fetched in `jscontext`, so it was brittle.
I intend to use the NamespaceSelector for more things that need these missing features and a more solid foundation (such as the ability to transfer saved searches to a different owner), so I started out by refactoring this to access data from the GraphQL API. I added a new `viewer { affiliatedNamespaces }` GraphQL API field to support this.
Test plan:
1. Go to the Batch Changes global create page. Create a batch change. Ensure it works if you use the initial/default selected namespace (your user account), and if you change it. Ensure the created batch change exists in the intended namespace.
2. Do the same, but from a user or org batch changes create page. Ensure the NamespaceSelector's initial option is the namespace you are visiting, and that creation succeeds.
Previously, the Batch Changes namespace selector looked at the user's settings that were loaded on the initial page load. This bypassed the GraphQL API, which should be the source of truth for "what namespaces can a user write to" (i.e., their user account and all orgs they are a member of), and introduced a reliance on non-type-safe
jscontextdata. Also, adding any other features to this NamespaceSelector would require changing the data fetched injscontext, so it was brittle.I intend to use the NamespaceSelector for more things that need these missing features and a more solid foundation (such as the ability to transfer saved searches to a different owner), so I started out by refactoring this to access data from the GraphQL API. I added a new
viewer { affiliatedNamespaces }GraphQL API field to support this.Also adds tests and a storybook for this NamespaceSelector component.
Test plan
Test plan: