Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Fix auth-related error in the SSC REST API client hook#62798

Merged
chrsmith merged 2 commits into
mainfrom
chrsmith/auth-error-for-api-client
May 20, 2024
Merged

Fix auth-related error in the SSC REST API client hook#62798
chrsmith merged 2 commits into
mainfrom
chrsmith/auth-error-for-api-client

Conversation

@chrsmith

Copy link
Copy Markdown
Contributor

While working on integrating the SSC REST API client hook (https://github.com/sourcegraph/sourcegraph/pull/62715) I spent longer than I would have liked trying to figure out why it wasn't working in some cases...

To cut to the punch line, there is a quirk of the fetch() API in that it won't send the "origin" header for GET or HEAD requests. And that lead to the Sourcegraph backend not treating the request as authenticated, even though we supplied the user's sgs session cookie.

After doing my best to understand how things all work, it looks like the simplest thing would be to just supply the X-Requested-With header. Which is how the backend authenticates HTTP requests based on the session cookie. (In addition to the Origin header if it is made available, which is why I didn't catch this when working on the initial PR -- because I was only using POST methods...)

Test Plan

Testing locally (how I found the book). CI/CD.

@chrsmith chrsmith requested review from taras-yemets and vdavid May 20, 2024 19:04
@cla-bot cla-bot Bot added the cla-signed label May 20, 2024
@chrsmith chrsmith force-pushed the chrsmith/auth-error-for-api-client branch from dc8a51e to 26a960a Compare May 20, 2024 19:05
@taras-yemets

Copy link
Copy Markdown
Contributor

I guess I faced this issue today, but "fixed" it in an ugly way to unblock myself.
Thanks for fixing it!

@chrsmith chrsmith merged commit 622cf7d into main May 20, 2024
@chrsmith chrsmith deleted the chrsmith/auth-error-for-api-client branch May 20, 2024 19:45
@chrsmith

Copy link
Copy Markdown
Contributor Author

but "fixed" it in an ugly way to unblock myself.

Sorry for not posting a PR to fix this sooner! 🙇 (Though to be fair, it was a PITA to track down.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants