Conversation
07dc2bc to
64143b9
Compare
92a1a22 to
c26ea43
Compare
19c9b08 to
9f77d4e
Compare
5dfa55e to
adc70e9
Compare
justin-stephenson
left a comment
There was a problem hiding this comment.
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
f5d64b3 to
b854636
Compare
411572a to
1c024ce
Compare
1c024ce to
283b664
Compare
src/tests/system/tests/test_ipa.py
Outdated
|
|
||
| @pytest.mark.importance("high") | ||
| @pytest.mark.topology(KnownTopology.IPA) | ||
| def test_ipa__hbac_category_all_users(client: Client, ipa: IPA): |
There was a problem hiding this comment.
I think this can be parameterize? The category_all tests?
There was a problem hiding this comment.
I tried but code looks complicated, please check
There was a problem hiding this comment.
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!"
There was a problem hiding this comment.
updated, please check
283b664 to
c076fe0
Compare
7fa09d6 to
df30a68
Compare
0a90533 to
6b89bfc
Compare
danlavu
left a comment
There was a problem hiding this comment.
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.
861cee7 to
13a0182
Compare
danlavu
left a comment
There was a problem hiding this comment.
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.
-
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.
-
Separate the code with a blank line, following the setup, steps and results
1: add users
2. Disable hbac rule
3. create hbac rule
13a0182 to
f7854bf
Compare
danlavu
left a comment
There was a problem hiding this comment.
- Move the restart SSSD to the setup steps, per the comments.
- Fix the services test.
Any other docstring comments are just things to consider.
…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>
Uh oh!
There was an error while loading. Please reload this page.