Skip to content

Commit edf3300

Browse files
gergosimonyigergosimonyi
authored
policies/reputation: limit reputation score (#14008)
* add limits to reputation score * limit reputation score limits Upper to non-negative, Lower to non-positive * simplify tests * "fix" bandit false-positives * move magic numbers to constants Is it too much to ask for a world in which I can just import these straight from Python?
1 parent 5d9c40e commit edf3300

File tree

7 files changed

+150
-20
lines changed

7 files changed

+150
-20
lines changed

‎authentik/policies/reputation/signals.py‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from django.contrib.auth.signals import user_logged_in
44
from django.db import transaction
5-
from django.db.models import F
65
from django.dispatch import receiver
76
from django.http import HttpRequest
87
from structlog.stdlib import get_logger
@@ -13,30 +12,45 @@
1312
from authentik.policies.reputation.models import Reputation, reputation_expiry
1413
from authentik.root.middleware import ClientIPMiddleware
1514
from authentik.stages.identification.signals import identification_failed
15+
from authentik.tenants.utils import get_current_tenant
1616

1717
LOGGER = get_logger()
1818

1919

20+
def clamp(value, min, max):
21+
return sorted([min, value, max])[1]
22+
23+
2024
def update_score(request: HttpRequest, identifier: str, amount: int):
2125
"""Update score for IP and User"""
2226
remote_ip = ClientIPMiddleware.get_client_ip(request)
27+
tenant = get_current_tenant()
28+
new_score = clamp(amount, tenant.reputation_lower_limit, tenant.reputation_upper_limit)
2329

2430
with transaction.atomic():
2531
reputation, created = Reputation.objects.select_for_update().get_or_create(
2632
ip=remote_ip,
2733
identifier=identifier,
2834
defaults={
29-
"score": amount,
35+
"score": clamp(
36+
amount, tenant.reputation_lower_limit, tenant.reputation_upper_limit
37+
),
3038
"ip_geo_data": GEOIP_CONTEXT_PROCESSOR.city_dict(remote_ip) or {},
3139
"ip_asn_data": ASN_CONTEXT_PROCESSOR.asn_dict(remote_ip) or {},
3240
"expires": reputation_expiry(),
3341
},
3442
)
3543

3644
if not created:
37-
reputation.score = F("score") + amount
45+
new_score = clamp(
46+
reputation.score + amount,
47+
tenant.reputation_lower_limit,
48+
tenant.reputation_upper_limit,
49+
)
50+
reputation.score = new_score
3851
reputation.save()
39-
LOGGER.info("Updated score", amount=amount, for_user=identifier, for_ip=remote_ip)
52+
53+
LOGGER.info("Updated score", amount=new_score, for_user=identifier, for_ip=remote_ip)
4054

4155

4256
@receiver(login_failed)

‎authentik/policies/reputation/tests.py‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
from authentik.lib.generators import generate_id
77
from authentik.policies.reputation.api import ReputationPolicySerializer
88
from authentik.policies.reputation.models import Reputation, ReputationPolicy
9+
from authentik.policies.reputation.signals import update_score
910
from authentik.policies.types import PolicyRequest
1011
from authentik.stages.password import BACKEND_INBUILT
1112
from authentik.stages.password.stage import authenticate
13+
from authentik.tenants.models import DEFAULT_REPUTATION_LOWER_LIMIT, DEFAULT_REPUTATION_UPPER_LIMIT
1214

1315

1416
class TestReputationPolicy(TestCase):
@@ -17,36 +19,48 @@ class TestReputationPolicy(TestCase):
1719
def setUp(self):
1820
self.request_factory = RequestFactory()
1921
self.request = self.request_factory.get("/")
20-
self.test_ip = "127.0.0.1"
21-
self.test_username = "test"
22+
self.ip = "127.0.0.1"
23+
self.username = "username"
24+
self.password = generate_id()
2225
# We need a user for the one-to-one in userreputation
23-
self.user = User.objects.create(username=self.test_username)
26+
self.user = User.objects.create(username=self.username)
27+
self.user.set_password(self.password)
2428
self.backends = [BACKEND_INBUILT]
2529

2630
def test_ip_reputation(self):
2731
"""test IP reputation"""
2832
# Trigger negative reputation
29-
authenticate(
30-
self.request, self.backends, username=self.test_username, password=self.test_username
31-
)
32-
self.assertEqual(Reputation.objects.get(ip=self.test_ip).score, -1)
33+
authenticate(self.request, self.backends, username=self.username, password=self.username)
34+
self.assertEqual(Reputation.objects.get(ip=self.ip).score, -1)
3335

3436
def test_user_reputation(self):
3537
"""test User reputation"""
3638
# Trigger negative reputation
37-
authenticate(
38-
self.request, self.backends, username=self.test_username, password=self.test_username
39-
)
40-
self.assertEqual(Reputation.objects.get(identifier=self.test_username).score, -1)
39+
authenticate(self.request, self.backends, username=self.username, password=self.username)
40+
self.assertEqual(Reputation.objects.get(identifier=self.username).score, -1)
4141

4242
def test_update_reputation(self):
4343
"""test reputation update"""
44-
Reputation.objects.create(identifier=self.test_username, ip=self.test_ip, score=43)
44+
Reputation.objects.create(identifier=self.username, ip=self.ip, score=4)
4545
# Trigger negative reputation
46-
authenticate(
47-
self.request, self.backends, username=self.test_username, password=self.test_username
46+
authenticate(self.request, self.backends, username=self.username, password=self.username)
47+
self.assertEqual(Reputation.objects.get(identifier=self.username).score, 3)
48+
49+
def test_reputation_lower_limit(self):
50+
"""test reputation lower limit"""
51+
Reputation.objects.create(identifier=self.username, ip=self.ip)
52+
update_score(self.request, identifier=self.username, amount=-1000)
53+
self.assertEqual(
54+
Reputation.objects.get(identifier=self.username).score, DEFAULT_REPUTATION_LOWER_LIMIT
55+
)
56+
57+
def test_reputation_upper_limit(self):
58+
"""test reputation upper limit"""
59+
Reputation.objects.create(identifier=self.username, ip=self.ip)
60+
update_score(self.request, identifier=self.username, amount=1000)
61+
self.assertEqual(
62+
Reputation.objects.get(identifier=self.username).score, DEFAULT_REPUTATION_UPPER_LIMIT
4863
)
49-
self.assertEqual(Reputation.objects.get(identifier=self.test_username).score, 42)
5064

5165
def test_policy(self):
5266
"""Test Policy"""

‎authentik/tenants/api/settings.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ class Meta:
2020
"default_user_change_email",
2121
"default_user_change_username",
2222
"event_retention",
23+
"reputation_lower_limit",
24+
"reputation_upper_limit",
2325
"footer_links",
2426
"gdpr_compliance",
2527
"impersonation",

‎authentik/tenants/migrations/0005_tenant_reputation_lower_limit_and_more.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Generated by Django 5.0.14 on 2025-04-14 07:50
2+
3+
import django.core.validators
4+
from django.db import migrations, models
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
("authentik_tenants", "0004_tenant_impersonation_require_reason"),
11+
]
12+
13+
operations = [
14+
migrations.AddField(
15+
model_name="tenant",
16+
name="reputation_lower_limit",
17+
field=models.IntegerField(
18+
default=-5,
19+
help_text="Reputation cannot decrease lower than this value. Zero or negative.",
20+
validators=[django.core.validators.MaxValueValidator(0)],
21+
),
22+
),
23+
migrations.AddField(
24+
model_name="tenant",
25+
name="reputation_upper_limit",
26+
field=models.IntegerField(
27+
default=5,
28+
help_text="Reputation cannot increase higher than this value. Zero or positive.",
29+
validators=[django.core.validators.MinValueValidator(0)],
30+
),
31+
),
32+
]

‎authentik/tenants/models.py‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from django.apps import apps
77
from django.core.exceptions import ValidationError
8-
from django.core.validators import MinValueValidator
8+
from django.core.validators import MaxValueValidator, MinValueValidator
99
from django.db import models
1010
from django.db.utils import IntegrityError
1111
from django.dispatch import receiver
@@ -25,6 +25,8 @@
2525

2626
DEFAULT_TOKEN_DURATION = "days=1" # nosec
2727
DEFAULT_TOKEN_LENGTH = 60
28+
DEFAULT_REPUTATION_LOWER_LIMIT = -5
29+
DEFAULT_REPUTATION_UPPER_LIMIT = 5
2830

2931

3032
def _validate_schema_name(name):
@@ -70,6 +72,16 @@ class Tenant(TenantMixin, SerializerModel):
7072
"Events will be deleted after this duration.(Format: weeks=3;days=2;hours=3,seconds=2)."
7173
),
7274
)
75+
reputation_lower_limit = models.IntegerField(
76+
help_text=_("Reputation cannot decrease lower than this value. Zero or negative."),
77+
default=DEFAULT_REPUTATION_LOWER_LIMIT,
78+
validators=[MaxValueValidator(0)],
79+
)
80+
reputation_upper_limit = models.IntegerField(
81+
help_text=_("Reputation cannot increase higher than this value. Zero or positive."),
82+
default=DEFAULT_REPUTATION_UPPER_LIMIT,
83+
validators=[MinValueValidator(0)],
84+
)
7385
footer_links = models.JSONField(
7486
help_text=_("The option configures the footer links on the flow executor pages."),
7587
default=list,

‎schema.yml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53723,6 +53723,17 @@ components:
5372353723
type: string
5372453724
minLength: 1
5372553725
description: 'Events will be deleted after this duration.(Format: weeks=3;days=2;hours=3,seconds=2).'
53726+
reputation_lower_limit:
53727+
type: integer
53728+
maximum: 0
53729+
minimum: -2147483648
53730+
description: Reputation cannot decrease lower than this value. Zero or negative.
53731+
reputation_upper_limit:
53732+
type: integer
53733+
maximum: 2147483647
53734+
minimum: 0
53735+
description: Reputation cannot increase higher than this value. Zero or
53736+
positive.
5372653737
footer_links:
5372753738
description: The option configures the footer links on the flow executor
5372853739
pages.
@@ -57775,6 +57786,17 @@ components:
5777557786
event_retention:
5777657787
type: string
5777757788
description: 'Events will be deleted after this duration.(Format: weeks=3;days=2;hours=3,seconds=2).'
57789+
reputation_lower_limit:
57790+
type: integer
57791+
maximum: 0
57792+
minimum: -2147483648
57793+
description: Reputation cannot decrease lower than this value. Zero or negative.
57794+
reputation_upper_limit:
57795+
type: integer
57796+
maximum: 2147483647
57797+
minimum: 0
57798+
description: Reputation cannot increase higher than this value. Zero or
57799+
positive.
5777857800
footer_links:
5777957801
description: The option configures the footer links on the flow executor
5778057802
pages.
@@ -57818,6 +57840,17 @@ components:
5781857840
type: string
5781957841
minLength: 1
5782057842
description: 'Events will be deleted after this duration.(Format: weeks=3;days=2;hours=3,seconds=2).'
57843+
reputation_lower_limit:
57844+
type: integer
57845+
maximum: 0
57846+
minimum: -2147483648
57847+
description: Reputation cannot decrease lower than this value. Zero or negative.
57848+
reputation_upper_limit:
57849+
type: integer
57850+
maximum: 2147483647
57851+
minimum: 0
57852+
description: Reputation cannot increase higher than this value. Zero or
57853+
positive.
5782157854
footer_links:
5782257855
description: The option configures the footer links on the flow executor
5782357856
pages.

‎web/src/admin/admin-settings/AdminSettingsForm.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import { AdminApi, FooterLink, Settings, SettingsRequest } from "@goauthentik/ap
2323
import "./AdminSettingsFooterLinks.js";
2424
import { IFooterLinkInput, akFooterLinkInput } from "./AdminSettingsFooterLinks.js";
2525

26+
const DEFAULT_REPUTATION_LOWER_LIMIT = -5;
27+
const DEFAULT_REPUTATION_UPPER_LIMIT = 5;
28+
2629
@customElement("ak-admin-settings-form")
2730
export class AdminSettingsForm extends Form<SettingsRequest> {
2831
//
@@ -177,6 +180,26 @@ export class AdminSettingsForm extends Form<SettingsRequest> {
177180
<ak-utils-time-delta-help></ak-utils-time-delta-help>`}
178181
>
179182
</ak-text-input>
183+
<ak-number-input
184+
label=${msg("Reputation: lower limit")}
185+
required
186+
name="reputationLowerLimit"
187+
value="${first(
188+
this._settings?.reputationLowerLimit,
189+
DEFAULT_REPUTATION_LOWER_LIMIT,
190+
)}"
191+
help=${msg("Reputation cannot decrease lower than this value. Zero or negative.")}
192+
></ak-number-input>
193+
<ak-number-input
194+
label=${msg("Reputation: upper limit")}
195+
required
196+
name="reputationUpperLimit"
197+
value="${first(
198+
this._settings?.reputationUpperLimit,
199+
DEFAULT_REPUTATION_UPPER_LIMIT,
200+
)}"
201+
help=${msg("Reputation cannot increase higher than this value. Zero or positive.")}
202+
></ak-number-input>
180203
<ak-form-element-horizontal label=${msg("Footer links")} name="footerLinks">
181204
<ak-array-input
182205
.items=${this._settings?.footerLinks ?? []}

0 commit comments

Comments
 (0)