Skip to content

Add tests for aliases#28

Merged
jakub-vavra-cz merged 1 commit into
RedHat-SP-Security:masterfrom
jakub-vavra-cz:aliases
Apr 29, 2026
Merged

Add tests for aliases#28
jakub-vavra-cz merged 1 commit into
RedHat-SP-Security:masterfrom
jakub-vavra-cz:aliases

Conversation

@jakub-vavra-cz
Copy link
Copy Markdown
Collaborator

@jakub-vavra-cz jakub-vavra-cz commented Apr 21, 2026

Add tests for aliases on local provider (sudoers files).
Add enable netgroup test for local provider.
Add flake8 line length rule to pytest/pyproject.toml

Depends on: SSSD/sssd-test-framework#242

Summary by Sourcery

Add coverage for sudoers alias handling and enable netgroup testing on bare client topology while aligning flake8 configuration with Black’s line length.

Build:

  • Configure flake8 max line length to match Black (119 characters) in pytest/pyproject.toml.

Tests:

  • Add sudoers tests for user, command, host, and runas aliases on the local (bare client) provider.
  • Enable the single netgroup sudo test to run on BareClient topology.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 21, 2026

Reviewer's Guide

Extends sudo-related pytest coverage for the local (bare client) provider by enabling an existing netgroup test and adding focused tests for sudoers aliases (user, command, host, and runas), and aligns flake8’s max line length with Black via pyproject.toml updates.

File-Level Changes

Change Details Files
Enable netgroup-based sudo test for the BareClient topology.
  • Remove commented-out BareClient topology marker from the netgroup test.
  • Add BareClient to the list of supported topologies for the single netgroup test.
pytest/tests/test_basic.py
Add sudo alias coverage for local provider across user, command, host, and runas dimensions.
  • Introduce tests for sudo User_Alias with a single user and with multiple members, ensuring only aliased users can execute /bin/ls via sudo.
  • Add a test verifying User_Alias entries can reference POSIX groups and that group membership allows sudo access.
  • Add a test ensuring sudo rules can reference a Cmnd_Alias for allowed commands.
  • Add a test ensuring sudo rules can reference a Host_Alias for sudoHost matching the client’s short hostname.
  • Add a test validating Runas_Alias usage for sudoRunAsUser, allowing execution as allowed runas targets and denying others.
pytest/tests/test_basic.py
Configure flake8 to use the same maximum line length as Black.
  • Add a [tool.flake8] section to pyproject.toml with max-line-length set to 119 to match Black’s configuration.
pytest/pyproject.toml

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. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the 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 exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

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

Copy link
Copy Markdown

@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 - I've found 3 issues, and left some high level feedback:

  • Several tests hard-code the password string "Secret123" multiple times; consider centralizing this into a shared constant or fixture to avoid repetition and ease future changes.
  • The alias-based sudo tests share a lot of setup logic (user creation, alias creation, sudo rule creation, and SSSD restart); extracting common helpers or fixtures would make the tests more concise and easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Several tests hard-code the password string "Secret123" multiple times; consider centralizing this into a shared constant or fixture to avoid repetition and ease future changes.
- The alias-based sudo tests share a lot of setup logic (user creation, alias creation, sudo rule creation, and SSSD restart); extracting common helpers or fixtures would make the tests more concise and easier to maintain.

## Individual Comments

### Comment 1
<location path="pytest/tests/test_basic.py" line_range="1108-1117" />
<code_context>
+def test_basic__user_alias_with_group_member(
</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative case for a user not in the group-based User_Alias.

Consider also adding a user who is not in `group-1` and asserting that sudo is denied for that user, to confirm the `User_Alias` is not applied to non-members.

Suggested implementation:

```python
    :setup:
        1. Create users "user-1" and "user-2" and group "group-1" with user-1 as member and user-2 not a member
        2. Define ``User_Alias SUDO_SUBJECTS`` listing ``%group-1``
        3. Create sudorule for ``SUDO_SUBJECTS`` to run /bin/ls on all hosts
        4. Enable SSSD sudo responder and start SSSD
        5. Verify that sudo is allowed for "user-1" and denied for "user-2", confirming that the User_Alias is not applied to non-members of ``group-1``

```

To fully implement the negative case:
1. In `test_basic__user_alias_with_group_member`, ensure a second user (e.g. `user_2` or similar) is created who is *not* added to `group-1` (matching the naming and helper APIs already used in this file for creating users and groups).
2. After the existing positive sudo assertion for the group member, add an assertion similar to:
   ```python
   assert not client.auth.sudo.run(
       user_2.name,
       "Secret123",
       command="/bin/ls /root",
   ), f"User {user_2.name} should be denied (not in group-1 User_Alias)"
   ```
   adapting `user_2`/password/command to whatever variables and helpers are already used in this test file.
3. Keep the style consistent with the earlier test that asserts a non-member is denied (the `u3` negative case shown at the top of your snippet), including any helper functions or fixtures used to provision users and set passwords.
</issue_to_address>

### Comment 2
<location path="pytest/tests/test_basic.py" line_range="1141-1150" />
<code_context>
+def test_basic__command_alias(
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the Cmnd_Alias test with a command that should be denied.

To better verify the alias behavior, please also assert that a different command (e.g. `/bin/id` or `/bin/cat`) is denied for the same user and rule. This ensures only the aliased command is permitted and guards against overly broad aliases.

Suggested implementation:

```python
    """
    :title: Sudo rule may reference a Cmnd_Alias
    :setup:
        1. Create user "user-1"
        2. Define ``Cmnd_Alias LSHELP`` as /bin/ls
        3. Create sudorule allowing user-1 to run ``LSHELP`` on all hosts
        4. Enable SSSD sudo responder and start SSSD
        5. Verify that the same user is *denied* sudo for a non-aliased command (e.g. ``/bin/id``)

```

To fully implement the suggestion, you should also add a negative assertion in the body of `test_basic__command_alias`, reusing the same user and rule that allow running the `LSHELP` alias. For example, after the existing positive check that `/bin/ls` (or `LSHELP`) works, add something like:

```python
    assert not client.auth.sudo.run(
        user.name, "Secret123", command="/bin/id"
    ), f"User {user.name} should not be allowed to run /bin/id via sudo (Cmnd_Alias)"
```

Replace `user` and the password with the actual variable names/values used in the test for the allowed command. Place this assertion close to the existing successful sudo assertion so that the test clearly verifies both permitted and denied behavior for the same rule.
</issue_to_address>

### Comment 3
<location path="pytest/tests/test_basic.py" line_range="1173-1182" />
<code_context>
+def test_basic__host_alias(
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a negative assertion for Host_Alias with a non-matching host.

Since this only covers the positive case, please also add a `Host_Alias` that does not match the current host (e.g., a bogus hostname) and assert that a rule using that alias is not applied, to confirm that host alias matching can fail as expected.

Suggested implementation:

```python
    """
    :title: Sudo rule may reference a Host_Alias for sudoHost
    :setup:
        1. Create user "user-1"
        2. Define ``Host_Alias TRUSTED`` with the client's short hostname
        3. Create sudorule allowing user-1 to run /bin/ls on ``TRUSTED``
        4. Enable SSSD sudo responder and start SSSD
        5. Define ``Host_Alias UNTRUSTED`` with a bogus/non-matching hostname
        6. Create a sudorule using ``UNTRUSTED`` and assert that it is not applied

```

To fully implement the requested negative assertion, extend the body of `test_basic__host_alias` to:
1. Create a second Host_Alias (e.g. `UNTRUSTED`) with a hostname that does not match the client (for example `"definitely-not-" + client.system.hostname` or a fixed bogus hostname).
2. Create a sudo rule for the same user and command, but with `sudoHost` set to the `UNTRUSTED` Host_Alias.
3. After configuring SSSD and refreshing sudo rules (using whatever helper is already present in this test file, e.g. `client.sssd.restart()` or a dedicated sudo refresh), assert that:
   - The positive case still works when using the `TRUSTED` alias (existing assertion).
   - A sudo attempt matching the `UNTRUSTED` rule does *not* succeed. Depending on your helper API this will likely look like:
     - `assert not client.auth.sudo.run(u.name, "Secret123", command="/bin/ls /root", expect_failure=True)` or
     - wrapping `client.auth.sudo.run(...)` in a `with pytest.raises(...)` or checking the returned object’s exit code, following the patterns used elsewhere in `test_basic.py`.
Make sure the negative assertion explicitly exercises the rule that references the non-matching `UNTRUSTED` Host_Alias, so the test confirms that host alias matching can fail as expected.
</issue_to_address>

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.

Comment thread pytest/tests/test_basic.py
Comment thread pytest/tests/test_basic.py
Comment thread pytest/tests/test_basic.py
Comment thread pytest/tests/test_basic.py
Comment thread pytest/tests/test_basic.py Outdated
Copy link
Copy Markdown
Collaborator

@shridhargadekar shridhargadekar left a comment

Choose a reason for hiding this comment

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

apart from minor change request. rest looks good.

Add tests for aliases on local provider (sudoers files).
Add enable netgroup test for local provider.
Add flake8 line length rule to pytest/pyproject.toml
Copy link
Copy Markdown
Collaborator

@shridhargadekar shridhargadekar left a comment

Choose a reason for hiding this comment

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

lgtm

@jakub-vavra-cz jakub-vavra-cz merged commit 860e3bb into RedHat-SP-Security:master Apr 29, 2026
4 checks passed
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.

2 participants