[issue15014] smtplib: add support for arbitrary auth methods
Changes by Barry A. Warsaw ba...@python.org: -- assignee: - barry status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Roundup Robot added the comment: New changeset 97a29b86a2dc by Barry Warsaw in branch '3.5': - Issue #15014: SMTP.auth() and SMTP.login() now support RFC 4954's optional https://hg.python.org/cpython/rev/97a29b86a2dc New changeset 2d9003d44694 by Barry Warsaw in branch 'default': - Issue #15014: SMTP.auth() and SMTP.login() now support RFC 4954's optional https://hg.python.org/cpython/rev/2d9003d44694 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: Only one question, about a possible missing test. Otherwise this looks good to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: Attached patch includes test, documentation, and implementation. While this is technically a new feature, it fixes a regression in Python 3.5 w.r.t. 3.4. I'll email python-dev with a request for beta exemption. -- Added file: http://bugs.python.org/file39882/15014-auth-init-resp.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: On Jul 07, 2015, at 05:59 PM, R. David Murray wrote: I don't see any need to add the is_initial_auth_ok flag. Either the auth method returns something that is not None (initial auth is OK), or it doesn't (initial auth is not OK). I added this because test_smtplib itself expects some challenge/response even for AUTH PLAIN. I could have done more surgery on test_smtplib to avoid this, but on thinking about it, I decided that it makes sense to allow for the override. smtplib is often used in test scenarios, so imagine testing an SMTP server implementation. To get full coverage you'd want to test both initial-response and challenge/response even for auth methods like PLAIN. Indeed, I've been thinking about writing an asyncio-based smtpd, and it would be nice to be able to handle both cases without having to derive from class SMTP and re-implementing auth_plain and auth_login. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: I don't see any need to add the is_initial_auth_ok flag. Either the auth method returns something that is not None (initial auth is OK), or it doesn't (initial auth is not OK). Providing for that initial return value from the authobj is still a change to the new feature, but I don't think fixes to API bugs require an exception to the features rule, especially when they represent a fix to a regression. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: OK, that makes sense. I'll try to give this a thorough review soon. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: I have a patch to support initial-response, which I'll be posting here after a bit of clean up and a full (local) test run, with documentation. I ended up adding a keyword argument `initial_response_ok=True` to .login() and .auth(). The reason for this is that some clients may wish to force smtplib to do challenge/response. By default, mechanisms that support it (e.g. AUTH PLAIN) will send the initial response. Setting `initial_response_ok=False` in either SMTP.auth() or SMTP.login() will prevent smtplib from sending the initial response, and instead deal with challenge/response. I made initial_response_ok a keyword argument. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: Thanks! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Changes by Larry Hastings la...@hastings.org: -- priority: release blocker - normal ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: I believe this change broke RFC 4954's AUTH command when the optional initial-response is expected. $4 The AUTH Command says: AUTH mechanism [initial-response] ... initial-response: An optional initial client response. If present, this response MUST be encoded as described in Section 4 of [BASE64] or contain a single character = It's possible that some SMTP servers only look for a string like AUTH PLAIN base64-gobbledygook and won't issue a challenge if it's missing. Such an example is found in the lazr.smtptest and used by the testing SMTPd in GNU Mailman. It's possible that the servers are not standards compliant (I haven't completely groked RFC 4422), but still, since this is a backward incompatible change and breaks existing code, there should be some way of making smtplib.login() provide the initial-response, and it probably ought to be the default for backward compatibility. -- status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: Here's a rough thought for a fix. Some auth_*() methods require a challenge, but some don't, e.g. auth_plain(). Let's allow authobject() to be called with challenge=None. If they allow an initial-response, then they can just return the response bytes. If they return None, then they don't allow an initial-response and so must wait for the challenge (i.e. 334 response code). I think that would at least restore backward compatibility for AUTH PLAIN. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Barry A. Warsaw added the comment: Also, smtpd is not compatible with auth challenges because found_terminator() doesn't know that the response its getting isn't a command but instead a challenge response. So really we need another bug to track fixes to smtpd.py to handle challenge responses. It makes no sense for smtpd and smtplib not to be compatible. (Who wants to reimplement smtpd in asyncio? :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Roundup Robot added the comment: New changeset 42917d774476 by R David Murray in branch 'default': #15014: Add 'auth' command to implement auth mechanisms and use it in login. http://hg.python.org/cpython/rev/42917d774476 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: Thanks, Milan. I made some tweaks...mostly documentation and code style (your code style wasn't wrong, I just have some slightly different preferences with regards to line folding). I also eliminated the authobjects test method, since I didn't see that it added anything to duplicate the code from the login method to test them...they get tested via the login methods. Instead I added the loop to the test for the auth method, so that we test that calling the public API with the auth objects that are documented as part of the public API work. That is, if someone were to change login such that the method names changed but login still worked, the auth test method will throw an error because the documented method names changed. I'm attaching the final patch here so you can look at the differences via the reitveld patch-diff function, if you want to. -- resolution: - fixed stage: needs patch - resolved status: open - closed Added file: http://bugs.python.org/file35842/smtplib_auth.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: Ah, you are right, I wasn't looking at the full context of the diff when I made the comment about backward compatibility. So ignore that part :) On the other hand, exposing perferred_auth on the class would be a simple API for allowing auth extension (since one really needs to subclass to add an authobject, given that it needs access to self.user and self.password). But, like I said before, that can be a separate issue. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Milan Oberkirch added the comment: Here comes the patch implementing your suggestions. Changing the API to make adding new auth methods and still using login() would only require to make preferred_auth accessable as you mentioned. Using custom authobjects is possible with this patch. -- Added file: http://bugs.python.org/file35500/smtplib_auth_060614.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: I made some review comments. Also, we need doc updates, including a what's new entry. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Milan Oberkirch added the comment: Done. -- Added file: http://bugs.python.org/file35392/smtplib_auth.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Changes by Milan Oberkirch milan...@oberkirch.org: -- nosy: +jesstess ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: OK, I've finally had time to review this, sorry for the delay. The impalib mechanism is tailored to how imap works (that's the whole thing about continuation response. smtplib auth works a bit differently, and your adaptation looks OK to me. The accompanying docstrings are a bit confusing, though, since they copy the imaplib language, which isn't entirely applicable. Instead of talking about a 'continuation response', it should talk about a '334 challenge response', I think. Also, both the imaplib and smptlib methods are named after the corresponding command names in the protocol. So for imaplib we have 'authenticate', but for smtplib it should be 'auth'. I also think we should expose the supported auth methods as part of the public API. I think exposing the preferred auth list is premature, since if we are going to do that we need to have a way to add elements to that list for the users's own auth methods and we don't have that. So let's confine this issue to adding the 'auth' method and using it in the login method. The tests should include a new test of calling the auth method directly. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Milan Oberkirch added the comment: I looked into imaplib a bit to see how the problem is solved there; what I observed: - login() does 'PLAIN' only (and does not use authobj but smtplib would) - there exists an extra function login_cram_md5() for 'CRAM-MD5' So I guess the right way to do it would be to write an authenticate() method as in imaplib and use it in new member functions of the form login_[method](). In contrast to imaplib login() could still be used to probe through the three main methods (to avoid breaking backward compatibility). I'll try to implement this later today, so if you think it's completely dumb and read this before a patch is applied feel free to give me a quick reply to stop me from wasting my time :) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: Ah, I was going on imperfect memory of how imaplib worked without looking it up. And I won't have time to do so today probably. Probably we should decide on the exact API before you implement anything. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Milan Oberkirch added the comment: Maybe I am a bit hyperactive, but I already was at it when receiving your message, so I finished it. Anyhow most of what I have done is completely independent of the API chosen. What I did: 1. I implemented a new authenticate(self, mechanism, authobject) as in imaplib which can be used for all methods 2. I implemented authobjects for the three most important mechanisms 3. login() has a keyarg 'preferred_auth' which is a list of auth-methods (strings) I think 1. and 2. make sense regardless of how the API should look like in the end, because it's the only way without any code-redundancy. 3 is more a proposal. -- Added file: http://bugs.python.org/file34287/smtplibAuthobj.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: OK, that makes sense to me. I'll take a look at it as soon as I can. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Milan Oberkirch added the comment: I implemented one approach to solve this by writing new member functions for each method (see the patch attached). Bonus: It does not change the usage of login() in any way (which uses the new functions internally). Another option would be to make those functions private/put them into login() and provide an optional keyarg. Or auth objects, but as far as I can see only 3 methods are relevant so it might be an overkill? So that's the easy way to fix it, would be glad if it helps! -- keywords: +patch nosy: +zvyn Added file: http://bugs.python.org/file34279/smtplib.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: There is a lot of repeated code in those methods, which becomes a maintenance burden. That is one reason the imaplib authobj approach seems interesting to me. Also, going the authobj route would mean imaplib and smptlib had a consistent interface for this, and consistency is good where it makes sense. -- versions: +Python 3.5 -Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray added the comment: Oh, sorry. I meant to start out that message by saying, thanks for the patch. Your idea is indeed interesting from the standpoint of not needing to change the rest of the API, butand then the rest of my message. In other words, I appreciate your work and your approach, but I think the reasons I outline argue for a different approach. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Changes by Catalin Iacob iacobcata...@gmail.com: -- nosy: +catalin.iacob ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
R. David Murray rdmur...@bitdance.com added the comment: Yes, this is a need I also ran into, but hadn't gotten around to submitting a bug for. If we are going to do this, though, I think we should do it right and add the ability to support arbitrary login methods. An example of one way to do this is the imaplib authobj protocol. As things are, I wound up implementing XYMCOOKIE login using the even-more-primitive-than-SMTP-cmd-level operation 'do_cmd'. -- nosy: +r.david.murray title: smtplib: allow to choose auth method on login() - smtplib: add support for arbitrary auth methods type: - enhancement ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue15014] smtplib: add support for arbitrary auth methods
Changes by R. David Murray rdmur...@bitdance.com: -- components: +email nosy: +barry stage: - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue15014 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
