diff --git a/docs/security.rst b/docs/security.rst index f6ce4bedd1..7e8f08390b 100644 --- a/docs/security.rst +++ b/docs/security.rst @@ -374,6 +374,15 @@ Therefore, you can send tweets, post on the users Facebook, retrieve the user's Take a look at the `example `_ to get an idea of a simple use for this. +Authentication: Rate limiting +----------------------------- + +To prevent brute-forcing of credentials, FlaskApplicationBuilder applies rate limits to AuthViews in 4.2.0, so that +only 2 POST requests can be made every 5 seconds. This can be disabled by setting ``AUTH_RATE_LIMITED`` to +``False`` or can be changed by adjusting ``AUTH_RATE_LIMIT`` to, for example, ``1 per 10 seconds``. Take a look +at the `documentation `_ of Flask-Limiter for more options and +examples. + Role based ---------- diff --git a/flask_appbuilder/api/__init__.py b/flask_appbuilder/api/__init__.py index 31ef167bf8..c111b18309 100644 --- a/flask_appbuilder/api/__init__.py +++ b/flask_appbuilder/api/__init__.py @@ -76,6 +76,7 @@ from ..hooks import get_before_request_hooks, wrap_route_handler_with_hooks from ..models.filters import Filters from ..security.decorators import permission_name, protect +from ..utils.limit import Limit if TYPE_CHECKING: from flask_appbuilder import AppBuilder @@ -453,6 +454,18 @@ class GreetingApi(BaseApi): Use this attribute to override the tag name """ + limits: Optional[List[Limit]] = None + """ + List of limits for this api. + + Use it like this if you want to restrict the rate of requests to a view: + + class MyView(ModelView): + limits = [Limit("2 per 5 second")] + + or use the decorator @limit. + """ + def __init__(self) -> None: """ Initialization of base permissions @@ -490,7 +503,13 @@ def __init__(self) -> None: if self.base_permissions is None: self.base_permissions = set() is_add_base_permissions = True + + if self.limits is None: + self.limits = [] + for attr_name in dir(self): + if hasattr(getattr(self, attr_name), "_limit"): + self.limits.append(getattr(getattr(self, attr_name), "_limit")) # If include_route_methods is not None white list if ( self.include_route_methods is not None diff --git a/flask_appbuilder/base.py b/flask_appbuilder/base.py index 8776719f4c..96adf69330 100644 --- a/flask_appbuilder/base.py +++ b/flask_appbuilder/base.py @@ -446,6 +446,7 @@ def add_view( if self.app: self.register_blueprint(baseview) self._add_permission(baseview) + self.add_limits(baseview) self.add_link( name=name, href=href, @@ -564,6 +565,7 @@ def add_view_no_menu( baseview, endpoint=endpoint, static_folder=static_folder ) self._add_permission(baseview) + self.add_limits(baseview) else: log.warning(LOGMSG_WAR_FAB_VIEW_EXISTS.format(baseview.__class__.__name__)) return baseview @@ -644,6 +646,10 @@ def get_url_for_locale(self, lang: str) -> str: locale=lang, ) + def add_limits(self, baseview: "AbstractViewApi") -> None: + if hasattr(baseview, "limits"): + self.sm.add_limit_view(baseview) + def add_permissions(self, update_perms: bool = False) -> None: from flask_appbuilder.baseviews import AbstractViewApi diff --git a/flask_appbuilder/baseviews.py b/flask_appbuilder/baseviews.py index adf514109e..dec40b4023 100644 --- a/flask_appbuilder/baseviews.py +++ b/flask_appbuilder/baseviews.py @@ -181,8 +181,20 @@ class ContactModelView(ModelView): default_view = "list" """ the default view for this BaseView, to be used with url_for (method name) """ extra_args = None - """ dictionary for injecting extra arguments into template """ + + limits = None + """ + List of limits for this view. + + Use it like this if you want to restrict the rate of requests to a view: + + class MyView(ModelView): + limits = [Limit("2 per 5 second")] + + or use the decorator @limit. + """ + _apis = None def __init__(self): @@ -212,6 +224,9 @@ def __init__(self): self.base_permissions = set() is_add_base_permissions = True + if self.limits is None: + self.limits = [] + for attr_name in dir(self): # If include_route_methods is not None white list if ( @@ -239,6 +254,8 @@ def __init__(self): _extra = getattr(getattr(self, attr_name), "_extra") for key in _extra: self._apis[key] = _extra[key] + if hasattr(getattr(self, attr_name), "_limit"): + self.limits.append(getattr(getattr(self, attr_name), "_limit")) def create_blueprint(self, appbuilder, endpoint=None, static_folder=None): """ diff --git a/flask_appbuilder/security/decorators.py b/flask_appbuilder/security/decorators.py index 394423d06d..ba6e207b7f 100644 --- a/flask_appbuilder/security/decorators.py +++ b/flask_appbuilder/security/decorators.py @@ -1,5 +1,6 @@ import functools import logging +from typing import Optional, List, Union, Callable from flask import ( current_app, @@ -18,8 +19,10 @@ PERMISSION_PREFIX, ) from flask_jwt_extended import verify_jwt_in_request +from flask_limiter.wrappers import RequestLimit from flask_login import current_user +from flask_appbuilder.utils.limit import Limit log = logging.getLogger(__name__) @@ -228,3 +231,68 @@ def wraps(f): return f return wraps + + +def limit( + limit_value: Union[str, Callable[[], str]], + key_func: Optional[Callable[[], str]] = None, + per_method: bool = False, + methods: Optional[List[str]] = None, + error_message: Optional[str] = None, + exempt_when: Optional[Callable[[], bool]] = None, + override_defaults: bool = True, + deduct_when: Optional[Callable[[Response], bool]] = None, + on_breach: Optional[Callable[[RequestLimit], Optional[Response]]] = None, + cost: Union[int, Callable[[], int]] = 1, +): + """ + Decorator to be used for rate limiting individual routes or blueprints. + + :param limit_value: rate limit string or a callable that returns a + string. :ref:`ratelimit-string` for more details. + :param key_func: function/lambda to extract the unique + identifier for the rate limit. defaults to remote address of the + request. + :param per_method: whether the limit is sub categorized into the + http method of the request. + :param methods: if specified, only the methods in this list will + be rate limited (default: ``None``). + :param error_message: string (or callable that returns one) to override + the error message used in the response. + :param exempt_when: function/lambda used to decide if the rate + limit should skipped. + :param override_defaults: whether the decorated limit overrides + the default limits (Default: ``True``). + + .. note:: When used with a :class:`~BaseView` the meaning + of the parameter extends to any parents the blueprint instance is + registered under. For more details see :ref:`recipes:nested blueprints` + + :param deduct_when: a function that receives the current + :class:`flask.Response` object and returns True/False to decide if a + deduction should be done from the rate limit + :param on_breach: a function that will be called when this limit + is breached. If the function returns an instance of :class:`flask.Response` + that will be the response embedded into the :exc:`RateLimitExceeded` exception + raised. + :param cost: The cost of a hit or a function that + takes no parameters and returns the cost as an integer (Default: ``1``). + """ + + def wraps(f): + _limit = Limit( + limit_value=limit_value, + key_func=key_func, + per_method=per_method, + methods=methods, + error_message=error_message, + exempt_when=exempt_when, + override_defaults=override_defaults, + deduct_when=deduct_when, + on_breach=on_breach, + cost=cost, + ) + f._limit = _limit + return f + + return wraps diff --git a/flask_appbuilder/security/manager.py b/flask_appbuilder/security/manager.py index 4d3326364a..934cd4c7eb 100644 --- a/flask_appbuilder/security/manager.py +++ b/flask_appbuilder/security/manager.py @@ -9,6 +9,8 @@ from flask_babel import lazy_gettext as _ from flask_jwt_extended import current_user as current_user_jwt from flask_jwt_extended import JWTManager +from flask_limiter import Limiter +from flask_limiter.util import get_remote_address from flask_login import current_user, LoginManager from werkzeug.security import check_password_hash, generate_password_hash @@ -255,6 +257,10 @@ def __init__(self, appbuilder): app.config.setdefault("AUTH_LDAP_LASTNAME_FIELD", "sn") app.config.setdefault("AUTH_LDAP_EMAIL_FIELD", "mail") + # Rate limiting + app.config.setdefault("AUTH_RATE_LIMITED", True) + app.config.setdefault("AUTH_RATE_LIMIT", "2 per 5 second") + if self.auth_type == AUTH_OID: from flask_openid import OpenID @@ -285,6 +291,14 @@ def __init__(self, appbuilder): # Setup Flask-Jwt-Extended self.jwt_manager = self.create_jwt_manager(app) + # Setup Flask-Limiter + self.limiter = self.create_limiter(app) + + def create_limiter(self, app) -> Limiter: + limiter = Limiter(key_func=get_remote_address) + limiter.init_app(app) + return limiter + def create_login_manager(self, app) -> LoginManager: """ Override to implement your custom login manager instance @@ -489,6 +503,14 @@ def openid_providers(self): def oauth_providers(self): return self.appbuilder.get_app.config["OAUTH_PROVIDERS"] + @property + def is_auth_limited(self): + return self.appbuilder.get_app.config["AUTH_RATE_LIMITED"] + + @property + def auth_rate_limit(self): + return self.appbuilder.get_app.config["AUTH_RATE_LIMIT"] + @property def current_user(self): if current_user.is_authenticated: @@ -735,6 +757,12 @@ def register_views(self): self.appbuilder.add_view_no_menu(self.auth_view) + # this needs to be done after the view is added, otherwise the blueprint is not initialized + if self.is_auth_limited: + self.limiter.limit(self.auth_rate_limit, methods=["POST"])( + self.auth_view.blueprint + ) + self.user_view = self.appbuilder.add_view( self.user_view, "List Users", @@ -1548,6 +1576,22 @@ def get_user_menu_access(self, menu_names: List[str] = None) -> Set[str]: None, "menu_access", view_menus_name=menu_names ) + def add_limit_view(self, baseview): + if baseview.limits: + for limit in baseview.limits: + self.limiter.limit( + limit_value=limit.limit_value, + key_func=limit.key_func, + per_method=limit.per_method, + methods=limit.methods, + error_message=limit.error_message, + exempt_when=limit.exempt_when, + override_defaults=limit.override_defaults, + deduct_when=limit.deduct_when, + on_breach=limit.on_breach, + cost=limit.cost, + )(baseview.blueprint) + def add_permissions_view(self, base_permissions, view_menu): """ Adds a permission on a view menu to the backend diff --git a/flask_appbuilder/security/views.py b/flask_appbuilder/security/views.py index 3fe0b0b32f..9064d7e60a 100644 --- a/flask_appbuilder/security/views.py +++ b/flask_appbuilder/security/views.py @@ -9,7 +9,7 @@ from flask_appbuilder.baseviews import BaseView from flask_appbuilder.charts.views import DirectByChartView from flask_appbuilder.fieldwidgets import BS3PasswordFieldWidget -from flask_appbuilder.security.decorators import has_access +from flask_appbuilder.security.decorators import has_access, limit from flask_appbuilder.security.forms import ( DynamicForm, LoginForm_db, diff --git a/flask_appbuilder/tests/A_fixture/test_0_fixture.py b/flask_appbuilder/tests/A_fixture/test_0_fixture.py index 7627d2123f..269bf116f7 100644 --- a/flask_appbuilder/tests/A_fixture/test_0_fixture.py +++ b/flask_appbuilder/tests/A_fixture/test_0_fixture.py @@ -1,6 +1,8 @@ -from flask_appbuilder import SQLA -from freezegun import freeze_time +from datetime import datetime + +from hiro import Timeline +from flask_appbuilder import SQLA from ..base import FABTestCase from ..const import ( MODEL1_DATA_SIZE, @@ -23,15 +25,15 @@ def setUp(self): self.appbuilder = AppBuilder(self.app, self.db.session) def test_data(self): - with freeze_time("2020-01-01"): + with Timeline(start=datetime(2020, 1, 1)).freeze(): insert_data(self.db.session, MODEL1_DATA_SIZE) def test_create_admin(self): - with freeze_time("2020-01-01"): + with Timeline(start=datetime(2020, 1, 1)).freeze(): self.create_admin_user(self.appbuilder, USERNAME_ADMIN, PASSWORD_ADMIN) def test_create_ro_user(self): - with freeze_time("2020-01-01"): + with Timeline(start=datetime(2020, 1, 1)).freeze(): self.create_user( self.appbuilder, USERNAME_READONLY, diff --git a/flask_appbuilder/tests/config_api.py b/flask_appbuilder/tests/config_api.py index a22ada4225..7141ba73dc 100644 --- a/flask_appbuilder/tests/config_api.py +++ b/flask_appbuilder/tests/config_api.py @@ -18,3 +18,5 @@ [".*", "can_show"], ] } + +RATELIMIT_ENABLED = False diff --git a/flask_appbuilder/tests/config_oauth.py b/flask_appbuilder/tests/config_oauth.py index e162563d05..45ba8328ec 100644 --- a/flask_appbuilder/tests/config_oauth.py +++ b/flask_appbuilder/tests/config_oauth.py @@ -34,3 +34,5 @@ # The default user self registration role for all users AUTH_USER_REGISTRATION_ROLE = "Admin" + +RATELIMIT_ENABLED = False diff --git a/flask_appbuilder/tests/config_security.py b/flask_appbuilder/tests/config_security.py index 8c1624e0c1..cbba135ee6 100644 --- a/flask_appbuilder/tests/config_security.py +++ b/flask_appbuilder/tests/config_security.py @@ -15,3 +15,4 @@ "FAB_ROLE1": [["Model1View", "can_list"], ["Model2View", "can_list"]], "FAB_ROLE2": [["Model3View", "can_list"], ["Model4View", "can_list"]], } +RATELIMIT_ENABLED = False diff --git a/flask_appbuilder/tests/config_security_api.py b/flask_appbuilder/tests/config_security_api.py index 53261d4176..d336a4d21a 100644 --- a/flask_appbuilder/tests/config_security_api.py +++ b/flask_appbuilder/tests/config_security_api.py @@ -20,3 +20,4 @@ [".*", "can_show"], ] } +RATELIMIT_ENABLED = False diff --git a/flask_appbuilder/tests/const.py b/flask_appbuilder/tests/const.py index bdc26c6be8..bdecf13f5c 100644 --- a/flask_appbuilder/tests/const.py +++ b/flask_appbuilder/tests/const.py @@ -7,3 +7,4 @@ MAX_PAGE_SIZE = 25 USERNAME_READONLY = "readonly" PASSWORD_READONLY = "readonly" +INVALID_LOGIN_STRING = "Invalid login" diff --git a/flask_appbuilder/tests/security/test_mvc_security.py b/flask_appbuilder/tests/security/test_mvc_security.py index a71be5ffa4..1e5e0567cf 100644 --- a/flask_appbuilder/tests/security/test_mvc_security.py +++ b/flask_appbuilder/tests/security/test_mvc_security.py @@ -3,12 +3,16 @@ from flask_appbuilder.models.sqla.filters import FilterEqual from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_appbuilder.security.sqla.models import User - from ..base import BaseMVCTestCase -from ..const import PASSWORD_ADMIN, PASSWORD_READONLY, USERNAME_ADMIN, USERNAME_READONLY +from ..const import ( + PASSWORD_ADMIN, + PASSWORD_READONLY, + USERNAME_ADMIN, + USERNAME_READONLY, + INVALID_LOGIN_STRING, +) from ..sqla.models import Model1, Model2 -INVALID_LOGIN_STRING = "Invalid login" PASSWORD_COMPLEXITY_ERROR = ( "Must have at least two capital letters, " "one special character, two digits, three lower case letters and " @@ -203,7 +207,7 @@ def test_db_login_invalid_control_characters_next_url(self): self.client, USERNAME_ADMIN, PASSWORD_ADMIN, - next_url=u"\u0001" + "sample.com", + next_url="\u0001" + "sample.com", follow_redirects=False, ) assert response.location == "http://localhost/" diff --git a/flask_appbuilder/tests/security/test_rate_limiter.py b/flask_appbuilder/tests/security/test_rate_limiter.py new file mode 100644 index 0000000000..29d94c0d2c --- /dev/null +++ b/flask_appbuilder/tests/security/test_rate_limiter.py @@ -0,0 +1,78 @@ +import logging + +import hiro +import jinja2 +from flask import Flask + +from flask_appbuilder import AppBuilder, SQLA, BaseView +from flask_appbuilder.tests.base import FABTestCase +from ..const import INVALID_LOGIN_STRING, USERNAME_ADMIN, PASSWORD_ADMIN +from ...api import BaseApi, expose +from ...security.decorators import limit + + +class LimiterTestCase(FABTestCase): + def setUp(self): + self.app = Flask(__name__) + self.app.jinja_env.undefined = jinja2.StrictUndefined + self.app.config.from_object("flask_appbuilder.tests.config_api") + self.app.config["RATELIMIT_ENABLED"] = True + logging.basicConfig(level=logging.ERROR) + + self.db = SQLA(self.app) + self.appbuilder = AppBuilder(self.app, self.db.session) + + class Base1Api(BaseApi): + @limit("2 per second") + @expose("/test1") + def test1(self, **kwargs): + return self.response(200, message="OK") + + class TestView(BaseView): + @limit("1 per second") + @expose("/test") + def test(self): + return self.render_template(template=self.appbuilder.base_template) + + self.appbuilder.add_api(Base1Api) + self.appbuilder.add_view(TestView, name="testview") + + def test_default_auth_rate_limit(self): + client = self.app.test_client() + + with hiro.Timeline().freeze(): + rv = self.browser_login(client, USERNAME_ADMIN, "wrong_password") + data = rv.data.decode("utf-8") + self.assertIn(INVALID_LOGIN_STRING, data) + + rv = self.browser_login(client, USERNAME_ADMIN, "wrong_password") + data = rv.data.decode("utf-8") + self.assertIn(INVALID_LOGIN_STRING, data) + + rv = self.browser_login(client, USERNAME_ADMIN, "wrong_password") + self.assertEqual(rv.status_code, 429) + + def test_api_rate_decorated_limit(self): + client = self.app.test_client() + token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) + uri = "api/v1/base1api/test1" + with hiro.Timeline().freeze(): + rv = self.auth_client_get(client, token, uri) + self.assertEqual(rv.status_code, 200) + + rv = self.auth_client_get(client, token, uri) + self.assertEqual(rv.status_code, 200) + + rv = self.auth_client_get(client, token, uri) + self.assertEqual(rv.status_code, 429) + + def view_rate_decorated_limit(self): + client = self.app.test_client() + token = self.login(client, USERNAME_ADMIN, PASSWORD_ADMIN) + uri = "/test" + with hiro.Timeline().freeze(): + rv = self.auth_client_get(client, token, uri) + self.assertEqual(rv.status_code, 200) + + rv = self.auth_client_get(client, token, uri) + self.assertEqual(rv.status_code, 429) diff --git a/flask_appbuilder/utils/limit.py b/flask_appbuilder/utils/limit.py new file mode 100644 index 0000000000..def014182c --- /dev/null +++ b/flask_appbuilder/utils/limit.py @@ -0,0 +1,20 @@ +import dataclasses +from typing import Union, Callable, Optional, Tuple + +from flask import Response +from flask_limiter.wrappers import RequestLimit + + +@dataclasses.dataclass +class Limit: + limit_value: Union[Callable[[], str], str] + key_func: Callable[[], str] + scope: Optional[Union[str, Callable[[str], str]]] = None + methods: Optional[Tuple[str, ...]] = None + error_message: Optional[str] = None + exempt_when: Optional[Callable[[], bool]] = None + override_defaults: Optional[bool] = False + deduct_when: Optional[Callable[[Response], bool]] = None + on_breach: Optional[Callable[[RequestLimit], Optional[Response]]] = None + per_method: bool = False + cost: Optional[Union[Callable[[], int], int]] = None diff --git a/requirements-dev.txt b/requirements-dev.txt index cc210fed73..9bba01d1bf 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,6 +2,7 @@ black==19.3b0 coverage==5.5 flake8-import-order==0.18.1 flake8==3.9.2 +hiro==0.5.1 jmespath==0.9.5 mypy==0.910 mypy-extensions==0.4.3 @@ -10,5 +11,4 @@ nose==1.3.7 parameterized==0.8.1 pip-tools==6.8.0 tox==3.24.3 -freezegun==1.2.1 types-PyYAML==6.0.12.2 diff --git a/requirements.txt b/requirements.txt index f51c6764ee..3a2f97f17a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ # -# This file is autogenerated by pip-compile with python 3.9 -# To update, run: +# This file is autogenerated by pip-compile with Python 3.10 +# by the following command: # # pip-compile # @@ -16,6 +16,10 @@ click==8.0.4 # flask colorama==0.4.4 # via Flask-AppBuilder (setup.py) +commonmark==0.9.1 + # via rich +deprecated==1.2.13 + # via limits dnspython==2.2.1 # via email-validator email-validator==1.1.3 @@ -25,6 +29,7 @@ flask==2.0.3 # Flask-AppBuilder (setup.py) # flask-babel # flask-jwt-extended + # flask-limiter # flask-login # flask-sqlalchemy # flask-wtf @@ -32,14 +37,14 @@ flask-babel==2.0.0 # via Flask-AppBuilder (setup.py) flask-jwt-extended==4.3.1 # via Flask-AppBuilder (setup.py) +flask-limiter==3.1.0 + # via Flask-AppBuilder (setup.py) flask-login==0.6.0 # via Flask-AppBuilder (setup.py) flask-sqlalchemy==2.5.1 # via Flask-AppBuilder (setup.py) flask-wtf==1.0.1 # via Flask-AppBuilder (setup.py) -greenlet==1.1.2 - # via sqlalchemy idna==3.3 # via email-validator itsdangerous==2.1.1 @@ -52,6 +57,8 @@ jinja2==3.0.3 # flask-babel jsonschema==3.2.0 # via Flask-AppBuilder (setup.py) +limits==2.8.0 + # via flask-limiter markupsafe==2.1.1 # via # jinja2 @@ -65,10 +72,16 @@ marshmallow-enum==1.5.1 # via Flask-AppBuilder (setup.py) marshmallow-sqlalchemy==0.26.1 # via Flask-AppBuilder (setup.py) +ordered-set==4.1.0 + # via flask-limiter packaging==21.3 - # via marshmallow + # via + # limits + # marshmallow prison==0.2.1 # via Flask-AppBuilder (setup.py) +pygments==2.13.0 + # via rich pyjwt==2.3.0 # via # Flask-AppBuilder (setup.py) @@ -85,6 +98,8 @@ pytz==2021.1 # flask-babel pyyaml==5.4.1 # via apispec +rich==12.6.0 + # via flask-limiter six==1.16.0 # via # jsonschema @@ -99,11 +114,17 @@ sqlalchemy==1.4.29 # sqlalchemy-utils sqlalchemy-utils==0.37.8 # via Flask-AppBuilder (setup.py) +typing-extensions==4.4.0 + # via + # flask-limiter + # limits werkzeug==2.0.3 # via # flask # flask-jwt-extended # flask-login +wrapt==1.14.1 + # via deprecated wtforms==3.0.1 # via # Flask-AppBuilder (setup.py) diff --git a/setup.py b/setup.py index 0cc57bb4ae..f8b11978c4 100644 --- a/setup.py +++ b/setup.py @@ -51,6 +51,7 @@ def desc(): "email_validator>=1.0.5, <2", "Flask>=2, <3", "Flask-Babel>=1, <3", + "Flask-Limiter>3,<4", "Flask-Login>=0.3, <0.7", "Flask-SQLAlchemy>=2.4, <3", "Flask-WTF>=0.14.2, <2", @@ -72,7 +73,7 @@ def desc(): "oauth": ["Authlib>=0.14, <2.0.0"], "openid": ["Flask-OpenID>=1.2.5, <2"], }, - tests_require=["nose>=1.0", "mockldap>=0.3.0"], + tests_require=["nose>=1.0", "mockldap>=0.3.0", "hiro>=0.5.1"], classifiers=[ "Development Status :: 5 - Production/Stable", "Environment :: Web Environment",