Smartcard multi token tests by krishnavema · Pull Request #8519 · SSSD/sssd

krishnavema · 2026-03-15T16:12:25Z

No description provided.

gemini-code-assist

Code Review

This pull request introduces system tests for multi-token smartcard authentication. It includes new helper functions for setting up tokens and authenticating, along with several test cases covering different scenarios with two tokens. A key change is updating the sssd-test-framework dependency to a personal fork to support these new tests. My review focuses on the risk associated with this dependency and on improving the maintainability and robustness of the new test code by addressing magic numbers and polling logic.

gemini-code-assist · 2026-03-15T16:14:18Z

 git+https://github.com/next-actions/pytest-tier
 git+https://github.com/next-actions/pytest-output
-git+https://github.com/SSSD/sssd-test-framework
+git+https://github.com/krishnavema/sssd-test-framework@multi-token-smart-card-support


This change introduces a dependency on a personal fork (krishnavema/sssd-test-framework). While this might be acceptable for development, it poses a security and maintenance risk for the main branch. The changes from this fork should be merged into the upstream SSSD/sssd-test-framework repository, and the dependency should point to an official release or commit from the upstream repository before this pull request is merged.

git+https://github.com/SSSD/sssd-test-framework

spoore1

This is a good start. I've run into a couple snags with testing but basics are working so here's a start for review.

alexey-tikhonov · 2026-03-25T09:32:32Z

'changes requested': all platforms fail test_smartcard__two_tokens_match_on_both

spoore1 · 2026-03-26T12:56:53Z

'changes requested': all platforms fail test_smartcard__two_tokens_match_on_both

We're investigating this issue now. I hadn't looked at the test failures yet because I was hitting this one locally.

sumit-bose · 2026-04-22T11:07:28Z

Hi,

please try to run the tests with #8629, his PR should hopefully fix the issue with test_smartcard__two_tokens_match_on_both.

bye,
Sumit

spoore1 · 2026-04-22T15:12:12Z

Hi,

please try to run the tests with #8629, his PR should hopefully fix the issue with test_smartcard__two_tokens_match_on_both.

bye, Sumit

Yes, I can confirm that fixes the failure. I posted to that PR as well.

I tested on Fedora 42 containers and I can confirm this fixes the test_smartcard__two_tokens_match_on_both failure in PR#8519:

Version:

[root@client ~]# rpm -q sssd
sssd-2.13.0-99.20260422110653129739.pr8629.168.g4868b17eb.fc42.x86_64

Results:

tests/test_smartcard.py::test_smartcard__two_tokens_match_on_first (ipa) PASSED
tests/test_smartcard.py::test_smartcard__two_tokens_match_on_second (ipa) PASSED
tests/test_smartcard.py::test_smartcard__two_tokens_match_on_both (ipa) PASSED
==== 3 passed, 1 deselected in 200.01s (0:03:20) ====

spoore1

You can probably move the enroll_to_token() to the framework. It might be useful in other places (like the GDM tests) to consolidate and reduce duplicate code. Overall the tests look good. And with the fix in #8629, all the tests are working now.

spoore1 · 2026-04-22T22:09:57Z

+TOKEN_PIN = "123456"
+
+
+def enroll_to_token(


I'm wondering now if this would be better to add this to the smartcard util in the framework with this: SSSD/sssd-test-framework#239

Also, if this is moved to the framework, can the initialize_card() be included here to make this more generic?

Hi,

I see this same pattern in test_gdm.py and test_ipa.py so I agree that it makes sense to move it to the framework and I think initialize_card() can be added as well to cover the most common case. The variant in test_gdm.py even has an option to call it or not.

bye,
Sumit

spoore1

I think the only thing left with this one is to decide if you still need the intialize_card()'s in the setup_two_tokens() helper. I think it can be dropped now with the change in sssd-test-framework#239 right?

sumit-bose

Hi,

thank you for the updates, ACK.

Please do not forget to remove the first patch before setting 'Accepted'.

bye,
Sumit

spoore1 · 2026-04-27T18:23:29Z

Test failures unrelated:

ERROR tests/test_kcm.py::test_kcm__tgt_renewal_updates_ticket_as_configured (client) - TypeError: _FlakyPlugin._make_test_flaky() got an unexpected keyword argument 'reruns'

spoore1

LGTM

Reviewed-by: Scott Poore <[email protected]> Reviewed-by: Sumit Bose <[email protected]>

sssd-bot · 2026-04-28T20:36:02Z

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

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

krishnavema requested a review from spoore1 March 15, 2026 16:12

gemini-code-assist Bot reviewed Mar 15, 2026

View reviewed changes

krishnavema assigned spoore1 Mar 17, 2026

krishnavema requested a review from sumit-bose March 17, 2026 07:50

krishnavema assigned sumit-bose Mar 17, 2026

spoore1 mentioned this pull request Mar 18, 2026

Add support for multi-token management in SmartCardUtils SSSD/sssd-test-framework#239

Merged

spoore1 requested changes Mar 18, 2026

View reviewed changes

Comment thread src/tests/system/tests/test_smartcard.py Outdated

Comment thread src/tests/system/tests/test_smartcard.py Outdated

Comment thread src/tests/system/tests/test_smartcard.py Outdated

krishnavema force-pushed the smartcard-multi-token-tests branch from 2592a18 to 1322416 Compare March 20, 2026 08:01

krishnavema marked this pull request as ready for review March 20, 2026 08:02

krishnavema requested a review from spoore1 March 20, 2026 08:03

alexey-tikhonov added the Changes requested label Mar 25, 2026

krishnavema marked this pull request as draft March 27, 2026 12:20

alexey-tikhonov added the Tests label Apr 14, 2026

spoore1 mentioned this pull request Apr 22, 2026

krb5: restart krb5_child for Smartcard authentication #8629

Merged

spoore1 requested changes Apr 22, 2026

View reviewed changes

krishnavema marked this pull request as ready for review April 23, 2026 16:56

krishnavema force-pushed the smartcard-multi-token-tests branch from 1322416 to 44bb74a Compare April 23, 2026 17:09

krishnavema requested a review from spoore1 April 23, 2026 17:17

krishnavema force-pushed the smartcard-multi-token-tests branch from 44bb74a to 9d50c98 Compare April 24, 2026 08:34

sumit-bose reviewed Apr 24, 2026

View reviewed changes

Comment thread src/tests/system/tests/test_smartcard.py Outdated

sumit-bose reviewed Apr 24, 2026

View reviewed changes

Comment thread src/tests/system/tests/test_smartcard.py Outdated

krishnavema force-pushed the smartcard-multi-token-tests branch from 9d50c98 to 401bde4 Compare April 24, 2026 15:58

krishnavema requested a review from sumit-bose April 24, 2026 15:59

spoore1 reviewed Apr 24, 2026

View reviewed changes

Comment thread src/tests/system/tests/test_smartcard.py Outdated

krishnavema force-pushed the smartcard-multi-token-tests branch from 401bde4 to 49ea146 Compare April 25, 2026 11:17

sumit-bose approved these changes Apr 27, 2026

View reviewed changes

krishnavema force-pushed the smartcard-multi-token-tests branch from 49ea146 to bfcf14b Compare April 27, 2026 16:04

krishnavema force-pushed the smartcard-multi-token-tests branch 2 times, most recently from 21b2183 to 9ee62de Compare April 28, 2026 14:30

spoore1 added backport-to-sssd-2-9 backport-to-sssd-2-10 backport-to-sssd-2-12 backport-to-sssd-2-13 and removed Changes requested labels Apr 28, 2026

spoore1 approved these changes Apr 28, 2026

View reviewed changes

spoore1 added the Accepted label Apr 28, 2026

tests: implement multi-token support for smart card authentication

a108ebf

Reviewed-by: Scott Poore <[email protected]> Reviewed-by: Sumit Bose <[email protected]>

sssd-bot force-pushed the smartcard-multi-token-tests branch from 9ee62de to a108ebf Compare April 28, 2026 20:36

sumit-bose merged commit e5b6597 into SSSD:master Apr 29, 2026
15 checks passed

		TOKEN_PIN = "123456"


		def enroll_to_token(

Uh oh!

Conversation

krishnavema commented Mar 15, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

spoore1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexey-tikhonov commented Mar 25, 2026

Uh oh!

spoore1 commented Mar 26, 2026

Uh oh!

sumit-bose commented Apr 22, 2026

Uh oh!

spoore1 commented Apr 22, 2026

Uh oh!

spoore1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

spoore1 Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

sumit-bose Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

spoore1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sumit-bose left a comment

Choose a reason for hiding this comment

Uh oh!

spoore1 commented Apr 27, 2026

Uh oh!

spoore1 left a comment

Choose a reason for hiding this comment

Uh oh!

sssd-bot commented Apr 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants