diff --git a/querybook/server/app/auth/permission.py b/querybook/server/app/auth/permission.py index 5ee0bae7b..db62e3f7c 100644 --- a/querybook/server/app/auth/permission.py +++ b/querybook/server/app/auth/permission.py @@ -208,7 +208,7 @@ def verify_query_execution_access(execution_id, user_envs, session=None): session=session, ), "CANNOT_ACCESS_QUERY_EXECUTION", - 403, + ACCESS_RESTRICTED_STATUS_CODE, ) diff --git a/querybook/server/app/auth/utils.py b/querybook/server/app/auth/utils.py index 98b40f083..1a204c58c 100644 --- a/querybook/server/app/auth/utils.py +++ b/querybook/server/app/auth/utils.py @@ -59,7 +59,7 @@ def load_user(uid, session=None): if user is None: # Invalid user, clear session flask_session.clear() - flask.abort(401, description="Invalid cookie") + flask.abort(UNAUTHORIZED_STATUS_CODE, description="Invalid cookie") return AuthUser(user) @@ -74,9 +74,11 @@ def load_user_with_api_access_token(request): user = get_user_by_id(token_validation.creator_uid, session=session) return AuthUser(user) else: - flask.abort(401, description="Token is disabled.") + flask.abort( + UNAUTHORIZED_STATUS_CODE, description="Token is disabled." + ) else: - flask.abort(401, description="Token is invalid.") + flask.abort(UNAUTHORIZED_STATUS_CODE, description="Token is invalid.") return None diff --git a/querybook/server/app/datasource.py b/querybook/server/app/datasource.py index 6a7c96a17..e7cdfb1fe 100644 --- a/querybook/server/app/datasource.py +++ b/querybook/server/app/datasource.py @@ -10,7 +10,15 @@ from app.flask_app import flask_app, limiter from app.db import get_session -from const.datasources import DS_PATH +from const.datasources import ( + DS_PATH, + OK_STATUS_CODE, + UNAUTHORIZED_STATUS_CODE, + INVALID_SEMANTIC_STATUS_CODE, + ACCESS_RESTRICTED_STATUS_CODE, + UNKNOWN_CLIENT_ERROR_STATUS_CODE, + UNKNOWN_SERVER_ERROR_STATUS_CODE, +) from lib.logger import get_logger from logic.impression import create_impression from lib.event_logger import event_logger @@ -49,7 +57,7 @@ def wrapper(fn): @functools.wraps(fn) def handler(**kwargs): if require_auth and not current_user.is_authenticated: - flask.abort(401, description="Login required.") + flask.abort(UNAUTHORIZED_STATUS_CODE, description="Login required.") params = {} if flask.request.method == "GET": @@ -57,7 +65,7 @@ def handler(**kwargs): elif flask.request.is_json: params = flask.request.json - status = 200 + status = OK_STATUS_CODE try: kwargs.update(params) @@ -80,18 +88,18 @@ def handler(**kwargs): status = e.code results = {"host": _host, "error": e.description} except RequestException as e: - status = e.status_code or 500 + status = e.status_code or UNKNOWN_CLIENT_ERROR_STATUS_CODE results = {"host": _host, "error": str(e), "request_exception": True} except Exception as e: LOG.error(e, exc_info=True) - status = 500 + status = UNKNOWN_SERVER_ERROR_STATUS_CODE results = { "host": _host, "error": str(e), "traceback": traceback.format_exc(), } finally: - if status != 200 and "database_session" in flask.g: + if status != OK_STATUS_CODE and "database_session" in flask.g: flask.g.database_session.rollback() if custom_response: return results @@ -140,19 +148,21 @@ def handler(*args, **kwargs): if current_user.is_authenticated and current_user.is_admin: return fn(*args, **kwargs) else: - flask.abort(403) + flask.abort(ACCESS_RESTRICTED_STATUS_CODE) handler.__raw__ = fn return handler -def api_assert(value, message="Assertion has failed", status_code=500): +def api_assert( + value, message="Assertion has failed", status_code=INVALID_SEMANTIC_STATUS_CODE +): if not value: abort_request(status_code, message) def abort_request( - status_code=500, + status_code=UNKNOWN_CLIENT_ERROR_STATUS_CODE, message=None, ): raise RequestException(message, status_code) diff --git a/querybook/server/app/server.py b/querybook/server/app/server.py index 1c1f9c4a9..c83ccdf48 100644 --- a/querybook/server/app/server.py +++ b/querybook/server/app/server.py @@ -5,6 +5,10 @@ from app.datasource import register, abort_request from app.flask_app import flask_app, limiter from const.path import WEBAPP_INDEX_PATH +from const.datasources import ( + RESOURCE_NOT_FOUND_STATUS_CODE, + LOCKED_FOR_DEPLOYMENT_STATUS_CODE, +) import datasources @@ -18,7 +22,7 @@ @register("/") @limiter.exempt def datasource_four_oh_four(*args, **kwargs): - abort_request(404) + abort_request(RESOURCE_NOT_FOUND_STATUS_CODE) @flask_app.route("/ping/") @@ -26,7 +30,7 @@ def datasource_four_oh_four(*args, **kwargs): def get_health_check(): """This is a health check endpoint""" if os.path.exists("/tmp/querybook/deploying"): - abort(503) + abort(LOCKED_FOR_DEPLOYMENT_STATUS_CODE) return "pong" diff --git a/querybook/server/const/datasources.py b/querybook/server/const/datasources.py index a7029df2c..e312cfe31 100644 --- a/querybook/server/const/datasources.py +++ b/querybook/server/const/datasources.py @@ -1,5 +1,10 @@ +OK_STATUS_CODE = 200 +UNKNOWN_CLIENT_ERROR_STATUS_CODE = 400 +UNAUTHORIZED_STATUS_CODE = 401 ACCESS_RESTRICTED_STATUS_CODE = 403 RESOURCE_NOT_FOUND_STATUS_CODE = 404 -UNAUTHORIZED_STATUS_CODE = 401 +INVALID_SEMANTIC_STATUS_CODE = 422 +LOCKED_FOR_DEPLOYMENT_STATUS_CODE = 423 +UNKNOWN_SERVER_ERROR_STATUS_CODE = 500 DS_PATH = "/ds" diff --git a/querybook/server/datasources/board.py b/querybook/server/datasources/board.py index cce89fc91..9e2dd8f48 100644 --- a/querybook/server/datasources/board.py +++ b/querybook/server/datasources/board.py @@ -8,6 +8,7 @@ get_data_doc_environment_ids, get_board_environment_ids, ) +from const.datasources import RESOURCE_NOT_FOUND_STATUS_CODE from logic import board as logic from logic import user as user_logic from logic.board_permission import assert_can_read, assert_can_edit, assert_is_owner @@ -51,7 +52,9 @@ def get_board_by_id(board_id, environment_id): assert_can_read(board_id, session=session) board = Board.get(id=board_id, session=session) - api_assert(board is not None, "Invalid board id", 404) + api_assert( + board is not None, "Invalid board id", RESOURCE_NOT_FOUND_STATUS_CODE + ) verify_environment_permission([board.environment_id]) return board.to_dict( extra_fields=["docs", "tables", "boards", "queries", "items"] diff --git a/querybook/server/datasources/metastore.py b/querybook/server/datasources/metastore.py index bc10a65bb..8310b2655 100644 --- a/querybook/server/datasources/metastore.py +++ b/querybook/server/datasources/metastore.py @@ -15,6 +15,7 @@ from const.impression import ImpressionItemType from const.metastore import DataTableWarningSeverity, MetadataType from const.time import seconds_in_a_day +from const.datasources import RESOURCE_NOT_FOUND_STATUS_CODE from lib.lineage.utils import lineage from lib.metastore.utils import DataTableFinder from lib.metastore import get_metastore_loader @@ -75,7 +76,7 @@ def get_table(table_id, with_schema=True, with_column=True, with_warnings=True): api_assert( table, "Table doesn't exist or has been deleted from Metastore", - status_code=404, + status_code=RESOURCE_NOT_FOUND_STATUS_CODE, ) verify_data_schema_permission(table.schema_id, session=session) result = table.to_dict(with_schema, with_column, with_warnings) diff --git a/querybook/server/datasources/query_execution.py b/querybook/server/datasources/query_execution.py index 2d97c5689..86cdcaf05 100644 --- a/querybook/server/datasources/query_execution.py +++ b/querybook/server/datasources/query_execution.py @@ -21,7 +21,10 @@ from lib.data_doc.meta import var_config_to_var_dict from lib.data_doc.doc_types import DataDocMetaVarConfig from const.query_execution import QueryExecutionExportStatus, QueryExecutionStatus -from const.datasources import RESOURCE_NOT_FOUND_STATUS_CODE +from const.datasources import ( + RESOURCE_NOT_FOUND_STATUS_CODE, + INVALID_SEMANTIC_STATUS_CODE, +) from logic import ( query_execution as logic, datadoc as datadoc_logic, @@ -458,7 +461,7 @@ def get_templated_query( query, var_config_to_var_dict(var_config), engine_id ) except QueryTemplatingError as e: - raise RequestException(e) + raise RequestException(e, status_code=INVALID_SEMANTIC_STATUS_CODE) @register("/query_execution/templated_query_params/", methods=["POST"]) diff --git a/querybook/server/logic/board_permission.py b/querybook/server/logic/board_permission.py index d0ddda017..51f0443bd 100644 --- a/querybook/server/logic/board_permission.py +++ b/querybook/server/logic/board_permission.py @@ -3,6 +3,10 @@ from app.datasource import api_assert from app.db import with_session from models.board import Board, BoardEditor +from const.datasources import ( + ACCESS_RESTRICTED_STATUS_CODE, + RESOURCE_NOT_FOUND_STATUS_CODE, +) class BoardDoesNotExist(Exception): @@ -58,10 +62,10 @@ def assert_can_edit(board_id, session=None): api_assert( user_can_edit(board_id, uid=current_user.id, session=session), "CANNOT_EDIT_BOARD", - 403, + ACCESS_RESTRICTED_STATUS_CODE, ) except BoardDoesNotExist: - api_assert(False, "BOARD_DNE", 404) + api_assert(False, "BOARD_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) @with_session @@ -70,10 +74,10 @@ def assert_can_read(board_id, session=None): api_assert( user_can_read(board_id, uid=current_user.id, session=session), "CANNOT_READ_BOARD", - 403, + ACCESS_RESTRICTED_STATUS_CODE, ) except BoardDoesNotExist: - api_assert(False, "BOARD_DNE", 404) + api_assert(False, "BOARD_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) @with_session @@ -85,7 +89,7 @@ def assert_is_owner(board_id, session=None): api_assert( board.owner_uid == current_user.id, "NOT_BOARD_OWNER", - 403, + ACCESS_RESTRICTED_STATUS_CODE, ) except BoardDoesNotExist: - api_assert(False, "BOARD_DNE", 404) + api_assert(False, "BOARD_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) diff --git a/querybook/server/logic/datadoc_permission.py b/querybook/server/logic/datadoc_permission.py index 475800673..d4e6265fa 100644 --- a/querybook/server/logic/datadoc_permission.py +++ b/querybook/server/logic/datadoc_permission.py @@ -4,6 +4,7 @@ from app.datasource import api_assert from app.db import with_session from models.datadoc import DataDoc, DataDocEditor +from const.datasources import UNAUTHORIZED_STATUS_CODE, RESOURCE_NOT_FOUND_STATUS_CODE class DocDoesNotExist(Exception): @@ -48,10 +49,10 @@ def assert_can_read(doc_id, session=None): api_assert( user_can_read(doc_id, uid=current_user.id, session=session), "CANNOT_READ_DATADOC", - 403, + UNAUTHORIZED_STATUS_CODE, ) except DocDoesNotExist: - api_assert(False, "DOC_DNE", 404) + api_assert(False, "DOC_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) @with_session @@ -60,10 +61,10 @@ def assert_can_write(doc_id, session=None): api_assert( user_can_write(doc_id, uid=current_user.id, session=session), "CANNOT_WRITE_DATADOC", - 403, + UNAUTHORIZED_STATUS_CODE, ) except DocDoesNotExist: - api_assert(False, "DOC_DNE", 404) + api_assert(False, "DOC_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) @with_session @@ -75,7 +76,7 @@ def assert_is_owner(doc_id, session=None): api_assert( doc.owner_uid == current_user.id, "NOT_DATADOC_OWNER", - 403, + UNAUTHORIZED_STATUS_CODE, ) except DocDoesNotExist: - api_assert(False, "DOC_DNE", 404) + api_assert(False, "DOC_DNE", RESOURCE_NOT_FOUND_STATUS_CODE) diff --git a/querybook/tests/test_app/test_server.py b/querybook/tests/test_app/test_server.py index f6acae8ab..186a5996b 100644 --- a/querybook/tests/test_app/test_server.py +++ b/querybook/tests/test_app/test_server.py @@ -15,7 +15,7 @@ def test_ping_deploy_api(flask_client, monkeypatch): "exists", lambda p: True if p == "/tmp/querybook/deploying" else False, ) - assert 503 == flask_client.get("/ping/").status_code + assert 423 == flask_client.get("/ping/").status_code def test_four_oh_four(flask_client, fake_user):