Skip to content

Commit

Permalink
[REF] auth_brute_force: Cover all auth entrypoints (OCA/server-tools#…
Browse files Browse the repository at this point in the history
…1219)

To fix OCA/server-tools#1125 I needed to refactor the addon. To whitelist IPs now you use a config parameter, which renders res.banned.remote model unneeded.

The fix is affected by odoo/odoo#24183 and will not work until it gets fixed upstream due to the technical limitations implied.
  • Loading branch information
yajo authored and nileshsheliya committed Sep 5, 2022
1 parent 7ad12b6 commit d1f7a51
Show file tree
Hide file tree
Showing 18 changed files with 834 additions and 292 deletions.
53 changes: 30 additions & 23 deletions auth_brute_force/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,18 @@ extra information about remote IP.
Configuration
=============

Once installed, you can change the ir.config_parameter value for the key
'auth_brute_force.max_attempt_qty' (10 by default) that define the max number
of attempts allowed before the user was banned.
You can use these configuration parameters that control this addon behavior:

* ``auth_brute_force.whitelist_remotes`` is a comma-separated list of
whitelisted IPs. Failures from these remotes are ignored.

* ``auth_brute_force.max_by_ip`` defaults to 50, and indicates the maximum
successive failures allowed for an IP. After hitting the limit, the IP gets
banned.

* ``auth_brute_force.max_by_ip_user`` defaults to 10, and indicates the
maximum successive failures allowed for any IP and user combination.
After hitting the limit, that user and IP combination is banned.

Usage
=====
Expand All @@ -34,17 +43,14 @@ Admin user have the possibility to unblock a banned IP.
Logging
-------

This module generates some WARNING logs, in the three following cases:
This module generates some WARNING logs, in the following cases:

* Authentication failed from remote '127.0.0.1'. Login tried : 'admin'.
Attempt 1 / 10.
* When the IP limit is reached: *Authentication failed from remote 'x.x.x.x'.
The remote has been banned. Login tried: xxxx.*

* Authentication failed from remote '127.0.0.1'. The remote has been banned.
Login tried : 'admin'.

* Authentication tried from remote '127.0.0.1'. The request has been ignored
because the remote has been banned after 10 attempts without success. Login
tried : 'admin'.
* When the IP+user combination limit is reached:
*Authentication failed from remote 'x.x.x.x'.
The remote and login combination has been banned. Login tried: xxxx.*

Screenshot
----------
Expand All @@ -53,13 +59,9 @@ Screenshot

.. image:: /auth_brute_force/static/description/screenshot_attempts_list.png

**Detail of a banned IP**

.. image:: /auth_brute_force/static/description/screenshot_custom_ban.png


.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
:alt: Try me on Runbot
:alt: Try me on Runbot
:target: https://runbot.odoo-community.org/runbot/149/10.0

For further information, please visit:
Expand All @@ -69,14 +71,18 @@ For further information, please visit:
Known issues / Roadmap
======================

* The ID used to identify a remote request is the IP provided in the request
(key 'REMOTE_ADDR').
* Remove 🐒 patch for https://github.com/odoo/odoo/issues/24183 in v12.

* Depending of server and / or user network configuration, the idenfication
of the user can be wrong, and mainly in the following cases:
* If the Odoo server is behind an Apache / NGinx proxy without redirection,
all the request will be have the value '127.0.0.1' for the REMOTE_ADDR key;
* If some users are behind the same Internet Service Provider, if a user is
banned, all the other users will be banned too;

* If the Odoo server is behind an Apache / NGinx proxy and it is not properly
configured, all requests will use the same IP address. Blocking such IP
could render Odoo unusable for all users! **Make sure your logs output the
correct IP for werkzeug traffic before installing this addon.**

* The IP metadata retrieval should use a better system. `See details here
<https://github.com/OCA/server-tools/pull/1219/files#r187014504>`_.

Bug Tracker
===========
Expand All @@ -94,6 +100,7 @@ Contributors

* Sylvain LE GAL (https://twitter.com/legalsylvain)
* David Vidal <david.vidal@tecnativa.com>
* Jairo Llopis <jairo.llopis@tecnativa.com>

Maintainer
----------
Expand Down
3 changes: 1 addition & 2 deletions auth_brute_force/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
# -*- encoding: utf-8 -*-
# -*- coding: utf-8 -*-

from . import models
from . import controllers
15 changes: 7 additions & 8 deletions auth_brute_force/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,21 @@
# Copyright 2017 Tecnativa - David Vidal
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
{
'name': 'Authentification - Brute-force Attack',
'version': '10.0.1.0.0',
'name': 'Authentification - Brute-Force Filter',
'version': '10.0.2.0.0',
'category': 'Tools',
'summary': "Tracks Authentication Attempts and Prevents Brute-force"
" Attacks module",
'summary': "Track Authentication Attempts and Prevent Brute-force Attacks",
'author': "GRAP, "
"Tecnativa, "
"Odoo Community Association (OCA)",
'website': 'http://www.grap.coop',
'website': 'https://github.com/OCA/server-tools',
'license': 'AGPL-3',
'depends': [
'web',
],
# If we don't depend on it, it would inhibit this addon
"auth_crypt",
],
'data': [
'security/ir_model_access.yml',
'data/ir_config_parameter.xml',
'views/view.xml',
'views/action.xml',
'views/menu.xml',
Expand Down
3 changes: 0 additions & 3 deletions auth_brute_force/controllers/__init__.py

This file was deleted.

76 changes: 0 additions & 76 deletions auth_brute_force/controllers/main.py

This file was deleted.

15 changes: 0 additions & 15 deletions auth_brute_force/data/ir_config_parameter.xml

This file was deleted.

50 changes: 50 additions & 0 deletions auth_brute_force/migrations/10.0.2.0.0/pre-migrate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# -*- coding: utf-8 -*-
# Copyright 2018 Tecnativa - Jairo Llopis
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).

from psycopg2 import IntegrityError


def migrate(cr, version):
# Fix typo across DB
cr.execute(
""" UPDATE res_authentication_attempt
SET result = 'successful'
WHERE result = 'successfull'""",
)
# Store whitelist IPs in new format
cr.execute(
""" SELECT remote
FROM res_banned_remote
WHERE active IS FALSE""",
)
remotes = {record[0] for record in cr.fetchall()}
try:
with cr.savepoint():
cr.execute(
"INSERT INTO ir_config_parameter (key, value) VALUES (%s, %s)",
(
"auth_brute_force.whitelist_remotes",
",".join(remotes),
),
)
except IntegrityError:
# Parameter already exists
cr.execute(
"SELECT value FROM ir_config_parameter WHERE key = %s",
("auth_brute_force.whitelist_remotes",)
)
current = set(cr.fetchall()[0][0].split(","))
cr.execute(
"UPDATE ir_config_parameter SET value = %s WHERE key = %s",
(",".join(current | remotes),
"auth_brute_force.whitelist_remotes"),
)
# Update the configured IP limit parameter
cr.execute(
"UPDATE ir_config_parameter SET key = %s WHERE key = %s",
(
"auth_brute_force.whitelist_remotes",
"auth_brute_force.max_by_ip",
)
)
2 changes: 1 addition & 1 deletion auth_brute_force/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# -*- encoding: utf-8 -*-

from . import res_banned_remote
from . import res_authentication_attempt
from . import res_users

0 comments on commit d1f7a51

Please sign in to comment.