Skip to content

IPA HBAC test cases#7987

Merged
madhuriupadhye merged 1 commit intoSSSD:masterfrom
madhuriupadhye:ipa-hbac
Dec 8, 2025
Merged

IPA HBAC test cases#7987
madhuriupadhye merged 1 commit intoSSSD:masterfrom
madhuriupadhye:ipa-hbac

Conversation

@madhuriupadhye
Copy link
Contributor

@madhuriupadhye madhuriupadhye commented Jun 5, 2025

A comprehensive set of IPA HBAC (Host-Based Access Control)
test cases to validate HBAC rule creation, modification,
conflict resolution, and access permission enforcement.
The tests cover scenarios including user and group-based
access, nested groups, service and host-specific rules,
category-based access, and authorization priority resolution.

@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 3 times, most recently from 07dc2bc to 64143b9 Compare July 14, 2025 12:13
@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 2 times, most recently from 92a1a22 to c26ea43 Compare July 14, 2025 16:26
@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 4 times, most recently from 19c9b08 to 9f77d4e Compare October 12, 2025 11:58
Copy link
Contributor

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

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

Please also check system test failure in PRCI

FAILED tests/test_ipa.py::test_ipa__validate_hbac_rule_check_access_sshd_service (ipa) - sssd_test_framework.misc.errors.ExpectScriptError: Unexpected end of file

@pbrezina pbrezina force-pushed the master branch 2 times, most recently from f5d64b3 to b854636 Compare November 4, 2025 14:27
@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 2 times, most recently from 411572a to 1c024ce Compare November 21, 2025 18:11
@madhuriupadhye madhuriupadhye changed the title Ipa hbac IPA HBAC test cases Nov 21, 2025

@pytest.mark.importance("high")
@pytest.mark.topology(KnownTopology.IPA)
def test_ipa__hbac_category_all_users(client: Client, ipa: IPA):
Copy link

Choose a reason for hiding this comment

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

I think this can be parameterize? The category_all tests?

Copy link
Contributor Author

@madhuriupadhye madhuriupadhye Nov 21, 2025

Choose a reason for hiding this comment

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

I tried but code looks complicated, please check

Copy link

@danlavu danlavu Nov 22, 2025

Choose a reason for hiding this comment

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

I was thinking something like this.

@pytest.parametrize("hbac", ["user", "host", "service"])
@pytest.mark.importance("medium")
@pytest.mark.topology(KnownTopology.IPA)
def test_ipa__hbac_service_category_all(client: Client, ipa: IPA, hbac: str):
    users = ["user1", "user2"]
    for user in users:
        ipa.user(user).add()
    ipa.hbac("allow_all").disable()
    ipa_rule = (ipa.hbac(f"user1_all_{hbac}_access"))

    if hbac == "user":
        ipa_rule.create(description="Allow user1 access to all services", usercat="all", users="user1", hosts="client.test")
    elif hbac == "host":
        ipa_rule.create(description = "Allow user1 access to all services", hostcat = "all", users = "user1",
                        hosts = "client.test")
    elif hbac == "service":
        ipa_rule.create(description="Allow user1 access to all services", servicecat="all", users="user1", hosts="client.test")

    client.sssd.restart()
    assert client.auth.ssh.password("user1", "Secret123"), "user1 SSH should succeed!"
    assert not client.auth.ssh.password("user2", "Secret123"), "user2 SSH should be denied!"

    if hbac == "user":
        ipa_rule.modify(servicecat="")
    elif hbac == "host":
        ipa_rule.modify(hostcat = "")
    elif hbac == "service":
        ipa_rule.modify(servicecat = "")

    client.sssd.restart()
    assert not client.auth.ssh.password("user1", "Secret123"), "user1 SSH should be denied!"
    assert not client.auth.ssh.password("user2", "Secret123"), "user2 SSH should be denied!"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, please check

@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 3 times, most recently from 7fa09d6 to df30a68 Compare November 24, 2025 15:49
@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 2 times, most recently from 0a90533 to 6b89bfc Compare November 24, 2025 18:08
Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

I don't like conflicting tests, it's unclear to me. Reading the test cases, the long and different variable names are confusing. Things, dev_group is a nested group, qa_group is a nested group, if we name them nested_group1 and nested_group2 there is no misaking what those groups are.

@madhuriupadhye madhuriupadhye force-pushed the ipa-hbac branch 3 times, most recently from 861cee7 to 13a0182 Compare November 25, 2025 11:26
Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

I know a lot has been going on with this PR, so I'm going to put everything in this comment. I think most of these things are oversights. Still, we need to follow these guidelines.

  1. Setup steps, contain everything before the first test step. Test step is the first thing that is related to the actual scenario. With access control, it's logging in. i.e. line 995, Restart is part of step 1, should be in setup.

  2. Separate the code with a blank line, following the setup, steps and results

1: add users
2. Disable hbac rule
3. create hbac rule

* blank line * blank line etc etc

Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

  1. Move the restart SSSD to the setup steps, per the comments.
  2. Fix the services test.

Any other docstring comments are just things to consider.

Copy link

@danlavu danlavu left a comment

Choose a reason for hiding this comment

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

These look great, thank you

…d Group Membership Refresh

A comprehensive set of IPA HBAC (Host-Based Access Control)
test cases to validate HBAC rule creation, modification,
conflict resolution, and access permission enforcement.
The tests cover scenarios including user and group-based
access, nested groups, service and host-specific rules,
category-based access, and authorization priority resolution.

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
Reviewed-by: Dan Lavu <dlavu@redhat.com>
Reviewed-by: Justin Stephenson <jstephen@redhat.com>
@sssd-bot
Copy link
Contributor

sssd-bot commented Dec 4, 2025

The pull request was accepted by @madhuriupadhye with the following PR CI status:


🟢 CodeFactor (success)
🟢 CodeQL (success)
🟡 osh-diff-scan:fedora-rawhide-x86_64:upstream (in_progress)
🟢 rpm-build:centos-stream-10-x86_64:upstream (success)
🟢 rpm-build:fedora-41-x86_64:upstream (success)
🟢 rpm-build:fedora-42-x86_64:upstream (success)
🟢 rpm-build:fedora-43-x86_64:upstream (success)
🟢 rpm-build:fedora-rawhide-x86_64:upstream (success)
🟢 Analyze (target) / cppcheck (success)
🔴 ci / intgcheck (centos-10) (failure)
🔴 ci / intgcheck (fedora-41) (failure)
🔴 ci / intgcheck (fedora-42) (failure)
🟢 ci / intgcheck (fedora-43) (success)
🔴 ci / intgcheck (fedora-44) (failure)
🟢 ci / prepare (success)
🟢 ci / system (centos-10) (success)
🟢 ci / system (fedora-41) (success)
🟢 ci / system (fedora-42) (success)
🟢 ci / system (fedora-43) (success)
🟡 ci / system (fedora-44) (in_progress)
➖ Coverity scan / coverity (skipped)
🟢 Static code analysis / codeql (success)
🟢 Static code analysis / pre-commit (success)
🟢 Static code analysis / python-system-tests (success)


There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants