Test: combine gdm tests into one file

[spoore1]

Combining all the GDM tests into a single test module to simplify management of these specific tests.

Refactoring setup helper functions and adding xidp one.

Marking critical tests as well.

Removing some unnecessary comments and adding blank lines to make setup steps match code.

Renaming some test cases to make purpose more clear.

(cherry picked from commit 17390fd)

[gemini-code-assist]

Code Review

This pull request consolidates SSSD Passwordless GDM tests from several specialized files into a single test_gdm.py file and introduces reusable helper functions for client setup and smart card enrollment. The reviewer feedback focuses on enhancing test robustness and maintainability by replacing hardcoded domain strings with dynamic attributes from provider objects, implementing safer dictionary access for passkey mappings to prevent potential KeyError or IndexError exceptions, and removing a redundant time.sleep() call to reduce test flakiness.

[gemini-code-assist]

The IPA domain is hardcoded as "ipa.test". This makes the helper function less robust and reusable for topologies with different IPA domain names. It's better to use the domain attribute from the ipa object, similar to how it's done in client_setup_sssd.

Suggested change

	client.sssd.import_domain("ipa.test", ipa)
	client.sssd.import_domain(ipa.domain, ipa)

[gemini-code-assist]

The check for the passkey mapping could be made more robust. The get method might return a dictionary that doesn't contain the ipapasskey key, or the value could be an empty list. Accessing result["ipapasskey"][0] directly could lead to a KeyError or IndexError. It's safer to check for the existence of the key and that the list is not empty before accessing its first element.

Suggested change

	result = ipa.user(testuser).get(["ipapasskey"])
	if result is not None:
	ipa.user(testuser).passkey_remove(result["ipapasskey"][0])
	else:
	raise ValueError(f"ipa.user({testuser}) passkey mapping not found")
	result = ipa.user(testuser).get(["ipapasskey"])
	if result and result.get("ipapasskey"):
	ipa.user(testuser).passkey_remove(result["ipapasskey"][0])
	else:
	raise ValueError(f"ipa.user({testuser}) passkey mapping not found")

[gemini-code-assist]

Similar to a previous section, this check for the passkey mapping could be more robust. Accessing result["ipapasskey"][0] directly is risky if the key is missing or the list is empty. A safer check would prevent potential KeyError or IndexError exceptions and make the test more reliable.

Suggested change

	result = ipa.user(testuser).get(["ipapasskey"])
	if result is not None:
	ipa.user(testuser).passkey_remove(result["ipapasskey"][0])
	else:
	raise ValueError(f"ipa.user({testuser}) passkey mapping not found")
	result = ipa.user(testuser).get(["ipapasskey"])
	if result and result.get("ipapasskey"):
	ipa.user(testuser).passkey_remove(result["ipapasskey"][0])
	else:
	raise ValueError(f"ipa.user({testuser}) passkey mapping not found")

[gemini-code-assist]

The domain is hardcoded as "ldap.test". This makes the test less robust. It's better to use the domain attribute from the ldap provider object, like ldap.domain.

Suggested change

	mapping = client.sssctl.passkey_register(username=testuser, domain="ldap.test", pin=123456, virt_type="vfido")
	mapping = client.sssctl.passkey_register(username=testuser, domain=ldap.domain, pin=123456, virt_type="vfido")

[spoore1]

This is a backport from the master commit that was merged from PR8543

[ikerexxe]

LGTM, but did you take a look at Gemini's review? I'm assuming we must have rejected part of it in the original PR

[spoore1]

LGTM, but did you take a look at Gemini's review? I'm assuming we must have rejected part of it in the original PR

I looked at some of Gemini's reviews. I don't remember these things being found in the original but, I think some of them should be implemented. I was thinking I'd work through this backport and then open a new PR to master with these updates and autobackport to 2-12.

[ikerexxe]

LGTM