Fold Token and Registration validation errors into one error

flaskbb/auth/services/registration.py

@@ -13,7 +13,8 @@ from collections import namedtuple
 
 from sqlalchemy import func
 
-from ...core.auth.registration import UserRegistrationError, UserValidator
+from ...core.auth.registration import UserValidator
+from ...core.exceptions import ValidationError
 
 __all__ = (
     "UsernameRequirements", "UsernameValidator", "EmailUniquenessValidator",
@@ -33,7 +34,7 @@ class UsernameValidator(UserValidator):
     def validate(self, user_info):
         if not (self._requirements.min <= len(user_info.username) <=
                 self._requirements.max):
-            raise UserRegistrationError(
+            raise ValidationError(
                 'username',
                 'Username must be between {} and {} characters long'.format(
                     self._requirements.min, self._requirements.max
@@ -42,7 +43,7 @@ class UsernameValidator(UserValidator):
 
         is_blacklisted = user_info.username in self._requirements.blacklist
         if is_blacklisted:  # pragma: no branch
-            raise UserRegistrationError(
+            raise ValidationError(
                 'username',
                 '{} is a forbidden username'.format(user_info.username)
             )
@@ -58,7 +59,7 @@ class UsernameUniquenessValidator(UserValidator):
             func.lower(self.users.username) == user_info.username
         ).count()
         if count != 0:  # pragma: no branch
-            raise UserRegistrationError(
+            raise ValidationError(
                 'username',
                 '{} is already registered'.format(user_info.username)
             )
@@ -74,6 +75,6 @@ class EmailUniquenessValidator(UserValidator):
             func.lower(self.users.email) == user_info.email
         ).count()
         if count != 0:  # pragma: no branch
-            raise UserRegistrationError(
+            raise ValidationError(
                 'email', '{} is already registered'.format(user_info.email)
             )

flaskbb/auth/views.py

@@ -36,9 +36,9 @@ from flaskbb.utils.tokens import get_token_status
 
 from .services import registration
 from ..core.auth.password import ResetPasswordService
-from ..core.auth.registration import (RegistrationService, StopRegistration,
-                                      UserRegistrationInfo)
-from ..core.tokens import StopTokenVerification, TokenError
+from ..core.auth.registration import (RegistrationService, UserRegistrationInfo)
+from ..core.exceptions import ValidationError, StopValidation
+from ..core.tokens import TokenError
 from ..tokens import FlaskBBTokenSerializer
 from ..tokens.verifiers import EmailMatchesUserToken
 from ..user.repo import UserRepository
@@ -139,7 +139,7 @@ class Register(MethodView):
             service = self.registration_service_factory()
             try:
                 service.register(registration_info)
-            except StopRegistration as e:
+            except StopValidation as e:
                 form.populate_errors(e.reasons)
                 return render_template("auth/register.html", form=form)
 
@@ -218,7 +218,7 @@ class ResetPassword(MethodView):
             except TokenError as e:
                 flash(_(e.reason), 'danger')
                 return redirect(url_for('auth.forgot_password'))
-            except StopTokenVerification as e:
+            except StopValidation as e:
                 form.populate_errors(e.reasons)
                 form.token.data = token
                 return render_template("auth/reset_password.html", form=form)

flaskbb/core/auth/password.py

@@ -9,8 +9,8 @@
     :license: BSD, see LICENSE for more details
 """
 
-from ..tokens import (StopTokenVerification, TokenActions, TokenError,
-                      TokenVerificationError)
+from ..exceptions import StopValidation, ValidationError
+from ..tokens import TokenActions, TokenError
 
 
 class ResetPasswordService(object):
@@ -26,11 +26,11 @@ class ResetPasswordService(object):
         for verifier in self.token_verifiers:
             try:
                 verifier(token, email=email)
-            except TokenVerificationError as e:
+            except ValidationError as e:
                 errors.append((e.attribute, e.reason))
 
         if errors:
-            raise StopTokenVerification(errors)
+            raise StopValidation(errors)
 
     def reset_password(self, token, email, new_password):
         token = self.token_serializer.loads(token)

flaskbb/core/auth/registration.py

@@ -15,7 +15,7 @@ from abc import abstractmethod
 import attr
 
 from ..._compat import ABC
-from ...exceptions import BaseFlaskBBError
+from ..exceptions import ValidationError, StopValidation
 
 
 @attr.s(hash=True, cmp=False, repr=True, frozen=True)
@@ -27,37 +27,12 @@ class UserRegistrationInfo(object):
     group = attr.ib()
 
 
-class RegistrationError(BaseFlaskBBError):
-    pass
-
-
-class UserRegistrationError(RegistrationError):
-    """
-    Thrown when a user attempts to register but should
-    not be allowed to complete registration.
-
-    If the reason is not tied to a specific attribute then
-    the attribute property should be set to None.
-    """
-
-    def __init__(self, attribute, reason):
-        super(UserRegistrationError, self).__init__(reason)
-        self.attribute = attribute
-        self.reason = reason
-
-
-class StopRegistration(RegistrationError):
-    def __init__(self, reasons):
-        super(StopRegistration, self).__init__()
-        self.reasons = reasons
-
-
 class UserValidator(ABC):
     @abstractmethod
     def validate(self, user_info):
         """
         Used to check if a user should be allowed to register.
-        Should raise UserRegistrationError if the user should not be
+        Should raise ValidationError if the user should not be
         allowed to register.
         """
         return True
@@ -77,10 +52,10 @@ class RegistrationService(object):
         for v in self.validators:
             try:
                 v(user_info)
-            except UserRegistrationError as e:
+            except ValidationError as e:
                 failures.append((e.attribute, e.reason))
 
         if failures:
-            raise StopRegistration(failures)
+            raise StopValidation(failures)
 
         self.user_repo.add(user_info)

flaskbb/core/exceptions.py

@@ -0,0 +1,45 @@
+# -*- coding: utf-8 -*-
+"""
+    flaskbb.core.exceptions
+    ~~~~~~~~~~~~~~~~~~~~~~~
+
+    Exceptions raised by flaskbb.core,
+    forms the root of all exceptions in
+    FlaskBB.
+
+    :copyright: (c) 2014-2018 the FlaskBB Team
+    :license: BSD, see LICENSE for more details
+"""
+
+class BaseFlaskBBError(Exception):
+    "Root exception for FlaskBB"
+
+
+class ValidationError(BaseFlaskBBError):
+    """
+    Used to signal validation errors for things such as
+    token verification, user registration, etc.
+    """
+
+    def __init__(self, attribute, reason):
+        self.attribute = attribute
+        self.reason = reason
+        super(ValidationError, self).__init__((attribute, reason))
+
+
+class StopValidation(BaseFlaskBBError):
+    """
+    Raised from validation handlers to signal that
+    validation should end immediately and no further
+    processing should be done.
+
+    The reasons passed should be an iterable of
+    tuples consisting of `(attribute, reason)`
+
+    Can also be used to communicate all errors
+    raised during a validation run.
+    """
+
+    def __init__(self, reasons):
+        self.reasons = reasons
+        super(StopValidation, self).__init__(reasons)

flaskbb/core/tokens.py

@@ -14,8 +14,7 @@ from abc import abstractmethod
 import attr
 
 from .._compat import ABC
-from ..exceptions import BaseFlaskBBError
-
+from .exceptions import BaseFlaskBBError
 
 class TokenError(BaseFlaskBBError):
     """
@@ -43,35 +42,6 @@ class TokenError(BaseFlaskBBError):
         return cls('Token cannot be processed')
 
 
-class TokenVerificationError(BaseFlaskBBError):
-    """
-    Raised from token verifiers in order to signal an error.
-
-    This is not an exception representing an invalid token
-    because it is malformed, expired, etc. This is used
-    for issues such as the token's user doesn't match
-    other information provided.
-    """
-    def __init__(self, attribute, reason):
-        self.reason = reason
-        self.attribute = attribute
-        super(TokenVerificationError, self).__init__((attribute, reason))
-
-
-class StopTokenVerification(BaseFlaskBBError):
-    """
-    Raised from services using token verifies to signal all
-    errors associated with verifiying a token.
-
-    Alternatively, can be raised from a token verifier
-    to halt all further validation and immediately
-    signify a major error.
-    """
-    def __init__(self, reasons):
-        self.reasons = reasons
-        super(StopTokenVerification, self).__init__(reasons)
-
-
 # holder for token actions
 # not an enum so plugins can add to it
 class TokenActions:
@@ -109,6 +79,9 @@ class TokenVerifier(ABC):
     Used to verify the validatity of tokens post
     deserialization, such as an email matching the
     user id in the provided token.
+
+    Should raise a flaskbb.core.exceptions.ValidationError
+    if verification fails.
     """
     @abstractmethod
     def verify_token(self, token, **kwargs):

flaskbb/exceptions.py

@@ -8,15 +8,12 @@
     :license: BSD, see LICENSE for more details
 """
 from werkzeug.exceptions import HTTPException, Forbidden
+from .core.exceptions import BaseFlaskBBError
 
 
-class BaseFlaskBBError(Exception):
-    "Root exception for FlaskBB"
-    description = "An internal error has occured"
-
 
 class FlaskBBHTTPError(BaseFlaskBBError, HTTPException):
-    pass
+    description = "An internal error has occured"
 
 
 FlaskBBError = FlaskBBHTTPError

flaskbb/tokens/verifiers.py

@@ -8,7 +8,8 @@
     :license: BSD, see LICENSE for more details
 """
 
-from ..core.tokens import TokenVerifier, TokenVerificationError
+from ..core.tokens import TokenVerifier
+from ..core.exceptions import ValidationError
 
 
 class EmailMatchesUserToken(TokenVerifier):
@@ -24,4 +25,4 @@ class EmailMatchesUserToken(TokenVerifier):
     def verify_token(self, token, email, **kwargs):
         user = self.users.query.get(token.user_id)
         if user.email.lower() != email.lower():
-            raise TokenVerificationError("email", "Wrong email")
+            raise ValidationError("email", "Wrong email")

tests/core/auth/test_password.py

@@ -1,15 +1,15 @@
 import json
 
 import pytest
-from werkzeug.security import check_password_hash
-
 from flaskbb.core.auth import password
-from flaskbb.core.tokens import (StopTokenVerification, Token, TokenActions,
-                                 TokenError, TokenVerificationError)
+from flaskbb.core.exceptions import StopValidation, ValidationError
+from flaskbb.core.tokens import Token, TokenActions, TokenError
 from flaskbb.user.models import User
+from werkzeug.security import check_password_hash
 
 
 class SimpleTokenSerializer:
+
     @staticmethod
     def dumps(token):
         return json.dumps({'user_id': token.user_id, 'op': token.operation})
@@ -21,38 +21,46 @@ class SimpleTokenSerializer:
 
 
 class TestPasswordReset(object):
+
     def test_raises_token_error_if_not_a_password_reset(self):
-        service = password.ResetPasswordService(SimpleTokenSerializer, User,
-                                                [])
+        service = password.ResetPasswordService(
+            SimpleTokenSerializer, User, []
+        )
         raw_token = SimpleTokenSerializer.dumps(
-            Token(user_id=1, operation=TokenActions.ACTIVATE_ACCOUNT))
+            Token(user_id=1, operation=TokenActions.ACTIVATE_ACCOUNT)
+        )
 
         with pytest.raises(TokenError) as excinfo:
-            service.reset_password(raw_token, "some@e.mail",
-                                   "a great password!")
+            service.reset_password(
+                raw_token, "some@e.mail", "a great password!"
+            )
 
         assert "invalid" in str(excinfo.value)
 
-    def test_raises_StopTokenVerification_if_verifiers_fail(self):
+    def test_raises_StopValidation_if_verifiers_fail(self):
         token = SimpleTokenSerializer.dumps(
-            Token(user_id=1, operation=TokenActions.RESET_PASSWORD))
+            Token(user_id=1, operation=TokenActions.RESET_PASSWORD)
+        )
 
         def verifier(*a, **k):
-            raise TokenVerificationError('attr', 'no')
+            raise ValidationError('attr', 'no')
 
-        service = password.ResetPasswordService(SimpleTokenSerializer, User,
-                                                [verifier])
+        service = password.ResetPasswordService(
+            SimpleTokenSerializer, User, [verifier]
+        )
 
-        with pytest.raises(StopTokenVerification) as excinfo:
+        with pytest.raises(StopValidation) as excinfo:
             service.reset_password(token, "an@e.mail", "great password!")
         assert ("attr", "no") in excinfo.value.reasons
 
     def test_sets_user_password_to_provided_if_verifiers_pass(self, Fred):
         token = SimpleTokenSerializer.dumps(
-            Token(user_id=Fred.id, operation=TokenActions.RESET_PASSWORD))
+            Token(user_id=Fred.id, operation=TokenActions.RESET_PASSWORD)
+        )
 
-        service = password.ResetPasswordService(SimpleTokenSerializer, User,
-                                                [])
+        service = password.ResetPasswordService(
+            SimpleTokenSerializer, User, []
+        )
 
         service.reset_password(token, Fred.email, "newpasswordwhodis")
         assert check_password_hash(Fred.password, "newpasswordwhodis")

tests/core/auth/test_registration.py

@@ -1,45 +1,53 @@
 import pytest
-
 from flaskbb.core.auth import registration
+from flaskbb.core.exceptions import StopValidation, ValidationError
 from flaskbb.core.user.repo import UserRepository
 
 
 class RaisingValidator(registration.UserValidator):
+
     def validate(self, user_info):
-        raise registration.UserRegistrationError(
-            'test', 'just a little whoopsie-diddle')
+        raise ValidationError(
+            'test', 'just a little whoopsie-diddle'
+        )
 
 
-def test_doesnt_register_user_if_validator_fails_with_UserRegistrationError(
-        mocker):
+def test_doesnt_register_user_if_validator_fails_with_ValidationError(
+        mocker
+):
     repo = mocker.Mock(UserRepository)
     service = registration.RegistrationService([RaisingValidator()], repo)
 
-    with pytest.raises(registration.StopRegistration):
+    with pytest.raises(StopValidation):
         service.register(
             registration.UserRegistrationInfo(
                 username='fred',
                 password='lol',
                 email='fred@fred.fred',
                 language='fredspeak',
-                group=4))
+                group=4
+            )
+        )
 
     repo.add.assert_not_called()
 
 
 def test_gathers_up_all_errors_during_registration(mocker):
     repo = mocker.Mock(UserRepository)
-    service = registration.RegistrationService(
-        [RaisingValidator(), RaisingValidator()], repo)
+    service = registration.RegistrationService([
+        RaisingValidator(), RaisingValidator()
+    ], repo)
 
-    with pytest.raises(registration.StopRegistration) as excinfo:
+    with pytest.raises(StopValidation) as excinfo:
         service.register(
             registration.UserRegistrationInfo(
                 username='fred',
                 password='lol',
                 email='fred@fred.fred',
                 language='fredspeak',
-                group=4))
+                group=4
+            )
+        )
 
     repo.add.assert_not_called()
     assert len(excinfo.value.reasons) == 2
@@ -55,6 +63,7 @@ def test_registers_user_if_no_errors_occurs(mocker):
         password='lol',
         email='fred@fred.fred',
         language='fredspeak',
-        group=4)
+        group=4
+    )
     service.register(user_info)
     repo.add.assert_called_with(user_info)

tests/unit/auth/test_validators.py

@@ -3,20 +3,20 @@ from flaskbb.auth.services.registration import (EmailUniquenessValidator,
                                                 UsernameRequirements,
                                                 UsernameUniquenessValidator,
                                                 UsernameValidator)
-from flaskbb.core.auth.registration import (UserRegistrationError,
-                                            UserRegistrationInfo)
+from flaskbb.core.auth.registration import UserRegistrationInfo
+from flaskbb.core.exceptions import ValidationError
 from flaskbb.user.models import User
 
 
 def test_raises_if_username_too_short():
-    requirements = UsernameRequirements(
-        min=4, max=100, blacklist=set())
+    requirements = UsernameRequirements(min=4, max=100, blacklist=set())
     validator = UsernameValidator(requirements)
 
     registration = UserRegistrationInfo(
-        username='no', password='no', email='no@no.no', group=4, language='no')
+        username='no', password='no', email='no@no.no', group=4, language='no'
+    )
 
-    with pytest.raises(UserRegistrationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         validator(registration)
 
     assert excinfo.value.attribute == 'username'
@@ -24,14 +24,14 @@ def test_raises_if_username_too_short():
 
 
 def test_raises_if_username_too_long():
-    requirements = UsernameRequirements(
-        min=0, max=1, blacklist=set())
+    requirements = UsernameRequirements(min=0, max=1, blacklist=set())
     validator = UsernameValidator(requirements)
 
     registration = UserRegistrationInfo(
-        username='no', password='no', email='no@no.no', group=4, language='no')
+        username='no', password='no', email='no@no.no', group=4, language='no'
+    )
 
-    with pytest.raises(UserRegistrationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         validator(registration)
 
     assert excinfo.value.attribute == 'username'
@@ -39,14 +39,14 @@ def test_raises_if_username_too_long():
 
 
 def test_raises_if_username_in_blacklist():
-    requirements = UsernameRequirements(
-        min=1, max=100, blacklist=set(['no']))
+    requirements = UsernameRequirements(min=1, max=100, blacklist=set(['no']))
     validator = UsernameValidator(requirements)
 
     registration = UserRegistrationInfo(
-        username='no', password='no', email='no@no.no', group=4, language='no')
+        username='no', password='no', email='no@no.no', group=4, language='no'
+    )
 
-    with pytest.raises(UserRegistrationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         validator(registration)
 
     assert excinfo.value.attribute == 'username'
@@ -61,9 +61,10 @@ def test_raises_if_user_already_registered(Fred):
         email='fred@fred.fred',
         language='fred',
         group=4,
-        password='fred')
+        password='fred'
+    )
 
-    with pytest.raises(UserRegistrationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         validator(registration)
 
     assert excinfo.value.attribute == 'username'
@@ -77,9 +78,10 @@ def test_raises_if_user_email_already_registered(Fred):
         email='fred@fred.fred',
         language='fred',
         group=4,
-        password='fred')
+        password='fred'
+    )
 
-    with pytest.raises(UserRegistrationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         validator(registration)
 
     assert excinfo.value.attribute == 'email'

tests/unit/tokens/test_verifiers.py

@@ -1,6 +1,7 @@
 import pytest
 
-from flaskbb.core.tokens import Token, TokenActions, TokenVerificationError
+from flaskbb.core.tokens import Token, TokenActions
+from flaskbb.core.exceptions import ValidationError
 from flaskbb.tokens import verifiers
 from flaskbb.user.models import User
 
@@ -9,7 +10,7 @@ def test_raises_if_email_doesnt_match_token_user(Fred):
     verifier = verifiers.EmailMatchesUserToken(User)
     token = Token(user_id=1, operation=TokenActions.RESET_PASSWORD)
 
-    with pytest.raises(TokenVerificationError) as excinfo:
+    with pytest.raises(ValidationError) as excinfo:
         verifier(token, email="not really")
 
     assert excinfo.value.attribute == "email"