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.

Sign up for GitHub

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

Jump to bottom

Tests: Add following test cases for passkey with LDAP. IPA, AD and Samba #6634

Closed
wants to merge 1 commit into from
Closed

Tests: Add following test cases for passkey with LDAP. IPA, AD and Samba #6634

wants to merge 1 commit into from

Conversation

madhuriupadhye
Copy link
Contributor

@madhuriupadhye madhuriupadhye commented Mar 20, 2023 •
edited
Loading

  1. Register a key with sssctl.
  2. Register a key with IPA sssctl command.
  3. Check authentication of user with IPA, LDAP, AD and Samba.

All tests cases automated with umockdev.

@madhuriupadhye madhuriupadhye force-pushed the passkey_tests_part1 branch 3 times, most recently from 12acbda to 97e6184 Compare March 22, 2023 11:29
ikerexxe
ikerexxe requested changes Mar 22, 2023
View reviewed changes
Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

I checked the first four test cases, and I have added some minor comments. On top of that, would it be possible to reuse the test cases among different servers (IPA, LDAP, AD)? There are minor differences that could be tackled programmatically.

Finally, this is a note for future PRs. It would be nice to divide the changes in several commits. For this PR I would have added one commit to add the changes to the testing framework, and then one commit per test case which could include the scripts. This way you help in the review process by providing small chunks of changes instead of a big commit.

src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
@ikerexxe
Copy link
Contributor

By the way, you should check the CI output, as it's reporting some errors to your changes.

spoore1
spoore1 reviewed Mar 22, 2023
View reviewed changes
Copy link
Contributor

@spoore1 spoore1 left a comment

Choose a reason for hiding this comment

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

Most of my comments here center around trying to combine the LDAP/IPA test cases. I think it should be possible if you add the same methods to LDAPUser that you added to IPAUser.

src/tests/system/lib/sssd/roles/ldap.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
pbrezina
pbrezina requested changes Mar 23, 2023
View reviewed changes
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.

Hi, this is a very good start, kudos. I haven't gone trough the whole thing, I will wait until current comments are resolved to continue with the review.

src/tests/system/lib/sssd/roles/ipa.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/roles/ipa.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/roles/ldap.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
@madhuriupadhye madhuriupadhye force-pushed the passkey_tests_part1 branch 3 times, most recently from b3dd65f to d7c9362 Compare March 31, 2023 16:17
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 31, 2023
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

ikerexxe
ikerexxe requested changes Apr 3, 2023
View reviewed changes
Copy link
Contributor

@ikerexxe ikerexxe left a comment

Choose a reason for hiding this comment

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

The PR is progressing towards an optimal state, but there are still some things to improve.

src/tests/system/lib/sssd/utils/authentication.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
@madhuriupadhye madhuriupadhye added the passkey Issues and PRs related to 'passkey' feature label Apr 4, 2023
spoore1
spoore1 reviewed Apr 4, 2023
View reviewed changes
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
src/tests/system/tests/test_passkey.py Outdated Show resolved Hide resolved
@madhuriupadhye madhuriupadhye force-pushed the passkey_tests_part1 branch 3 times, most recently from b62b143 to 9bdbcdf Compare April 6, 2023 06:41
@pbrezina
Copy link
Member

pbrezina commented Oct 3, 2023

  • sssd-2-9
    • df709da - tests: add passkey tests for sssctl and non-kerberos authentication
  • master
    • 66c0a2d - tests: add passkey tests for sssctl and non-kerberos authentication

@pbrezina pbrezina closed this Oct 3, 2023
@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Oct 3, 2023
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
84f1e46 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
ikerexxe pushed a commit to ikerexxe/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye @ikerexxe
Tests: Add passkey test cases for following scenario.
3b87c24 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
339fe33 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
c75d93d 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
1f6aa3b 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
2ed0b59 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
11c0b3f 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
d4f1f94 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 23, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
7cb24fe 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 24, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
7cb59eb 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 24, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
e67c96f 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 24, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
4150763 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
ba41564 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
ab175ba 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
6485f73 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
4f96319 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
6727c97 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
a9387c7 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 27, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
243f729 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
madhuriupadhye added a commit to madhuriupadhye/sssd that referenced this pull request Nov 28, 2023
@madhuriupadhye
Tests: Add passkey test cases for following scenario.
41574ef 
  Test cases are as follows:
  4. Check auth deny for incorrect pin for LDAP, IPA, Ad and Samba.
  5. Check auth deny for incorrect passkey mapping for LDAP, IPA, AD and Samba.
  6. Check auth of user when server is not resolvable for IPA, LDAP, AD and Samba.

  First PR is under review, SSSD#6634

Signed-off-by: Madhuri Upadhye <mupadhye@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikerexxe ikerexxe ikerexxe approved these changes

@spoore1 spoore1 spoore1 approved these changes

@justin-stephenson justin-stephenson justin-stephenson approved these changes

@pbrezina pbrezina pbrezina approved these changes

@jakub-vavra-cz jakub-vavra-cz jakub-vavra-cz approved these changes

Assignees

@ikerexxe ikerexxe

@spoore1 spoore1

@pbrezina pbrezina

Labels
passkey Issues and PRs related to 'passkey' feature Pushed Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@madhuriupadhye @ikerexxe @pbrezina @jakub-vavra-cz @justin-stephenson @spoore1