Skip to content
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

fix: rework sys_admin authorizations #249

Open
wants to merge 8 commits into
base: release/2.1.1
Choose a base branch
from

Conversation

psyray
Copy link
Contributor

@psyray psyray commented Jan 17, 2025

Fix #241

This change introduces several fixes to the sys_admin role.

  • Better check the project list to prevent new sys admins to be stucked on Welcome page

  • Refines the permissions of the user administration actions to prevent superuser profile edition by sys_admin, only superuser can edit superuser

  • Clarify user roles in the administrator interface.

    • Test onboarding page after sys_admin creation
    • Test permissions with each roles

This change fixes user access control for "sys_admin" users to have access to all projects.
It also introduces a get_user_groups utility function to determine a user's role and refactors project access logic to use this function.
This change fix several bugs to the admin interface, focusing on improved security and user management. Specifically, it refines the permissions for managing user accounts, adds visual distinctions for superuser roles
@psyray psyray self-assigned this Jan 17, 2025
web/dashboard/views.py Fixed Show fixed Hide fixed
web/dashboard/views.py Fixed Show fixed Hide fixed
Copy link
Contributor

sourcery-ai bot commented Jan 17, 2025

Reviewer's Guide by Sourcery

This pull request refines the system administrator authorizations and user interface, addressing issues where new system administrators were getting stuck on the welcome page and preventing system administrators from editing superuser profiles.

Sequence diagram for user modification authorization flow

sequenceDiagram
    participant A as Administrator
    participant S as System
    participant T as Target User

    A->>S: Request to modify user
    S->>S: Check modification mode
    alt Mode is create
        S->>S: Skip user checks
        S-->>A: Allow modification
    else Other modes
        S->>S: Get current user role
        S->>S: Get target user role
        alt Target is superuser & Current is not superuser
            S-->>A: Deny modification (403)
        else Self-modification for admin roles
            S-->>A: Deny self deletion/deactivation (403)
        else Valid modification
            S-->>A: Allow modification
        end
    end
Loading

Class diagram for user role and permissions structure

classDiagram
    class User {
        +is_superuser: boolean
        +is_active: boolean
        +username: string
        +date_joined: datetime
    }

    class UserGroup {
        +name: string
    }

    class Project {
        +name: string
        +slug: string
        +insert_date: datetime
    }

    User "*" -- "*" Project: belongs to
    User "*" -- "1" UserGroup: has role

    note for UserGroup "Available roles:\n- superuser\n- sys_admin\n- auditor\n- penetration_tester"

    note for User "New restrictions:\n- Only superuser can modify superuser\n- Admins cannot delete themselves"
Loading

State diagram for user access control

stateDiagram-v2
    [*] --> CheckUserRole
    CheckUserRole --> SuperUser: is_superuser
    CheckUserRole --> SysAdmin: in sys_admin group
    CheckUserRole --> Auditor: in auditor group
    CheckUserRole --> PenTester: in penetration_tester group

    SuperUser --> FullAccess: Can modify all users
    SysAdmin --> RestrictedAccess: Cannot modify superusers
    Auditor --> LimitedAccess
    PenTester --> LimitedAccess

    state RestrictedAccess {
        [*] --> CanModifyRegularUsers
        CanModifyRegularUsers --> CannotModifySuperUsers
        CannotModifySuperUsers --> CannotModifySelf
    }
Loading

File-Level Changes

Change Details Files
Implement permission checks to prevent sys_admins from modifying superusers.
  • Added tests to verify that sys_admins cannot modify superuser accounts.
  • Implemented a check in the admin_interface_update view to prevent sys_admins from modifying superusers.
  • Added a check to prevent administrators from deleting or deactivating themselves.
web/dashboard/tests/test_dashboard.py
web/dashboard/views.py
Modify the admin interface to clarify user roles and permissions.
  • Updated the admin.html template to display superusers with a distinct badge.
  • Modified the get_user_role filter to use the get_user_groups function.
  • Modified the get_user_projects function to return all projects for sys_admins.
  • Modified the admin.html template to only show action buttons for superuser->superuser or sys_admin->non-superuser.
web/dashboard/templates/dashboard/admin.html
web/commonFilters/templatetags/custom_filters.py
web/dashboard/utils.py
Fix the issue where new sys admins were stuck on the welcome page.
  • Modified the onboarding view to redirect sys_admins to the first available project.
  • Modified the get_user_projects function to return all projects for sys_admins.
web/dashboard/views.py
web/dashboard/utils.py

Assessment against linked issues

Issue Objective Addressed Explanation
#241 Resolve the issue where new sysadmins get stuck on the welcome page after login
#241 Prevent new sysadmins from being unable to proceed past the welcome screen
#241 Improve user role and permission management for sysadmins

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@psyray psyray linked an issue Jan 17, 2025 that may be closed by this pull request
3 tasks
@psyray psyray marked this pull request as ready for review January 17, 2025 18:51
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @psyray - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

web/dashboard/utils.py Show resolved Hide resolved
web/dashboard/utils.py Show resolved Hide resolved
web/dashboard/tests/test_dashboard.py Show resolved Hide resolved
web/dashboard/views.py Outdated Show resolved Hide resolved
web/dashboard/views.py Show resolved Hide resolved
@psyray
Copy link
Contributor Author

psyray commented Jan 17, 2025

@AnonymousWP
Ready for review and merge

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.

bug(scope): Sysadmins stuck at the welcome page
1 participant