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

added support for authenticating updates from github #4

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
10 changes: 8 additions & 2 deletions cinch/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

from collections import namedtuple
import logging
from flask import request
from flask import abort, request
from nameko.standalone.events import event_dispatcher
from sqlalchemy.orm.exc import NoResultFound
from werkzeug.security import safe_str_cmp

from cinch import app, db
from cinch.models import Project, PullRequest
Expand Down Expand Up @@ -120,7 +121,12 @@ def handle_github_webhok():
"""We always return a 200 to keep github happy, but include info about
actions taken
"""
# TODO: verify request is from github
update_secret = app.config.get('GITHUB_UPDATE_SECRET')
if (
update_secret and
not safe_str_cmp(request.args.get('secret', ''), update_secret)
):
abort(401)

parser = GithubHookParser(request)

Expand Down
4 changes: 4 additions & 0 deletions setup_env.sample.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ export CINCH_SENTRY_DSN=

# A sqlalchemy parseable database identifier
export CINCH_DB_URI=

# For authenticating updates sent from github.
# This must be included in the callback url under the param `secret`
export CINCH_GITHUB_UPDATE_SECRET=
12 changes: 12 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def pytest_configure(config):
if db_uri:
os.environ['CINCH_DB_URI'] = db_uri

# disable error catching for better traceback
from cinch import app
app.testing = True


@pytest.fixture
def session():
Expand All @@ -32,3 +36,11 @@ def app_context():
from cinch import app
with app.test_request_context():
yield app


@pytest.yield_fixture(autouse=True)
def temp_config():
from cinch import app
original_config = app.config.copy()
yield
app.config = original_config
21 changes: 19 additions & 2 deletions tests/test_github.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,31 @@
import pytest

from cinch import app
from cinch.github import Responses
from cinch.github import HookEvents, Responses
from cinch.models import Project, PullRequest
from cinch.worker import MasterMoved, PullRequestMoved

URL = '/api/github/update'


@pytest.mark.parametrize(('args', 'status_code'), [
({}, 401),
({'secret': 'incorrect secret'}, 401),
({'secret': 'secret'}, 200),
])
def test_updates_require_secret(args, status_code):
app.config['GITHUB_TOKEN'] = 'abc'
app.config['GITHUB_UPDATE_SECRET'] = 'secret'

with app.test_client() as client:
res = client.post(
'/api/github/update', query_string=args, data={'payload': '{}'},
headers={HookEvents.HEADER_KEY: HookEvents.PING},
)

assert res.status_code == status_code


@pytest.fixture(autouse=True)
def propagate_exceptions():
app.config['PROPAGATE_EXCEPTIONS'] = True
Expand All @@ -35,7 +53,6 @@ def poster(data, event_type):
return poster



def test_ping():
client = app.test_client()
res = client.post(URL, headers={'X-GitHub-Event': 'ping'})
Expand Down