diff --git a/oauth/loginmanager.py b/oauth/loginmanager.py index 457fc2343..2504d661b 100644 --- a/oauth/loginmanager.py +++ b/oauth/loginmanager.py @@ -12,7 +12,7 @@ PREFIX_BLACKLIST = ['ldap', 'jwt', 'keystone'] class OAuthLoginManager(object): """ Helper class which manages all registered OAuth login services. """ - def __init__(self, config): + def __init__(self, config, client=None): self.services = [] # Register the endpoints for each of the OAuth login services. @@ -28,7 +28,7 @@ class OAuthLoginManager(object): if prefix in PREFIX_BLACKLIST: raise Exception('Cannot use reserved config name %s' % key) - self.services.append(OIDCLoginService(config, key)) + self.services.append(OIDCLoginService(config, key, client=client)) def get_service(self, service_id): for service in self.services: diff --git a/oauth/oidc.py b/oauth/oidc.py index f858bb1b3..f3dab2032 100644 --- a/oauth/oidc.py +++ b/oauth/oidc.py @@ -34,12 +34,12 @@ class PublicKeyLoadException(Exception): class OIDCLoginService(OAuthService): """ Defines a generic service for all OpenID-connect compatible login services. """ - def __init__(self, config, key_name): + def __init__(self, config, key_name, client=None): super(OIDCLoginService, self).__init__(config, key_name) self._public_key_cache = TTLCache(1, PUBLIC_KEY_CACHE_TTL, missing=self._load_public_key) self._id = key_name[0:key_name.find('_')].lower() - self._http_client = config['HTTPCLIENT'] + self._http_client = client or config['HTTPCLIENT'] self._mailing = config.get('FEATURE_MAILING', False) def service_id(self): @@ -71,6 +71,9 @@ class OIDCLoginService(OAuthService): def user_endpoint(self): return self._oidc_config().get('userinfo_endpoint') + def validate(self): + return bool(self.user_endpoint()) + def validate_client_id_and_secret(self, http_client, app_config): # TODO: find a way to verify client secret too. check_auth_url = http_client.get(self.get_auth_url()) diff --git a/util/config/validator.py b/util/config/validator.py index 9dd9246ae..ae3f02be8 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -18,6 +18,7 @@ from util.config.validators.validate_google_login import GoogleLoginValidator from util.config.validators.validate_bitbucket_trigger import BitbucketTriggerValidator from util.config.validators.validate_gitlab_trigger import GitLabTriggerValidator from util.config.validators.validate_github import GitHubLoginValidator, GitHubTriggerValidator +from util.config.validators.validate_oidc import OIDCLoginValidator logger = logging.getLogger(__name__) @@ -51,6 +52,7 @@ VALIDATORS = { SignerValidator.name: SignerValidator.validate, SecurityScannerValidator.name: SecurityScannerValidator.validate, BittorrentValidator.name: BittorrentValidator.validate, + OIDCLoginValidator.name: OIDCLoginValidator.validate, } def validate_service_for_config(service, config, password=None): diff --git a/util/config/validators/test/test_validate_oidc.py b/util/config/validators/test/test_validate_oidc.py new file mode 100644 index 000000000..0bc3c89a6 --- /dev/null +++ b/util/config/validators/test/test_validate_oidc.py @@ -0,0 +1,38 @@ +import json +import pytest + +from httmock import urlmatch, HTTMock + +from oauth.oidc import OIDC_WELLKNOWN +from util.config.validators import ConfigValidationException +from util.config.validators.validate_oidc import OIDCLoginValidator + +@pytest.mark.parametrize('unvalidated_config', [ + ({'SOMETHING_LOGIN_CONFIG': {}}), +]) +def test_validate_invalid_oidc_login_config(unvalidated_config): + validator = OIDCLoginValidator() + + with pytest.raises(ConfigValidationException): + validator.validate(unvalidated_config, None, None) + +def test_validate_oidc_login(): + url_hit = [False] + @urlmatch(netloc=r'someserver', path=r'/\.well-known/openid-configuration') + def handler(_, __): + url_hit[0] = True + data = { + 'userinfo_endpoint': 'foobar', + } + return {'status_code': 200, 'content': json.dumps(data)} + + with HTTMock(handler): + validator = OIDCLoginValidator() + validator.validate({ + 'SOMETHING_LOGIN_CONFIG': { + 'OIDC_SERVER': 'http://someserver', + 'DEBUGGING': True, # Allows for HTTP. + }, + }, None, None) + + assert url_hit[0] diff --git a/util/config/validators/validate_oidc.py b/util/config/validators/validate_oidc.py new file mode 100644 index 000000000..84e626b46 --- /dev/null +++ b/util/config/validators/validate_oidc.py @@ -0,0 +1,27 @@ +from app import app +from oauth.loginmanager import OAuthLoginManager +from oauth.oidc import OIDCLoginService, DiscoveryFailureException +from util.config.validators import BaseValidator, ConfigValidationException + +class OIDCLoginValidator(BaseValidator): + name = "oidc-login" + + @classmethod + def validate(cls, config, user, user_password): + client = app.config['HTTPCLIENT'] + login_manager = OAuthLoginManager(config, client=client) + for service in login_manager.services: + if not isinstance(service, OIDCLoginService): + continue + + if service.config.get('OIDC_SERVER') is None: + msg = 'Missing OIDC_SERVER on OIDC service %s' % service.service_id() + raise ConfigValidationException(msg) + + try: + if not service.validate(): + msg = 'Could not validate OIDC service %s' % service.service_id() + raise ConfigValidationException(msg) + except DiscoveryFailureException as dfe: + msg = 'Could not validate OIDC service %s: %s' % (service.service_id(), dfe.message) + raise ConfigValidationException(msg)