-
Notifications
You must be signed in to change notification settings - Fork 6
Switch to auth headers #10
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
Conversation
@@ -3,7 +3,7 @@ DATABASE_URL="postgresql://johndoe:randompassword@localhost:5432/mydb?schema=${P | |||
|
|||
|
|||
# Gitea Webhook Configuration | |||
GITEA_WEBHOOK_SECRET="your_webhook_secret_here" | |||
GITEA_WEBHOOK_AUTH="your_webhook_secret_here" |
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.
Consider renaming the environment variable GITEA_WEBHOOK_AUTH
to something more descriptive if it is intended to store an authentication header value, as AUTH
might be too generic and could lead to confusion.
@@ -15,7 +15,7 @@ import { ReviewHistoryController } from './review-history/reviewHistory.controll | |||
import { ChallengeApiService } from 'src/shared/modules/global/challenge.service'; | |||
import { WebhookController } from './webhook/webhook.controller'; | |||
import { WebhookService } from './webhook/webhook.service'; | |||
import { GiteaSignatureGuard } from '../shared/guards/gitea-signature.guard'; | |||
import { GiteaWebhookAuthGuard } from '../shared/guards/gitea-webhook-auth.guard'; |
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.
The import statement has been updated to use GiteaWebhookAuthGuard
instead of GiteaSignatureGuard
. Ensure that the new guard GiteaWebhookAuthGuard
is correctly implemented and provides the necessary functionality that was previously handled by GiteaSignatureGuard
. Also, verify that all references to GiteaSignatureGuard
in the codebase have been updated accordingly.
@@ -13,7 +13,7 @@ import { | |||
WebhookEventDto, | |||
WebhookResponseDto, | |||
} from '../../dto/webhook-event.dto'; | |||
import { GiteaSignatureGuard } from '../../shared/guards/gitea-signature.guard'; | |||
import { GiteaWebhookAuthGuard } from '../../shared/guards/gitea-webhook-auth.guard'; |
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.
The import statement has been updated to use GiteaWebhookAuthGuard
instead of GiteaSignatureGuard
. Ensure that the new guard GiteaWebhookAuthGuard
is correctly implemented and that all necessary changes in the codebase have been made to accommodate this switch. Verify that the new guard provides the required authentication functionality and that any dependencies or configurations have been updated accordingly.
|
||
@Injectable() | ||
export class GiteaWebhookAuthGuard implements CanActivate { | ||
private readonly logger = LoggerService.forRoot('GiteaWebhookAuthGuard'); |
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.
Consider using dependency injection for the LoggerService instead of calling forRoot
. This will make the service easier to test and mock.
const authHeader = request.headers['authorization'] as string; | ||
|
||
// Check if GITEA_WEBHOOK_AUTH is configured | ||
const auth = process.env.GITEA_WEBHOOK_AUTH; |
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 might be more secure to use a configuration service or a secrets manager to handle sensitive environment variables like GITEA_WEBHOOK_AUTH
instead of accessing them directly from process.env
.
throw new BadRequestException('Missing authorization header'); | ||
} | ||
|
||
if (authHeader !== `Bearer ${auth}`) { |
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.
Consider using a more secure method for comparing the authorization header, such as a constant-time comparison function, to prevent timing attacks.
No description provided.