diff --git a/.gitignore b/.gitignore index 40985af..0d3ad0a 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ *.eggs /dist /build +venv +.venv diff --git a/conftest.py b/conftest.py index 271be78..3a6e2b8 100644 --- a/conftest.py +++ b/conftest.py @@ -3,10 +3,8 @@ import sys import os -os.environ.setdefault('DB', 'postgres') +os.environ.setdefault("DB", "postgres") sys.path.insert(0, os.path.join(os.path.dirname(__file__))) -pytest_plugins = [ - 'sentry.utils.pytest' -] +pytest_plugins = ["sentry.utils.pytest"] diff --git a/oidc/apps.py b/oidc/apps.py index c36737b..48a8f8d 100644 --- a/oidc/apps.py +++ b/oidc/apps.py @@ -9,4 +9,4 @@ def ready(self): from sentry import auth from .provider import OIDCProvider - auth.register('oidc', OIDCProvider) + auth.register("oidc", OIDCProvider) diff --git a/oidc/constants.py b/oidc/constants.py index f4eba7d..8c41b58 100644 --- a/oidc/constants.py +++ b/oidc/constants.py @@ -4,29 +4,31 @@ import requests -AUTHORIZATION_ENDPOINT = getattr(settings, 'OIDC_AUTHORIZATION_ENDPOINT', None) -TOKEN_ENDPOINT = getattr(settings, 'OIDC_TOKEN_ENDPOINT', None) -CLIENT_ID = getattr(settings, 'OIDC_CLIENT_ID', None) -CLIENT_SECRET = getattr(settings, 'OIDC_CLIENT_SECRET', None) -USERINFO_ENDPOINT = getattr(settings, 'OIDC_USERINFO_ENDPOINT', None) -SCOPE = getattr(settings, 'OIDC_SCOPE', 'openid email') +AUTHORIZATION_ENDPOINT = getattr(settings, "OIDC_AUTHORIZATION_ENDPOINT", None) +TOKEN_ENDPOINT = getattr(settings, "OIDC_TOKEN_ENDPOINT", None) +CLIENT_ID = getattr(settings, "OIDC_CLIENT_ID", None) +CLIENT_SECRET = getattr(settings, "OIDC_CLIENT_SECRET", None) +USERINFO_ENDPOINT = getattr(settings, "OIDC_USERINFO_ENDPOINT", None) +SCOPE = getattr(settings, "OIDC_SCOPE", "openid email") WELL_KNOWN_SCHEME = "/.well-known/openid-configuration" -ERR_INVALID_RESPONSE = 'Unable to fetch user information from provider. Please check the log.' +ERR_INVALID_RESPONSE = ( + "Unable to fetch user information from provider. Please check the log." +) ISSUER = None -DATA_VERSION = '1' +DATA_VERSION = "1" -OIDC_DOMAIN = getattr(settings, 'OIDC_DOMAIN', None) +OIDC_DOMAIN = getattr(settings, "OIDC_DOMAIN", None) if OIDC_DOMAIN: WELL_KNOWN_URL = OIDC_DOMAIN.strip("/") + WELL_KNOWN_SCHEME well_known_values = requests.get(WELL_KNOWN_URL, timeout=2.0).json() if well_known_values: - USERINFO_ENDPOINT = well_known_values['userinfo_endpoint'] - AUTHORIZATION_ENDPOINT = well_known_values['authorization_endpoint'] - TOKEN_ENDPOINT = well_known_values['token_endpoint'] - ISSUER = well_known_values['issuer'] + USERINFO_ENDPOINT = well_known_values["userinfo_endpoint"] + AUTHORIZATION_ENDPOINT = well_known_values["authorization_endpoint"] + TOKEN_ENDPOINT = well_known_values["token_endpoint"] + ISSUER = well_known_values["issuer"] -config_issuer = getattr(settings, 'OIDC_ISSUER', None) +config_issuer = getattr(settings, "OIDC_ISSUER", None) if config_issuer: ISSUER = config_issuer diff --git a/oidc/provider.py b/oidc/provider.py index e4f9961..cab320c 100644 --- a/oidc/provider.py +++ b/oidc/provider.py @@ -2,20 +2,21 @@ import requests -from sentry.auth.providers.oauth2 import ( - OAuth2Callback, OAuth2Provider, OAuth2Login -) +from sentry.auth.provider import MigratingIdentityId +from sentry.auth.providers.oauth2 import OAuth2Callback, OAuth2Provider, OAuth2Login + from .constants import ( AUTHORIZATION_ENDPOINT, USERINFO_ENDPOINT, - ISSUER, TOKEN_ENDPOINT, + ISSUER, + TOKEN_ENDPOINT, CLIENT_SECRET, CLIENT_ID, - SCOPE, DATA_VERSION + SCOPE, + DATA_VERSION, ) from .views import FetchUser, OIDCConfigureView -import logging -logger = logging.getLogger('sentry.auth.oidc') +import time class OIDCLogin(OAuth2Login): @@ -23,19 +24,17 @@ class OIDCLogin(OAuth2Login): client_id = CLIENT_ID scope = SCOPE - def __init__(self, domains=None): + def __init__(self, client_id, domains=None): self.domains = domains - super(OIDCLogin, self).__init__() + super(OIDCLogin, self).__init__(client_id=client_id) def get_authorize_params(self, state, redirect_uri): - params = super(OIDCLogin, self).get_authorize_params( - state, redirect_uri - ) + params = super(OIDCLogin, self).get_authorize_params(state, redirect_uri) # TODO(dcramer): ideally we could look at the current resulting state # when an existing auth happens, and if they're missing a refresh_token # we should re-prompt them a second time with ``approval_prompt=force`` - params['approval_prompt'] = 'force' - params['access_type'] = 'offline' + params["approval_prompt"] = "force" + params["access_type"] = "offline" return params @@ -67,7 +66,7 @@ def get_configure_view(self): def get_auth_pipeline(self): return [ - OIDCLogin(domains=self.domains), + OIDCLogin(self.client_id, domains=self.domains), OAuth2Callback( access_token_url=TOKEN_ENDPOINT, client_id=self.client_id, @@ -84,30 +83,44 @@ def get_refresh_token_url(self): def build_config(self, state): return { - 'domains': [state['domain']], - 'version': DATA_VERSION, + "domains": [state["domain"]], + "version": DATA_VERSION, } def get_user_info(self, bearer_token): endpoint = USERINFO_ENDPOINT - bearer_auth = 'Bearer ' + bearer_token - return requests.get(endpoint + "?schema=openid", - headers={'Authorization': bearer_auth}, - timeout=2.0).json() + bearer_auth = "Bearer " + bearer_token + retry_codes = [429, 500, 502, 503, 504] + for retry in range(10): + if 10 < retry: + return {} + r = requests.get( + endpoint + "?schema=openid", + headers={"Authorization": bearer_auth}, + timeout=2.0, + ) + if r.status_code in retry_codes: + wait_time = 2 ** retry * 0.1 + time.sleep(wait_time) + continue + return r.json() def build_identity(self, state): - data = state['data'] - bearer_token = data['access_token'] + data = state["data"] + user_data = state["user"] + + bearer_token = data["access_token"] user_info = self.get_user_info(bearer_token) - if not user_info.get('email'): - logger.error('Missing email in user endpoint: %s' % data) - user_data = state['user'] + # XXX(epurkhiser): We initially were using the email as the id key. + # This caused account dupes on domain changes. Migrate to the + # account-unique sub key. + user_id = MigratingIdentityId(id=user_data["sub"], legacy_id=user_data["email"]) + return { - 'id': user_data.get('sub'), - 'email': user_info.get('email'), - 'email_verified': user_info.get('email_verified'), - 'nickname': user_info.get('nickname'), - 'name': user_info.get('name'), - 'data': self.get_oauth_data(data), + "id": user_id, + "email": user_info.get("email"), + "name": user_info.get("name"), + "data": self.get_oauth_data(data), + "email_verified": user_info.get("email_verified"), } diff --git a/oidc/views.py b/oidc/views.py index 2e29811..661c701 100644 --- a/oidc/views.py +++ b/oidc/views.py @@ -4,14 +4,16 @@ from sentry.auth.view import AuthView, ConfigureView from sentry.utils import json - -from .constants import ( - ERR_INVALID_RESPONSE, - ISSUER -) from sentry.utils.signing import urlsafe_b64decode +from six.moves import map as _map + +from .constants import ERR_INVALID_RESPONSE, ISSUER + +logger = logging.getLogger("sentry.auth.oidc") + -logger = logging.getLogger('sentry.auth.oidc') +def map(a, b, *c): + return list(_map(a, b, *c)) class FetchUser(AuthView): @@ -21,83 +23,54 @@ def __init__(self, domains, version, *args, **kwargs): super(FetchUser, self).__init__(*args, **kwargs) def dispatch(self, request, helper): - data = helper.fetch_state('data') + data = helper.fetch_state("data") try: - id_token = data['id_token'] + id_token = data["id_token"] except KeyError: - logger.error('Missing id_token in OAuth response: %s' % data) + logger.error("Missing id_token in OAuth response: %s" % data) return helper.error(ERR_INVALID_RESPONSE) try: - _, payload, _ = map(urlsafe_b64decode, id_token.split('.', 2)) + _, payload, _ = map(urlsafe_b64decode, id_token.split(".", 2)) except Exception as exc: - logger.error(u'Unable to decode id_token: %s' % exc, exc_info=True) + logger.error(u"Unable to decode id_token: %s" % exc, exc_info=True) return helper.error(ERR_INVALID_RESPONSE) try: payload = json.loads(payload) except Exception as exc: - logger.error(u'Unable to decode id_token payload: %s' % exc, exc_info=True) + logger.error(u"Unable to decode id_token payload: %s" % exc, exc_info=True) return helper.error(ERR_INVALID_RESPONSE) - if not payload.get('sub'): - logger.error('Missing sub in id_token payload: %s' % id_token) + if not payload.get("email"): + logger.error("Missing email in id_token payload: %s" % id_token) return helper.error(ERR_INVALID_RESPONSE) - domain = payload.get('iss') + # support legacy style domains with pure domain regexp + if self.version is None: + domain = extract_domain(payload["email"]) + else: + domain = payload.get("hd") + + helper.bind_state("domain", domain) + helper.bind_state("user", payload) - helper.bind_state('domain', domain) - helper.bind_state('user', payload) return helper.next_step() class OIDCConfigureView(ConfigureView): def dispatch(self, request, organization, auth_provider): config = auth_provider.config - if config.get('domain'): - domains = [config['domain']] + if config.get("domain"): + domains = [config["domain"]] else: - domains = config.get('domains') - return self.render('oidc/configure.html', { - 'provider_name': ISSUER or "", - 'domains': domains or [] - }) + domains = config.get("domains") + return self.render( + "oidc/configure.html", + {"provider_name": ISSUER or "", "domains": domains or []}, + ) -class FetchDetailedUserInfo(AuthView): - def __init__(self, domains, version, *args, **kwargs): - self.domains = domains - self.version = version - super(FetchDetailedUserInfo, self).__init__(*args, **kwargs) - - def dispatch(self, request, helper): - data = helper.fetch_state('data') - - try: - id_token = data['id_token'] - except KeyError: - logger.error('Missing id_token in OAuth response: %s' % data) - return helper.error(ERR_INVALID_RESPONSE) - - try: - _, payload, _ = map(urlsafe_b64decode, id_token.split('.', 2)) - except Exception as exc: - logger.error(u'Unable to decode id_token: %s' % exc, exc_info=True) - return helper.error(ERR_INVALID_RESPONSE) - - try: - payload = json.loads(payload) - except Exception as exc: - logger.error(u'Unable to decode id_token payload: %s' % exc, exc_info=True) - return helper.error(ERR_INVALID_RESPONSE) - - if not payload.get('sub'): - logger.error('Missing sub in id_token payload: %s' % id_token) - return helper.error(ERR_INVALID_RESPONSE) - - domain = payload.get('iss') - - helper.bind_state('domain', domain) - helper.bind_state('user', payload) - return helper.next_step() +def extract_domain(email): + return email.rsplit("@", 1)[-1] diff --git a/setup.py b/setup.py index fe31140..a51910a 100644 --- a/setup.py +++ b/setup.py @@ -2,41 +2,39 @@ from __future__ import absolute_import from setuptools import setup, find_packages -with open('README.rst') as readme_file: +with open("README.rst") as readme_file: readme = readme_file.read() -install_requires = [ - 'requests<3.0.0' -] +install_requires = ["requests<3.0.0", "six>=1.15.0"] tests_require = [ - 'flake8<3.6.0,>=3.5.0', + "flake8<3.6.0,>=3.5.0", ] setup( - name='sentry-auth-oidc', - version='3.0.0', - author='Max Wittig', - author_email='max.wittig@siemens.com', - url='https://www.getsentry.com', - description='OpenID Connect authentication provider for Sentry', + name="sentry-auth-oidc", + version="4.0.0", + author="Max Wittig", + author_email="max.wittig@siemens.com", + url="https://www.getsentry.com", + description="OpenID Connect authentication provider for Sentry", long_description=readme, - license='Apache 2.0', - packages=find_packages(exclude=['tests']), + license="Apache 2.0", + packages=find_packages(exclude=["tests"]), zip_safe=False, install_requires=install_requires, tests_require=tests_require, - extras_require={'tests': tests_require}, + extras_require={"tests": tests_require}, include_package_data=True, entry_points={ - 'sentry.apps': [ - 'oidc = oidc.apps.Config', + "sentry.apps": [ + "oidc = oidc.apps.Config", ], }, classifiers=[ - 'Intended Audience :: Developers', - 'Intended Audience :: System Administrators', - 'Operating System :: OS Independent', - 'Topic :: Software Development', + "Intended Audience :: Developers", + "Intended Audience :: System Administrators", + "Operating System :: OS Independent", + "Topic :: Software Development", ], ) diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_provider.py b/tests/test_provider.py index efd0acb..c1ed982 100644 --- a/tests/test_provider.py +++ b/tests/test_provider.py @@ -13,13 +13,13 @@ class OIDCProviderTest(TestCase): def setUp(self): - self.user = self.create_user('foo@example.com') + self.user = self.create_user("foo@example.com") self.org = self.create_organization(owner=self.user) self.auth_provider = AuthProvider.objects.create( - provider='oidc', + provider="oidc", organization=self.org, ) - auth.register('oidc', OIDCProvider) + auth.register("oidc", OIDCProvider) super(OIDCProviderTest, self).setUp() def test_refresh_identity_without_refresh_token(self): @@ -27,8 +27,8 @@ def test_refresh_identity_without_refresh_token(self): auth_provider=self.auth_provider, user=self.user, data={ - 'access_token': 'access_token', - } + "access_token": "access_token", + }, ) provider = self.auth_provider.get_provider() @@ -38,39 +38,39 @@ def test_refresh_identity_without_refresh_token(self): def test_handles_multiple_domains(self): self.auth_provider.update( - config={'domains': ['example.com']}, + config={"domains": ["example.com"]}, ) provider = self.auth_provider.get_provider() - assert provider.domains == ['example.com'] + assert provider.domains == ["example.com"] def test_handles_legacy_single_domain(self): self.auth_provider.update( - config={'domain': 'example.com'}, + config={"domain": "example.com"}, ) provider = self.auth_provider.get_provider() - assert provider.domains == ['example.com'] + assert provider.domains == ["example.com"] def test_build_config(self): provider = self.auth_provider.get_provider() state = { - 'domain': 'example.com', - 'user': { - 'iss': 'accounts.google.com', - 'at_hash': 'HK6E_P6Dh8Y93mRNtsDB1Q', - 'email_verified': 'true', - 'sub': '10769150350006150715113082367', - 'azp': '1234987819200.apps.googleusercontent.com', - 'email': 'jsmith@example.com', - 'aud': '1234987819200.apps.googleusercontent.com', - 'iat': 1353601026, - 'exp': 1353604926, - 'hd': 'example.com' + "domain": "example.com", + "user": { + "iss": "accounts.google.com", + "at_hash": "HK6E_P6Dh8Y93mRNtsDB1Q", + "email_verified": "true", + "sub": "10769150350006150715113082367", + "azp": "1234987819200.apps.googleusercontent.com", + "email": "jsmith@example.com", + "aud": "1234987819200.apps.googleusercontent.com", + "iat": 1353601026, + "exp": 1353604926, + "hd": "example.com", }, } result = provider.build_config(state) assert result == { - 'domains': ['example.com'], - 'version': DATA_VERSION, + "domains": ["example.com"], + "version": DATA_VERSION, }