darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-01-13 14:11:15 -05:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #100 from alphagov/service-not-found-returns-404

Browse Source 
Fix get_users_by_service to return 404 if service does not exist.
This commit is contained in:
Adam Shimali
2016-02-25 15:47:11 +00:00
parent ab495d983e 52ab17e718
commit abb46e0a9f
6 changed files with 109 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
app/errors.py
View File

@@ -10,42 +10,42 @@ def register_errors(blueprint):
@blueprint.app_errorhandler(400)
def bad_request(e):
return jsonify(error=str(e.description)), 400
return jsonify(result='error', message=str(e.description)), 400
@blueprint.app_errorhandler(401)
def unauthorized(e):
error_message = "Unauthorized, authentication token must be provided"
return jsonify(error=error_message), 401, [('WWW-Authenticate', 'Bearer')]
return jsonify(result='error', message=error_message), 401, [('WWW-Authenticate', 'Bearer')]
@blueprint.app_errorhandler(403)
def forbidden(e):
error_message = "Forbidden, invalid authentication token provided"
return jsonify(error=error_message), 403
return jsonify(result='error', message=error_message), 403
@blueprint.app_errorhandler(404)
def page_not_found(e):
return jsonify(error=e.description or "Not found"), 404
return jsonify(result='error', message=e.description or "Not found"), 404
@blueprint.app_errorhandler(429)
def limit_exceeded(e):
return jsonify(error=str(e.description)), 429
return jsonify(result='error', message=str(e.description)), 429
@blueprint.app_errorhandler(500)
def internal_server_error(e):
current_app.logger.exception(e)
return jsonify(error="Internal error"), 500
return jsonify(result='error', message="Internal server error"), 500
@blueprint.app_errorhandler(NoResultFound)
def no_result_found(e):
current_app.logger.error(e)
return jsonify(error="No result found"), 404
return jsonify(result='error', message="No result found"), 404
@blueprint.app_errorhandler(DataError)
def no_result_found(e):
def data_error(e):
current_app.logger.error(e)
return jsonify(result="error", message="No result found"), 404
return jsonify(result='error', message="No result found"), 404
@blueprint.app_errorhandler(SQLAlchemyError)
def db_error(e):
current_app.logger.error(e)
return jsonify(error="Database error occurred"), 500
return jsonify(result='error', message=str(e)), 500

5
app/models.py
View File

@@ -58,7 +58,10 @@ user_to_service = db.Table(
'user_to_service',
db.Model.metadata,
db.Column('user_id', db.Integer, db.ForeignKey('users.id')),
db.Column('service_id', UUID(as_uuid=True), db.ForeignKey('services.id'))
db.Column('service_id', UUID(as_uuid=True), db.ForeignKey('services.id')),
UniqueConstraint('user_id', 'service_id', name='uix_user_to_service')
)

72
app/service/rest.py
View File

@@ -1,14 +1,18 @@
from datetime import datetime
from flask import Blueprint
from flask import (
jsonify,
request
)
from sqlalchemy.exc import DataError
from sqlalchemy.orm.exc import NoResultFound
from app.dao import DAOException
from app.dao.users_dao import get_model_users
from app import email_safe
from app.dao.api_key_dao import (
save_model_api_key,
get_model_api_keys,
get_unsigned_secret
)
from app.dao.services_dao import (
dao_fetch_service_by_id_and_user,
dao_fetch_service_by_id,
@@ -17,25 +21,17 @@ from app.dao.services_dao import (
dao_update_service,
dao_fetch_all_services_by_user
)
from app.dao.api_key_dao import (
save_model_api_key,
get_model_api_keys,
get_unsigned_secret
)
from app.dao.users_dao import get_model_users
from app.models import ApiKey
from app.schemas import (
services_schema,
service_schema,
api_keys_schema,
users_schema)
from app import email_safe
from flask import Blueprint
from app.errors import register_errors
service = Blueprint('service', __name__)
from app.errors import register_errors
register_errors(service)
@@ -59,7 +55,10 @@ def get_service_by_id(service_id):
else:
fetched = dao_fetch_service_by_id(service_id)
if not fetched:
return jsonify(result="error", message="not found"), 404
message_with_user_id = 'and for user id: {}'.format(user_id) if user_id else ''
return jsonify(result="error",
message="Service not found for service id: {0} {1}".format(service_id,
message_with_user_id)), 404
data, errors = service_schema.dump(fetched)
return jsonify(data=data)
@@ -91,7 +90,7 @@ def create_service():
def update_service(service_id):
fetched_service = dao_fetch_service_by_id(service_id)
if not fetched_service:
return jsonify(result="error", message="not found"), 404
return _service_not_found(service_id)
current_data = dict(service_schema.dump(fetched_service).data.items())
current_data.update(request.get_json())
@@ -107,28 +106,21 @@ def update_service(service_id):
def renew_api_key(service_id=None):
fetched_service = dao_fetch_service_by_id(service_id=service_id)
if not fetched_service:
return jsonify(result="error", message="Service not found"), 404
return _service_not_found(service_id)
# create a new one
# TODO: what validation should be done here?
secret_name = request.get_json()['name']
key = ApiKey(service=fetched_service, name=secret_name)
save_model_api_key(key)
try:
# create a new one
# TODO: what validation should be done here?
secret_name = request.get_json()['name']
key = ApiKey(service=fetched_service, name=secret_name)
save_model_api_key(key)
except DAOException as e:
return jsonify(result='error', message=str(e)), 500
unsigned_api_key = get_unsigned_secret(key.id)
return jsonify(data=unsigned_api_key), 201
@service.route('/<service_id>/api-key/revoke/<int:api_key_id>', methods=['POST'])
def revoke_api_key(service_id, api_key_id):
try:
service_api_key = get_model_api_keys(service_id=service_id, id=api_key_id)
except DataError:
return jsonify(result="error", message="Invalid api key for service"), 400
except NoResultFound:
return jsonify(result="error", message="Api key not found for service"), 404
service_api_key = get_model_api_keys(service_id=service_id, id=api_key_id)
save_model_api_key(service_api_key, update_dict={'id': service_api_key.id, 'expiry_date': datetime.utcnow()})
return jsonify(), 202
@@ -137,22 +129,16 @@ def revoke_api_key(service_id, api_key_id):
@service.route('/<service_id>/api-keys', methods=['GET'])
@service.route('/<service_id>/api-keys/<int:key_id>', methods=['GET'])
def get_api_keys(service_id, key_id=None):
try:
service = dao_fetch_service_by_id(service_id=service_id)
except DataError:
return jsonify(result="error", message="Invalid service id"), 400
except NoResultFound:
return jsonify(result="error", message="Service not found"), 404
fetched_service = dao_fetch_service_by_id(service_id=service_id)
if not fetched_service:
return _service_not_found(service_id)
try:
if key_id:
api_keys = [get_model_api_keys(service_id=service_id, id=key_id)]
else:
api_keys = get_model_api_keys(service_id=service_id)
except DAOException as e:
return jsonify(result='error', message=str(e)), 500
except NoResultFound:
return jsonify(result="error", message="API key not found"), 404
return jsonify(result="error", message="API key not found for id: {}".format(service_id)), 404
return jsonify(apiKeys=api_keys_schema.dump(api_keys).data), 200
@@ -161,7 +147,13 @@ def get_api_keys(service_id, key_id=None):
def get_users_for_service(service_id):
fetched = dao_fetch_service_by_id(service_id)
if not fetched:
return _service_not_found(service_id)
if not fetched.users:
return jsonify(data=[])
result = users_schema.dump(fetched.users)
return jsonify(data=result.data)
def _service_not_found(service_id):
return jsonify(result='error', message='Service not found for id: {}'.format(service_id)), 404

26
migrations/versions/0024_uix_user_to_service.py Normal file
View File

@@ -0,0 +1,26 @@
"""empty message
Revision ID: 0024_uix_user_to_service
Revises: 0023_drop_token
Create Date: 2016-02-25 11:15:06.088508
"""
# revision identifiers, used by Alembic.
revision = '0024_uix_user_to_service'
down_revision = '0023_drop_token'
from alembic import op
import sqlalchemy as sa
def upgrade():
### commands auto generated by Alembic - please adjust! ###
op.create_unique_constraint('uix_user_to_service', 'user_to_service', ['user_id', 'service_id'])
### end Alembic commands ###
def downgrade():
### commands auto generated by Alembic - please adjust! ###
op.drop_constraint('uix_user_to_service', 'user_to_service', type_='unique')
### end Alembic commands ###

55
tests/app/service/test_rest.py
View File

@@ -1,10 +1,9 @@
import json
import uuid
from app.dao.users_dao import save_model_user
from app.dao.services_dao import dao_remove_user_from_service
from app.models import User
from tests import create_authorization_header
from tests.app.conftest import sample_service as create_service
def test_get_service_list(notify_api, service_factory):
@@ -13,7 +12,6 @@ def test_get_service_list(notify_api, service_factory):
service_factory.get('one')
service_factory.get('two')
service_factory.get('three')
auth_header = create_authorization_header(
path='/service',
method='GET'
@@ -30,7 +28,8 @@ def test_get_service_list(notify_api, service_factory):
assert json_resp['data'][2]['name'] == 'three'
def test_get_service_list_by_user(notify_api, service_factory, sample_user):
def test_get_service_list_by_user(notify_api, sample_user, service_factory):
with notify_api.test_request_context():
with notify_api.test_client() as client:
service_factory.get('one', sample_user)
@@ -81,7 +80,7 @@ def test_get_service_list_by_user_should_return_empty_list_if_no_services(notify
assert len(json_resp['data']) == 0
def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db):
def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db, notify_db_session):
with notify_api.test_request_context():
with notify_api.test_client() as client:
auth_header = create_authorization_header(
@@ -129,7 +128,7 @@ def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db):
assert resp.status_code == 404
json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['result'] == 'error'
assert json_resp['message'] == 'not found'
assert json_resp['message'] == 'Service not found for service id: {} '.format(service_id)
def test_get_service_by_id_and_user(notify_api, service_factory, sample_user):
@@ -165,7 +164,8 @@ def test_get_service_by_id_should_404_if_no_service_for_user(notify_api, sample_
assert resp.status_code == 404
json_resp = json.loads(resp.get_data(as_text=True))
assert json_resp['result'] == 'error'
assert json_resp['message'] == 'not found'
assert json_resp['message'] == \
'Service not found for service id: {0} and for user id: {1}'.format(service_id, sample_user.id)
def test_create_service(notify_api, sample_user):
@@ -233,7 +233,7 @@ def test_should_not_create_service_with_missing_user_id_field(notify_api):
assert 'Missing data for required field.' in json_resp['message']['user_id']
def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, notify_db):
def test_should_not_create_service_with_missing_if_user_id_is_not_in_database(notify_api, notify_db, notify_db_session):
with notify_api.test_request_context():
with notify_api.test_client() as client:
data = {
@@ -320,10 +320,9 @@ def test_update_service(notify_api, sample_service):
assert result['data']['name'] == 'updated service name'
def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db):
def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db, notify_db_session):
with notify_api.test_request_context():
with notify_api.test_client() as client:
data = {
'name': 'updated service name'
}
@@ -344,12 +343,10 @@ def test_update_service_should_404_if_id_is_invalid(notify_api, notify_db):
assert resp.status_code == 404
def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_user):
def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_service):
with notify_api.test_request_context():
with notify_api.test_client() as client:
sample_service = create_service(notify_db=notify_db, notify_db_session=notify_db_session,
service_name='Sample service', user=sample_user)
user_on_service = sample_service.users[0]
auth_header = create_authorization_header(
path='/service/{}/users'.format(sample_service.id),
method='GET'
@@ -363,19 +360,18 @@ def test_get_users_by_service(notify_api, notify_db, notify_db_session, sample_u
assert resp.status_code == 200
result = json.loads(resp.get_data(as_text=True))
assert len(result['data']) == 1
assert result['data'][0]['name'] == sample_user.name
assert result['data'][0]['email_address'] == sample_user.email_address
assert result['data'][0]['mobile_number'] == sample_user.mobile_number
assert result['data'][0]['name'] == user_on_service.name
assert result['data'][0]['email_address'] == user_on_service.email_address
assert result['data'][0]['mobile_number'] == user_on_service.mobile_number
def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_service(notify_api,
notify_db,
notify_db_session,
sample_service,
sample_user):
sample_service):
with notify_api.test_request_context():
with notify_api.test_client() as client:
sample_service.users.remove(sample_user)
dao_remove_user_from_service(sample_service, sample_service.users[0])
auth_header = create_authorization_header(
path='/service/{}/users'.format(sample_service.id),
method='GET'
@@ -388,3 +384,22 @@ def test_get_users_for_service_returns_empty_list_if_no_users_associated_with_se
result = json.loads(response.get_data(as_text=True))
assert response.status_code == 200
assert result['data'] == []
def test_get_users_for_service_returns_404_when_service_does_not_exist(notify_api, notify_db, notify_db_session):
with notify_api.test_request_context():
with notify_api.test_client() as client:
service_id = uuid.uuid4()
auth_header = create_authorization_header(
path='/service/{}/users'.format(service_id),
method='GET'
)
response = client.get(
'/service/{}/users'.format(service_id),
headers=[('Content-Type', 'application/json'), auth_header]
)
assert response.status_code == 404
result = json.loads(response.get_data(as_text=True))
assert result['result'] == 'error'
assert result['message'] == 'Service not found for id: {}'.format(service_id)

4
tests/conftest.py
View File

@@ -52,8 +52,8 @@ def notify_db_session(request):
def teardown():
db.session.remove()
for tbl in reversed(meta.sorted_tables):
if tbl.fullname not in ['roles']:
db.engine.execute(tbl.delete())
db.engine.execute(tbl.delete())
db.session.commit()
meta = MetaData(bind=db.engine, reflect=True)
request.addfinalizer(teardown)