Better test auth flows

flaskbb/auth/plugins.py

@@ -7,17 +7,21 @@
     :copyright: (c) 2014-2018 the FlaskBB Team.
     :license: BSD, see LICENSE for more details
 """
-from flask import flash
+from flask import flash, redirect, url_for
 from flask_babelplus import gettext as _
-from flask_login import login_user
+from flask_login import current_user, login_user, logout_user
 
 from . import impl
+from ..core.auth.authentication import ForceLogout
 from ..user.models import User
 from ..utils.settings import flaskbb_config
 from .services.authentication import (BlockUnactivatedUser, ClearFailedLogins,
                                       DefaultFlaskBBAuthProvider,
                                       MarkFailedLogin)
 from .services.factories import account_activator_factory
+from .services.reauthentication import (ClearFailedLoginsOnReauth,
+                                        DefaultFlaskBBReauthProvider,
+                                        MarkFailedReauth)
 
 
 @impl
@@ -53,3 +57,30 @@ def flaskbb_post_authenticate(user):
 @impl
 def flaskbb_authentication_failed(identifier):
     MarkFailedLogin().handle_authentication_failure(identifier)
+
+
+@impl(trylast=True)
+def flaskbb_reauth_attempt(user, secret):
+    return DefaultFlaskBBReauthProvider().reauthenticate(user, secret)
+
+
+@impl
+def flaskbb_reauth_failed(user):
+    MarkFailedReauth().handle_reauth_failure(user)
+
+
+@impl
+def flaskbb_post_reauth(user):
+    ClearFailedLoginsOnReauth().handle_post_reauth(user)
+
+
+@impl
+def flaskbb_errorhandlers(app):
+
+    @app.errorhandler(ForceLogout)
+    def handle_force_logout(error):
+        if current_user:
+            logout_user()
+            if error.reason:
+                flash(error.reason, 'danger')
+        return redirect(url_for('forum.index'))

flaskbb/auth/services/__init__.py

@@ -12,8 +12,16 @@
 """
 
 from .activation import AccountActivator
+from .authentication import (BlockTooManyFailedLogins, BlockUnactivatedUser,
+                             DefaultFlaskBBAuthProvider,
+                             FailedLoginConfiguration, MarkFailedLogin,
+                             PluginAuthenticationManager)
 from .factories import (account_activator_factory,
+                        authentication_manager_factory,
+                        reauthentication_manager_factory,
                         registration_service_factory, reset_service_factory)
 from .password import ResetPasswordService
-from .registration import (EmailUniquenessValidator, UsernameRequirements,
-                           UsernameUniquenessValidator, UsernameValidator)
+from .registration import (
+    EmailUniquenessValidator, UsernameRequirements,
+    UsernameUniquenessValidator, UsernameValidator
+)

flaskbb/auth/services/authentication.py

@@ -9,7 +9,7 @@
     :copyright: (c) 2014-2018 the FlaskBB Team.
     :license: BSD, see LICENSE for more details
 """
-
+import logging
 from datetime import datetime
 
 import attr
@@ -18,6 +18,7 @@ from pytz import UTC
 from werkzeug import check_password_hash
 
 from ...core.auth.authentication import (AuthenticationFailureHandler,
+                                         AuthenticationManager,
                                          AuthenticationProvider,
                                          PostAuthenticationHandler,
                                          StopAuthentication)
@@ -25,6 +26,8 @@ from ...extensions import db
 from ...user.models import User
 from ...utils.helpers import time_utcnow
 
+logger = logging.getLogger(__name__)
+
 
 @attr.s(frozen=True)
 class FailedLoginConfiguration(object):
@@ -106,3 +109,32 @@ class ClearFailedLogins(PostAuthenticationHandler):
 
     def handle_post_auth(self, user):
         user.login_attempts = 0
+
+
+class PluginAuthenticationManager(AuthenticationManager):
+
+    def __init__(self, plugin_manager, session):
+        self.plugin_manager = plugin_manager
+        self.session = session
+
+    def authenticate(self, identifier, secret):
+        try:
+            user = self.plugin_manager.hook.flaskbb_authenticate(
+                identifier=identifier, secret=secret
+            )
+            if user is None:
+                raise StopAuthentication(_("Wrong username or password."))
+            self.plugin_manager.hook.flaskbb_post_authenticate(user=user)
+            return user
+        except StopAuthentication as e:
+            self.plugin_manager.hook.flaskbb_authentication_failed(
+                identifier=identifier
+            )
+            raise
+        finally:
+            try:
+                self.session.commit()
+            except Exception:
+                logger.exception("Exception while processing login")
+                self.session.rollback()
+                raise

flaskbb/auth/services/factories.py

@@ -20,7 +20,9 @@ from ...user.models import User
 from ...user.repo import UserRepository
 from ...utils.settings import flaskbb_config
 from .activation import AccountActivator
+from .authentication import PluginAuthenticationManager
 from .password import ResetPasswordService
+from .reauthentication import PluginReauthenticationManager
 from .registration import (EmailUniquenessValidator, RegistrationService,
                            UsernameRequirements, UsernameUniquenessValidator,
                            UsernameValidator)
@@ -62,3 +64,10 @@ def account_activator_factory():
         current_app.config['SECRET_KEY'], expiry=timedelta(hours=1)
     )
     return AccountActivator(token_serializer, User)
+
+
+def authentication_manager_factory():
+    return PluginAuthenticationManager(current_app.pluggy, db.session)
+
+def reauthentication_manager_factory():
+    return PluginReauthenticationManager(current_app.pluggy, db.session)

flaskbb/auth/services/reauthentication.py

@@ -0,0 +1,69 @@
+# -*- coding: utf-8 -*-
+"""
+    flaskbb.auth.services.reauthentication
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+    Tools for handling reauthentication needs inside FlaskBB.
+
+    :copyright: (c) 2014-2018 the FlaskBB Team
+    :license: BSD, see LICENSE for more details
+"""
+
+import logging
+
+from flask_babelplus import gettext as _
+from werkzeug.security import check_password_hash
+
+from ...core.auth.authentication import (PostReauthenticateHandler,
+                                         ReauthenticateFailureHandler,
+                                         ReauthenticateManager,
+                                         ReauthenticateProvider,
+                                         StopAuthentication)
+from ...utils.helpers import time_utcnow
+
+logger = logging.getLogger(__name__)
+
+
+class DefaultFlaskBBReauthProvider(ReauthenticateProvider):
+
+    def reauthenticate(self, user, secret):
+        if check_password_hash(user.password, secret):  # pragma: no branch
+            return True
+
+
+class ClearFailedLoginsOnReauth(PostReauthenticateHandler):
+
+    def handle_post_reauth(self, user):
+        user.login_attempts = 0
+
+
+class MarkFailedReauth(ReauthenticateFailureHandler):
+
+    def handle_reauth_failure(self, user):
+        user.login_attempts += 1
+        user.last_failed_login = time_utcnow()
+
+
+class PluginReauthenticationManager(ReauthenticateManager):
+
+    def __init__(self, plugin_manager, session):
+        self.plugin_manager = plugin_manager
+        self.session = session
+
+    def reauthenticate(self, user, secret):
+        try:
+            result = self.plugin_manager.hook.flaskbb_reauth_attempt(
+                user=user, secret=secret
+            )
+            if not result:
+                raise StopAuthentication(_("Wrong password."))
+            self.plugin_manager.hook.flaskbb_post_reauth(user=user)
+        except StopAuthentication as e:
+            self.plugin_manager.hook.flaskbb_reauth_failed(user=user)
+            raise
+        finally:
+            try:
+                self.session.commit()
+            except Exception:
+                logger.exception("Exception while processing login")
+                self.session.rollback()
+                raise

flaskbb/auth/views.py

@@ -17,7 +17,6 @@ from flask.views import MethodView
 from flask_babelplus import gettext as _
 from flask_login import (confirm_login, current_user, login_fresh,
                          login_required, login_user, logout_user)
-
 from flaskbb.auth.forms import (ForgotPasswordForm, LoginForm,
                                 LoginRecaptchaForm, ReauthForm, RegisterForm,
                                 RequestActivationForm, ResetPasswordForm)
@@ -34,8 +33,10 @@ from ..core.auth.registration import UserRegistrationInfo
 from ..core.exceptions import StopValidation, ValidationError
 from ..core.tokens import TokenError
 from .plugins import impl
-from .services import (account_activator_factory, registration_service_factory,
-                       reset_service_factory)
+from .services import (account_activator_factory,
+                       authentication_manager_factory,
+                       reauthentication_manager_factory,
+                       registration_service_factory, reset_service_factory)
 
 logger = logging.getLogger(__name__)
 
@@ -52,6 +53,9 @@ class Logout(MethodView):
 class Login(MethodView):
     decorators = [anonymous_required]
 
+    def __init__(self, authentication_manager_factory):
+        self.authentication_manager_factory = authentication_manager_factory
+
     def form(self):
         if enforce_recaptcha(limiter):
             return LoginRecaptchaForm()
@@ -63,27 +67,17 @@ class Login(MethodView):
     def post(self):
         form = self.form()
         if form.validate_on_submit():
+            auth_manager = self.authentication_manager_factory()
             try:
-                user = current_app.pluggy.hook.flaskbb_authenticate(
+                user = auth_manager.authenticate(
                     identifier=form.login.data, secret=form.password.data
                 )
-                if user is None:
-                    raise StopAuthentication(_("Wrong username or password."))
-                current_app.pluggy.hook.flaskbb_post_authenticate(user=user)
                 login_user(user, remember=form.remember_me.data)
                 return redirect_or_next(url_for("forum.index"))
             except StopAuthentication as e:
                 flash(e.reason, "danger")
-                current_app.pluggy.hook.flaskbb_authentication_failed(
-                    identifier=form.login.data
-                )
-            finally:
-                try:
-                    db.session.commit()
-                except Exception:
-                    logger.exception("Exception while processing login")
-                    db.session.rollback()
-                    flash(_("Unrecoverable error while handling login"))
+            except Exception:
+                flash(_("Unrecoverable error while handling login"))
 
         return render_template("auth/login.html", form=form)
 
@@ -92,6 +86,9 @@ class Reauth(MethodView):
     decorators = [login_required, limiter.exempt]
     form = ReauthForm
 
+    def __init__(self, reauthentication_factory):
+        self.reauthentication_factory = reauthentication_factory
+
     def get(self):
         if not login_fresh():
             return render_template("auth/reauth.html", form=self.form())
@@ -100,12 +97,21 @@ class Reauth(MethodView):
     def post(self):
         form = self.form()
         if form.validate_on_submit():
-            if current_user.check_password(form.password.data):
+
+            reauth_manager = self.reauthentication_factory()
+            try:
+                user = reauth_manager.reauthenticate(
+                    user=current_user, secret=form.password.data
+                )
                 confirm_login()
                 flash(_("Reauthenticated."), "success")
                 return redirect_or_next(current_user.url)
+            except StopAuthentication as e:
+                flash(e.reason, "danger")
+            except Exception:
+                flash(_("Unrecoverable error while handling reauthentication"))
+                raise
 
-            flash(_("Wrong password."), "danger")
         return render_template("auth/reauth.html", form=form)
 
 
@@ -361,8 +367,22 @@ def flaskbb_load_blueprints(app):
     )(auth)
 
     register_view(auth, routes=['/logout'], view_func=Logout.as_view('logout'))
-    register_view(auth, routes=['/login'], view_func=Login.as_view('login'))
-    register_view(auth, routes=['/reauth'], view_func=Reauth.as_view('reauth'))
+    register_view(
+        auth,
+        routes=['/login'],
+        view_func=Login.as_view(
+            'login',
+            authentication_manager_factory=authentication_manager_factory
+        )
+    )
+    register_view(
+        auth,
+        routes=['/reauth'],
+        view_func=Reauth.as_view(
+            'reauth',
+            reauthentication_factory=reauthentication_manager_factory
+        )
+    )
     register_view(
         auth,
         routes=['/register'],
@@ -406,4 +426,5 @@ def flaskbb_load_blueprints(app):
             account_activator_factory=account_activator_factory
         )
     )
+
     app.register_blueprint(auth, url_prefix=app.config['AUTH_URL_PREFIX'])

flaskbb/core/auth/authentication.py

@@ -23,7 +23,31 @@ class StopAuthentication(BaseFlaskBBError):
         self.reason = reason
 
 
+class ForceLogout(BaseFlaskBBError):
+    """
+    Used to forcefully log a user out.
+    """
+
+    def __init__(self, reason):
+        super(ForceLogout, self).__init__(reason)
+        self.reason = reason
+
+
+class AuthenticationManager(ABC):
+
+    @abstractmethod
+    def authenticate(self, identifier, secret):
+        """
+        Manages the entire authentication process in FlaskBB.
+
+        If a user is successfully authenticated, it is returned
+        from this method.
+        """
+        pass
+
+
 class AuthenticationProvider(ABC):
+
     @abstractmethod
     def authenticate(self, identifier, secret):
         pass
@@ -33,6 +57,7 @@ class AuthenticationProvider(ABC):
 
 
 class AuthenticationFailureHandler(ABC):
+
     @abstractmethod
     def handle_authentication_failure(self, identifier):
         pass
@@ -42,9 +67,50 @@ class AuthenticationFailureHandler(ABC):
 
 
 class PostAuthenticationHandler(ABC):
+
     @abstractmethod
     def handle_post_auth(self, user):
         pass
 
     def __call__(self, user):
         self.handle_post_auth(user)
+
+
+class ReauthenticateManager(ABC):
+
+    @abstractmethod
+    def reauthenticate(self, user, secret):
+        pass
+
+    def __call__(self, user, secret):
+        pass
+
+
+class ReauthenticateProvider(ABC):
+
+    @abstractmethod
+    def reauthenticate(self, user, secret):
+        pass
+
+    def __call__(self, user, secret):
+        self.handle_reauth(user, secret)
+
+
+class ReauthenticateFailureHandler(ABC):
+
+    @abstractmethod
+    def handle_reauth_failure(self, user):
+        pass
+
+    def __call__(self, user):
+        self.handle_reauth_failure(user)
+
+
+class PostReauthenticateHandler(ABC):
+
+    @abstractmethod
+    def handle_post_reauth(self, user):
+        pass
+
+    def __call__(self, user):
+        self.handle_post_reauth(user)

flaskbb/plugins/spec.py

@@ -262,6 +262,62 @@ def flaskbb_authentication_failed(identifier):
     """
 
 
+@spec(firstresult=True)
+def flaskbb_reauth_attempt(user, secret):
+    """Hook for handling reauth in FlaskBB
+
+    These hooks receive the currently authenticated user
+    and the entered secret. Only the first response from
+    this hook is considered -- similar to the authenticate
+    hooks. A successful attempt should return True, otherwise
+    None for an unsuccessful or untried reauth from an
+    implementation. Reauth will be considered a failure if
+    no implementation return True.
+
+    If a hook decides that a reauthenticate attempt should
+    cease, it may raise StopAuthentication.
+
+    Example of checking secret or passing to the next implementer::
+
+        @impl
+        def flaskbb_reauth_attempt(user, secret):
+            if check_password(user.password, secret):
+                return True
+
+
+    Example of forcefully ending reauth::
+
+        @impl
+        def flaskbb_reauth_attempt(user, secret):
+            if user.login_attempts > 5:
+                raise StopAuthentication(_("Too many failed authentication attempts"))
+    """
+
+
+@spec
+def flaskbb_post_reauth(user):
+    """Hook called after successfully reauthenticating.
+
+    These hooks are called a user has passed the flaskbb_reauth_attempt
+    hooks but before their reauth is confirmed so a post reauth implementer
+    may still force a reauth to fail by raising StopAuthentication.
+
+    Results from these hooks are not considered.
+    """
+
+@spec
+def flaskbb_reauth_failed(user):
+    """Hook called if a reauth fails.
+
+    These hooks will only be called if no implementation
+    for flaskbb_reauth_attempt returns a True result or if
+    an implementation raises StopAuthentication.
+
+    If an implementation raises ForceLogout it should register
+    itself as trylast to give other reauth failed handlers an
+    opprotunity to run first.
+    """
+
 # Form hooks
 @spec
 def flaskbb_form_new_post(form):

tests/unit/auth/test_authentication.py

@@ -1,12 +1,16 @@
 from datetime import datetime, timedelta
 
 import pytest
+from flaskbb.auth.services import authentication as auth
+from flaskbb.core.auth.authentication import (AuthenticationFailureHandler,
+                                              AuthenticationProvider,
+                                              PostAuthenticationHandler,
+                                              StopAuthentication)
+from flaskbb.plugins import spec
 from freezegun import freeze_time
+from pluggy import HookimplMarker
 from pytz import UTC
 
-from flaskbb.auth.services import authentication as auth
-from flaskbb.core.auth.authentication import StopAuthentication
-
 pytestmark = pytest.mark.usefixtures('default_settings')
 
 
@@ -105,3 +109,91 @@ class TestBlockUnactivatedUser(object):
             self.handler.handle_post_auth(unactivated_user)
 
         assert 'In order to use your account' in excinfo.value.reason
+
+
+class TestPluginAuthenticationManager(object):
+
+    def raises_stop_authentication_if_user_isnt_authenticated(
+            self, plugin_manager, mocker, database
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        auth = mocker.MagicMock(spec=AuthenticationProvider)
+        plugin_manager.register(self.impls(auth=auth))
+
+        with pytest.raises(StopAuthentication) as excinfo:
+            service.authenticate('doesnt exist', 'nope')
+
+        auth.assert_called_once_with(identifier='doesnt exist', secret='nope')
+        assert excinfo.value.reason == "Wrong username or password."
+
+    def test_runs_failed_hooks_when_stopauthentication_is_raised(
+            self, plugin_manager, mocker, database
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        failure = mocker.MagicMock(spec=AuthenticationFailureHandler)
+        plugin_manager.register(self.impls(failure=failure))
+
+        with pytest.raises(StopAuthentication):
+            service.authenticate('doesnt exist', 'nope')
+
+        failure.assert_called_once_with(identifier='doesnt exist')
+
+    def test_runs_post_auth_handler_if_user_authenticates(
+            self, plugin_manager, mocker, Fred, database
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        auth = mocker.MagicMock(spec=AuthenticationProvider, return_value=Fred)
+        post_auth = mocker.MagicMock(spec=PostAuthenticationHandler)
+        plugin_manager.register(self.impls(auth=auth, post_auth=post_auth))
+
+        service.authenticate(Fred.email, 'fred')
+
+        auth.assert_called_once_with(identifier=Fred.email, secret='fred')
+        post_auth.assert_called_once_with(user=Fred)
+
+    def test_reraises_if_session_commit_fails(
+            self, mocker, plugin_manager, Fred
+    ):
+
+        class NotAnActualException(Exception):
+            pass
+
+        db = mocker.Mock()
+        db.session.commit.side_effect = NotAnActualException
+        service = self._get_auth_manager(plugin_manager, db)
+
+        with pytest.raises(NotAnActualException):
+            service.authenticate('doesnt exist', 'nope')
+
+        db.session.rollback.assert_called_once_with()
+
+    def _get_auth_manager(self, plugin_manager, db):
+        plugin_manager.add_hookspecs(spec)
+        return auth.PluginAuthenticationManager(
+            plugin_manager, session=db.session
+        )
+
+    @staticmethod
+    def impls(auth=None, post_auth=None, failure=None):
+        impl = HookimplMarker('flaskbb')
+
+        class Impls:
+            if auth is not None:
+
+                @impl
+                def flaskbb_authenticate(self, identifier, secret):
+                    return auth(identifier=identifier, secret=secret)
+
+            if post_auth is not None:
+
+                @impl
+                def flaskbb_post_authenticate(self, user):
+                    post_auth(user=user)
+
+            if failure is not None:
+
+                @impl
+                def flaskbb_authentication_failed(self, identifier):
+                    failure(identifier=identifier)
+
+        return Impls()

tests/unit/auth/test_reauthentication.py

@@ -0,0 +1,131 @@
+from datetime import datetime
+
+import pytest
+from flaskbb.auth.services import reauthentication as reauth
+from flaskbb.core.auth.authentication import (PostReauthenticateHandler,
+                                              ReauthenticateFailureHandler,
+                                              ReauthenticateProvider,
+                                              StopAuthentication)
+from flaskbb.plugins import spec
+from freezegun import freeze_time
+from pluggy import HookimplMarker
+from pytz import UTC
+
+pytestmark = pytest.mark.usefixtures('default_settings')
+
+
+def test_default_reauth_returns_true_if_secret_matches_user(Fred):
+    service = reauth.DefaultFlaskBBReauthProvider()
+
+    assert service.reauthenticate(Fred, 'fred')
+
+
+def test_clears_failed_logins_attempts(Fred):
+    service = reauth.ClearFailedLoginsOnReauth()
+    Fred.login_attempts = 1000
+    service.handle_post_reauth(Fred)
+    assert Fred.login_attempts == 0
+
+
+@freeze_time(datetime(2018, 1, 1, 13, 30))
+def test_marks_failed_login_attempt(Fred):
+    service = reauth.MarkFailedReauth()
+    Fred.login_attempts = 1
+    Fred.last_failed_login = datetime.min.replace(tzinfo=UTC)
+
+    service.handle_reauth_failure(Fred)
+
+    assert Fred.login_attempts == 2
+    assert Fred.last_failed_login == datetime(2018, 1, 1, 13, 30, tzinfo=UTC)
+
+
+class TestPluginAuthenticationManager(object):
+
+    def raises_stop_authentication_if_user_isnt_reauthenticated(
+            self, plugin_manager, mocker, database, Fred
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        reauth = mocker.MagicMock(spec=ReauthenticateProvider)
+        plugin_manager.register(self.impls(reauth=reauth))
+
+        with pytest.raises(StopAuthentication) as excinfo:
+            service.reauthenticate(Fred, 'nope')
+
+        reauth.assert_called_once_with(user=Fred, secret='nope')
+        assert excinfo.value.reason == "Wrong password."
+
+    def test_runs_failed_hooks_when_stopauthentication_is_raised(
+            self, plugin_manager, mocker, database, Fred
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        failure = mocker.MagicMock(spec=ReauthenticateFailureHandler)
+        plugin_manager.register(self.impls(failure=failure))
+
+        with pytest.raises(StopAuthentication):
+            service.reauthenticate(Fred, 'nope')
+
+        failure.assert_called_once_with(user=Fred)
+
+    def test_runs_post_reauth_handler_if_user_authenticates(
+            self, plugin_manager, mocker, Fred, database
+    ):
+        service = self._get_auth_manager(plugin_manager, database)
+        reauth = mocker.MagicMock(
+            spec=ReauthenticateProvider, return_value=Fred
+        )
+        post_reauth = mocker.MagicMock(spec=PostReauthenticateHandler)
+        plugin_manager.register(
+            self.impls(reauth=reauth, post_reauth=post_reauth)
+        )
+
+        service.reauthenticate(Fred, 'fred')
+
+        reauth.assert_called_once_with(user=Fred, secret='fred')
+        post_reauth.assert_called_once_with(user=Fred)
+
+    def test_reraises_if_session_commit_fails(
+            self, mocker, plugin_manager, Fred
+    ):
+
+        class NotAnActualException(Exception):
+            pass
+
+        db = mocker.Mock()
+        db.session.commit.side_effect = NotAnActualException
+        service = self._get_auth_manager(plugin_manager, db)
+
+        with pytest.raises(NotAnActualException):
+            service.reauthenticate('doesnt exist', 'nope')
+
+        db.session.rollback.assert_called_once_with()
+
+    def _get_auth_manager(self, plugin_manager, db):
+        plugin_manager.add_hookspecs(spec)
+        return reauth.PluginReauthenticationManager(
+            plugin_manager, session=db.session
+        )
+
+    @staticmethod
+    def impls(reauth=None, post_reauth=None, failure=None):
+        impl = HookimplMarker('flaskbb')
+
+        class Impls:
+            if reauth is not None:
+
+                @impl
+                def flaskbb_reauth_attempt(self, user, secret):
+                    return reauth(user=user, secret=secret)
+
+            if post_reauth is not None:
+
+                @impl
+                def flaskbb_post_reauth(self, user):
+                    post_reauth(user=user)
+
+            if failure is not None:
+
+                @impl
+                def flaskbb_reauth_failed(self, user):
+                    failure(user=user)
+
+        return Impls()