Skip to content

Port user permission privilege escalation protections#72

Open
sergei-maertens wants to merge 5 commits into
mainfrom
feature/18-port-user-perm-priv-escalation-protections
Open

Port user permission privilege escalation protections#72
sergei-maertens wants to merge 5 commits into
mainfrom
feature/18-port-user-perm-priv-escalation-protections

Conversation

@sergei-maertens
Copy link
Copy Markdown
Member

Closes #18

Ported the tests and code, and cleaned them up + added type annotations.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.33%. Comparing base (08d8068) to head (2dcd602).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #72      +/-   ##
==========================================
+ Coverage   99.27%   99.33%   +0.06%     
==========================================
  Files          34       36       +2     
  Lines         826      908      +82     
  Branches       92       98       +6     
==========================================
+ Hits          820      902      +82     
  Misses          4        4              
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sergei-maertens sergei-maertens force-pushed the feature/18-port-user-perm-priv-escalation-protections branch from 52bc726 to ca33888 Compare May 13, 2026 17:02
Comment thread tests/base/test_checks.py
Comment on lines +33 to +48
@pytest.fixture
def unregister_user_model():
_original = type(admin.site.get_model_admin(User))
admin.site.unregister(User)
yield
admin.site.register(User, _original)


@pytest.fixture
def revert_useradmin_patch(unregister_user_model):
"""
Restore the default, unpatched user admin.
"""
admin.site.register(User, UserAdmin)
yield
admin.site.unregister(User)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Onions are tasty

@alextreme
Copy link
Copy Markdown
Member

Good that this is tackled, but I'll let you two do the first review together

@alextreme alextreme removed their request for review May 13, 2026 18:16
@sergei-maertens sergei-maertens force-pushed the feature/18-port-user-perm-priv-escalation-protections branch from 2534eac to 494510d Compare May 13, 2026 20:15
@sergei-maertens sergei-maertens marked this pull request as ready for review May 13, 2026 20:15
* Verify that standard admin pages still work
* Verify that users cannot make themselves superuser
* Verify that users cannot give themselves or other more permissions
  than they currently have
* Verify that you cannot change passwords of users with more permissions
  than the current user

This also wires up the custom admin that will hold our implementation.
It was checked that the smoke tests fail when only the user model is
unregistered, and they pass again when the custom admin class is
registered.
Ported from default-project, but rewritten to be more explicit and a
tad more ✨ magic ✨ for easy integration in existing
projects.

The code is now heavily annotated with types and hopefully a bit easier
to read and maintain.
Ensure that a warning is emitted if we notice the mixin is missing on
the user admin. Users can silence this warning if it's deliberate.

For projects that have the protections via default-project, this can
be a nice signal to replace that code with the equivalent from
maykin-common.

Additionally, this adds a missing period for the end of the sentence to
the existing check.
@sergei-maertens sergei-maertens force-pushed the feature/18-port-user-perm-priv-escalation-protections branch from 494510d to 2dcd602 Compare May 14, 2026 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move the privilege escalation protection from default-project to this library

2 participants