Skip to content

Commit

Permalink
Enable authentication via Github access token (#247)
Browse files Browse the repository at this point in the history
* Make naming of responses imports consistent

* Enable authentication via Github access token

* Use Github authentication for live service tests

* Make live/local test mode distinction clearer
  • Loading branch information
c-w authored Nov 22, 2019
1 parent c3ebfe9 commit 4ae4d70
Show file tree
Hide file tree
Showing 10 changed files with 284 additions and 21 deletions.
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ LOKOLE_EMAIL_SERVER_QUEUES_SAS_NAME=
LOKOLE_EMAIL_SERVER_QUEUES_SAS_KEY=
LOKOLE_EMAIL_SERVER_QUEUES_NAMESPACE=
LOKOLE_SENDGRID_KEY=
REGISTRATION_CREDENTIALS=admin:password

CLOUDBROWSER_PORT=10001
AZURITE_PORT=10000
Expand Down
17 changes: 12 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ language: minimal

env:
matrix:
- LOKOLE_QUEUE_BROKER_SCHEME=azureservicebus
- LOKOLE_QUEUE_BROKER_SCHEME=amqp
- TEST_MODE=live
- TEST_MODE=local

services:
- docker
Expand All @@ -21,7 +21,14 @@ script:
else
echo "Skipping cert renewal for on-demand build"
fi
if [[ "$TRAVIS_PULL_REQUEST" = "false" ]] && [[ "$LOKOLE_QUEUE_BROKER_SCHEME" = "azureservicebus" ]]; then
if [[ "$TEST_MODE" = "live" ]]; then
export REGISTRATION_CREDENTIALS="$GITHUB_AUTH_USERNAME:$GITHUB_AUTH_TOKEN"
export LOKOLE_QUEUE_BROKER_SCHEME="azureservicebus"
else
export REGISTRATION_CREDENTIALS="admin:password"
export LOKOLE_QUEUE_BROKER_SCHEME="amqp"
fi
if [[ "$TRAVIS_PULL_REQUEST" = "false" ]] && [[ "$TEST_MODE" = "live" ]]; then
echo "Skipping live service test for branch build"
else
make build verify-build
Expand All @@ -31,7 +38,7 @@ script:
after_success:
- |
if [[ "$LOKOLE_QUEUE_BROKER_SCHEME" = "amqp" ]]; then
if [[ "$TEST_MODE" = "local" ]]; then
bash <(curl -s https://codecov.io/bash)
fi
Expand All @@ -50,4 +57,4 @@ deploy:
on: # yamllint disable rule:truthy
repo: ascoderu/opwen-cloudserver
tags: true
condition: $LOKOLE_QUEUE_BROKER_SCHEME = azureservicebus
condition: $TEST_MODE = live
2 changes: 2 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ services:
context: .
dockerfile: docker/integtest/Dockerfile
command: echo skipped
environment:
REGISTRATION_CREDENTIALS: ${REGISTRATION_CREDENTIALS}
volumes:
- /var/run/docker.sock:/var/run/docker.sock

Expand Down
4 changes: 2 additions & 2 deletions docker/integtest/1-register-client.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mkdir -p "${out_dir}"
# normally this endpoint would be called during a new lokole device setup
curl -fs \
-H "Content-Type: application/json" \
-u "admin:password" \
-u "${REGISTRATION_CREDENTIALS}" \
-d '{"domain":"developer1.lokole.ca"}' \
"http://nginx:8888/api/email/register/" \
| tee "${out_dir}/register1.json"
Expand All @@ -27,7 +27,7 @@ if curl -fs \
# also register another client to simulate multi-client emails
curl -fs \
-H "Content-Type: application/json" \
-u "admin:password" \
-u "${REGISTRATION_CREDENTIALS}" \
-d '{"domain":"developer2.lokole.ca"}' \
"http://nginx:8888/api/email/register/" \
| tee "${out_dir}/register2.json"
2 changes: 2 additions & 0 deletions opwen_email_server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@

REGISTRATION_USERNAME = env('REGISTRATION_USERNAME', '')
REGISTRATION_PASSWORD = env('REGISTRATION_PASSWORD', '')
REGISTRATION_GITHUB_ORGANIZATION = env('REGISTRATION_GITHUB_ORGANIZATION', 'ascoderu')
REGISTRATION_GITHUB_TEAM = env('REGISTRATION_GITHUB_ORGANIZATION', 'lokole-registration')

MAX_WIDTH_IMAGES = env.int('LOKOLE_MAX_WIDTH_EMAIL_IMAGES', 200)
MAX_HEIGHT_IMAGES = env.int('LOKOLE_MAX_HEIGHT_EMAIL_IMAGES', 200)
Expand Down
3 changes: 3 additions & 0 deletions opwen_email_server/constants/github.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from typing_extensions import Final # noqa: F401

GRAPHQL_URL = 'https://api.github.com/graphql' # type: Final # noqa: E501
14 changes: 11 additions & 3 deletions opwen_email_server/integration/connexion.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
from opwen_email_server.integration.azure import get_raw_email_storage
from opwen_email_server.integration.celery import inbound_store
from opwen_email_server.integration.celery import written_store
from opwen_email_server.services.auth import AnyOfBasicAuth
from opwen_email_server.services.auth import BasicAuth
from opwen_email_server.services.auth import GithubBasicAuth

email_receive = ReceiveInboundEmail(
auth=get_auth(),
Expand Down Expand Up @@ -46,8 +48,14 @@
pending_factory=get_pending_storage,
)

basic_auth = BasicAuth(users={config.REGISTRATION_USERNAME: {
'password': config.REGISTRATION_PASSWORD,
}})
basic_auth = AnyOfBasicAuth(auths=[
BasicAuth(users={
config.REGISTRATION_USERNAME: {'password': config.REGISTRATION_PASSWORD},
}),
GithubBasicAuth(
organization=config.REGISTRATION_GITHUB_ORGANIZATION,
team=config.REGISTRATION_GITHUB_TEAM,
),
])

healthcheck = Ping()
92 changes: 92 additions & 0 deletions opwen_email_server/services/auth.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,34 @@
from functools import lru_cache
from typing import Callable
from typing import Dict
from typing import Iterable
from typing import List
from typing import Optional

from libcloud.storage.types import ObjectDoesNotExistError
from requests import RequestException
from requests import post as http_post

from opwen_email_server.constants import events
from opwen_email_server.constants import github
from opwen_email_server.constants.cache import AUTH_DOMAIN_CACHE_SIZE
from opwen_email_server.services.storage import AzureTextStorage
from opwen_email_server.utils.log import LogMixin


class AnyOfBasicAuth(LogMixin):
def __init__(self, auths: Iterable[Callable[[str, str, Optional[List[str]]], Optional[Dict[str, str]]]]):
self._auths = list(auths)

def __call__(self, username, password, required_scopes=None):
for auth in self._auths:
user = auth(username, password, required_scopes)
if user is not None:
return user

return None


class BasicAuth(LogMixin):
def __init__(self, users: dict):
self._users = dict(users)
Expand Down Expand Up @@ -41,6 +61,78 @@ def _has_scopes(cls, user, required_scopes) -> bool:
return set(required_scopes).issubset(user.get('scopes', []))


class GithubBasicAuth(LogMixin):
def __init__(self, organization: str, team: str, page_size: int = 50):
self._organization = organization
self._team = team
self._page_size = page_size

def __call__(self, username, password, required_scopes=None):
if not username or not password or not self._organization or not self._team:
return None

try:
team_members = self._fetch_team_members(access_token=password)
user_exists = any(username == team_member for team_member in team_members)
except RequestException:
self.log_event(events.BAD_PASSWORD, {'username': username}) # noqa: E501 # yapf: disable
return None

if not user_exists:
self.log_event(events.UNKNOWN_USER, {'username': username}) # noqa: E501 # yapf: disable
return None

return {'sub': username}

def _fetch_team_members(self, access_token: str) -> Iterable[str]:
cursor = None

while True:
response = http_post(
url=github.GRAPHQL_URL,
json={
'query': '''
query($organization:String!, $team:String!, $cursor:String, $first:Int!) {
organization(login:$organization) {
team(slug:$team) {
members(after:$cursor, first:$first, orderBy:{ field:LOGIN, direction:DESC }) {
edges {
cursor
}
nodes {
login
}
}
}
}
}
''',
'variables': {
'organization': self._organization,
'team': self._team,
'cursor': cursor,
'first': self._page_size,
},
},
headers={
'Authorization': f'Bearer {access_token}',
},
)
response.raise_for_status()

members = response.json()['data']['organization']['team']['members']
nodes = members['nodes']
edges = members['edges']

for member in nodes:
yield member['login']

if len(nodes) < self._page_size:
break

cursor = edges[-1]['cursor']


class AzureAuth(LogMixin):
def __init__(self, storage: AzureTextStorage) -> None:
self._storage = storage
Expand Down
146 changes: 146 additions & 0 deletions tests/opwen_email_server/services/test_auth.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,68 @@
from shutil import rmtree
from tempfile import mkdtemp
from unittest import TestCase
from unittest.mock import Mock

from responses import mock as mock_responses

from opwen_email_server.constants import github
from opwen_email_server.services.auth import AzureAuth
from opwen_email_server.services.auth import AnyOfBasicAuth
from opwen_email_server.services.auth import BasicAuth
from opwen_email_server.services.auth import GithubBasicAuth
from opwen_email_server.services.storage import AzureTextStorage


class _MockResponses:
def __init__(self, responses):
self._i = 0
self._responses = responses

def __call__(self, *args, **kwargs):
try:
status, headers, body = self._responses[self._i]
except ValueError:
body = self._responses[self._i]
status = 200
headers = {}

self._i += 1

return status, headers, body


class AnyOfBasicAuthTests(TestCase):
def setUp(self):
self._auth1 = Mock()
self._auth2 = Mock()
self._auth3 = Mock()
self._auth = AnyOfBasicAuth(auths=[self._auth1, self._auth2, self._auth3])

def test_with_failing_sub_auth(self):
self._auth1.return_value = None
self._auth2.return_value = None
self._auth3.return_value = None

user = self._auth('username', 'password')

self.assertIsNone(user)
self.assertEqual(self._auth1.call_count, 1)
self.assertEqual(self._auth2.call_count, 1)
self.assertEqual(self._auth3.call_count, 1)

def test_with_passing_sub_auth(self):
self._auth1.return_value = None
self._auth2.return_value = {'sub': 'username'}
self._auth3.return_value = None

user = self._auth('username', 'password')

self.assertIsNotNone(user)
self.assertEqual(self._auth1.call_count, 1)
self.assertEqual(self._auth2.call_count, 1)
self.assertEqual(self._auth3.call_count, 0)


class BasicAuthTests(TestCase):
def setUp(self):
self._auth = BasicAuth({
Expand Down Expand Up @@ -38,6 +94,96 @@ def test_with_correct_password(self):
self.assertIsNotNone(self._auth(username='user1', password='pass', required_scopes=['scope1', 'scopeA']))


class GithubBasicAuthTests(TestCase):
def setUp(self):
self._auth = GithubBasicAuth(organization='organization', team='team', page_size=2)

def test_with_bad_user(self):
self.assertIsNone(self._auth(username='', password='pass'))

@mock_responses.activate
def test_with_missing_user(self):
mock_responses.add(
mock_responses.POST,
github.GRAPHQL_URL,
body='''{
"data": {
"organization": {
"team": {
"members": {
"edges": [
{"cursor": "cursor1"}
],
"nodes": [
{"login": "user1"}
]
}
}
}
}
}''',
status=200,
)

self.assertIsNone(self._auth(username='does-not-exist', password='pass'))

@mock_responses.activate
def test_with_bad_password(self):
mock_responses.add(
mock_responses.POST,
github.GRAPHQL_URL,
json={'message': 'Bad credentials'},
status=401,
)

self.assertIsNone(self._auth(username='user1', password='incorrect'))

@mock_responses.activate
def test_with_correct_password(self):
mock_responses.add_callback(
mock_responses.POST,
github.GRAPHQL_URL,
callback=_MockResponses([
'''{
"data": {
"organization": {
"team": {
"members": {
"edges": [
{"cursor": "cursor1"},
{"cursor": "cursor2"}
],
"nodes": [
{"login": "user1"},
{"login": "user2"}
]
}
}
}
}
}''',
'''{
"data": {
"organization": {
"team": {
"members": {
"edges": [
{"cursor": "cursor3"}
],
"nodes": [
{"login": "user3"}
]
}
}
}
}
}''',
]),
)

self.assertIsNotNone(self._auth(username='user3', password='pass'))


class AzureAuthTests(TestCase):
def setUp(self):
self._folder = mkdtemp()
Expand Down
Loading

0 comments on commit 4ae4d70

Please sign in to comment.