Skip to content

[autobackport: sssd-2-10] Smartcard multi token tests#8663

Open
sssd-bot wants to merge 1 commit into
SSSD:sssd-2-10from
sssd-bot:SSSD-sssd-backport-pr8519-to-sssd-2-10
Open

[autobackport: sssd-2-10] Smartcard multi token tests#8663
sssd-bot wants to merge 1 commit into
SSSD:sssd-2-10from
sssd-bot:SSSD-sssd-backport-pr8519-to-sssd-2-10

Conversation

@sssd-bot

Copy link
Copy Markdown
Contributor

This is an automatic backport of PR#8519 Smartcard multi token tests to branch sssd-2-10, created by @krishnavema.

Please make sure this backport is correct.

Note

The commits were cherry-picked without conflicts.

You can push changes to this pull request

git remote add sssd-bot git@github.com:sssd-bot/sssd.git
git fetch sssd-bot refs/heads/SSSD-sssd-backport-pr8519-to-sssd-2-10
git checkout SSSD-sssd-backport-pr8519-to-sssd-2-10
git push sssd-bot SSSD-sssd-backport-pr8519-to-sssd-2-10 --force

Original commits
e5b6597 - tests: implement multi-token support for smart card authentication

Backported commits

  • c8e6ba7 - tests: implement multi-token support for smart card authentication

Original Pull Request Body

Reviewed-by: Scott Poore <spoore@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
(cherry picked from commit e5b6597)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces new system tests for smartcard authentication scenarios involving multiple SoftHSM tokens, along with a helper function to initialize these tokens with IPA-signed certificates. A critical issue was identified in the parametrized test test_smartcard__two_tokens_match_on_both, where the attempt to create the same IPA user in every iteration would lead to a failure; a suggestion was provided to ensure unique usernames are used for each test run.

Comment on lines +145 to +146
username = "scuser_both"
ipa.user(username).add()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The test test_smartcard__two_tokens_match_on_both is parametrized by cert_selection, which causes the entire test function to run twice. On the second iteration, ipa.user(username).add() will fail because the user scuser_both already exists in the IPA server. To ensure test isolation and avoid this failure, you should use a unique username for each iteration.

Suggested change
username = "scuser_both"
ipa.user(username).add()
username = f"scuser_both_{cert_selection}"
ipa.user(username).add()

@sumit-bose sumit-bose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi,

original patch and backport matches, ACK.

Imo the Gemini comment is not correct because the IPA server is refreshed during the tests.

If this test is needed in sssd-2-10 it can be merged.

bye,
Sumit

@sumit-bose sumit-bose self-requested a review June 12, 2026 14:22
@sumit-bose

Copy link
Copy Markdown
Contributor

Ah, sorry, I realized too late that the tests were failing in the CI run. Will check this ....

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants