Merge pull request #2899 from coreos-inc/joseph.schorr/QS-36/appr-auth-improvement

Allow app registry to use robots and tokens to login

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,8 @@ 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)
+  result, _ = validate_credentials(auth_username, auth_password_or_token)
+  return result.with_kind(AuthKind.basic)
 
 
 def _parse_basic_auth_header(auth):

auth/credentials.py

@@ -0,0 +1,62 @@
+import logging
+
+from enum import Enum
+
+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'
+
+
+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.
+  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), 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'),
+              CredentialKind.token)
+
+  # Check for OAuth tokens.
+  if auth_username == OAUTH_TOKEN_USERNAME:
+    return validate_oauth_token(auth_password_or_token), CredentialKind.oauth_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), 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), 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), CredentialKind.user
+  else:
+    logger.warning('Failed to validate basic auth for user %s: %s', auth_username, err)
+    return ValidateResult(AuthKind.credentials, error_message=err), CredentialKind.user

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)

auth/test/test_credentials.py

@@ -0,0 +1,36 @@
+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, 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, 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, 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, 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, kind = validate_credentials('devtable', 'somepassword')
+  assert kind == CredentialKind.user
+  assert result == ValidateResult(AuthKind.credentials,
+                                  error_message='Invalid Username or Password')

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. """

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

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,32 @@ 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')
+    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')
 
-  (verified, error_message) = authentication.verify_and_link_user(username, password,
-                                                                  basic_auth=True)
-  if verified:
+    # Default case: Just fail.
+    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
-  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')
+
+  return success
 
 
 @v1_bp.route('/users', methods=['GET'])