Skip to content
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

Fixes webhooks attempting to authenticate #129

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

simonbs
Copy link
Contributor

@simonbs simonbs commented Nov 7, 2023

This PR fixes an issue where our web hooks would attempt to access the user session, however, no user is logged in when executing a webhook.

The regression was introduced in #113 where we decorate our GitHubClient with a AccessTokenRefreshingGitHubClient which makes all invocations of GitHubClient authenticated. However, that causes errors when using the GitHub client through a webhook.

To fix the issue we ensure that our webhooks do not use an instance of AccessTokenRefreshingGitHubClient but rather uses an instance of GitHubClient directly.

@simonbs simonbs self-assigned this Nov 7, 2023
Copy link
Contributor

@ulrikandersen ulrikandersen left a comment

Choose a reason for hiding this comment

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

Dependency injection 💯

@simonbs simonbs merged commit 833a378 into develop Nov 7, 2023
3 checks passed
@simonbs simonbs deleted the bugfix/authenticating-webhooks branch November 7, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants