Tests: Adding GDM Passkey tests #8150

spoore1 · 2025-10-18T17:07:01Z

No description provided.

gemini-code-assist

Code Review

This pull request adds a comprehensive suite of system tests for GDM Passkey login functionality. The tests cover various scenarios, including login with PIN, without PIN, with password fallback, multiple keys, and handling of unregistered or removed passkeys.

My review has identified a few critical and high-severity issues that should be addressed:

A pytest.set_trace() call needs to be removed.
The use of time.sleep() should be replaced with more robust polling mechanisms to avoid flaky tests.
Fragile shell commands for parsing ipa output should be replaced with more stable API calls.
There's an incorrect usage of assert with a custom assertion method, which could lead to misleading test failures.

Overall, this is a valuable addition for ensuring the quality of the GDM Passkey feature. Addressing these points will improve the reliability and maintainability of the new tests.

src/tests/system/tests/test_gdm_passkey.py

ikerexxe

Some minor comments inline

src/tests/system/tests/test_gdm_passkey.py

ikerexxe · 2025-10-29T11:12:24Z

By the way, CI failures are related to your changes so review them as well

justin-stephenson · 2025-11-05T13:27:13Z

Hi @spoore1 would it be possible for you to add a GDM Passkey test with local (non-IPA) auth?

$ sssctl passkey-exec --register --username=alice --domain=ldap.test

Then on the ldap container 

$ vim user.ldif
# alice, ldap.test
# replace passkey data below
dn: uid=alice,dc=ldap,dc=test
mail: alice@ldap.test
uid: alice
givenName: Alice
objectClass: top
objectClass: person
objectClass: organizationalPerson
objectClass: inetorgperson
objectClass: posixAccount
objectClass: inetuser
objectClass: passkeyUser
sn: Moreau
cn: Alice Moreau
uidNumber: 2000
gidNumber: 2000
homeDirectory: /home/alice
loginShell: /bin/bash
gecos: alice
passkey: passkey:zSidKsavt3TudFuW9Hl4oEzif6SzPenk4LuWjQmUZB8USfhJW5NfA7faOOr9Z
 tarevL0GwKL9oytC7yPVIIPGg==,MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE+J5jOEO7E6qMa
 LA8n1oyYKVG3w0af+mrXAjybl3qdLEsSBCsk9SyuqXMbK1xnrBCyL5bgIX6ONeULx86YkP05w==

$ ldapadd -D "cn=Directory Manager" -w Secret123 -H ldap://localhost -x -f user.ldif


And on the client with `local_auth_policy = enable:passkey` set

 # su - alice@ldap.test

src/tests/system/tests/test_gdm_doc.py

ikerexxe

What does the test_gdm_doc.py file do? Is it some sort of extra documentation?

Apart from that I left several comments inline

src/tests/system/mhc_2.yaml

src/tests/system/tests/test_gdm_kcacl.py

src/tests/system/tests/test_gdm_passkey.py

src/tests/system/tests/test_gdm_smartcard.py

src/tests/system/tests/test_gdm_kcacl.py

src/tests/system/tests/test_gdm_passkey.py

src/tests/system/tests/test_gdm_smartcard.py

spoore1 · 2025-11-10T15:28:29Z

What does the test_gdm_doc.py file do? Is it some sort of extra documentation?

Apart from that I left several comments inline

Sorry about the confusion here. It seems I ran a git add . instead of the specific file I was updating and brought in some extra files/notes I didn't intend to include here. I've removed them again from the commit now.

src/tests/system/tests/test_gdm_passkey.py

src/tests/system/mhc.yaml

jakub-vavra-cz

sss mhc.yaml comment

madhuriupadhye

Looks good to me; I plan to test this in IDM-CI.

ikerexxe

LGTM! Thank you for your hard work getting passkey tests for this feature

jakub-vavra-cz

LGTM

justin-stephenson

Ack, thank you.

alexey-tikhonov · 2025-11-25T14:06:54Z

@spoore1, does this depend on product patches / PRs, that were not merged yet?

alexey-tikhonov · 2025-11-26T11:59:56Z

C10S CI is red due to known issue:

ERROR tests/test_feature.py::test_feature__presence[Fedora-39-0-2-9-passkey-True]
...
cat: \'/tmp/tmp.UDjJVryOAU/audit/audit.log*\': No such file or directory\n') [single exception in TeardownExceptionGroup]

The new "bare" topologies are missing from the system tests' mhc.yaml file in provisioned_topologies. These need to be there for upstream CI tests to work with tests using those topologies. Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Jakub Vávra <jvavra@redhat.com> Reviewed-by: Justin Stephenson <jstephen@redhat.com> Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Jakub Vávra <jvavra@redhat.com> Reviewed-by: Justin Stephenson <jstephen@redhat.com> Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>

sssd-bot · 2025-11-26T12:00:26Z

The pull request was accepted by @alexey-tikhonov 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.

gemini-code-assist bot reviewed Oct 18, 2025

View reviewed changes

spoore1 force-pushed the test_gdm_passkey branch from 2691304 to 4e3e389 Compare October 21, 2025 15:50

alexey-tikhonov requested a review from ikerexxe October 23, 2025 12:39

alexey-tikhonov assigned ikerexxe Oct 23, 2025

alexey-tikhonov requested a review from jakub-vavra-cz October 23, 2025 12:40

alexey-tikhonov assigned jakub-vavra-cz Oct 23, 2025

spoore1 force-pushed the test_gdm_passkey branch from 4e3e389 to 6119206 Compare October 26, 2025 15:49

ikerexxe reviewed Oct 29, 2025

View reviewed changes

src/tests/system/tests/test_gdm_passkey.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_gdm_passkey.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_gdm_passkey.py Outdated Show resolved Hide resolved

pbrezina force-pushed the master branch 2 times, most recently from f5d64b3 to b854636 Compare November 4, 2025 14:27

justin-stephenson self-assigned this Nov 5, 2025

justin-stephenson self-requested a review November 5, 2025 13:27

spoore1 force-pushed the test_gdm_passkey branch 5 times, most recently from c193edb to eea3a7e Compare November 8, 2025 16:43

spoore1 requested a review from ikerexxe November 8, 2025 16:48

spoore1 force-pushed the test_gdm_passkey branch from eea3a7e to f2bf490 Compare November 8, 2025 17:07

github-advanced-security bot found potential problems Nov 8, 2025

View reviewed changes

ikerexxe reviewed Nov 10, 2025

View reviewed changes

src/tests/system/tests/test_gdm_kcacl.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_gdm_passkey.py Outdated Show resolved Hide resolved

src/tests/system/tests/test_gdm_smartcard.py Outdated Show resolved Hide resolved

spoore1 force-pushed the test_gdm_passkey branch 3 times, most recently from 97ef4a5 to b3341f2 Compare November 10, 2025 15:23

spoore1 force-pushed the test_gdm_passkey branch from b3341f2 to 302bd12 Compare November 10, 2025 15:29

madhuriupadhye reviewed Nov 11, 2025

View reviewed changes

src/tests/system/tests/test_gdm_passkey.py Show resolved Hide resolved

spoore1 force-pushed the test_gdm_passkey branch 6 times, most recently from 034996a to 882fe7c Compare November 23, 2025 13:54

jakub-vavra-cz reviewed Nov 24, 2025

View reviewed changes

src/tests/system/mhc.yaml Outdated Show resolved Hide resolved

jakub-vavra-cz requested changes Nov 24, 2025

View reviewed changes

spoore1 force-pushed the test_gdm_passkey branch 4 times, most recently from dbb1690 to 398172a Compare November 24, 2025 15:54

spoore1 requested review from ikerexxe, jakub-vavra-cz and madhuriupadhye November 24, 2025 15:55

madhuriupadhye approved these changes Nov 24, 2025

View reviewed changes

ikerexxe approved these changes Nov 25, 2025

View reviewed changes

jakub-vavra-cz approved these changes Nov 25, 2025

View reviewed changes

justin-stephenson approved these changes Nov 25, 2025

View reviewed changes

spoore1 added Tests no-backport This should go to target branch only. labels Nov 25, 2025

alexey-tikhonov added the Accepted label Nov 26, 2025

spoore1 added 2 commits November 26, 2025 12:00

Tests: Adding GDM Passkey tests

5635e5f

Reviewed-by: Iker Pedrosa <ipedrosa@redhat.com> Reviewed-by: Jakub Vávra <jvavra@redhat.com> Reviewed-by: Justin Stephenson <jstephen@redhat.com> Reviewed-by: Madhuri Upadhye <mupadhye@redhat.com>

sssd-bot force-pushed the test_gdm_passkey branch from 398172a to 5635e5f Compare November 26, 2025 12:00

alexey-tikhonov merged commit 05fa421 into SSSD:master Nov 26, 2025
9 of 14 checks passed

ikerexxe mentioned this pull request Nov 26, 2025

Passwordless-gdm #8212

Merged

Tests: Adding GDM Passkey tests #8150

Tests: Adding GDM Passkey tests #8150

Uh oh!

Conversation

spoore1 commented Oct 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ikerexxe commented Oct 29, 2025

Uh oh!

justin-stephenson commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spoore1 commented Nov 10, 2025

Uh oh!

Uh oh!

Uh oh!

jakub-vavra-cz left a comment

Choose a reason for hiding this comment

Uh oh!

madhuriupadhye left a comment

Choose a reason for hiding this comment

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

jakub-vavra-cz left a comment

Choose a reason for hiding this comment

Uh oh!

justin-stephenson left a comment

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexey-tikhonov commented Nov 26, 2025

Uh oh!

sssd-bot commented Nov 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

justin-stephenson commented Nov 5, 2025 •

edited

Loading

alexey-tikhonov commented Nov 25, 2025 •

edited

Loading