-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
events: add reason to the MODEL_UPDATED event for password hash updates #18068
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
danidee10
wants to merge
9
commits into
goauthentik:main
Choose a base branch
from
danidee10:feat/add-reason-password-hash-update
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+102
−3
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
147cd34
feat: Add reason to the MODEL_UPDATED event for password hash updates
danidee10 edf3aaf
fix: Make the test_docker.sh script POSIX compliant
danidee10 1682c77
Merge remote-tracking branch 'origin/main' into feat/add-reason-passw…
danidee10 c173c38
fix: Fix bandit lint error
danidee10 ff8ff4f
chore: Add test for password hash without the request context
danidee10 7a3dcda
chore: simplify the Implementation with model_utils FieldTracker
danidee10 d3118c8
chore: Remove unnecessary test case
danidee10 d6afe94
Merge remote-tracking branch 'origin/main' into feat/add-reason-passw…
danidee10 391a3d3
feat: Distinguish between set-password and other hash upgrades
danidee10 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| PASSWORD_HASH_UPGRADE_REASON = "Password hash upgraded" # noqa # nosec |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| from unittest.mock import PropertyMock, patch | ||
|
|
||
| from django.contrib.auth.hashers import PBKDF2PasswordHasher | ||
| from django.urls import reverse | ||
| from rest_framework.test import APITestCase | ||
|
|
||
| from authentik.core.tests.utils import create_test_admin_user, create_test_flow | ||
| from authentik.events.constants import PASSWORD_HASH_UPGRADE_REASON | ||
| from authentik.events.models import Event, EventAction | ||
| from authentik.flows.models import FlowDesignation, FlowStageBinding | ||
| from authentik.lib.generators import generate_id | ||
| from authentik.stages.identification.models import IdentificationStage, UserFields | ||
| from authentik.stages.password import BACKEND_INBUILT | ||
| from authentik.stages.password.models import PasswordStage | ||
|
|
||
|
|
||
| class TestAudit(APITestCase): | ||
| """Test audit middleware""" | ||
|
|
||
| def setUp(self) -> None: | ||
| self.user = create_test_admin_user() | ||
| # Set up a flow for authentication | ||
| self.flow = create_test_flow(FlowDesignation.AUTHENTICATION) | ||
| self.stage = IdentificationStage.objects.create( | ||
| name="identification", | ||
| user_fields=[UserFields.USERNAME], | ||
| pretend_user_exists=False, | ||
| ) | ||
| pw_stage = PasswordStage.objects.create(name="password", backends=[BACKEND_INBUILT]) | ||
| self.stage.password_stage = pw_stage | ||
| self.stage.save() | ||
| FlowStageBinding.objects.create( | ||
| target=self.flow, | ||
| stage=self.stage, | ||
| order=0, | ||
| ) | ||
|
|
||
| def test_password_hash_updated(self): | ||
| """ | ||
| When Django is updated, it's possible that the password hash is also updated. | ||
|
|
||
| Due to an increase in the password hash rounds. | ||
| When this happens, we should log a MODEL_UPDATED event with a reason | ||
| explaining the password hash upgrade. | ||
| """ | ||
| with patch.object( | ||
| PBKDF2PasswordHasher, | ||
| "iterations", | ||
| new_callable=PropertyMock, | ||
| return_value=PBKDF2PasswordHasher.iterations + 100_000, | ||
| ): | ||
| # During authentication, | ||
| # Django should detect that the hash needs to be updated and update it | ||
| form_data = {"uid_field": self.user.username, "password": self.user.username} | ||
| url = reverse("authentik_api:flow-executor", kwargs={"flow_slug": self.flow.slug}) | ||
| response = self.client.post(url, form_data) | ||
| self.assertEqual(response.status_code, 200) | ||
|
|
||
| events = Event.objects.filter( | ||
| action=EventAction.MODEL_UPDATED, | ||
| context__model__app="authentik_core", | ||
| context__model__model_name="user", | ||
| context__model__pk=self.user.pk, | ||
| context__reason=PASSWORD_HASH_UPGRADE_REASON, | ||
| ) | ||
| self.assertTrue(events.exists()) | ||
|
|
||
| def test_set_password_no_password_upgrade_reason(self): | ||
| """Ensure that setting a password is not detected as a password hash upgrade.""" | ||
| self.client.login(username=self.user.username, password=self.user.username) | ||
| response = self.client.post( | ||
| reverse("authentik_api:user-set-password", kwargs={"pk": self.user.pk}), | ||
| data={"password": generate_id()}, | ||
| ) | ||
| self.assertEqual(response.status_code, 204) | ||
|
|
||
| events = Event.objects.filter( | ||
| action=EventAction.MODEL_UPDATED, | ||
| context__model__app="authentik_core", | ||
| context__model__model_name="user", | ||
| context__model__pk=self.user.pk, | ||
| context__reason=PASSWORD_HASH_UPGRADE_REASON, | ||
| ) | ||
| self.assertFalse(events.exists()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When trying to test with
docker. I discovered that this syntax wasn't POSIX compliant and didn't work on macos (with bash 3.2)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm the same with zsh 5.9 (arm64-apple-darwin25.0)