From 6f3d9a6fce890c6cb49b4b5b00845a4099dff1d3 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 27 Oct 2017 14:05:53 -0400 Subject: [PATCH 1/4] Extract credential handling into its own module Will be used in Docker V1 and APPR protocols --- auth/basic.py | 48 ++------------------------------ auth/credentials.py | 52 +++++++++++++++++++++++++++++++++++ auth/test/test_basic.py | 7 +++-- auth/test/test_credentials.py | 30 ++++++++++++++++++++ auth/validateresult.py | 6 ++++ 5 files changed, 94 insertions(+), 49 deletions(-) create mode 100644 auth/credentials.py create mode 100644 auth/test/test_credentials.py diff --git a/auth/basic.py b/auth/basic.py index a6c621da6..d551e957e 100644 --- a/auth/basic.py +++ b/auth/basic.py @@ -3,18 +3,11 @@ import logging from base64 import b64decode from flask import request -from app import authentication -from auth.oauth import validate_oauth_token +from auth.credentials import validate_credentials from auth.validateresult import ValidateResult, AuthKind -from data import model -from util.names import parse_robot_username logger = logging.getLogger(__name__) -ACCESS_TOKEN_USERNAME = '$token' -OAUTH_TOKEN_USERNAME = '$oauthtoken' - - def has_basic_auth(username): """ Returns true if a basic auth header exists with a username and password pair that validates against the internal authentication system. Returns True on full success and False on any @@ -41,44 +34,7 @@ def validate_basic_auth(auth_header): return ValidateResult(AuthKind.basic, missing=True) auth_username, auth_password_or_token = credentials - - # Check for access tokens. - if auth_username == ACCESS_TOKEN_USERNAME: - logger.debug('Found basic auth header for access token') - try: - token = model.token.load_token_data(auth_password_or_token) - logger.debug('Successfully validated basic auth for access token %s', token.id) - return ValidateResult(AuthKind.basic, token=token) - except model.DataModelException: - logger.warning('Failed to validate basic auth for access token %s', auth_password_or_token) - return ValidateResult(AuthKind.basic, error_message='Invalid access token') - - # Check for OAuth tokens. - if auth_username == OAUTH_TOKEN_USERNAME: - return validate_oauth_token(auth_password_or_token) - - # Check for robots and users. - is_robot = parse_robot_username(auth_username) - if is_robot: - logger.debug('Found basic auth header for robot %s', auth_username) - try: - robot = model.user.verify_robot(auth_username, auth_password_or_token) - - logger.debug('Successfully validated basic auth for robot %s', auth_username) - return ValidateResult(AuthKind.basic, robot=robot) - except model.InvalidRobotException as ire: - logger.warning('Failed to validate basic auth for robot %s: %s', auth_username, ire.message) - return ValidateResult(AuthKind.basic, error_message=ire.message) - - # Otherwise, treat as a standard user. - (authenticated, err) = authentication.verify_and_link_user(auth_username, auth_password_or_token, - basic_auth=True) - if authenticated: - logger.debug('Successfully validated basic auth for user %s', authenticated.username) - return ValidateResult(AuthKind.basic, user=authenticated) - else: - logger.warning('Failed to validate basic auth for user %s: %s', auth_username, err) - return ValidateResult(AuthKind.basic, error_message=err) + return validate_credentials(auth_username, auth_password_or_token).with_kind(AuthKind.basic) def _parse_basic_auth_header(auth): diff --git a/auth/credentials.py b/auth/credentials.py new file mode 100644 index 000000000..daff7eaec --- /dev/null +++ b/auth/credentials.py @@ -0,0 +1,52 @@ +import logging + +from app import authentication +from auth.oauth import validate_oauth_token +from auth.validateresult import ValidateResult, AuthKind +from data import model +from util.names import parse_robot_username + +logger = logging.getLogger(__name__) + +ACCESS_TOKEN_USERNAME = '$token' +OAUTH_TOKEN_USERNAME = '$oauthtoken' + +def validate_credentials(auth_username, auth_password_or_token): + """ Validates a pair of auth username and password/token credentials. """ + # Check for access tokens. + if auth_username == ACCESS_TOKEN_USERNAME: + logger.debug('Found basic auth header for access token') + try: + token = model.token.load_token_data(auth_password_or_token) + logger.debug('Successfully validated basic auth for access token %s', token.id) + return ValidateResult(AuthKind.credentials, token=token) + except model.DataModelException: + logger.warning('Failed to validate basic auth for access token %s', auth_password_or_token) + return ValidateResult(AuthKind.credentials, error_message='Invalid access token') + + # Check for OAuth tokens. + if auth_username == OAUTH_TOKEN_USERNAME: + return validate_oauth_token(auth_password_or_token) + + # Check for robots and users. + is_robot = parse_robot_username(auth_username) + if is_robot: + logger.debug('Found basic auth header for robot %s', auth_username) + try: + robot = model.user.verify_robot(auth_username, auth_password_or_token) + + logger.debug('Successfully validated basic auth for robot %s', auth_username) + return ValidateResult(AuthKind.credentials, robot=robot) + except model.InvalidRobotException as ire: + logger.warning('Failed to validate basic auth for robot %s: %s', auth_username, ire.message) + return ValidateResult(AuthKind.credentials, error_message=ire.message) + + # Otherwise, treat as a standard user. + (authenticated, err) = authentication.verify_and_link_user(auth_username, auth_password_or_token, + basic_auth=True) + if authenticated: + logger.debug('Successfully validated basic auth for user %s', authenticated.username) + return ValidateResult(AuthKind.credentials, user=authenticated) + else: + logger.warning('Failed to validate basic auth for user %s: %s', auth_username, err) + return ValidateResult(AuthKind.credentials, error_message=err) diff --git a/auth/test/test_basic.py b/auth/test/test_basic.py index 042901bb1..54e4db8a4 100644 --- a/auth/test/test_basic.py +++ b/auth/test/test_basic.py @@ -2,7 +2,8 @@ import pytest from base64 import b64encode -from auth.basic import validate_basic_auth, ACCESS_TOKEN_USERNAME, OAUTH_TOKEN_USERNAME +from auth.basic import validate_basic_auth +from auth.credentials import ACCESS_TOKEN_USERNAME, OAUTH_TOKEN_USERNAME from auth.validateresult import AuthKind, ValidateResult from data import model @@ -23,7 +24,7 @@ def _token(username, password): (_token(ACCESS_TOKEN_USERNAME, 'invalid'), ValidateResult(AuthKind.basic, error_message='Invalid access token')), (_token(OAUTH_TOKEN_USERNAME, 'invalid'), - ValidateResult(AuthKind.oauth, error_message='OAuth access token could not be validated')), + ValidateResult(AuthKind.basic, error_message='OAuth access token could not be validated')), (_token('devtable', 'invalid'), ValidateResult(AuthKind.basic, error_message='Invalid Username or Password')), (_token('devtable+somebot', 'invalid'), ValidateResult( @@ -62,4 +63,4 @@ def test_valid_oauth(app): oauth_token = list(model.oauth.list_access_tokens_for_user(user))[0] token = _token(OAUTH_TOKEN_USERNAME, oauth_token.access_token) result = validate_basic_auth(token) - assert result == ValidateResult(AuthKind.oauth, oauthtoken=oauth_token) + assert result == ValidateResult(AuthKind.basic, oauthtoken=oauth_token) diff --git a/auth/test/test_credentials.py b/auth/test/test_credentials.py new file mode 100644 index 000000000..b4108f167 --- /dev/null +++ b/auth/test/test_credentials.py @@ -0,0 +1,30 @@ +from auth.credentials import ACCESS_TOKEN_USERNAME, OAUTH_TOKEN_USERNAME, validate_credentials +from auth.validateresult import AuthKind, ValidateResult +from data import model + +from test.fixtures import * + +def test_valid_user(app): + result = validate_credentials('devtable', 'password') + assert result == ValidateResult(AuthKind.credentials, user=model.user.get_user('devtable')) + +def test_valid_robot(app): + robot, password = model.user.create_robot('somerobot', model.user.get_user('devtable')) + result = validate_credentials(robot.username, password) + assert result == ValidateResult(AuthKind.credentials, robot=robot) + +def test_valid_token(app): + access_token = model.token.create_delegate_token('devtable', 'simple', 'sometoken') + result = validate_credentials(ACCESS_TOKEN_USERNAME, access_token.code) + assert result == ValidateResult(AuthKind.credentials, token=access_token) + +def test_valid_oauth(app): + user = model.user.get_user('devtable') + oauth_token = list(model.oauth.list_access_tokens_for_user(user))[0] + result = validate_credentials(OAUTH_TOKEN_USERNAME, oauth_token.access_token) + assert result == ValidateResult(AuthKind.oauth, oauthtoken=oauth_token) + +def test_invalid_user(app): + result = validate_credentials('devtable', 'somepassword') + assert result == ValidateResult(AuthKind.credentials, + error_message='Invalid Username or Password') diff --git a/auth/validateresult.py b/auth/validateresult.py index f6d08a9a8..d285a2c88 100644 --- a/auth/validateresult.py +++ b/auth/validateresult.py @@ -13,6 +13,7 @@ class AuthKind(Enum): basic = 'basic' oauth = 'oauth' signed_grant = 'signed_grant' + credentials = 'credentials' class ValidateResult(object): @@ -56,6 +57,11 @@ class ValidateResult(object): if self.identity: identity_changed.send(app, identity=self.identity) + def with_kind(self, kind): + """ Returns a copy of this result, but with the kind replaced. """ + return ValidateResult(kind, self.missing, self.user, self.token, self.oauthtoken, self.robot, + self.signed_data, self.error_message) + @property def authed_user(self): """ Returns the authenticated user, whether directly, or via an OAuth token. """ From 0bcda90c6e01e6883899bb493f10459ddb6f7051 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 27 Oct 2017 14:11:19 -0400 Subject: [PATCH 2/4] Add kind to credentials validate call --- auth/basic.py | 3 ++- auth/credentials.py | 25 +++++++++++++++++-------- auth/test/test_credentials.py | 18 ++++++++++++------ 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/auth/basic.py b/auth/basic.py index d551e957e..dfb945acf 100644 --- a/auth/basic.py +++ b/auth/basic.py @@ -34,7 +34,8 @@ def validate_basic_auth(auth_header): return ValidateResult(AuthKind.basic, missing=True) auth_username, auth_password_or_token = credentials - return validate_credentials(auth_username, auth_password_or_token).with_kind(AuthKind.basic) + result, _ = validate_credentials(auth_username, auth_password_or_token) + return result.with_kind(AuthKind.basic) def _parse_basic_auth_header(auth): diff --git a/auth/credentials.py b/auth/credentials.py index daff7eaec..e1e6b5bf0 100644 --- a/auth/credentials.py +++ b/auth/credentials.py @@ -1,5 +1,7 @@ import logging +from enum import Enum + from app import authentication from auth.oauth import validate_oauth_token from auth.validateresult import ValidateResult, AuthKind @@ -11,6 +13,13 @@ logger = logging.getLogger(__name__) ACCESS_TOKEN_USERNAME = '$token' OAUTH_TOKEN_USERNAME = '$oauthtoken' +class CredentialKind(Enum): + user = 'user' + robot = 'robot' + token = ACCESS_TOKEN_USERNAME + oauth_token = OAUTH_TOKEN_USERNAME + + def validate_credentials(auth_username, auth_password_or_token): """ Validates a pair of auth username and password/token credentials. """ # Check for access tokens. @@ -19,14 +28,15 @@ def validate_credentials(auth_username, auth_password_or_token): try: token = model.token.load_token_data(auth_password_or_token) logger.debug('Successfully validated basic auth for access token %s', token.id) - return ValidateResult(AuthKind.credentials, token=token) + return ValidateResult(AuthKind.credentials, token=token), CredentialKind.token except model.DataModelException: logger.warning('Failed to validate basic auth for access token %s', auth_password_or_token) - return ValidateResult(AuthKind.credentials, error_message='Invalid access token') + return (ValidateResult(AuthKind.credentials, error_message='Invalid access token'), + CredentialKind.token) # Check for OAuth tokens. if auth_username == OAUTH_TOKEN_USERNAME: - return validate_oauth_token(auth_password_or_token) + return validate_oauth_token(auth_password_or_token), CredentialKind.oauth_token # Check for robots and users. is_robot = parse_robot_username(auth_username) @@ -34,19 +44,18 @@ def validate_credentials(auth_username, auth_password_or_token): logger.debug('Found basic auth header for robot %s', auth_username) try: robot = model.user.verify_robot(auth_username, auth_password_or_token) - logger.debug('Successfully validated basic auth for robot %s', auth_username) - return ValidateResult(AuthKind.credentials, robot=robot) + return ValidateResult(AuthKind.credentials, robot=robot), CredentialKind.robot except model.InvalidRobotException as ire: logger.warning('Failed to validate basic auth for robot %s: %s', auth_username, ire.message) - return ValidateResult(AuthKind.credentials, error_message=ire.message) + return ValidateResult(AuthKind.credentials, error_message=ire.message), CredentialKind.robot # Otherwise, treat as a standard user. (authenticated, err) = authentication.verify_and_link_user(auth_username, auth_password_or_token, basic_auth=True) if authenticated: logger.debug('Successfully validated basic auth for user %s', authenticated.username) - return ValidateResult(AuthKind.credentials, user=authenticated) + return ValidateResult(AuthKind.credentials, user=authenticated), CredentialKind.user else: logger.warning('Failed to validate basic auth for user %s: %s', auth_username, err) - return ValidateResult(AuthKind.credentials, error_message=err) + return ValidateResult(AuthKind.credentials, error_message=err), CredentialKind.user diff --git a/auth/test/test_credentials.py b/auth/test/test_credentials.py index b4108f167..78438b143 100644 --- a/auth/test/test_credentials.py +++ b/auth/test/test_credentials.py @@ -1,30 +1,36 @@ -from auth.credentials import ACCESS_TOKEN_USERNAME, OAUTH_TOKEN_USERNAME, validate_credentials +from auth.credentials import (ACCESS_TOKEN_USERNAME, OAUTH_TOKEN_USERNAME, validate_credentials, + CredentialKind) from auth.validateresult import AuthKind, ValidateResult from data import model from test.fixtures import * def test_valid_user(app): - result = validate_credentials('devtable', 'password') + result, kind = validate_credentials('devtable', 'password') + assert kind == CredentialKind.user assert result == ValidateResult(AuthKind.credentials, user=model.user.get_user('devtable')) def test_valid_robot(app): robot, password = model.user.create_robot('somerobot', model.user.get_user('devtable')) - result = validate_credentials(robot.username, password) + result, kind = validate_credentials(robot.username, password) + assert kind == CredentialKind.robot assert result == ValidateResult(AuthKind.credentials, robot=robot) def test_valid_token(app): access_token = model.token.create_delegate_token('devtable', 'simple', 'sometoken') - result = validate_credentials(ACCESS_TOKEN_USERNAME, access_token.code) + result, kind = validate_credentials(ACCESS_TOKEN_USERNAME, access_token.code) + assert kind == CredentialKind.token assert result == ValidateResult(AuthKind.credentials, token=access_token) def test_valid_oauth(app): user = model.user.get_user('devtable') oauth_token = list(model.oauth.list_access_tokens_for_user(user))[0] - result = validate_credentials(OAUTH_TOKEN_USERNAME, oauth_token.access_token) + result, kind = validate_credentials(OAUTH_TOKEN_USERNAME, oauth_token.access_token) + assert kind == CredentialKind.oauth_token assert result == ValidateResult(AuthKind.oauth, oauthtoken=oauth_token) def test_invalid_user(app): - result = validate_credentials('devtable', 'somepassword') + result, kind = validate_credentials('devtable', 'somepassword') + assert kind == CredentialKind.user assert result == ValidateResult(AuthKind.credentials, error_message='Invalid Username or Password') From aa49b37ad2117461ff1160d0d5e90a33a428cabf Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 27 Oct 2017 14:37:23 -0400 Subject: [PATCH 3/4] Change Docker V1 index to use verify_credentials --- endpoints/v1/index.py | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index 07aa00e67..30aa72235 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -8,6 +8,7 @@ from flask import request, make_response, jsonify, session from app import authentication, userevents, metric_queue from auth.auth_context import get_authenticated_user, get_validated_token, get_validated_oauth_token +from auth.credentials import validate_credentials, CredentialKind from auth.decorators import process_auth from auth.permissions import ( ModifyRepositoryPermission, UserAdminPermission, ReadRepositoryPermission, @@ -84,34 +85,28 @@ def create_user(): # UGH! we have to use this response when the login actually worked, in order # to get the CLI to try again with a get, and then tell us login succeeded. success = make_response('"Username or email already exists"', 400) + result, kind = validate_credentials(username, password) + if not result.auth_valid: + if kind == CredentialKind.token: + abort(400, 'Invalid access token.', issue='invalid-access-token') - if username == '$token': - if model.load_token(password): - return success - abort(400, 'Invalid access token.', issue='invalid-access-token') + if kind == CredentialKind.robot: + abort(400, 'Invalid robot account or password.', issue='robot-login-failure') - elif username == '$oauthtoken': - if model.validate_oauth_token(password): - return success - abort(400, 'Invalid oauth access token.', issue='invalid-oauth-access-token') + if kind == CredentialKind.oauth_token: + abort(400, 'Invalid oauth access token.', issue='invalid-oauth-access-token') - elif '+' in username: - if model.verify_robot(username, password): - return success - abort(400, 'Invalid robot account or password.', issue='robot-login-failure') - - (verified, error_message) = authentication.verify_and_link_user(username, password, - basic_auth=True) - if verified: - # Mark that the user was logged in. - event = userevents.get_event(username) - event.publish_event_data('docker-cli', {'action': 'login'}) - return success - else: # Mark that the login failed. event = userevents.get_event(username) event.publish_event_data('docker-cli', {'action': 'loginfailure'}) - abort(400, error_message, issue='login-failure') + abort(400, result.error_message, issue='login-failure') + + if result.has_user: + # Mark that the user was logged in. + event = userevents.get_event(username) + event.publish_event_data('docker-cli', {'action': 'login'}) + + return success @v1_bp.route('/users', methods=['GET']) From 3bf8973fd9f76e66601affb443ea9c0fc4a81724 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 27 Oct 2017 14:55:49 -0400 Subject: [PATCH 4/4] Change app registry to use the credentials verification system Allows for tokens, OAuth tokens and robot accounts to be used as well Fixes https://jira.prod.coreos.systems/browse/QS-36 --- auth/credentials.py | 1 + endpoints/appr/registry.py | 9 +++++---- endpoints/v1/index.py | 10 +++++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/auth/credentials.py b/auth/credentials.py index e1e6b5bf0..4b15a2cbe 100644 --- a/auth/credentials.py +++ b/auth/credentials.py @@ -13,6 +13,7 @@ logger = logging.getLogger(__name__) ACCESS_TOKEN_USERNAME = '$token' OAUTH_TOKEN_USERNAME = '$oauthtoken' + class CredentialKind(Enum): user = 'user' robot = 'robot' diff --git a/endpoints/appr/registry.py b/endpoints/appr/registry.py index 8d7e988b3..4c73fcffb 100644 --- a/endpoints/appr/registry.py +++ b/endpoints/appr/registry.py @@ -11,6 +11,7 @@ from cnr.exception import ( from flask import jsonify, request from auth.auth_context import get_authenticated_user +from auth.credentials import validate_credentials from auth.decorators import process_auth from auth.permissions import CreateRepositoryPermission, ModifyRepositoryPermission from endpoints.appr import appr_bp, require_app_repo_read, require_app_repo_write @@ -56,11 +57,11 @@ def login(): if not username or not password: raise InvalidUsage('Missing username or password') - user, err = User.get_user(username, password) - if err is not None: - raise UnauthorizedAccess(err) + result, _ = validate_credentials(username, password) + if not result.auth_valid: + raise UnauthorizedAccess(result.error_message) - return jsonify({'token': "basic " + b64encode("%s:%s" % (user.username, password))}) + return jsonify({'token': "basic " + b64encode("%s:%s" % (username, password))}) # @TODO: Redirect to S3 url diff --git a/endpoints/v1/index.py b/endpoints/v1/index.py index 30aa72235..42e1b26d0 100644 --- a/endpoints/v1/index.py +++ b/endpoints/v1/index.py @@ -96,9 +96,13 @@ def create_user(): if kind == CredentialKind.oauth_token: abort(400, 'Invalid oauth access token.', issue='invalid-oauth-access-token') - # Mark that the login failed. - event = userevents.get_event(username) - event.publish_event_data('docker-cli', {'action': 'loginfailure'}) + if kind == CredentialKind.user: + # Mark that the login failed. + event = userevents.get_event(username) + event.publish_event_data('docker-cli', {'action': 'loginfailure'}) + abort(400, result.error_message, issue='login-failure') + + # Default case: Just fail. abort(400, result.error_message, issue='login-failure') if result.has_user: