diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py index 02a66a20..ab5bf1c3 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/exceptions/api_error.py @@ -20,6 +20,11 @@ from SpiffWorkflow.exceptions import WorkflowTaskException from SpiffWorkflow.specs.base import TaskSpec # type: ignore from SpiffWorkflow.task import Task # type: ignore +from spiffworkflow_backend.services.authentication_service import NotAuthorizedError +from spiffworkflow_backend.services.authentication_service import TokenInvalidError +from spiffworkflow_backend.services.authentication_service import TokenNotProvidedError +from spiffworkflow_backend.services.authentication_service import UserNotLoggedInError + api_error_blueprint = Blueprint("api_error_blueprint", __name__) @@ -172,7 +177,12 @@ def handle_exception(exception: Exception) -> flask.wrappers.Response: set_user_sentry_context() sentry_link = None - if not isinstance(exception, ApiError) or exception.error_code != "invalid_token": + # we want to capture_exception to log the exception to sentry, but we don't want to log: + # 1. ApiErrors that are just invalid tokens + # 2. NotAuthorizedError + if ( + not isinstance(exception, ApiError) or exception.error_code != "invalid_token" + ) and not isinstance(exception, NotAuthorizedError): id = capture_exception(exception) if isinstance(exception, ApiError): @@ -193,17 +203,30 @@ def handle_exception(exception: Exception) -> flask.wrappers.Response: # an event id or send out tags like username current_app.logger.exception(exception) + error_code = "internal_server_error" + status_code = 500 + if ( + isinstance(exception, NotAuthorizedError) + or isinstance(exception, TokenNotProvidedError) + or isinstance(exception, TokenInvalidError) + ): + error_code = "not_authorized" + status_code = 403 + if isinstance(exception, UserNotLoggedInError): + error_code = "not_authenticated" + status_code = 401 + # set api_exception like this to avoid confusing mypy - # and what type the object is + # about what type the object is api_exception = None if isinstance(exception, ApiError): api_exception = exception else: api_exception = ApiError( - error_code="internal_server_error", + error_code=error_code, message=f"{exception.__class__.__name__}", sentry_link=sentry_link, - status_code=500, + status_code=status_code, ) return make_response(jsonify(api_exception), api_exception.status_code) diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py index 1793aab6..5c9c4708 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authentication_service.py @@ -11,7 +11,6 @@ from flask import current_app from flask import redirect from werkzeug.wrappers import Response -from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.refresh_token import RefreshTokenModel @@ -20,7 +19,21 @@ class MissingAccessTokenError(Exception): """MissingAccessTokenError.""" +class NotAuthorizedError(Exception): + pass + + +class RefreshTokenStorageError(Exception): + pass + + +class UserNotLoggedInError(Exception): + pass + + # These could be either 'id' OR 'access' tokens and we can't always know which + + class TokenExpiredError(Exception): """TokenExpiredError.""" @@ -29,6 +42,10 @@ class TokenInvalidError(Exception): """TokenInvalidError.""" +class TokenNotProvidedError(Exception): + pass + + class AuthenticationProviderTypes(enum.Enum): """AuthenticationServiceProviders.""" @@ -183,9 +200,8 @@ class AuthenticationService: db.session.commit() except Exception as e: db.session.rollback() - raise ApiError( - error_code="store_refresh_token_error", - message=f"We could not store the refresh token. Original error is {e}", + raise RefreshTokenStorageError( + f"We could not store the refresh token. Original error is {e}", ) from e @staticmethod diff --git a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py index 19f9f418..a72effd4 100644 --- a/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py +++ b/spiffworkflow-backend/src/spiffworkflow_backend/services/authorization_service.py @@ -21,7 +21,6 @@ from SpiffWorkflow.task import Task as SpiffTask # type: ignore from sqlalchemy import or_ from sqlalchemy import text -from spiffworkflow_backend.exceptions.api_error import ApiError from spiffworkflow_backend.helpers.api_version import V1_API_PATH_PREFIX from spiffworkflow_backend.models.db import db from spiffworkflow_backend.models.group import GroupModel @@ -34,6 +33,11 @@ from spiffworkflow_backend.models.user import UserModel from spiffworkflow_backend.models.user import UserNotFoundError from spiffworkflow_backend.models.user_group_assignment import UserGroupAssignmentModel from spiffworkflow_backend.routes.openid_blueprint import openid_blueprint +from spiffworkflow_backend.services.authentication_service import NotAuthorizedError +from spiffworkflow_backend.services.authentication_service import TokenExpiredError +from spiffworkflow_backend.services.authentication_service import TokenInvalidError +from spiffworkflow_backend.services.authentication_service import TokenNotProvidedError +from spiffworkflow_backend.services.authentication_service import UserNotLoggedInError from spiffworkflow_backend.services.group_service import GroupService from spiffworkflow_backend.services.user_service import UserService @@ -98,20 +102,16 @@ class AuthorizationService: def verify_sha256_token(cls, auth_header: Optional[str]) -> None: """Verify_sha256_token.""" if auth_header is None: - raise ApiError( - error_code="unauthorized", - message="", - status_code=403, + raise TokenNotProvidedError( + "unauthorized", ) received_sign = auth_header.split("sha256=")[-1].strip() secret = current_app.config["GITHUB_WEBHOOK_SECRET"].encode() expected_sign = HMAC(key=secret, msg=request.data, digestmod=sha256).hexdigest() if not compare_digest(received_sign, expected_sign): - raise ApiError( - error_code="unauthorized", - message="", - status_code=403, + raise TokenInvalidError( + "unauthorized", ) @classmethod @@ -393,10 +393,8 @@ class AuthorizationService: authorization_exclusion_list = ["permissions_check"] if not hasattr(g, "user"): - raise ApiError( - error_code="user_not_logged_in", - message="User is not logged in. Please log in", - status_code=401, + raise UserNotLoggedInError( + "User is not logged in. Please log in", ) api_view_function = current_app.view_functions[request.endpoint] @@ -416,13 +414,11 @@ class AuthorizationService: if has_permission: return None - raise ApiError( - error_code="unauthorized", - message=( + raise NotAuthorizedError( + ( f"User {g.user.username} is not authorized to perform requested action:" f" {permission_string} - {request.path}" ), - status_code=403, ) @staticmethod @@ -440,13 +436,11 @@ class AuthorizationService: payload = jwt.decode(auth_token, options={"verify_signature": False}) return payload except jwt.ExpiredSignatureError as exception: - raise ApiError( - "token_expired", + raise TokenExpiredError( "The Authentication token you provided expired and must be renewed.", ) from exception except jwt.InvalidTokenError as exception: - raise ApiError( - "token_invalid", + raise TokenInvalidError( ( "The Authentication token you provided is invalid. You need a new" " token. " diff --git a/spiffworkflow-frontend/src/components/ProcessGroupForm.tsx b/spiffworkflow-frontend/src/components/ProcessGroupForm.tsx index 79ab8253..c82157d7 100644 --- a/spiffworkflow-frontend/src/components/ProcessGroupForm.tsx +++ b/spiffworkflow-frontend/src/components/ProcessGroupForm.tsx @@ -35,7 +35,7 @@ export default function ProcessGroupForm({ }; const hasValidIdentifier = (identifierToCheck: string) => { - return identifierToCheck.match(/^[a-z0-9][0-9a-z-]+[a-z0-9]$/); + return identifierToCheck.match(/^[a-z0-9][0-9a-z-]*[a-z0-9]$/); }; const handleFormSubmission = (event: any) => {