Address a variety of small review issues

This commit is contained in:
Jeff Bradberry 2019-09-27 15:07:23 -04:00
parent 6aa6471b7c
commit b6b70e55fb
6 changed files with 22 additions and 18 deletions

View File

@ -8,4 +8,5 @@ webhook events. The response will include the following fields:
webhook configuration of the service this template will be receiving
webhook events from (string, read-only)
Make an empty POST request to this resource to cycle in a new `webhook_key`.
Make an empty POST request to this resource to generate a new
replacement `webhook_key`.

View File

@ -5,6 +5,7 @@ import logging
import urllib.parse
from django.utils.encoding import force_bytes
from django.utils.translation import ugettext_lazy as _
from django.views.decorators.csrf import csrf_exempt
from rest_framework import status
@ -139,7 +140,7 @@ class WebhookReceiverBase(APIView):
if WorkflowJob.objects.filter(**kwargs).exists() or Job.objects.filter(**kwargs).exists():
# Short circuit if this webhook has already been received and acted upon.
logger.debug("Webhook previously received, returning without action.")
return Response({'message': "Webhook previously received, aborting."},
return Response({'message': _("Webhook previously received, aborting.")},
status=status.HTTP_202_ACCEPTED)
kwargs = {
@ -192,9 +193,11 @@ class GithubWebhookReceiver(WebhookReceiverBase):
def get_signature(self):
header_sig = self.request.META.get('HTTP_X_HUB_SIGNATURE')
if not header_sig:
logger.debug("Expected signature missing from header key HTTP_X_HUB_SIGNATURE")
raise PermissionDenied
hash_alg, signature = header_sig.split('=')
if hash_alg != 'sha1':
logger.debug("Unsupported signature type, expected: sha1, received: {}".format(hash_alg))
raise PermissionDenied
return force_bytes(signature)
@ -212,7 +215,7 @@ class GitlabWebhookReceiver(WebhookReceiverBase):
return self.request.META.get('HTTP_X_GITLAB_EVENT')
def get_event_guid(self):
# Gitlab does not provide a unique identifier on events, so construct one.
# GitLab does not provide a unique identifier on events, so construct one.
h = sha1()
h.update(force_bytes(self.request.body))
return h.hexdigest()
@ -230,13 +233,13 @@ class GitlabWebhookReceiver(WebhookReceiverBase):
parsed.scheme, parsed.netloc, project['id'], self.get_event_ref())
def get_signature(self):
return force_bytes(self.request.META.get('HTTP_X_GITLAB_TOKEN'))
return force_bytes(self.request.META.get('HTTP_X_GITLAB_TOKEN') or '')
def check_signature(self, obj):
if not obj.webhook_key:
raise PermissionDenied
# Gitlab only returns the secret token, not an hmac hash. Use
# GitLab only returns the secret token, not an hmac hash. Use
# the hmac `compare_digest` helper function to prevent timing
# analysis by attackers.
if not hmac.compare_digest(force_bytes(obj.webhook_key), self.get_signature()):

View File

@ -24,7 +24,7 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name='jobtemplate',
name='webhook_service',
field=models.CharField(blank=True, choices=[('github', 'Github'), ('gitlab', 'Gitlab')], help_text='Service that webhook requests will be accepted from', max_length=16),
field=models.CharField(blank=True, choices=[('github', 'GitHub'), ('gitlab', 'GitLab')], help_text='Service that webhook requests will be accepted from', max_length=16),
),
migrations.AddField(
model_name='workflowjobtemplate',
@ -39,7 +39,7 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name='workflowjobtemplate',
name='webhook_service',
field=models.CharField(blank=True, choices=[('github', 'Github'), ('gitlab', 'Gitlab')], help_text='Service that webhook requests will be accepted from', max_length=16),
field=models.CharField(blank=True, choices=[('github', 'GitHub'), ('gitlab', 'GitLab')], help_text='Service that webhook requests will be accepted from', max_length=16),
),
migrations.AlterField(
model_name='unifiedjob',

View File

@ -24,7 +24,7 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name='job',
name='webhook_service',
field=models.CharField(blank=True, choices=[('github', 'Github'), ('gitlab', 'Gitlab')], help_text='Service that webhook requests will be accepted from', max_length=16),
field=models.CharField(blank=True, choices=[('github', 'GitHub'), ('gitlab', 'GitLab')], help_text='Service that webhook requests will be accepted from', max_length=16),
),
migrations.AddField(
model_name='workflowjob',
@ -39,6 +39,6 @@ class Migration(migrations.Migration):
migrations.AddField(
model_name='workflowjob',
name='webhook_service',
field=models.CharField(blank=True, choices=[('github', 'Github'), ('gitlab', 'Gitlab')], help_text='Service that webhook requests will be accepted from', max_length=16),
field=models.CharField(blank=True, choices=[('github', 'GitHub'), ('gitlab', 'GitLab')], help_text='Service that webhook requests will be accepted from', max_length=16),
),
]

View File

@ -972,7 +972,7 @@ ManagedCredentialType(
ManagedCredentialType(
namespace='github_token',
kind='token',
name=ugettext_noop('Github Personal Access Token'),
name=ugettext_noop('GitHub Personal Access Token'),
managed_by_tower=True,
inputs={
'fields': [{
@ -980,7 +980,7 @@ ManagedCredentialType(
'label': ugettext_noop('Token'),
'type': 'string',
'secret': True,
'help_text': ugettext_noop('This token needs to come from your profile settings in Github')
'help_text': ugettext_noop('This token needs to come from your profile settings in GitHub')
}],
'required': ['token'],
},
@ -989,7 +989,7 @@ ManagedCredentialType(
ManagedCredentialType(
namespace='gitlab_token',
kind='token',
name=ugettext_noop('Gitlab Personal Access Token'),
name=ugettext_noop('GitLab Personal Access Token'),
managed_by_tower=True,
inputs={
'fields': [{
@ -997,7 +997,7 @@ ManagedCredentialType(
'label': ugettext_noop('Token'),
'type': 'string',
'secret': True,
'help_text': ugettext_noop('This token needs to come from your profile settings in Gitlab')
'help_text': ugettext_noop('This token needs to come from your profile settings in GitLab')
}],
'required': ['token'],
},

View File

@ -496,8 +496,8 @@ class WebhookTemplateMixin(models.Model):
abstract = True
SERVICES = [
('github', "Github"),
('gitlab', "Gitlab"),
('github', "GitHub"),
('gitlab', "GitLab"),
]
webhook_service = models.CharField(
@ -583,7 +583,7 @@ class WebhookMixin(models.Model):
'pending': 'pending',
'successful': 'success',
'failed': 'failure',
'canceled': 'failure', # Github doesn't have a 'canceled' status :(
'canceled': 'failure', # GitHub doesn't have a 'canceled' status :(
'error': 'error',
},
'gitlab': {
@ -591,7 +591,7 @@ class WebhookMixin(models.Model):
'running': 'running',
'successful': 'success',
'failed': 'failed',
'error': 'failed', # Gitlab doesn't have an 'error' status distinct from 'failed' :(
'error': 'failed', # GitLab doesn't have an 'error' status distinct from 'failed' :(
'canceled': 'canceled',
},
}
@ -611,7 +611,7 @@ class WebhookMixin(models.Model):
k: v.format(self.webhook_credential.get_input('token')),
'Content-Type': 'application/json'
}
response = requests.post(status_api, data=json.dumps(data), headers=headers)
response = requests.post(status_api, data=json.dumps(data), headers=headers, timeout=30)
except Exception:
logger.exception("Posting webhook status caused an error.")
return