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

imaplib.IMAP4.authenticate authobject does not work correctly in python3 #57909

Closed
etukia mannequin opened this issue Jan 3, 2012 · 18 comments
Closed

imaplib.IMAP4.authenticate authobject does not work correctly in python3 #57909

etukia mannequin opened this issue Jan 3, 2012 · 18 comments
Assignees
Labels
topic-email type-bug An unexpected behavior, bug, or error

Comments

@etukia
Copy link
Mannequin

etukia mannequin commented Jan 3, 2012

BPO 13700
Nosy @warsaw, @bitdancer, @demianbrecht
Files
  • issue13700.patch
  • cram_md5.patch
  • imaplib_authenticate.patch
  • imaplib_authenticate_v2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/bitdancer'
    closed_at = <Date 2013-02-19.17:22:45.561>
    created_at = <Date 2012-01-03.01:11:28.557>
    labels = ['type-bug', 'expert-email']
    title = 'imaplib.IMAP4.authenticate authobject does not work correctly in python3'
    updated_at = <Date 2013-02-19.17:22:45.559>
    user = 'https://bugs.python.org/etukia'

    bugs.python.org fields:

    activity = <Date 2013-02-19.17:22:45.559>
    actor = 'r.david.murray'
    assignee = 'r.david.murray'
    closed = True
    closed_date = <Date 2013-02-19.17:22:45.561>
    closer = 'r.david.murray'
    components = ['email']
    creation = <Date 2012-01-03.01:11:28.557>
    creator = 'etukia'
    dependencies = []
    files = ['24132', '24134', '25109', '26227']
    hgrepos = []
    issue_num = 13700
    keywords = ['patch']
    message_count = 18.0
    messages = ['150489', '150491', '150492', '150493', '150494', '150496', '150500', '150505', '150510', '150530', '150536', '152608', '157436', '158262', '164486', '182391', '182398', '182399']
    nosy_count = 7.0
    nosy_names = ['barry', 'r.david.murray', 'python-dev', 'etukia', 'joebauer', 'nagisa', 'demian.brecht']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue13700'
    versions = ['Python 3.2', 'Python 3.3', 'Python 3.4']

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    >> import imaplib
    >> imap = imaplib.IMAP4_SSL("imap.example.com")

    >>> authcb = lambda resp: "{0}\x00{0}\x00{1}".format("username","password")
    >>> imap.authenticate("PLAIN", authcb)
    Traceback (most recent call last):
      File "<pyshell#3>", line 1, in <module>
        imap.authenticate("PLAIN", authcb)
      File "/usr/lib/python3.1/imaplib.py", line 361, in authenticate
        typ, dat = self._simple_command('AUTHENTICATE', mech)
      File "/usr/lib/python3.1/imaplib.py", line 1075, in _simple_command
        return self._command_complete(name, self._command(name, *args))
      File "/usr/lib/python3.1/imaplib.py", line 889, in _command
        literal = literator(self.continuation_response)
      File "/usr/lib/python3.1/imaplib.py", line 1238, in process
        return self.encode(ret)
      File "/usr/lib/python3.1/imaplib.py", line 1257, in encode
        e = binascii.b2a_base64(t)
    TypeError: must be bytes or buffer, not str

    ... and ...

    >>> authcb = lambda resp: "{0}\x00{0}\x00{1}".format("username","password").encode()
    >>> imap.authenticate("PLAIN", authcb)
    Traceback (most recent call last):
      File "<pyshell#8>", line 1, in <module>
        imap.authenticate("PLAIN", authcb)
      File "/usr/lib/python3.1/imaplib.py", line 361, in authenticate
        typ, dat = self._simple_command('AUTHENTICATE', mech)
      File "/usr/lib/python3.1/imaplib.py", line 1075, in _simple_command
        return self._command_complete(name, self._command(name, *args))
      File "/usr/lib/python3.1/imaplib.py", line 889, in _command
        literal = literator(self.continuation_response)
      File "/usr/lib/python3.1/imaplib.py", line 1238, in process
        return self.encode(ret)
      File "/usr/lib/python3.1/imaplib.py", line 1259, in encode
        oup = oup + e[:-1]
    TypeError: Can't convert 'bytes' object to str implicitly

    @etukia etukia mannequin added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 3, 2012
    @bitdancer
    Copy link
    Member

    The first argument to authenticate must be bytes. This is not well documented. It might also be a bug, since I'm not sure anyone has done a thorough audit of what should be bytes and what should be string in imaplib.

    3.1 no longer gets bug fixes, so I'm removing it from versions. Likewise I remove 3.4 since that applies only to changes that will *not* be put in 3.3 for some reason.

    @bitdancer bitdancer added the docs Documentation in the Doc dir label Jan 3, 2012
    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    The same problems exists even if I use b"PLAIN" as the first argument in authenticate().

    @bitdancer
    Copy link
    Member

    Gah, I was looking at the wrong source code when I wrote that. A string first argument is indeed valid. I'm not sure where the problem is coming from since the internal CRAM_MD5 returns a string and that seems to work...

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    File "/usr/lib/python3.1/imaplib.py", line 1257, in encode
    e = binascii.b2a_base64(t)

    imaplib._Authenticator.encode() calls binascii.b2a_base64() function. In Python 2.6 that function returns a string, and in Python 3.1 it returns bytes. What is returned from b2a_base64() function is later in the encode() function concatenated with a string, with bytes that is not possible.

    Should binascii.b2a_base64() return a string (2.6) or bytes (3.1)?

    @bitdancer
    Copy link
    Member

    Bytes definitely. We hashed that out a while ago.

    My point is that CRAM_MD5 login calls authenticate, and its authenticator returns a string, just like your example does. So it ought to be going through the same code path. I haven't followed the logic in detail, though, so there must be some difference...I'm pretty sure the MD5 login has a test now (but not 100% sure...)

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    In Python 2.6 PLAIN authentication works, in Python 3.1 not.

    Lib/test/test_imaplib.py does not test IMAP4.authenticate() or IMAP4.login_cram_md5() functions, only IMAP4.login().

    I would still like to go back to imaplib._Authenticator.encode() function. The function is below.

    # inp = authobject(response)
    def encode(self, inp):
        oup = ''
        while inp:
            if len(inp) > 48:
                t = inp[:48]
                inp = inp[48:]
            else:
                t = inp
                inp = ''
            e = binascii.b2a_base64(t)
            if e:
                oup = oup + e[:-1]
        return oup

    binascii.b2a_base64() takes bytes, so inp must therefore be bytes, and returns bytes (Python 3). Then str + bytes (out + e[:-1]) fails.

    The fix would then be changing
    oup = oup + e[:-1]
    to
    oup = oup + e[:-1].decode()

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    I tried to fix the problem and the correct fix is to change
    oup = ''
    to
    oup = b''

    in imaplib._Authenticator.encode() function, and not what I suggested in my previous post.

    After changing that PLAIN authentication works.

    @bitdancer
    Copy link
    Member

    Would you be interested in providing a patch that includes tests? I think Antoine set up a test framework for testing the login as part of bpo-4471.

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    Here's a patch with test.

    I am not an IMAP guru, so please verify my patch.

    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jan 3, 2012

    Here's another patch that should fix the CRAM-MD5 authentication. My previous patch is required with this one. The patch includes a test.

    @joebauer
    Copy link
    Mannequin

    joebauer mannequin commented Feb 4, 2012

    Issue also affects Python3.1. Hunk succeeds against 3.1 imaplib.py and works for me.

    @bitdancer
    Copy link
    Member

    I made some time to work on this today. Attached is a new patch. I've incorporated the tests from the existing patches (though I'm doing the infrastructure a bit differently). PLAIN seems to be a specific case of the general authenticate, so I just have a generalized test.

    My fix is slightly different. I'm changing the _Authenticate class to except either bytes or string as input. String because that's more natural in Python3, bytes because there might be auth mechanisms that require bytes (since implementations can define 'X' auth mechanisms).

    The authentication callback always gets passed the data as bytes, for the same reasons of generality. So I'd be OK with the idea that the authentication handler always has to return bytes...which would require a different fix in login_cram_md5.

    My login_cram_md5 test passes, so I *think* the fix I made there is OK. However, I'm getting a traceback from SSL about the socket being shut down, which seems to arise from an imaplib.abort resulting from an unexpected 'b''' value. I'm out of time for working in this right now, so I'm uploading the patch to see if anyone else has time to figure it out. I'll come back to it as some point but I don't know when.

    @bitdancer bitdancer removed the docs Documentation in the Doc dir label Apr 3, 2012
    @bitdancer bitdancer assigned bitdancer and unassigned docspython Apr 3, 2012
    @nagisa
    Copy link
    Mannequin

    nagisa mannequin commented Apr 14, 2012

    Exactly same thing happens with XOAUTH mechanism too, so this bug report should be made more general. (Py3.2.2)

    @bitdancer bitdancer changed the title imaplib.IMAP4.authenticate authobject fails with PLAIN mechanism imaplib.IMAP4.authenticate authobject does not work correctly in python3 Apr 14, 2012
    @bitdancer bitdancer added topic-email and removed stdlib Python modules in the Lib dir labels May 28, 2012
    @etukia
    Copy link
    Mannequin Author

    etukia mannequin commented Jul 1, 2012

    Here's the updated patch. Tests now works. PLAIN works for me, that's only I can test against live system.

    test_login_cram_md5 test had extra \r\n in _send_tagged.

    diff imaplib_authenticate.patch imaplib_authenticate_v2.patch
    < + self._send_tagged(tag, 'OK', 'CRAM-MD5 successful\r\n')
    ---

    • self._send_tagged(tag, 'OK', 'CRAM-MD5 successful')

    @demianbrecht
    Copy link
    Mannequin

    demianbrecht mannequin commented Feb 19, 2013

    Has there been any further work/review done on this issue? I just ran into the problem myself on 3.4(dev) and can verify that the patch (imaplib_authenticate_v.patch) fixes the issue.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 19, 2013

    New changeset 3d4302718e7c by R David Murray in branch '3.2':
    bpo-13700: Make imap.authenticate with authobject work.
    http://hg.python.org/cpython/rev/3d4302718e7c

    New changeset b21f955b8ba2 by R David Murray in branch '3.3':
    Merge: bpo-13700: Make imap.authenticate with authobject work.
    http://hg.python.org/cpython/rev/b21f955b8ba2

    New changeset d404d33a999c by R David Murray in branch 'default':
    Merge: bpo-13700: Make imap.authenticate with authobject work.
    http://hg.python.org/cpython/rev/d404d33a999c

    @bitdancer
    Copy link
    Member

    Demian: thanks for the reminder, and the confirmation that it works on a real server.

    Erno: thanks for the test fix. That was a pretty stupid mistake on my part :)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-email type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant