Skip to content

test rewrite: legacy intg test_pam_responder.py - #8873

Draft
danlavu wants to merge 1 commit into
SSSD:masterfrom
danlavu:rewrite-test_pam_responder.py
Draft

test rewrite: legacy intg test_pam_responder.py#8873
danlavu wants to merge 1 commit into
SSSD:masterfrom
danlavu:rewrite-test_pam_responder.py

Conversation

@danlavu

@danlavu danlavu commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Port sssd/src/tests/intg/test_pam_responder.py to test_smartcard.py, test_authentication.py, and test_ldap_krb5.py.

  • test_smartcard__wrong_pin_entered_is_rejected
  • test_smartcard__try_cert_auth_fails_when_certificate_not_mapped
  • test_smartcard__pam_p11_allowed_services_controls_fallback
  • test_smartcard__require_cert_auth_succeeds_with_card
  • test_smartcard__require_cert_auth_fails_without_card
  • test_authentication__custom_password_prompt_is_shown_at_login
  • test_ldap_krb5__pam_sss_domains_option_skips_non_matching_domains

Co-authored-by: Cursor cursoragent@cursor.com
Model used: Claude Sonnet 4.6

@danlavu danlavu changed the title tests: legacy intg test_pam_responder.py tests test rewrite: legacy intg test_pam_responder.py Jul 1, 2026

@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 adds several system tests for SSSD, covering custom password prompts, PAM domain filtering, and various smart card authentication scenarios such as incorrect PIN handling, unmapped certificates, and required certificate authentication. The feedback recommends using inspect.cleandoc when writing multi-line PAM configuration files to prevent leading indentation from causing malformed configuration files.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/tests/system/tests/test_ldap_krb5.py Outdated
Comment thread src/tests/system/tests/test_smartcard.py Outdated
Comment thread src/tests/system/tests/test_smartcard.py
Comment thread src/tests/system/tests/test_ldap_krb5.py Outdated
@danlavu
danlavu force-pushed the rewrite-test_pam_responder.py branch from 342e202 to 652c0c5 Compare July 10, 2026 18:02
@danlavu
danlavu requested a review from krishnavema July 10, 2026 19:27

@krishnavema krishnavema 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.

LGTM!

@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,

thank you for the tests. The ones where I commented "This test replaces ..." are fine and the mentioned integration tests can be removed.

I'm not sure how easy it is to fix the other tests. But imo it would be ok if you split out the "good" ones in a new PR which also contains a pathc to remove the related integration tests and keep the others here for further improvement?

bye,
Sumit

Comment thread src/tests/system/tests/test_authentication.py
@pytest.mark.importance("medium")
@pytest.mark.authentication
@pytest.mark.topology(KnownTopology.LDAP_KRB5)
def test_ldap_krb5__pam_sss_domains_option_skips_non_matching_domains(

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.

'The purpose of the integration test test_krb5_auth_domains is different. The imporatn part is that the other domains are configured in sssd.conf and handle the same users but only a single domain (the last in the PAM configuration) can actually authenticate the user. The use-case is Kerberos authentication for local users where the users are coming from different Kerberos realms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was reworking this test, there is something I dont understand. Are the other domains working or non-working domains? So this was the configuration from the intg test.

[sssd]
debug_level = 10
domains = wrong.dom1, wrong.dom2, krb5_auth, wrong.dom3
services = pam, nss
[nss]
debug_level = 10
[pam]
debug_level = 10
[domain/wrong.dom1]
debug_level = 10
id_provider = proxy
proxy_lib_name = call
auth_provider = krb5
krb5_realm = WRONG1REALM
krb5_server = localhost:10888
[domain/wrong.dom2]
debug_level = 10
id_provider = proxy
proxy_lib_name = call
auth_provider = krb5
krb5_realm = WRONG2REALM
krb5_server = localhost:10888
[domain/wrong.dom3]
debug_level = 10
id_provider = proxy
proxy_lib_name = call
auth_provider = krb5
krb5_realm = WRONG3REALM
krb5_server = localhost:10888
[domain/krb5_auth]
debug_level = 10
id_provider = proxy
proxy_lib_name = call
auth_provider = krb5
krb5_realm = PAMKRB5TEST
krb5_server = localhost:10888
/etc/pam.d/pam_sss_domains (from Makefile.am, target pam_sss_domains):

auth     sufficient       /usr/lib64/security/pam_sss.so forward_pass domains=wrong.dom1
auth     sufficient       /usr/lib64/security/pam_sss.so forward_pass domains=wrong.dom2
auth     sufficient       /usr/lib64/security/pam_sss.so forward_pass domains=wrong.dom3
auth     sufficient       /usr/lib64/security/pam_sss.so forward_pass domains=krb5_auth
auth     required         pam_deny.so
account  required         /usr/lib64/security/pam_sss.so
password required         /usr/lib64/security/pam_sss.so
session  required         /usr/lib64/security/pam_sss.so

If all the other domains are valid and exist, say we used Samba, IPA, and in the test environment as the.wrongdom1 and 2 and they have conflicting users. i.e.

    for name, role in (("samba", samba), ("ipa", ipa), ("krb5", kdc)):
        client.sssd.dom(name).update(
            enabled="true",
            id_provider="proxy",
            proxy_lib_name="files",
            auth_provider="krb5",
            krb5_realm=role.realm,
            krb5_server=role.host.hostname,
        )
    client.sssd.sssd["domains"] = "samba, ipa, krb5"
    client.sssd.default_domain = "krb5"
    client.sssd.start()

    client.fs.backup("/etc/pam.d/su-l")
    client.fs.write(
        "/etc/pam.d/su-l",
        cleandoc("""
            auth        required    pam_env.so
            auth        sufficient  pam_sss.so forward_pass domains=samba
            auth        sufficient  pam_sss.so forward_pass domains=ipa
            auth        sufficient  pam_sss.so forward_pass domains=krb5
            auth        required    pam_deny.so
            account     required    pam_sss.so
            password    required    pam_sss.so
            session     required    pam_sss.so
            """),
    )

I don't see what would prevent user1@samba.test or user1@ipa.test from authenticating in this pam stack if the domains are valid. It would hit samba, it's sufficient to auth, and let the samba user in. The test only uses krb5 because the rest of the realms don't work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@spoore1 and I talked about it. We concluded that while the scenario is slightly different but the outcome is the same. The integration test fails because the kerberos domains are invalid. In this scenario, the Kerberos domain will fail because the passwords for the users in the other domains are incorrect, skipping to the next. I think it is slightly more functional. What makes sense to me, is to remove the extra auth lines for samba and ipa so there is no way that they should be able to authenticate. Ultimately, the outcome is the same; it iterates through the failed domains and successful logins.

Comment thread src/tests/system/tests/test_smartcard.py
Comment thread src/tests/system/tests/test_smartcard.py
Comment thread src/tests/system/tests/test_smartcard.py
Comment thread src/tests/system/tests/test_smartcard.py
@danlavu
danlavu force-pushed the rewrite-test_pam_responder.py branch from 652c0c5 to 53dd5c2 Compare July 23, 2026 18:29
Port sssd/src/tests/intg/test_pam_responder.py to test_smartcard.py, test_authentication.py, and test_ldap_krb5.py.

- test_smartcard__login_fails_when_wrong_pin_is_entered
- test_smartcard__login_fails_when_card_is_not_mapped
- test_smartcard__cert_auth_limited_to_allowed_pam_services
- test_smartcard__login_succeeds_when_cert_auth_required
- test_smartcard__login_fails_when_cert_auth_required_without_card
- test_authentication__custom_password_prompt_is_shown_at_login
- test_ldap_krb5__pam_sss_domains_option_skips_non_matching_domains

Co-authored-by: Cursor <cursoragent@cursor.com>
Model used: Claude Sonnet 4.6
@danlavu
danlavu force-pushed the rewrite-test_pam_responder.py branch 2 times, most recently from 114930d to ad4d39b Compare July 24, 2026 16:33
@danlavu
danlavu marked this pull request as draft July 24, 2026 16:46
danlavu pushed a commit to danlavu/sssd that referenced this pull request Jul 24, 2026
Port sssd/src/tests/intg/test_pam_responder.py to test_smartcard.py,
test_authentication.py

- test_smartcard__login_fails_when_wrong_pin_is_entered
- test_smartcard__login_fails_when_card_is_not_mapped
- test_smartcard__cert_auth_limited_to_allowed_pam_services
- test_smartcard__login_succeeds_when_cert_auth_required
- test_smartcard__login_fails_when_cert_auth_required_without_card
- test_authentication__custom_password_prompt_is_shown_at_login

Peeling out the reviewed test cases into it's own PR, from
SSSD#8873

Co-authored-by: Cursor <cursoragent@cursor.com>
Model used: Claude Sonnet 4.6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants