Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sssd remove use of python-pam #12179

Closed
wants to merge 1 commit into from
Closed

Conversation

tonyyuan1
Copy link
Contributor

sssd test used python-pam, however it's unmantained and it was removed from package hub repo

# Install test subjects and test scripts
my @test_subjects = qw(
sssd sssd-krb5 sssd-krb5-common sssd-ldap sssd-tools
openldap2 openldap2-client
krb5 krb5-client krb5-server krb5-plugin-kdb-ldap
);

# for sle 12 we still use and support python2
Copy link
Contributor

Choose a reason for hiding this comment

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

Update also test description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Updated.

Copy link
Member

@foursixnine foursixnine left a comment

Choose a reason for hiding this comment

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

You are removing all the testing done via PAM.

In the end, this might be a complete rewrite of the sssd tests, rendering tests/sysauth/sssd.pm useless. A bit of background, those tests are a bit extensive but they were written by a developer that is long gone from the company, investing time in fixing or looking for an alternative to python-pam and rewrite the test suite is more expensive than switching to directly testing the service against the system (by means of directly using pam/passwd/getent)

You can adapt these tests to use directly openldap + sssd + pam via getent for instance to test the autentication.

But I'd propose a full module rewrite with a much simpler approach (like the pr mentioned in the ticket)

@tonyyuan1
Copy link
Contributor Author

You are removing all the testing done via PAM.

In the end, this might be a complete rewrite of the sssd tests, rendering tests/sysauth/sssd.pm useless. A bit of background, those tests are a bit extensive but they were written by a developer that is long gone from the company, investing time in fixing or looking for an alternative to python-pam and rewrite the test suite is more expensive than switching to directly testing the service against the system (by means of directly using pam/passwd/getent)

You can adapt these tests to use directly openldap + sssd + pam via getent for instance to test the autentication.

But I'd propose a full module rewrite with a much simpler approach (like the pr mentioned in the ticket)

This test uses some "su" commands which use PAM but they run su as root to verify password. It seems incorrect. root can su any user without password. I did not try them manually.
I like your proposal to rewrite it. I can rewrite it after hackweek. This PR is no use. I will close it if you have no objection.

@foursixnine
Copy link
Member

@tonyyuan1 sure thing :) Thanks!

@tonyyuan1 tonyyuan1 closed this Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants