1:19 p.m.
URL: https://github.com/SSSD/sssd/pull/911
Author: alexal
Title: #911: Update pam_sss.8.xml
Action: opened
PR body:
"""
pam_sss: Added return values on a man page
Resolves: https://pagure.io/SSSD/sssd/issue/3672
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/911/head:pr911
git checkout pr911
1:19 p.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
centos-ci commented:
"""
Can one of the admins verify this patch?
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-544642148
3:58 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
I plan to review this today or tomorrow.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-544867903
7 a.m.
New subject: [sssd PR#911][+Waiting for review] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Waiting for review
7:01 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
alexey-tikhonov commented:
"""
ok to test
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-544928286
4:32 p.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
Here are my comments for each entry you added:
- PAM_SUCCESS - OK
- PAM_USER_UNKNOWN - ... or the SSSD's PAM responder is not running.
- PAM_IGNORE - Simply write "See options ignore_unknown_user and
ignore_authinfo_unavail."
- PAM_AUTHTOK_ERR - I think this is OK... maybe add that it could be returned also when
user authenticates with certificates and multiple certificates are available but installed
version of gdm does not support selection from multiple certificates.
- PAM_AUTHINFO_UNAVAIL - OK (I think)
- PAM_BUF_ERR - Ok, but also add that this could be returned also when options
use_first_pass or use_authtok were set, but no password from previously stacked pam module
was found."
- PAM_SYSTEM_ERR - OK
- PAM_CRED_ERR - OK
- PAM_CRED_INSUFFICIENT Ok, but add "For example missing PIN during smartcard
authentication or missing factor during 2-factor authentication."
```
I hope it is understandable. Ask if something is not clear.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-545645733
4:33 p.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
Here are my comments for each entry you added:
- PAM_SUCCESS - OK
- PAM_USER_UNKNOWN - ... or the SSSD's PAM responder is not running.
- PAM_IGNORE - Simply write "See options ignore_unknown_user and
ignore_authinfo_unavail."
- PAM_AUTHTOK_ERR - I think this is OK... maybe add that it could be returned also when
user authenticates with certificates and multiple certificates are available but installed
version of gdm does not support selection from multiple certificates.
- PAM_AUTHINFO_UNAVAIL - OK (I think)
- PAM_BUF_ERR - Ok, but also add that this could be returned also when options
use_first_pass or use_authtok were set, but no password from previously stacked pam module
was found."
- PAM_SYSTEM_ERR - OK
- PAM_CRED_ERR - OK
- PAM_CRED_INSUFFICIENT Ok, but add "For example missing PIN during smartcard
authentication or missing factor during 2-factor authentication."
I hope it is understandable. Ask if something is not clear.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-545645733
4:33 p.m.
New subject: [sssd PR#911][+Changes requested] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Changes requested
4:33 p.m.
New subject: [sssd PR#911][-Waiting for review] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Waiting for review
4:35 p.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
Here are my comments for each entry you added:
- PAM_SUCCESS - OK
- PAM_USER_UNKNOWN - ... or the SSSD's PAM responder is not running.
- PAM_IGNORE - Simply write "See options ignore_unknown_user and
ignore_authinfo_unavail."
- PAM_AUTHTOK_ERR - I think this is OK... maybe add that it could be returned also when
user authenticates with certificates and multiple certificates are available but installed
version of gdm does not support selection from multiple certificates.
- PAM_AUTHINFO_UNAVAIL - OK (I think)
- PAM_BUF_ERR - Ok, but also add that this could be returned also when options
use_first_pass or use_authtok were set, but no password from previously stacked pam module
was found.
- PAM_SYSTEM_ERR - OK
- PAM_CRED_ERR - OK
- PAM_CRED_INSUFFICIENT Ok, but add "For example missing PIN during smartcard
authentication or missing factor during two-factor authentication."
I hope it is understandable. Ask if something is not clear.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-545645733
5:39 p.m.
New subject: [sssd PR#911][synchronized] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Author: alexal
Title: #911: Update pam_sss.8.xml
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/911/head:pr911
git checkout pr911
5:40 p.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
alexal commented:
"""
@mzidek-rh thanks for your comments. I've made those changes, please see the updated
commit.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-545666034
3:25 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
@alexal Thanks, it looks good to me now.
@sumit-bose Can I borrow your pair of eyes for these pam_sss man page changes?
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-545805841
3:28 a.m.
New subject: [sssd PR#911][-Changes requested] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Changes requested
5:52 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
sumit-bose commented:
"""
Hi,
thank you for the patch.
you wording is mainly about authentication, but pam_sss handles authorization and session
setup as well. Maybe you can check if you can make at least some of the descriptions more
general.
You didn't mention return codes like e.g. `PAM_PERM_DENIED` or `PAM_AUTH_ERR`. Is this
because you didn't find a good description or are you planning to add them in a later
pull-request?
bye,
Sumit
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-546306056
10:39 a.m.
New subject: [sssd PR#911][synchronized] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Author: alexal
Title: #911: Update pam_sss.8.xml
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/911/head:pr911
git checkout pr911
10:40 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
alexal commented:
"""
@sumit-bose ,
> I think it would be good to say that this means an error on the
SSSD side and the SSSD logs should be checked why it occured.
I've added, "The SSSD log files may contain additional information about the
error."
> You didn't mention return codes like e.g. PAM_PERM_DENIED or
PAM_AUTH_ERR
I just missed these two return values. Added them, please see the updated commit.
> you wording is mainly about authentication, but pam_sss handles
authorization and session setup as well. Maybe you can check if you can make at least some
of the descriptions more general.
I would appreciate any suggestions.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-547006780
6:12 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
sumit-bose commented:
"""
@sumit-bose ,
> > I think it would be good to say that this means an error on the SSSD side and
the SSSD logs should be checked why it occured.
I've added, "The SSSD log files may contain additional information about the
error."
> > You didn't mention return codes like e.g. PAM_PERM_DENIED or PAM_AUTH_ERR
I just missed these two return values. Added them, please see the updated commit.
Thanks, I think there were important to mention as well.
For the record the following PAM return codes are used in the SSSD code as well, but
please do not feel obliged to add those to your PR as well, I think you covered all that
are important. I just want to record them somewhere to make it more easy to add them
later:
- PAM_SERVICE_ERR
- PAM_NEW_AUTHTOK_REQD
- PAM_ACCT_EXPIRED
- PAM_SESSION_ERR
- PAM_CRED_UNAVAIL
- PAM_NO_MODULE_DATA
- PAM_CONV_ERR
- PAM_AUTHTOK_LOCK_BUSY
- PAM_ABORT
- PAM_MODULE_UNKNOWN
- PAM_BAD_ITEM
> > you wording is mainly about authentication, but pam_sss
handles authorization and session setup as well. Maybe you can check if you can make at
least some of the descriptions more general.
I would appreciate any suggestions.
This is mostly about `PAM_SUCCESS`. Here something like `The PAM operation finished
successfully.` would be better imo.
bye,
Sumit
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-547370620
5:18 a.m.
New subject: [sssd PR#911][+Changes requested] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Changes requested
5:19 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
mzidek-rh commented:
"""
Adding changes requested as per @sumit-bose's comment.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-547831486
9:32 a.m.
New subject: [sssd PR#911][synchronized] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Author: alexal
Title: #911: Update pam_sss.8.xml
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/911/head:pr911
git checkout pr911
9:35 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
alexal commented:
"""
@sumit-bose , @mzidek-rh I apologize for the delay. I had too much work. I've updated
a man page with all of the requested changes. Please take a look and let me know if
something needs to be changed.
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-564090022
6:45 p.m.
New subject: [sssd PR#911][+Waiting for review] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Waiting for review
6:45 p.m.
New subject: [sssd PR#911][-Changes requested] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Changes requested
4:23 a.m.
New subject: [sssd PR#911][-Waiting for review] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Waiting for review
4:23 a.m.
New subject: [sssd PR#911][+Accepted] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Accepted
5:16 a.m.
New subject: [sssd PR#911][+Ready to push] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Ready to push
5:19 a.m.
New subject: [sssd PR#911][comment] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
pbrezina commented:
"""
* `master`
* 192eadaaf8ac6a427533612362f012dcbd7ce776 - Update pam_sss.8.xml
"""
See the full comment at https://github.com/SSSD/sssd/pull/911#issuecomment-581364133
5:19 a.m.
New subject: [sssd PR#911][+Pushed] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: +Pushed
5:19 a.m.
New subject: [sssd PR#911][-Accepted] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Accepted
5:19 a.m.
New subject: [sssd PR#911][-Ready to push] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Title: #911: Update pam_sss.8.xml
Label: -Ready to push
5:19 a.m.
New subject: [sssd PR#911][closed] Update pam_sss.8.xml
URL: https://github.com/SSSD/sssd/pull/911
Author: alexal
Title: #911: Update pam_sss.8.xml
Action: closed
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/911/head:pr911
git checkout pr911
1570
days inactive
1675
days old
sssd-devel@lists.fedorahosted.org
31 comments
6 participants
participants (6)
-
alexal -
alexey-tikhonov
-
centos-ci
-
mzidek-rh
-
pbrezina
-
sumit-bose