From 3b7b12085d5b4297e177c5335c30c4d666bfe381 Mon Sep 17 00:00:00 2001 From: jakedt Date: Wed, 19 Mar 2014 18:09:09 -0400 Subject: [PATCH] User scope objects everywhere. Switch scope objects to namedtuples. Pass the user when validating whether the user has authorized such scopes in the past. Make sure we calculate the scope string using all user scopes form all previously granted tokens. --- auth/permissions.py | 18 ++++--- auth/scopes.py | 106 ++++++++++++++++++++++--------------- data/model/oauth.py | 40 +++++++------- endpoints/api/__init__.py | 4 +- endpoints/api/discovery.py | 8 ++- endpoints/web.py | 3 +- 6 files changed, 103 insertions(+), 76 deletions(-) diff --git a/auth/permissions.py b/auth/permissions.py index 15512faa5..2e8ded91e 100644 --- a/auth/permissions.py +++ b/auth/permissions.py @@ -4,6 +4,8 @@ from flask.ext.principal import identity_loaded, Permission, Identity, identity_ from collections import namedtuple, defaultdict from functools import partial +import scopes + from data import model from app import app @@ -31,22 +33,22 @@ TEAM_REPO_ROLES = { SCOPE_MAX_REPO_ROLES = defaultdict(lambda: None) SCOPE_MAX_REPO_ROLES.update({ - 'repo:read': 'read', - 'repo:write': 'write', - 'repo:admin': 'admin', - 'direct_user_login': 'admin', + scopes.READ_REPO: 'read', + scopes.WRITE_REPO: 'write', + scopes.ADMIN_REPO: 'admin', + scopes.DIRECT_LOGIN: 'admin', }) SCOPE_MAX_TEAM_ROLES = defaultdict(lambda: None) SCOPE_MAX_TEAM_ROLES.update({ - 'repo:create': 'creator', - 'direct_user_login': 'admin', + scopes.CREATE_REPO: 'creator', + scopes.DIRECT_LOGIN: 'admin', }) SCOPE_MAX_USER_ROLES = defaultdict(lambda: None) SCOPE_MAX_USER_ROLES.update({ - 'user:read': 'admin', - 'direct_user_login': 'admin', + scopes.READ_USER: 'read', + scopes.DIRECT_LOGIN: 'admin', }) diff --git a/auth/scopes.py b/auth/scopes.py index b6f66ce8e..aad91182b 100644 --- a/auth/scopes.py +++ b/auth/scopes.py @@ -1,49 +1,65 @@ -READ_REPO = { - 'scope': 'repo:read', - 'icon': 'fa-hdd-o', - 'title': 'View all visible repositories', - 'description': ('This application will be able to view and pull all repositories visible to the ' - 'granting user or robot account') -} +from collections import namedtuple -WRITE_REPO = { - 'scope': 'repo:write', - 'icon': 'fa-hdd-o', - 'title': 'Read/Write to any accessible repositories', - 'description': ('This application will be able to view, push and pull to all repositories to ' - 'which the granting user or robot account has write access') -} -ADMIN_REPO = { - 'scope': 'repo:admin', - 'icon': 'fa-hdd-o', - 'title': 'Administer Repositories', - 'description': ('This application will have administrator access to all repositories to which ' - 'the granting user or robot account has access') -} +Scope = namedtuple('scope', ['scope', 'icon', 'title', 'description']) -CREATE_REPO = { - 'scope': 'repo:create', - 'icon': 'fa-plus', - 'title': 'Create Repositories', - 'description': ('This application will be able to create repositories in to any namespaces that ' - 'the granting user or robot account is allowed to create repositories') -} -READ_USER = { - 'scope': 'user:read', - 'icon': 'fa-user', - 'title': 'Read User Information', - 'description': ('This application will be able to read user information such as username and ' - 'email address.'), -} +READ_REPO = Scope(scope='repo:read', + icon='fa-hdd-o', + title='View all visible repositories', + description=('This application will be able to view and pull all repositories ' + 'visible to the granting user or robot account')) -ALL_SCOPES = {scope['scope']:scope for scope in (READ_REPO, WRITE_REPO, ADMIN_REPO, CREATE_REPO, - READ_USER)} +WRITE_REPO = Scope(scope='repo:write', + icon='fa-hdd-o', + title='Read/Write to any accessible repositories', + description=('This application will be able to view, push and pull to all ' + 'repositories to which the granting user or robot account has ' + 'write access')) + +ADMIN_REPO = Scope(scope='repo:admin', + icon='fa-hdd-o', + title='Administer Repositories', + description=('This application will have administrator access to all ' + 'repositories to which the granting user or robot account has ' + 'access')) + +CREATE_REPO = Scope(scope='repo:create', + icon='fa-plus', + title='Create Repositories', + description=('This application will be able to create repositories in to any ' + 'namespaces that the granting user or robot account is allowed to ' + 'create repositories')) + +READ_USER = Scope(scope= 'user:read', + icon='fa-user', + title='Read User Information', + description=('This application will be able to read user information such as ' + 'username and email address.')) + + +DIRECT_LOGIN = Scope(scope='direct_user_login', + icon='fa-exclamation-triangle', + title='Full Access', + description=('This scope should not be available to OAuth applications. ' + 'Never approve a request for this scope!')) + + +ALL_SCOPES = {scope.scope:scope for scope in (READ_REPO, WRITE_REPO, ADMIN_REPO, CREATE_REPO, + READ_USER)} + +IMPLIED_SCOPES = { + ADMIN_REPO: {ADMIN_REPO, WRITE_REPO, READ_REPO}, + WRITE_REPO: {WRITE_REPO, READ_REPO}, + READ_REPO: {READ_REPO}, + CREATE_REPO: {CREATE_REPO}, + READ_USER: {READ_USER}, + None: set(), +} def scopes_from_scope_string(scopes): - return {ALL_SCOPES.get(scope, {}).get('scope', None) for scope in scopes.split(',')} + return {ALL_SCOPES.get(scope, None) for scope in scopes.split(',')} def validate_scope_string(scopes): @@ -56,8 +72,10 @@ def is_subset_string(full_string, expected_string): in full_string. """ full_scopes = scopes_from_scope_string(full_string) + full_implied_scopes = set.union(*[IMPLIED_SCOPES[scope] for scope in full_scopes]) expected_scopes = scopes_from_scope_string(expected_string) - return expected_scopes.issubset(full_scopes) + return expected_scopes.issubset(full_implied_scopes) + def get_scope_information(scopes_string): scopes = scopes_from_scope_string(scopes_string) @@ -65,10 +83,10 @@ def get_scope_information(scopes_string): for scope in scopes: if scope: scope_info.append({ - 'title': ALL_SCOPES[scope]['title'], - 'scope': ALL_SCOPES[scope]['scope'], - 'description': ALL_SCOPES[scope]['description'], - 'icon': ALL_SCOPES[scope]['icon'], - }) + 'title': scope.title, + 'scope': scope.scope, + 'description': scope.description, + 'icon': scope.icon, + }) return scope_info diff --git a/data/model/oauth.py b/data/model/oauth.py index 75596c535..7dd4445c8 100644 --- a/data/model/oauth.py +++ b/data/model/oauth.py @@ -1,3 +1,5 @@ +import logging + from datetime import datetime, timedelta from oauth2lib.provider import AuthorizationProvider from oauth2lib import utils @@ -6,6 +8,9 @@ from data.database import OAuthApplication, OAuthAuthorizationCode, OAuthAccessT from auth import scopes +logger = logging.getLogger(__name__) + + class DatabaseAuthorizationProvider(AuthorizationProvider): def get_authorized_user(self): raise NotImplementedError('Subclasses must fill in the ability to get the authorized_user.') @@ -41,28 +46,25 @@ class DatabaseAuthorizationProvider(AuthorizationProvider): def validate_access(self): return self.get_authorized_user() is not None - def lookup_access_token(self, client_id): - try: - found = (OAuthAccessToken - .select() - .join(OAuthApplication) - .where(OAuthApplication.client_id == client_id) - .get()) - return found - except OAuthAccessToken.DoesNotExist: - return None + def load_authorized_scope_string(self, client_id, username): + found = (OAuthAccessToken + .select() + .join(OAuthApplication) + .switch(OAuthAccessToken) + .join(User) + .where(OAuthApplication.client_id == client_id, User.username == username, + OAuthAccessToken.expires_at > datetime.now())) + found = list(found) + logger.debug('Found %s matching tokens.', len(found)) + long_scope_string = ','.join([token.scope for token in found]) + logger.debug('Computed long scope string: %s', long_scope_string) + return long_scope_string - def validate_has_scopes(self, client_id, scope): - access_token = self.lookup_access_token(client_id) - if not access_token: - return False - - # Make sure the token is not expired. - if access_token.expires_at <= datetime.now(): - return False + def validate_has_scopes(self, client_id, username, scope): + long_scope_string = self.load_authorized_scope_string(client_id, username) # Make sure the token contains the given scopes (at least). - return scopes.is_subset_string(access_token.scope, scope) + return scopes.is_subset_string(long_scope_string, scope) def from_authorization_code(self, client_id, code, scope): try: diff --git a/endpoints/api/__init__.py b/endpoints/api/__init__.py index 5155dfa4e..43627663d 100644 --- a/endpoints/api/__init__.py +++ b/endpoints/api/__init__.py @@ -197,7 +197,7 @@ def require_user_permission(permission_class, scope=None): if not user: raise Unauthorized() - logger.debug('Checking permission %s for user', permission_class, user.username) + logger.debug('Checking permission %s for user %s', permission_class, user.username) permission = permission_class(user.username) if permission.can(): return func(self, *args, **kwargs) @@ -212,7 +212,7 @@ require_user_admin = require_user_permission(UserAdminPermission, None) def require_scope(scope_object): def wrapper(func): - @add_method_metadata('oauth2_scope', scope_object['scope']) + @add_method_metadata('oauth2_scope', scope_object) @wraps(func) def wrapped(*args, **kwargs): return func(*args, **kwargs) diff --git a/endpoints/api/discovery.py b/endpoints/api/discovery.py index dc5ee9178..2378f1969 100644 --- a/endpoints/api/discovery.py +++ b/endpoints/api/discovery.py @@ -100,7 +100,11 @@ def swagger_route_data(include_internal=False, compact=False): scope = method_metadata(method, 'oauth2_scope') if scope and not compact: new_operation['authorizations'] = { - 'oauth2': [scope], + 'oauth2': [ + { + 'scope': scope.scope + } + ], } internal = method_metadata(method, 'internal') @@ -148,7 +152,7 @@ def swagger_route_data(include_internal=False, compact=False): }, 'authorizations': { 'oauth2': { - 'scopes': list(scopes.ALL_SCOPES.values()), + 'scopes': [scope._asdict() for scope in scopes.ALL_SCOPES.values()], 'grantTypes': { "implicit": { "tokenName": "access_token", diff --git a/endpoints/web.py b/endpoints/web.py index d5e9a3ce7..b9d5726dc 100644 --- a/endpoints/web.py +++ b/endpoints/web.py @@ -277,7 +277,8 @@ def request_authorization_code(): redirect_uri = request.args.get('redirect_uri', None) scope = request.args.get('scope', None) - if not provider.validate_has_scopes(client_id, scope): + if (not current_user.is_authenticated() or + not provider.validate_has_scopes(client_id, current_user.db_user().username, scope)): if not provider.validate_redirect_uri(client_id, redirect_uri): abort(404) return