diff --git a/conf/gunicorn_local.py b/conf/gunicorn_local.py index bf312a5ef..a992326b7 100644 --- a/conf/gunicorn_local.py +++ b/conf/gunicorn_local.py @@ -2,8 +2,8 @@ import sys import os sys.path.append(os.path.join(os.path.dirname(__file__), "../")) -from util.log import logfile_path from Crypto import Random +from util.log import logfile_path logconfig = logfile_path(debug=True) diff --git a/conf/gunicorn_registry.py b/conf/gunicorn_registry.py index dce4c7ac8..b821d3d72 100644 --- a/conf/gunicorn_registry.py +++ b/conf/gunicorn_registry.py @@ -2,8 +2,8 @@ import sys import os sys.path.append(os.path.join(os.path.dirname(__file__), "../")) -from util.log import logfile_path from Crypto import Random +from util.log import logfile_path logconfig = logfile_path(debug=False) diff --git a/conf/gunicorn_secscan.py b/conf/gunicorn_secscan.py index 04b7cdce9..eea9afbf3 100644 --- a/conf/gunicorn_secscan.py +++ b/conf/gunicorn_secscan.py @@ -2,8 +2,8 @@ import sys import os sys.path.append(os.path.join(os.path.dirname(__file__), "../")) -from util.log import logfile_path from Crypto import Random +from util.log import logfile_path logconfig = logfile_path(debug=False) diff --git a/conf/gunicorn_verbs.py b/conf/gunicorn_verbs.py index 9f2c5aef1..fedf21cfb 100644 --- a/conf/gunicorn_verbs.py +++ b/conf/gunicorn_verbs.py @@ -2,9 +2,8 @@ import sys import os sys.path.append(os.path.join(os.path.dirname(__file__), "../")) -from util.log import logfile_path from Crypto import Random - +from util.log import logfile_path logconfig = logfile_path(debug=False) diff --git a/conf/gunicorn_web.py b/conf/gunicorn_web.py index 411ae1190..57e2eb17b 100644 --- a/conf/gunicorn_web.py +++ b/conf/gunicorn_web.py @@ -2,8 +2,8 @@ import sys import os sys.path.append(os.path.join(os.path.dirname(__file__), "../")) -from util.log import logfile_path from Crypto import Random +from util.log import logfile_path logconfig = logfile_path(debug=False) diff --git a/config.py b/config.py index 79f2bf8a0..2c5104664 100644 --- a/config.py +++ b/config.py @@ -23,7 +23,7 @@ CLIENT_WHITELIST = ['SERVER_HOSTNAME', 'PREFERRED_URL_SCHEME', 'MIXPANEL_KEY', 'CONTACT_INFO', 'AVATAR_KIND', 'LOCAL_OAUTH_HANDLER', 'DOCUMENTATION_LOCATION', 'DOCUMENTATION_METADATA', 'SETUP_COMPLETE', 'DEBUG', 'MARKETO_MUNCHKIN_ID', 'STATIC_SITE_BUCKET', 'RECAPTCHA_SITE_KEY', 'CHANNEL_COLORS', - 'TAG_EXPIRATION_OPTIONS'] + 'TAG_EXPIRATION_OPTIONS', 'INTERNAL_OIDC_SERVICE_ID'] def frontend_visible_config(config_dict): diff --git a/data/users/__init__.py b/data/users/__init__.py index 519947f8d..ccc9e7f7e 100644 --- a/data/users/__init__.py +++ b/data/users/__init__.py @@ -10,6 +10,7 @@ from data.users.database import DatabaseUsers from data.users.externalldap import LDAPUsers from data.users.externaljwt import ExternalJWTAuthN from data.users.keystone import get_keystone_users +from data.users.oidc import OIDCInternalAuth from util.security.aes import AESCipher logger = logging.getLogger(__name__) @@ -24,6 +25,12 @@ def get_federated_service_name(authentication_type): if authentication_type == 'Keystone': return 'keystone' + if authentication_type == 'OIDC': + return None + + if authentication_type == 'Database': + return None + raise Exception('Unknown auth type: %s' % authentication_type) @@ -74,8 +81,15 @@ def get_users_handler(config, _, override_config_dir): keystone_admin_password = config.get('KEYSTONE_ADMIN_PASSWORD') keystone_admin_tenant = config.get('KEYSTONE_ADMIN_TENANT') return get_keystone_users(auth_version, auth_url, keystone_admin_username, - keystone_admin_password, keystone_admin_tenant, timeout, - requires_email=features.MAILING) + keystone_admin_password, keystone_admin_tenant, timeout, + requires_email=features.MAILING) + + if authentication_type == 'OIDC': + if features.DIRECT_LOGIN: + raise Exception('Direct login feature must be disabled to use OIDC internal auth') + + login_service = config.get('INTERNAL_OIDC_SERVICE_ID') + return OIDCInternalAuth(config, login_service, requires_email=features.MAILING) raise RuntimeError('Unknown authentication type: %s' % authentication_type) @@ -160,6 +174,11 @@ class UserAuthentication(object): """ return self.state.federated_service + @property + def supports_encrypted_credentials(self): + """ Returns whether this auth system supports using encrypted credentials. """ + return self.state.supports_encrypted_credentials + def query_users(self, query, limit=20): """ Performs a lookup against the user system for the specified query. The returned tuple will be of the form (results, federated_login_id, err_msg). If the method is unsupported, diff --git a/data/users/database.py b/data/users/database.py index ce04b3c9d..09a8ccf7f 100644 --- a/data/users/database.py +++ b/data/users/database.py @@ -9,6 +9,10 @@ class DatabaseUsers(object): """ Always assumed to be working. If the DB is broken, other checks will handle it. """ return (True, None) + @property + def supports_encrypted_credentials(self): + return True + def verify_credentials(self, username_or_email, password): """ Simply delegate to the model implementation. """ result = model.user.verify_user(username_or_email, password) diff --git a/data/users/federated.py b/data/users/federated.py index f2b98ca82..7a79444e6 100644 --- a/data/users/federated.py +++ b/data/users/federated.py @@ -24,6 +24,10 @@ class FederatedUsers(object): def federated_service(self): return self._federated_service + @property + def supports_encrypted_credentials(self): + return True + def get_user(self, username_or_email): """ Retrieves the user with the given username or email, returning a tuple containing a UserInformation (if success) and the error message (on failure). diff --git a/data/users/oidc.py b/data/users/oidc.py new file mode 100644 index 000000000..5837a9e11 --- /dev/null +++ b/data/users/oidc.py @@ -0,0 +1,83 @@ +import logging + +from data import model +from oauth.loginmanager import OAuthLoginManager +from oauth.oidc import PublicKeyLoadException +from util.security.jwtutil import InvalidTokenError + + +logger = logging.getLogger(__name__) + + +class UnknownServiceException(Exception): + pass + + +class OIDCInternalAuth(object): + """ Handles authentication by delegating authentication to a signed OIDC JWT produced by the + configured OIDC service. + """ + def __init__(self, config, login_service_id, requires_email): + login_manager = OAuthLoginManager(config) + + self.login_service_id = login_service_id + self.login_service = login_manager.get_service(login_service_id) + if self.login_service is None: + raise UnknownServiceException('Unknown OIDC login service %s' % login_service_id) + + @property + def federated_service(self): + return None + + @property + def supports_encrypted_credentials(self): + # Since the "password" is already a signed JWT. + return False + + def verify_credentials(self, username_or_email, id_token): + # Parse the ID token. + try: + payload = self.login_service.decode_user_jwt(id_token) + except InvalidTokenError as ite: + logger.exception('Got invalid token error on OIDC decode: %s. Token: %s', ite.message, id_token) + return (None, 'Could not validate OIDC token') + except PublicKeyLoadException as pke: + logger.exception('Could not load public key during OIDC decode: %s. Token: %s', pke.message, id_token) + return (None, 'Could not validate OIDC token') + + # Find the user ID. + user_id = payload['sub'] + + # Lookup the federated login and user record with that matching ID and service. + user_found = model.user.verify_federated_login(self.login_service_id, user_id) + if user_found is None: + return (None, 'User does not exist') + + if not user_found.enabled: + return (None, 'User account is disabled. Please contact your administrator.') + + return (user_found, None) + + def verify_and_link_user(self, username_or_email, password): + return self.verify_credentials(username_or_email, password) + + def confirm_existing_user(self, username, password): + return self.verify_credentials(username, password) + + def link_user(self, username_or_email): + return (None, 'Unsupported for this authentication system') + + def get_and_link_federated_user_info(self, user_info): + return (None, 'Unsupported for this authentication system') + + def query_users(self, query, limit): + return (None, '', '') + + def check_group_lookup_args(self, group_lookup_args): + return (False, 'Not supported') + + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + return (None, 'Not supported') + + def service_metadata(self): + return {} diff --git a/data/users/test/test_oidc.py b/data/users/test/test_oidc.py new file mode 100644 index 000000000..23342316a --- /dev/null +++ b/data/users/test/test_oidc.py @@ -0,0 +1,38 @@ +import pytest + +from httmock import HTTMock + +from data import model +from data.users.oidc import OIDCInternalAuth +from oauth.test.test_oidc import * +from test.fixtures import * + +@pytest.mark.parametrize('username, expect_success', [ + ('devtable', True), + ('disabled', False) +]) +def test_oidc_login(username, expect_success, app_config, id_token, jwks_handler, + discovery_handler, app): + internal_auth = OIDCInternalAuth(app_config, 'someoidc', False) + with HTTMock(jwks_handler, discovery_handler): + # Try an invalid token. + (user, err) = internal_auth.verify_credentials('someusername', 'invalidtoken') + assert err is not None + assert user is None + + # Try a valid token for an unlinked user. + (user, err) = internal_auth.verify_credentials('someusername', id_token) + assert err is not None + assert user is None + + # Link the user to the service. + model.user.attach_federated_login(model.user.get_user(username), 'someoidc', 'cooluser') + + # Try a valid token for a linked user. + (user, err) = internal_auth.verify_credentials('someusername', id_token) + if expect_success: + assert err is None + assert user.username == username + else: + assert err is not None + assert user is None diff --git a/endpoints/api/suconfig.py b/endpoints/api/suconfig.py index dd90285bf..7221ac889 100644 --- a/endpoints/api/suconfig.py +++ b/endpoints/api/suconfig.py @@ -217,9 +217,10 @@ class SuperUserConfig(ApiResource): # Write the configuration changes to the config override file. config_provider.save_config(config_object) - # If the authentication system is not the database, link the superuser account to the + # If the authentication system is federated, link the superuser account to the # the authentication system chosen. - if config_object.get('AUTHENTICATION_TYPE', 'Database') != 'Database': + service_name = get_federated_service_name(config_object['AUTHENTICATION_TYPE']) + if service_name is not None: current_user = get_authenticated_user() if current_user is None: abort(401) diff --git a/endpoints/api/user.py b/endpoints/api/user.py index 4754ad905..aa9572808 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -529,6 +529,9 @@ class ClientKey(ApiResource): @validate_json_request('GenerateClientKey') def post(self): """ Return's the user's private client key. """ + if not authentication.supports_encrypted_credentials: + raise NotFound() + username = get_authenticated_user().username password = request.get_json()['password'] (result, error_message) = authentication.confirm_existing_user(username, password) @@ -744,7 +747,7 @@ class ExternalLoginInformation(ApiResource): 'kind': { 'type': 'string', 'description': 'The kind of URL', - 'enum': ['login', 'attach'], + 'enum': ['login', 'attach', 'cli'], }, }, }, @@ -762,7 +765,7 @@ class ExternalLoginInformation(ApiResource): csrf_token = generate_csrf_token(OAUTH_CSRF_TOKEN_NAME) kind = request.get_json()['kind'] - redirect_suffix = '/attach' if kind == 'attach' else '' + redirect_suffix = '' if kind == 'login' else '/' + kind try: login_scopes = login_service.get_login_scopes() diff --git a/endpoints/oauth/login.py b/endpoints/oauth/login.py index e6cfc0e75..54f0c7dc5 100644 --- a/endpoints/oauth/login.py +++ b/endpoints/oauth/login.py @@ -252,6 +252,24 @@ def _register_service(login_service): auth_url = login_service.get_auth_url(app.config, '', csrf_token, login_scopes) return redirect(auth_url) + @require_session_login + @oauthlogin_csrf_protect + def cli_token_func(): + # Check for a callback error. + error = request.args.get('error', None) + if error: + return _render_ologin_error(login_service.service_name(), error) + + # Exchange the OAuth code for the ID token. + code = request.args.get('code') + try: + idtoken, _ = login_service.exchange_code_for_tokens(app.config, client, code, '/cli') + except OAuthLoginException as ole: + return _render_ologin_error(login_service.service_name(), ole.message) + + user_obj = get_authenticated_user() + return redirect(url_for('web.user_view', path=user_obj.username, tab='settings', + idtoken=idtoken)) oauthlogin.add_url_rule('/%s/callback/captcha' % login_service.service_id(), '%s_oauth_captcha' % login_service.service_id(), @@ -268,6 +286,11 @@ def _register_service(login_service): attach_func, methods=['GET']) + oauthlogin.add_url_rule('/%s/callback/cli' % login_service.service_id(), + '%s_oauth_cli' % login_service.service_id(), + cli_token_func, + methods=['GET']) + # Register the routes for each of the login services. for current_service in oauth_login.services: _register_service(current_service) diff --git a/initdb.py b/initdb.py index c6b36e18e..2762751cf 100644 --- a/initdb.py +++ b/initdb.py @@ -267,6 +267,7 @@ def initialize_database(): LoginService.create(name='jwtauthn') LoginService.create(name='keystone') LoginService.create(name='dex') + LoginService.create(name='oidc') BuildTriggerService.create(name='github') BuildTriggerService.create(name='custom-git') diff --git a/oauth/base.py b/oauth/base.py index f50ae80d8..a699358bd 100644 --- a/oauth/base.py +++ b/oauth/base.py @@ -97,7 +97,6 @@ class OAuthService(object): def get_user_info(self, http_client, token): token_param = { - 'access_token': token, 'alt': 'json', } diff --git a/oauth/loginmanager.py b/oauth/loginmanager.py index 2504d661b..ea45890ea 100644 --- a/oauth/loginmanager.py +++ b/oauth/loginmanager.py @@ -1,7 +1,6 @@ from oauth.services.github import GithubOAuthService from oauth.services.google import GoogleOAuthService from oauth.oidc import OIDCLoginService -from data.users import UserAuthentication CUSTOM_LOGIN_SERVICES = { 'GITHUB_LOGIN_CONFIG': GithubOAuthService, diff --git a/oauth/oidc.py b/oauth/oidc.py index 2b6e7cbc6..2b7272652 100644 --- a/oauth/oidc.py +++ b/oauth/oidc.py @@ -39,7 +39,7 @@ class OIDCLoginService(OAuthService): 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 = client or config['HTTPCLIENT'] + self._http_client = client or config.get('HTTPCLIENT') self._mailing = config.get('FEATURE_MAILING', False) def service_id(self): @@ -89,7 +89,7 @@ class OIDCLoginService(OAuthService): 'OIDC': True, } - def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix): + def exchange_code_for_tokens(self, app_config, http_client, code, redirect_suffix): # Exchange the code for the access token and id_token try: json_data = self.exchange_code(app_config, http_client, code, @@ -109,9 +109,16 @@ class OIDCLoginService(OAuthService): logger.debug('Missing id_token in response: %s', json_data) raise OAuthLoginException('Missing `id_token` in OIDC response') + return id_token, access_token + + def exchange_code_for_login(self, app_config, http_client, code, redirect_suffix): + # Exchange the code for the access token and id_token + id_token, access_token = self.exchange_code_for_tokens(app_config, http_client, code, + redirect_suffix) + # Decode the id_token. try: - decoded_id_token = self._decode_user_jwt(id_token) + decoded_id_token = self.decode_user_jwt(id_token) except InvalidTokenError as ite: logger.exception('Got invalid token error on OIDC decode: %s', ite.message) raise OAuthLoginException('Could not decode OIDC token') @@ -181,7 +188,7 @@ class OIDCLoginService(OAuthService): logger.exception('Could not parse OIDC discovery for url: %s', discovery_url) raise DiscoveryFailureException("Could not parse OIDC discovery information") - def _decode_user_jwt(self, token): + def decode_user_jwt(self, token): """ Decodes the given JWT under the given provider and returns it. Raises an InvalidTokenError exception on an invalid token or a PublicKeyLoadException if the public key could not be loaded for decoding. diff --git a/oauth/test/__init__.py b/oauth/test/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/oauth/test/test_oidc.py b/oauth/test/test_oidc.py index 98e914638..b1ea38a83 100644 --- a/oauth/test/test_oidc.py +++ b/oauth/test/test_oidc.py @@ -60,7 +60,7 @@ def app_config(http_client, mailing_feature): 'SERVER_HOSTNAME': 'localhost', 'FEATURE_MAILING': mailing_feature, - 'SOMEOIDC_TEST_SERVICE': { + 'SOMEOIDC_LOGIN_CONFIG': { 'CLIENT_ID': 'foo', 'CLIENT_SECRET': 'bar', 'SERVICE_NAME': 'Some Cool Service', @@ -74,7 +74,7 @@ def app_config(http_client, mailing_feature): @pytest.fixture() def oidc_service(app_config): - return OIDCLoginService(app_config, 'SOMEOIDC_TEST_SERVICE') + return OIDCLoginService(app_config, 'SOMEOIDC_LOGIN_CONFIG') @pytest.fixture() def discovery_content(userinfo_supported): diff --git a/static/directives/config/config-setup-tool.html b/static/directives/config/config-setup-tool.html index 94c8eccd0..822737bf5 100644 --- a/static/directives/config/config-setup-tool.html +++ b/static/directives/config/config-setup-tool.html @@ -622,21 +622,23 @@

- Authentication for the registry can be handled by either the registry itself, LDAP or external JWT endpoint. + Authentication for the registry can be handled by either the registry itself, LDAP, Keystone, OIDC or external JWT endpoint.

Additional external authentication providers (such as GitHub) can be used in addition for login into the UI.

-
- It is highly recommended to require encrypted client passwords. External passwords used in the Docker client will be stored in plaintext! - Enable this requirement now. -
+
+
+ It is highly recommended to require encrypted client passwords. External passwords used in the Docker client will be stored in plaintext! + Enable this requirement now. +
-
- Note: The "Require Encrypted Client Passwords" feature is currently enabled which will - prevent passwords from being saved as plaintext by the Docker client. +
+ Note: The "Require Encrypted Client Passwords" feature is currently enabled which will + prevent passwords from being saved as plaintext by the Docker client. +
@@ -648,6 +650,7 @@ + @@ -687,6 +690,21 @@
+ + + + + + +
OIDC Provider: + +
+ An OIDC provider must be configured to use this authentication system +
+
+ @@ -1073,7 +1091,7 @@ (Delete)
-
+
Warning: This OIDC provider is not bound to your {{ config.AUTHENTICATION_TYPE }} authentication. Logging in via this provider will create a -only user, which is not the recommended approach. It is highly recommended to choose a "Binding Field" below.
@@ -1134,7 +1152,7 @@
- +
Binding Field: + + + + +
CLI Token: + +
+ + -
+

Docker CLI Password

The Docker CLI stores passwords entered on the command line in plaintext. It is therefore highly recommended to generate an an encrypted version of your password to use for docker login. @@ -216,4 +233,7 @@
+ + +
diff --git a/util/config/validator.py b/util/config/validator.py index dda3bd666..90582ccd1 100644 --- a/util/config/validator.py +++ b/util/config/validator.py @@ -22,6 +22,7 @@ from util.config.validators.validate_oidc import OIDCLoginValidator from util.config.validators.validate_timemachine import TimeMachineValidator from util.config.validators.validate_access import AccessSettingsValidator from util.config.validators.validate_actionlog_archiving import ActionLogArchivingValidator +from util.config.validators.validate_oidcauth import OIDCAuthValidator logger = logging.getLogger(__name__) @@ -59,6 +60,7 @@ VALIDATORS = { TimeMachineValidator.name: TimeMachineValidator.validate, AccessSettingsValidator.name: AccessSettingsValidator.validate, ActionLogArchivingValidator.name: ActionLogArchivingValidator.validate, + OIDCAuthValidator.name: OIDCAuthValidator.validate, } def validate_service_for_config(service, config, password=None): diff --git a/util/config/validators/test/test_validate_oidcauth.py b/util/config/validators/test/test_validate_oidcauth.py new file mode 100644 index 000000000..7c5609ccb --- /dev/null +++ b/util/config/validators/test/test_validate_oidcauth.py @@ -0,0 +1,32 @@ +import pytest + +from util.config.validators import ConfigValidationException +from util.config.validators.validate_oidcauth import OIDCAuthValidator + +from test.fixtures import * + +@pytest.mark.parametrize('unvalidated_config', [ + ({'AUTHENTICATION_TYPE': 'OIDC'}), + ({'AUTHENTICATION_TYPE': 'OIDC', 'INTERNAL_OIDC_SERVICE_ID': 'someservice'}), +]) +def test_validate_invalid_oidc_auth_config(unvalidated_config, app): + validator = OIDCAuthValidator() + + with pytest.raises(ConfigValidationException): + validator.validate(unvalidated_config, None, None) + + +def test_validate_oidc_auth(app): + config = { + 'AUTHENTICATION_TYPE': 'OIDC', + 'INTERNAL_OIDC_SERVICE_ID': 'someservice', + 'SOMESERVICE_LOGIN_CONFIG': { + 'CLIENT_ID': 'foo', + 'CLIENT_SECRET': 'bar', + 'OIDC_SERVER': 'http://someserver', + }, + 'HTTPCLIENT': None, + } + + validator = OIDCAuthValidator() + validator.validate(config, None, None) diff --git a/util/config/validators/validate_oidcauth.py b/util/config/validators/validate_oidcauth.py new file mode 100644 index 000000000..5d74fa23d --- /dev/null +++ b/util/config/validators/validate_oidcauth.py @@ -0,0 +1,21 @@ +from app import app +from data.users.oidc import OIDCInternalAuth, UnknownServiceException +from util.config.validators import BaseValidator, ConfigValidationException + +class OIDCAuthValidator(BaseValidator): + name = "oidc-auth" + + @classmethod + def validate(cls, config, user, user_password): + if config.get('AUTHENTICATION_TYPE', 'Database') != 'OIDC': + return + + login_service_id = config.get('INTERNAL_OIDC_SERVICE_ID') + if not login_service_id: + raise ConfigValidationException('Missing OIDC provider') + + # By instantiating the auth engine, it will check if the provider exists and works. + try: + OIDCInternalAuth(config, login_service_id, False) + except UnknownServiceException as use: + raise ConfigValidationException(use.message)