From 135a148101e6688925c3da986cfa839e5206e9b4 Mon Sep 17 00:00:00 2001 From: Erik Moeller Date: Wed, 16 Sep 2020 17:50:55 -0700 Subject: [PATCH] Add /users endpoint Resolves #5490 Includes the following tests: - Test that endpoint exists - Test that endpoint requires authorization - Test that endpoint enumerates all users - Test that endpoint returns only expected fields --- securedrop/journalist_app/api.py | 8 +++++++ securedrop/models.py | 16 +++++++++++--- securedrop/tests/test_journalist_api.py | 28 +++++++++++++++++++++---- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index 9ea7040093..7d3a83e9d1 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -66,6 +66,7 @@ def make_blueprint(config: SDConfig) -> Blueprint: def get_endpoints() -> Tuple[flask.Response, int]: endpoints = {'sources_url': '/api/v1/sources', 'current_user_url': '/api/v1/user', + 'all_users_url': '/api/v1/users', 'submissions_url': '/api/v1/submissions', 'replies_url': '/api/v1/replies', 'auth_token_url': '/api/v1/token'} @@ -323,6 +324,13 @@ def get_current_user() -> Tuple[flask.Response, int]: user = _authenticate_user_from_auth_header(request) return jsonify(user.to_json()), 200 + @api.route('/users', methods=['GET']) + @token_required + def get_all_users() -> Tuple[flask.Response, int]: + users = Journalist.query.all() + return jsonify( + {'users': [user.to_json(all_info=False) for user in users]}), 200 + @api.route('/logout', methods=['POST']) @token_required def logout() -> Tuple[flask.Response, int]: diff --git a/securedrop/models.py b/securedrop/models.py index 5af006fe6b..abd54cabc0 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -710,15 +710,25 @@ def validate_api_token_and_get_user(token: str) -> 'Optional[Journalist]': return Journalist.query.get(data['id']) - def to_json(self) -> 'Dict[str, Union[str, bool, str]]': + def to_json(self, all_info: bool = True) -> 'Dict[str, Union[str, bool, str]]': + """Returns a JSON representation of the journalist user. If all_info is + False, potentially sensitive or extraneous fields are excluded. Note + that both representations do NOT include credentials.""" + json_user = { 'username': self.username, - 'last_login': self.last_access.isoformat() + 'Z', - 'is_admin': self.is_admin, 'uuid': self.uuid, 'first_name': self.first_name, 'last_name': self.last_name } + + if all_info is True: + json_user['is_admin'] = self.is_admin + try: + json_user['last_login'] = self.last_access.isoformat() + 'Z' + except AttributeError: + json_user['last_login'] = None + return json_user diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 8d93c4ade9..b73cdb8d18 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -22,9 +22,9 @@ def test_unauthenticated_user_gets_all_endpoints(journalist_app): with journalist_app.test_client() as app: response = app.get(url_for('api.get_endpoints')) - expected_endpoints = ['current_user_url', 'submissions_url', - 'sources_url', 'auth_token_url', - 'replies_url'] + expected_endpoints = ['current_user_url', 'all_users_url', + 'submissions_url', 'sources_url', + 'auth_token_url', 'replies_url'] expected_endpoints.sort() sorted_observed_endpoints = list(response.json.keys()) sorted_observed_endpoints.sort() @@ -153,7 +153,8 @@ def test_user_without_token_cannot_get_protected_endpoints(journalist_app, url_for('api.single_reply', source_uuid=uuid, reply_uuid=test_files['replies'][0].uuid), url_for('api.all_source_replies', source_uuid=uuid), - url_for('api.get_current_user') + url_for('api.get_current_user'), + url_for('api.get_all_users'), ] with journalist_app.test_client() as app: @@ -639,6 +640,25 @@ def test_authorized_user_can_get_current_user_endpoint(journalist_app, assert response.json['last_name'] == test_journo['journalist'].last_name +def test_authorized_user_can_get_all_users(journalist_app, test_journo, test_admin, + journalist_api_token): + with journalist_app.test_client() as app: + response = app.get(url_for('api.get_all_users'), + headers=get_api_headers(journalist_api_token)) + + assert response.status_code == 200 + + # Ensure that all the users in the database are returned + observed_users = [user['uuid'] for user in response.json['users']] + expected_users = [user.uuid for user in Journalist.query.all()] + assert observed_users == expected_users + + # Ensure that no fields other than the expected ones are returned + expected_fields = ['first_name', 'last_name', 'username', 'uuid'] + for user in response.json['users']: + assert sorted(user.keys()) == expected_fields + + def test_request_with_missing_auth_header_triggers_403(journalist_app): with journalist_app.test_client() as app: response = app.get(url_for('api.get_current_user'),