Tests: cache_credentials = true not working by shridhargadekar · Pull Request #8020 · SSSD/sssd

shridhargadekar · 2025-07-04T13:39:10Z

Tests for cache_credentials = true not working in sssd, with specified PAM configuration in /etc/pam.d/system-auth and /etc/pam.d/password-auth

verifies #7968

gemini-code-assist

Code Review

The pull request adds a new test to verify that cache_credentials = true works correctly with a custom PAM configuration when the provider is offline. The test logic is sound and covers the intended scenario. My review focuses on improving the maintainability and clarity of the new test by suggesting a more descriptive name for the test function and updating its docstring to be accurate and complete.

src/tests/system/tests/test_authentication.py

thalman

Thanks for the effort.

src/tests/system/tests/test_authentication.py

sumit-bose

Hi,

thank you for the update, ACK.

Please note, this patch can only be committed if #7995 is committed, otherwise CI will always fail.

bye,
Sumit

alexey-tikhonov · 2025-07-07T15:23:54Z

Please note, this patch can only be committed if #7995 is committed, otherwise CI will always fail.

#7995 was merged. Re-running CI.

alexey-tikhonov · 2025-07-07T19:06:59Z

@shridhargadekar, please rebase.

alexey-tikhonov · 2025-07-08T09:19:14Z

tests/test_authentication.py::test_authentication__user_login_with_modified_PAM_stack_provider_is_offline[root-ssh] (ipa) PASSED [ 14%]
tests/test_authentication.py::test_authentication__user_login_with_modified_PAM_stack_provider_is_offline[root-su] (ipa) PASSED [ 14%]
tests/test_authentication.py::test_authentication__user_login_with_modified_PAM_stack_provider_is_offline[sssd-ssh] (ipa) PASSED [ 14%]
tests/test_authentication.py::test_authentication__user_login_with_modified_PAM_stack_provider_is_offline[sssd-su] (ipa) PASSED [ 14%]

Failing tests seems to be unrelated to this PR.

alexey-tikhonov · 2025-07-10T11:13:02Z

Conflict in sssd-2-9.
@shridhargadekar, please open explicit backport PR.

src/tests/system/tests/test_authentication.py

shridhargadekar · 2025-07-16T13:44:42Z

@sumit-bose latest changes are reverting PAM configuration to original state. Let me check with authselect way as well

shridhargadekar · 2025-07-16T13:48:45Z

Looks like previously failing tests are passing now.

shridhargadekar · 2025-07-22T18:54:08Z

Three test cases that are failing here, could not be related to this PR. Those test cases are running before current PR test cases.

alexey-tikhonov · 2025-07-24T12:15:54Z

Is this intentional:

pam_file = client.fs.read("/etc/pam.d/system-auth")
...
client.fs.write("/etc/pam.d/system-auth", pam_file)
client.fs.write("/etc/pam.d/password-auth", pam_file)

-- so that "password-auth" == "system-auth" after the test?

@sumit-bose , @thalman , are you fine with the recent updates?

shridhargadekar · 2025-07-24T13:54:30Z

As Sumit suggested earlier, implementing authselect utility to back-up and restore the PAM stack.

alexey-tikhonov · 2025-07-25T07:30:49Z

@shridhargadekar, is this ready?

shridhargadekar · 2025-07-25T09:50:53Z

@alexey-tikhonov Yes, it's ready.

src/tests/system/tests/test_authentication.py

Tests for cache_credentials = true not working in sssd, with specified PAM configuration in /etc/pam.d/system-auth and /etc/pam.d/password-auth verifies SSSD#7968

sumit-bose · 2025-07-29T15:26:21Z

Hi,

I'm still fine withthe current version of the test, CI failures do not look related.

bye,
Sumit

alexey-tikhonov · 2025-07-31T09:06:59Z

Pushed PR: #8020

master
- 9856b6d - Tests: cache_credentials = true not working
sssd-2-11
- 61a5ab6 - Tests: cache_credentials = true not working

gemini-code-assist bot reviewed Jul 4, 2025

View reviewed changes

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

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

shridhargadekar force-pushed the offline branch from 7c5b24c to 4fb820d Compare July 4, 2025 13:43

shridhargadekar marked this pull request as ready for review July 4, 2025 13:44

alexey-tikhonov added Waiting for review backport-to-sssd-2-9 backport-to-sssd-2-11 labels Jul 4, 2025

alexey-tikhonov requested review from sumit-bose and thalman July 4, 2025 15:26

thalman self-assigned this Jul 4, 2025

thalman requested changes Jul 4, 2025

View reviewed changes

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

alexey-tikhonov assigned sumit-bose Jul 4, 2025

shridhargadekar force-pushed the offline branch 2 times, most recently from e4a75f7 to f8fe707 Compare July 7, 2025 08:26

shridhargadekar requested a review from thalman July 7, 2025 08:26

thalman approved these changes Jul 7, 2025

View reviewed changes

sumit-bose reviewed Jul 7, 2025

View reviewed changes

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

shridhargadekar force-pushed the offline branch from f8fe707 to 6906cbe Compare July 7, 2025 10:14

sumit-bose approved these changes Jul 7, 2025

View reviewed changes

sumit-bose added Accepted Blocked and removed Waiting for review labels Jul 7, 2025

alexey-tikhonov mentioned this pull request Jul 7, 2025

krb5: offline with SSS_AUTHTOK_TYPE_PAM_STACKED #7995

Closed

alexey-tikhonov removed the Blocked label Jul 7, 2025

shridhargadekar force-pushed the offline branch from 6906cbe to 86265a7 Compare July 7, 2025 19:55

alexey-tikhonov added Ready to push and removed Ready to push labels Jul 8, 2025

github-advanced-security bot found potential problems Jul 16, 2025

View reviewed changes

src/tests/system/tests/test_authentication.py Fixed Show fixed Hide fixed

src/tests/system/tests/test_authentication.py Fixed Show fixed Hide fixed

shridhargadekar force-pushed the offline branch from 682a41a to 38b4f0f Compare July 16, 2025 12:48

shridhargadekar force-pushed the offline branch 3 times, most recently from a413452 to d4d21a2 Compare July 24, 2025 13:52

shridhargadekar force-pushed the offline branch from d4d21a2 to fd134e2 Compare July 24, 2025 13:55

shridhargadekar force-pushed the offline branch from fd134e2 to 50bd8bd Compare July 25, 2025 09:50

sumit-bose reviewed Jul 25, 2025

View reviewed changes

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

shridhargadekar force-pushed the offline branch from 50bd8bd to 5dc61b4 Compare July 25, 2025 12:52

Tests: cache_credentials = true not working

7baaec5

Tests for cache_credentials = true not working in sssd, with specified PAM configuration in /etc/pam.d/system-auth and /etc/pam.d/password-auth verifies SSSD#7968

shridhargadekar force-pushed the offline branch from 5dc61b4 to 7baaec5 Compare July 25, 2025 12:54

alexey-tikhonov added Tests Accepted Ready to push and removed Changes requested backport-to-sssd-2-9 labels Jul 30, 2025

alexey-tikhonov added Pushed and removed Accepted Ready to push labels Jul 31, 2025

alexey-tikhonov closed this Jul 31, 2025

shridhargadekar mentioned this pull request Nov 28, 2025

Tests:cache_credentials = true not working for 2-9 #8032

Merged

Conversation

shridhargadekar commented Jul 4, 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!

thalman 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!

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

Uh oh!

Uh oh!

alexey-tikhonov commented Jul 7, 2025

Uh oh!

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

Uh oh!

Uh oh!

alexey-tikhonov commented Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

shridhargadekar commented Jul 16, 2025

Uh oh!

shridhargadekar commented Jul 16, 2025

Uh oh!

shridhargadekar commented Jul 22, 2025

Uh oh!

alexey-tikhonov commented Jul 24, 2025

Uh oh!

shridhargadekar commented Jul 24, 2025

Uh oh!

alexey-tikhonov commented Jul 25, 2025

Uh oh!

shridhargadekar commented Jul 25, 2025

Uh oh!

Uh oh!

sumit-bose commented Jul 29, 2025

Uh oh!

alexey-tikhonov commented Jul 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexey-tikhonov commented Jul 7, 2025 •

edited

Loading

alexey-tikhonov commented Jul 8, 2025 •

edited

Loading