python-pam
python-pam copied to clipboard
Two suggestions for readme (PAM not thread-safe and sample)
It seems that python_pam is not thread-safe, or I am using it in a wrong way. At least adding an RLock solved one issue for me: https://github.com/mar10/wsgidav/issues/265
Maybe you want to add a remark in the readme (or give me a hint how to use it correctly)?
Currently I create an instance p = pam.pam()
on startup, then re-use it in request handlers (p.authenticate(...)
).
This led to errors, that I was able to fix by wrapping it like
with lock:
p.authenticate(...)
Should a new pam.pam() instance be created in every request-hander instead and would this impact performance?
Also, the readme says
>>> import pam
>>> p = pam.authenticate()
>>> p.authenticate('david', 'correctpassword')
True
...
which gives an exception. I assume it should be
>>> import pam
>>> p = pam
>>> p.authenticate('david', 'correctpassword')
True
...
instead.
The example in the README.md should actually be (found that in some older version, probably a copy&paste mistake):
>>> import pam
>>> p = pam.pam()
>>> p.authenticate('david', 'correctpassword')
True
...
And then it becomes rather obvious that you don't want to reuse the same PamAuthenticator
object (returned by pam.pam()
) for different and concurrent requests - the p.code
and p.reason
will for sure be overwritten. I think creating a new PamAuthenticator
for each request and discarding it with all it's internal states is the way to go, and I would be surprised when locking a single instance (or making a pool and take one that is free with all the organization to make that thread safe) is seriously less overhead.
Thanks, I was just curious, since the PamAuthenticator's constructor seems to call ctype's cdll.loadLibrary()
and find_library()
every time.