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

[github app] Encode App baseURL in state instead of using Referer#55305

Merged
pjlast merged 5 commits into
mainfrom
pjlast/gha-referer-url-fix
Jul 26, 2023
Merged

[github app] Encode App baseURL in state instead of using Referer#55305
pjlast merged 5 commits into
mainfrom
pjlast/gha-referer-url-fix

Conversation

@pjlast

@pjlast pjlast commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

Firefox's default settings do not forward Referer headers for privacy reasons. This means that GitHub App setups fail when done using Firefox.

This PR adds the GitHub App's base url to the state we store in Redis, so that we no longer rely on the browser sending the Referer header.

Test plan

Tests updated.

@sourcegraph-bot

sourcegraph-bot commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 9a01c36...5136d1b.

Notify File(s)
@unknwon enterprise/cmd/frontend/internal/auth/githubappauth/middleware.go
enterprise/cmd/frontend/internal/auth/githubappauth/middleware_test.go

@pjlast pjlast requested a review from a team July 26, 2023 08:46

@kopancek kopancek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, but would be great if we can have this as part of an existing unit test.

@kopancek

Copy link
Copy Markdown
Contributor

Looks good, but would be great if we can have this as part of an existing unit test.

While I was writing this comment the unit test magically appeared. 👏 🙇

@sourcegraph-bot

sourcegraph-bot commented Jul 26, 2023

Copy link
Copy Markdown
Contributor

📖 Storybook live preview

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we backport for todays patch?

@pjlast pjlast merged commit 02ee893 into main Jul 26, 2023
@pjlast pjlast deleted the pjlast/gha-referer-url-fix branch July 26, 2023 11:05
github-actions Bot pushed a commit that referenced this pull request Jul 26, 2023
BolajiOlajide pushed a commit that referenced this pull request Jul 26, 2023
…ing Referer (#55307)

Firefox's default settings do not forward Referer headers for privacy reasons. This means that GitHub App setups fail when done using Firefox.

This PR adds the GitHub App's base url to the state we store in Redis, so that we no longer rely on the browser sending the Referer header.

## Test plan

Tests updated.

<!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->
 <br> Backport 02ee893 from #55305
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.

6 participants