diff --git a/fittrackee/application/app_config.py b/fittrackee/application/app_config.py index 008abf4d..1a368c04 100644 --- a/fittrackee/application/app_config.py +++ b/fittrackee/application/app_config.py @@ -67,7 +67,7 @@ def get_application_config() -> Union[Dict, HttpResponse]: @config_blueprint.route('/config', methods=['PATCH']) -@require_auth(as_admin=True) +@require_auth(scopes='write', as_admin=True) def update_application_config(auth_user: User) -> Union[Dict, HttpResponse]: """ Update Application config diff --git a/fittrackee/oauth2/client.py b/fittrackee/oauth2/client.py index 93094fe5..aa0dee92 100644 --- a/fittrackee/oauth2/client.py +++ b/fittrackee/oauth2/client.py @@ -6,6 +6,28 @@ from werkzeug.security import gen_salt from fittrackee.oauth2.models import OAuth2Client from fittrackee.users.models import User +DEFAULT_SCOPE = 'read' +VALID_SCOPES = ['read', 'write'] + + +def check_scope(scope: str) -> str: + """ + Verify if provided scope is valid. + If not, it returns the default scope ('read'). + """ + valid_scopes = [] + if not isinstance(scope, str) or not scope: + return DEFAULT_SCOPE + + scopes = scope.split() + for value in scopes: + if value in VALID_SCOPES: + valid_scopes.append(value) + if len(valid_scopes) == 0: + valid_scopes.append(DEFAULT_SCOPE) + + return ' '.join(valid_scopes) + def create_oauth_client(metadata: Dict, user: User) -> OAuth2Client: """ @@ -18,7 +40,7 @@ def create_oauth_client(metadata: Dict, user: User) -> OAuth2Client: 'client_name': metadata['client_name'], 'client_uri': metadata['client_uri'], 'redirect_uris': metadata['redirect_uris'], - 'scope': metadata['scope'], + 'scope': check_scope(metadata['scope']), 'grant_types': ['authorization_code', 'refresh_token'], 'response_types': ['code'], 'token_endpoint_auth_method': 'client_secret_post', diff --git a/fittrackee/tests/mixins.py b/fittrackee/tests/mixins.py index 2c9c018f..1dd5a37e 100644 --- a/fittrackee/tests/mixins.py +++ b/fittrackee/tests/mixins.py @@ -63,24 +63,33 @@ class ApiTestCaseMixin(RandomMixin): @staticmethod def create_oauth_client( - user: User, metadata: Optional[Dict] = None + user: User, + metadata: Optional[Dict] = None, + scope: Optional[str] = None, ) -> OAuth2Client: - oauth_client = create_oauth_client( - TEST_OAUTH_CLIENT_METADATA if metadata is None else metadata, user + client_metadata = ( + TEST_OAUTH_CLIENT_METADATA if metadata is None else metadata ) + if scope is not None: + client_metadata['scope'] = scope + oauth_client = create_oauth_client(client_metadata, user) db.session.add(oauth_client) db.session.commit() return oauth_client @staticmethod def authorize_client( - client: FlaskClient, oauth_client: OAuth2Client, auth_token: str + client: FlaskClient, + oauth_client: OAuth2Client, + auth_token: str, + scope: Optional[str] = None, ) -> Union[List[str], str]: response = client.post( '/api/oauth/authorize', data={ 'client_id': oauth_client.client_id, 'response_type': 'code', + 'scope': 'read' if not scope else scope, }, headers=dict( Authorization=f'Bearer {auth_token}', @@ -92,13 +101,15 @@ class ApiTestCaseMixin(RandomMixin): return code def create_oauth_client_and_issue_token( - self, app: Flask, user: User + self, app: Flask, user: User, scope: Optional[str] = None ) -> Tuple[FlaskClient, OAuth2Client, str]: client, auth_token = self.get_test_client_and_auth_token( app, user.email ) - oauth_client = self.create_oauth_client(user) - code = self.authorize_client(client, oauth_client, auth_token) + oauth_client = self.create_oauth_client(user, scope=scope) + code = self.authorize_client( + client, oauth_client, auth_token, scope=scope + ) response = client.post( '/api/oauth/token', data={ @@ -217,6 +228,31 @@ class ApiTestCaseMixin(RandomMixin): ), ) + @staticmethod + def assert_insufficient_scope(response: TestResponse) -> Dict: + return assert_oauth_errored_response( + response, + 403, + error='insufficient_scope', + error_description=( + 'The request requires higher privileges than provided by ' + 'the access token.' + ), + ) + + @staticmethod + def assert_not_insufficient_scope_error(response: TestResponse) -> None: + assert response.status_code != 403 + if response.status_code != 204: + data = json.loads(response.data.decode()) + if 'error' in data: + assert 'insufficient_scope' not in data['error'] + if 'error_description' in data: + assert ( + 'The request requires higher privileges than provided by ' + 'the access token.' + ) != data['error_description'] + class CallArgsMixin: @staticmethod diff --git a/fittrackee/tests/oauth2/test_oauth2_client.py b/fittrackee/tests/oauth2/test_oauth2_client.py index 9628fb26..77129916 100644 --- a/fittrackee/tests/oauth2/test_oauth2_client.py +++ b/fittrackee/tests/oauth2/test_oauth2_client.py @@ -1,10 +1,11 @@ from time import time -from typing import Dict +from typing import Any, Dict from unittest.mock import patch +import pytest from flask import Flask -from fittrackee.oauth2.client import create_oauth_client +from fittrackee.oauth2.client import check_scope, create_oauth_client from fittrackee.oauth2.models import OAuth2Client from fittrackee.users.models import User @@ -106,7 +107,7 @@ class TestCreateOAuth2Client: def test_oauth_client_has_expected_scope( self, app: Flask, user_1: User ) -> None: - scope = 'profile' + scope = 'write' client_metadata: Dict = {**TEST_METADATA, 'scope': scope} oauth_client = create_oauth_client(client_metadata, user_1) @@ -138,3 +139,28 @@ class TestCreateOAuth2Client: oauth_client = create_oauth_client(TEST_METADATA, user_1) assert oauth_client.user_id == user_1.id + + +class TestOAuthCheckScopes: + @pytest.mark.parametrize( + 'input_scope', ['', 1, random_string(), [random_string(), 'readwrite']] + ) + def test_it_returns_read_if_scope_is_invalid( + self, input_scope: Any + ) -> None: + assert check_scope(input_scope) == 'read' + + @pytest.mark.parametrize( + 'input_scope,expected_scope', + [ + ('read', 'read'), + ('read ' + random_string(), 'read'), + ('write', 'write'), + ('write read', 'write read'), + ('write read ' + random_string(), 'write read'), + ], + ) + def test_it_return_only_valid_scopes( + self, input_scope: str, expected_scope: str + ) -> None: + assert check_scope(input_scope) == expected_scope diff --git a/fittrackee/tests/oauth2/test_oauth2_scopes.py b/fittrackee/tests/oauth2/test_oauth2_scopes.py new file mode 100644 index 00000000..15e3c1be --- /dev/null +++ b/fittrackee/tests/oauth2/test_oauth2_scopes.py @@ -0,0 +1,259 @@ +from json import dumps + +import pytest +from flask import Flask +from werkzeug.test import TestResponse + +from fittrackee.users.models import User + +from ..mixins import ApiTestCaseMixin +from ..utils import random_short_id, random_string + + +class OAuth2ScopesTestCase(ApiTestCaseMixin): + def assert_expected_response( + self, response: TestResponse, client_scope: str, endpoint_scope: str + ) -> None: + if client_scope == endpoint_scope: + self.assert_not_insufficient_scope_error(response) + else: + self.assert_insufficient_scope(response) + + +class TestOAuth2ScopesWithReadAccess(OAuth2ScopesTestCase): + scope = 'read' + + @pytest.mark.parametrize( + 'endpoint_url', + [ + '/api/auth/profile', + '/api/records', + '/api/sports', + '/api/sports/1', + f'/api/stats/{random_string()}/by_sport', + f'/api/stats/{random_string()}/by_time', + '/api/users/test', + '/api/workouts', + f'/api/workouts/{random_short_id()}', + f'/api/workouts/{random_short_id()}/chart_data', + f'/api/workouts/{random_short_id()}/chart_data/segment/1', + f'/api/workouts/{random_short_id()}/gpx', + f'/api/workouts/{random_short_id()}/gpx/download', + f'/api/workouts/{random_short_id()}/gpx/segment/1', + ], + ) + def test_access_to_get_endpoints( + self, app: Flask, user_1: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + + response = client.get( + endpoint_url, + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='read' + ) + + @pytest.mark.parametrize( + 'endpoint_url', + ['/api/users'], + ) + def test_access_to_endpoints_as_admin( + self, app: Flask, user_1_admin: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1_admin, scope=self.scope + ) + + response = client.get( + endpoint_url, + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='read' + ) + + @pytest.mark.parametrize( + 'endpoint_url', + [ + '/api/auth/picture', + '/api/auth/profile/edit', + '/api/auth/profile/edit/preferences', + '/api/auth/profile/edit/sports', + '/api/workouts', + '/api/workouts/no_gpx', + ], + ) + def test_access_post_endpoints( + self, app: Flask, user_1: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + + response = client.post( + endpoint_url, + data=dumps(dict()), + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='write' + ) + + @pytest.mark.parametrize( + 'endpoint_url', + [ + '/api/auth/profile/edit/account', + '/api/workouts/0', + ], + ) + def test_access_to_patch_endpoints( + self, app: Flask, user_1: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + + response = client.patch( + endpoint_url, + data=dumps(dict()), + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='write' + ) + + @pytest.mark.parametrize( + 'endpoint_url', + [ + '/api/config', + '/api/sports/1', + f'/api/users/{random_string()}', + ], + ) + def test_access_to_patch_endpoints_as_admin( + self, app: Flask, user_1_admin: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1_admin, scope=self.scope + ) + + response = client.patch( + endpoint_url, + data=dumps(dict()), + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='write' + ) + + @pytest.mark.parametrize( + 'endpoint_url', + [ + '/api/auth/picture', + '/api/auth/profile/reset/sports/1', + f'/api/users/{random_string()}', + '/api/workouts/0', + ], + ) + def test_access_to_delete_endpoints( + self, app: Flask, user_1: User, endpoint_url: str + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + user_1.picture = random_string() + + response = client.delete( + endpoint_url, + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_expected_response( + response, client_scope=self.scope, endpoint_scope='write' + ) + + +class TestOAuth2ScopesWithWriteAccess(TestOAuth2ScopesWithReadAccess): + scope = 'write' + + +class TestOAuth2ScopesWithReadAndWriteAccess(ApiTestCaseMixin): + scope = 'read write' + + def test_client_can_access_endpoint_with_read_scope( + self, app: Flask, user_1: User + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + + response = client.get( + '/api/auth/profile', + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_not_insufficient_scope_error(response) + + def test_client_with_read_can_access_endpoints_with_write_scope( + self, app: Flask, user_1: User + ) -> None: + ( + client, + oauth_client, + access_token, + ) = self.create_oauth_client_and_issue_token( + app, user_1, scope=self.scope + ) + + response = client.post( + '/api/auth/picture', + data=dumps(dict()), + content_type='application/json', + headers=dict(Authorization=f'Bearer {access_token}'), + ) + + self.assert_not_insufficient_scope_error(response) diff --git a/fittrackee/tests/utils.py b/fittrackee/tests/utils.py index d85e2580..f1d7963b 100644 --- a/fittrackee/tests/utils.py +++ b/fittrackee/tests/utils.py @@ -2,9 +2,12 @@ import random import string from json import loads from typing import Dict, Optional +from uuid import uuid4 from flask import json as flask_json +from fittrackee.workouts.utils.short_id import encode_uuid + def random_string( length: Optional[int] = None, @@ -32,6 +35,10 @@ def random_email() -> str: return random_string(suffix='@example.com') +def random_short_id() -> str: + return encode_uuid(uuid4()) + + def jsonify_dict(data: Dict) -> Dict: return loads(flask_json.dumps(data)) diff --git a/fittrackee/users/auth.py b/fittrackee/users/auth.py index f6a3335c..50325ed2 100644 --- a/fittrackee/users/auth.py +++ b/fittrackee/users/auth.py @@ -252,7 +252,7 @@ def login_user() -> Union[Dict, HttpResponse]: @auth_blueprint.route('/auth/profile', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_authenticated_user_profile( auth_user: User, ) -> Union[Dict, HttpResponse]: @@ -354,7 +354,7 @@ def get_authenticated_user_profile( @auth_blueprint.route('/auth/profile/edit', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def edit_user(auth_user: User) -> Union[Dict, HttpResponse]: """ edit authenticated user profile @@ -502,7 +502,7 @@ def edit_user(auth_user: User) -> Union[Dict, HttpResponse]: @auth_blueprint.route('/auth/profile/edit/account', methods=['PATCH']) -@require_auth() +@require_auth(scopes='write') def update_user_account(auth_user: User) -> Union[Dict, HttpResponse]: """ update authenticated user email and password @@ -712,7 +712,7 @@ def update_user_account(auth_user: User) -> Union[Dict, HttpResponse]: @auth_blueprint.route('/auth/profile/edit/preferences', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def edit_user_preferences(auth_user: User) -> Union[Dict, HttpResponse]: """ edit authenticated user preferences @@ -853,7 +853,7 @@ def edit_user_preferences(auth_user: User) -> Union[Dict, HttpResponse]: @auth_blueprint.route('/auth/profile/edit/sports', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def edit_user_sport_preferences( auth_user: User, ) -> Union[Dict, HttpResponse]: @@ -959,7 +959,7 @@ def edit_user_sport_preferences( @auth_blueprint.route( '/auth/profile/reset/sports/', methods=['DELETE'] ) -@require_auth() +@require_auth(scopes='write') def reset_user_sport_preferences( auth_user: User, sport_id: int ) -> Union[Tuple[Dict, int], HttpResponse]: @@ -1014,7 +1014,7 @@ def reset_user_sport_preferences( @auth_blueprint.route('/auth/picture', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def edit_picture(auth_user: User) -> Union[Dict, HttpResponse]: """ update authenticated user picture @@ -1102,7 +1102,7 @@ def edit_picture(auth_user: User) -> Union[Dict, HttpResponse]: @auth_blueprint.route('/auth/picture', methods=['DELETE']) -@require_auth() +@require_auth(scopes='write') def del_picture(auth_user: User) -> Union[Tuple[Dict, int], HttpResponse]: """ delete authenticated user picture diff --git a/fittrackee/users/users.py b/fittrackee/users/users.py index 47a4e1f1..f9c387f4 100644 --- a/fittrackee/users/users.py +++ b/fittrackee/users/users.py @@ -34,7 +34,7 @@ USER_PER_PAGE = 10 @users_blueprint.route('/users', methods=['GET']) -@require_auth(as_admin=True) +@require_auth(scopes='read', as_admin=True) def get_users(auth_user: User) -> Dict: """ Get all users (regardless their account status), if authenticated user @@ -235,7 +235,7 @@ def get_users(auth_user: User) -> Dict: @users_blueprint.route('/users/', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_single_user( auth_user: User, user_name: str ) -> Union[Dict, HttpResponse]: @@ -394,7 +394,7 @@ def get_picture(user_name: str) -> Any: @users_blueprint.route('/users/', methods=['PATCH']) -@require_auth(as_admin=True) +@require_auth(scopes='write', as_admin=True) def update_user(auth_user: User, user_name: str) -> Union[Dict, HttpResponse]: """ Update user account @@ -593,7 +593,7 @@ def update_user(auth_user: User, user_name: str) -> Union[Dict, HttpResponse]: @users_blueprint.route('/users/', methods=['DELETE']) -@require_auth() +@require_auth(scopes='write') def delete_user( auth_user: User, user_name: str ) -> Union[Tuple[Dict, int], HttpResponse]: diff --git a/fittrackee/workouts/records.py b/fittrackee/workouts/records.py index 408dca0d..e7519318 100644 --- a/fittrackee/workouts/records.py +++ b/fittrackee/workouts/records.py @@ -11,7 +11,7 @@ records_blueprint = Blueprint('records', __name__) @records_blueprint.route('/records', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_records(auth_user: User) -> Dict: """ Get all records for authenticated user. diff --git a/fittrackee/workouts/sports.py b/fittrackee/workouts/sports.py index 752d2504..8016ecb8 100644 --- a/fittrackee/workouts/sports.py +++ b/fittrackee/workouts/sports.py @@ -19,7 +19,7 @@ sports_blueprint = Blueprint('sports', __name__) @sports_blueprint.route('/sports', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_sports(auth_user: User) -> Dict: """ Get all sports @@ -195,7 +195,7 @@ def get_sports(auth_user: User) -> Dict: @sports_blueprint.route('/sports/', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_sport(auth_user: User, sport_id: int) -> Union[Dict, HttpResponse]: """ Get a sport @@ -304,7 +304,7 @@ def get_sport(auth_user: User, sport_id: int) -> Union[Dict, HttpResponse]: @sports_blueprint.route('/sports/', methods=['PATCH']) -@require_auth(as_admin=True) +@require_auth(scopes='write', as_admin=True) def update_sport(auth_user: User, sport_id: int) -> Union[Dict, HttpResponse]: """ Update a sport diff --git a/fittrackee/workouts/stats.py b/fittrackee/workouts/stats.py index c0f3776f..2a3d4ca1 100644 --- a/fittrackee/workouts/stats.py +++ b/fittrackee/workouts/stats.py @@ -174,7 +174,7 @@ def get_workouts( @stats_blueprint.route('/stats//by_time', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_workouts_by_time( auth_user: User, user_name: str ) -> Union[Dict, HttpResponse]: @@ -281,7 +281,7 @@ def get_workouts_by_time( @stats_blueprint.route('/stats//by_sport', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_workouts_by_sport( auth_user: User, user_name: str ) -> Union[Dict, HttpResponse]: diff --git a/fittrackee/workouts/workouts.py b/fittrackee/workouts/workouts.py index 78f7ccf6..3e15fb59 100644 --- a/fittrackee/workouts/workouts.py +++ b/fittrackee/workouts/workouts.py @@ -56,7 +56,7 @@ MAX_WORKOUTS_PER_PAGE = 100 @workouts_blueprint.route('/workouts', methods=['GET']) -@require_auth() +@require_auth(scopes='read') def get_workouts(auth_user: User) -> Union[Dict, HttpResponse]: """ Get workouts for the authenticated user. @@ -298,7 +298,7 @@ def get_workouts(auth_user: User) -> Union[Dict, HttpResponse]: @workouts_blueprint.route( '/workouts/', methods=['GET'] ) -@require_auth() +@require_auth(scopes='read') def get_workout( auth_user: User, workout_short_id: str ) -> Union[Dict, HttpResponse]: @@ -462,7 +462,7 @@ def get_workout_data( @workouts_blueprint.route( '/workouts//gpx', methods=['GET'] ) -@require_auth() +@require_auth(scopes='read') def get_workout_gpx( auth_user: User, workout_short_id: str ) -> Union[Dict, HttpResponse]: @@ -512,7 +512,7 @@ def get_workout_gpx( @workouts_blueprint.route( '/workouts//chart_data', methods=['GET'] ) -@require_auth() +@require_auth(scopes='read') def get_workout_chart_data( auth_user: User, workout_short_id: str ) -> Union[Dict, HttpResponse]: @@ -582,7 +582,7 @@ def get_workout_chart_data( '/workouts//gpx/segment/', methods=['GET'], ) -@require_auth() +@require_auth(scopes='read') def get_segment_gpx( auth_user: User, workout_short_id: str, segment_id: int ) -> Union[Dict, HttpResponse]: @@ -634,7 +634,7 @@ def get_segment_gpx( '', methods=['GET'], ) -@require_auth() +@require_auth(scopes='read') def get_segment_chart_data( auth_user: User, workout_short_id: str, segment_id: int ) -> Union[Dict, HttpResponse]: @@ -705,7 +705,7 @@ def get_segment_chart_data( @workouts_blueprint.route( '/workouts//gpx/download', methods=['GET'] ) -@require_auth() +@require_auth(scopes='read') def download_workout_gpx( auth_user: User, workout_short_id: str ) -> Union[HttpResponse, Response]: @@ -848,7 +848,7 @@ def get_map_tile(s: str, z: str, x: str, y: str) -> Tuple[Response, int]: @workouts_blueprint.route('/workouts', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def post_workout(auth_user: User) -> Union[Tuple[Dict, int], HttpResponse]: """ Post an workout with a gpx file @@ -1016,7 +1016,7 @@ def post_workout(auth_user: User) -> Union[Tuple[Dict, int], HttpResponse]: @workouts_blueprint.route('/workouts/no_gpx', methods=['POST']) -@require_auth() +@require_auth(scopes='write') def post_workout_no_gpx( auth_user: User, ) -> Union[Tuple[Dict, int], HttpResponse]: @@ -1164,7 +1164,7 @@ def post_workout_no_gpx( @workouts_blueprint.route( '/workouts/', methods=['PATCH'] ) -@require_auth() +@require_auth(scopes='write') def update_workout( auth_user: User, workout_short_id: str ) -> Union[Dict, HttpResponse]: @@ -1311,7 +1311,7 @@ def update_workout( @workouts_blueprint.route( '/workouts/', methods=['DELETE'] ) -@require_auth() +@require_auth(scopes='write') def delete_workout( auth_user: User, workout_short_id: str ) -> Union[Tuple[Dict, int], HttpResponse]: