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

tests: convert multihost/basic/test_basic to system/tests/test_basic #6806

Closed
wants to merge 1 commit into from

Conversation

patriki01
Copy link
Contributor

Another tests converted.
Simple tests for ssh_login, offline_ssh_login and simple_kinit.
I'd like to read your comments on my code.

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

test_basic.py is probably not a very good name.

  • kinit is most probably already tested in test_kcm.py, if not it belongs there
  • ssh cases can be also tested with su and we can put it to test_authentication.py?

Copy link
Contributor

@aplopez aplopez left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Please talk to Jakub and check the requirement.

AD test for offline authentication fails, I need to check why.

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

Please add a proper assert messages.
The kcm suite is missing requirement.

@pbrezina
Copy link
Member

Please, rebase on top of current master.

@patriki01 patriki01 force-pushed the basic_tests branch 2 times, most recently from 0285f9b to 0f394ac Compare July 25, 2023 12:15
@pbrezina
Copy link
Member

Please talk to Jakub and check the requirement.

AD test for offline authentication fails, I need to check why.

Ok, the offline authentication test actually did not test offline authenticate at all. There were several issues:

  • Adding a firewall rule does not terminate active connections so SSSD was still able to lookup stuff in LDAP
  • For IPA, Samba and AD, you need to also block KDC
  • For Samba and AD, you need to also block Global Catalog port
  • For AD case, authentication was successful but it failed because it could not lookup GPO rules for access control check

I added bring_offline method and documented it in how to guide (PR SSSD/sssd-test-framework#23):

Please change it like this (and update docstring)

diff --git a/src/tests/system/tests/test_authentication.py b/src/tests/system/tests/test_authentication.py
index 3f4727867..350783576 100644
--- a/src/tests/system/tests/test_authentication.py
+++ b/src/tests/system/tests/test_authentication.py
@@ -73,7 +73,13 @@ def test_authentication__offline_login(client: Client, provider: GenericProvider
     assert not client.auth.parametrize(method).password(user, wrong), "login with wrong password succeeded"
     assert client.auth.parametrize(method).password(user, correct), "login with correct password failed"
 
-    provider.firewall.drop(389)
+    # Block KDC, LDAP and Global Catalog ports.
+    provider.firewall.drop([88, 389, 3268])
+
+    # There might be active connections that are not terminated by creating
+    # firewall rule. We need to terminated it by bringing SSSD to offline state
+    # explicitly.
+    client.sssd.bring_offline()
 
     assert client.auth.parametrize(method).password(user, correct), "offline login with correct password failed"
     assert not client.auth.parametrize(method).password(user, wrong), "offline login with wrong password succeeded"

@pbrezina
Copy link
Member

This fails on Samba on rawhide, I can reproduce it locally and will check why.

@pbrezina
Copy link
Member

This fails on Samba on rawhide, I can reproduce it locally and will check why.

It looks to be only timeout issue. SSSD/sssd-test-framework@97a3994 should fix it. Re-running the test.

@pbrezina
Copy link
Member

@jakub-vavra-cz Can you please review this PR?

@jakub-vavra-cz jakub-vavra-cz self-assigned this Jul 28, 2023
Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

At leats the port blocking should be changed to IP blocking intstead.

@jakub-vavra-cz
Copy link
Contributor

Hello @patriki01 ,
are the tests passing also with AD, samba etc?
If they are working then I will approve them.

@patriki01
Copy link
Contributor Author

Hi,
these works:
image
I am not sure if ad tests works as well, @pbrezina will take a look at this.

@pbrezina
Copy link
Member

pbrezina commented Sep 5, 2023

All tests passed:

tests/test_authentication.py::test_authentication__login[su] (ad) PASSED                                                                                                                                                               [  3%]
tests/test_authentication.py::test_authentication__login[su] (ipa) PASSED                                                                                                                                                              [  7%]
tests/test_authentication.py::test_authentication__login[su] (ldap) PASSED                                                                                                                                                             [ 10%]
tests/test_authentication.py::test_authentication__login[su] (samba) PASSED                                                                                                                                                            [ 14%]
tests/test_authentication.py::test_authentication__login[ssh] (ad) PASSED                                                                                                                                                              [ 17%]
tests/test_authentication.py::test_authentication__login[ssh] (ipa) PASSED                                                                                                                                                             [ 21%]
tests/test_authentication.py::test_authentication__login[ssh] (ldap) PASSED                                                                                                                                                            [ 25%]
tests/test_authentication.py::test_authentication__login[ssh] (samba) PASSED                                                                                                                                                           [ 28%]
tests/test_authentication.py::test_authentication__offline_login[su] (ad) PASSED                                                                                                                                                       [ 32%]
tests/test_authentication.py::test_authentication__offline_login[su] (ipa) PASSED                                                                                                                                                      [ 35%]
tests/test_authentication.py::test_authentication__offline_login[su] (ldap) PASSED                                                                                                                                                     [ 39%]
tests/test_authentication.py::test_authentication__offline_login[su] (samba) PASSED                                                                                                                                                    [ 42%]
tests/test_authentication.py::test_authentication__offline_login[ssh] (ad) PASSED                                                                                                                                                      [ 46%]
tests/test_authentication.py::test_authentication__offline_login[ssh] (ipa) PASSED                                                                                                                                                     [ 50%]
tests/test_authentication.py::test_authentication__offline_login[ssh] (ldap) PASSED                                                                                                                                                    [ 53%]
tests/test_authentication.py::test_authentication__offline_login[ssh] (samba) PASSED                                                                                                                                                   [ 57%]
tests/test_kcm.py::test_kcm__kinit_overwrite[memory] (client) PASSED                                                                                                                                                                   [ 60%]
tests/test_kcm.py::test_kcm__kinit_overwrite[secdb] (client) PASSED                                                                                                                                                                    [ 64%]
tests/test_kcm.py::test_kcm__kinit_collection[memory] (client) PASSED                                                                                                                                                                  [ 67%]
tests/test_kcm.py::test_kcm__kinit_collection[secdb] (client) PASSED                                                                                                                                                                   [ 71%]
tests/test_kcm.py::test_kcm__kswitch[memory] (client) PASSED                                                                                                                                                                           [ 75%]
tests/test_kcm.py::test_kcm__kswitch[secdb] (client) PASSED                                                                                                                                                                            [ 78%]
tests/test_kcm.py::test_kcm__subsidiaries[memory] (client) PASSED                                                                                                                                                                      [ 82%]
tests/test_kcm.py::test_kcm__subsidiaries[secdb] (client) PASSED                                                                                                                                                                       [ 85%]
tests/test_kcm.py::test_kcm__kdestroy_nocache[memory] (client) PASSED                                                                                                                                                                  [ 89%]
tests/test_kcm.py::test_kcm__kdestroy_nocache[secdb] (client) PASSED                                                                                                                                                                   [ 92%]
tests/test_kcm.py::test_kcm__tgt_renewal (client) PASSED                                                                                                                                                                               [ 96%]
tests/test_kcm.py::test_kcm__simple_kinit (client) PASSED  

Copy link
Contributor

@jakub-vavra-cz jakub-vavra-cz left a comment

Choose a reason for hiding this comment

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

LGTM.

@pbrezina
Copy link
Member

pbrezina commented Sep 8, 2023

Pushed PR: #6806

  • master
    • 3765340 - tests: convert multihost/basic/test_basic to test_kcm and test_authentication
  • sssd-2-9
    • 0a42910 - tests: convert multihost/basic/test_basic to test_kcm and test_authentication

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Sep 8, 2023
@pbrezina pbrezina closed this Sep 8, 2023
@patriki01 patriki01 deleted the basic_tests branch September 12, 2023 06:08
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