twisted.spread.pb.IUsernameMD5Password's docstring should say it accepts an MD5 digest of a password, not plaintext

[exarkun]

This interface seems to be entirely redundant with IUsernameHashedPassword.

[jesstess]

Some questions on this ticket:

• I'm not sure how to eke a DeprecationWarning out of an interface to make a test case. Implementing the interface doesn't cause one. Is there a standard way to do this?
• Should I be doing something to class _PortalAuthChallenger as well since it implements IUsernameMD5Password?

[exarkun]

1. twisted.python.deprecate.deprecatedModuleAttributes will let you deprecate the attribute (this results in a deprecation warning when the attribute is accessed, either via '.' or an import statement). I'm not sure if there's a way to hook into implements() or the other zope.interface declaration APIs; that'd be cool, but probably isn't totally necessary. Emitting a deprecation from __init__ won't do anything, I think, as interfaces aren't intended to be instantiated (and perhaps cannot be instantiated (I hope)).
2. Yea. It'd be good to eliminate any uses of this interface within Twisted, as long as doing so doesn't introduce backwards compatibility issues. In the case of _PortalAuthChallenger, this is a private class, so we can do pretty much anything we want to it without worrying about compatibility issues.

[jesstess]

(In [28129]) Branching to 'pb-md5-deprecate-3056'

[jesstess]

(In [28132]) Deprecate IUsernameMD5Password using deprecatedModuleAttribute and add a unit test for the deprecation.

refs #3056

[jesstess]

(In [28133]) Remove references to IUsernameMD5Password in _PortalAuthChallenger.

refs #3056

[jesstess]

I left checkPassword and checkMD5Password as distinct functions in _PortalAuthChallenger to be conservative about compatibility, but I can roll checkMD5Password into checkPassword if people prefer that.

[thijs]

Can you open a ticket for the removal of this interface?

[jesstess]

#4267 was opened for the removal.

[exarkun]

1. There's some weird quoting in test_pb.py, in the docstring for the new test, test_IUsernameMD5Password.
2. You can just do pb.IUsernameMD5Password in that test, rather than using getattr, can't you? It should do the same thing, I think, but it's simpler.
3. I suppose that _PortalAuthChallenger.checkMD5Password should be deprecated as well. Someone might get their hands on an instance of _PortalAuthChallenger via cred (eg, by implementing a checker for it) and call the method. They should get warned in that case. As long as we're doing that, I guess there's also no reason to remove the IUsernameMD5Password implements declaration from _PortalAuthChallenger right now either. We can delete it when we delete the interface definition (and delete checkMD5Password then as well). I think the only annoying thing about this is that to avoid a deprecation warning when calling checkPassword the implementation will have to shift around a bit.

[exarkun]

Errgh. In responding to a recent post on tpml about this code, I realized that IUsernameHashedPassword and IUsernameMD5Password actually have a difference. IUsernameMD5Password.checkMD5Password accepts an MD5 digest of a password, while IUsernameHashedPassword.checkPassword accepts a plaintext password. Thus, if you don't want to store plaintext passwords on your server, IUsernameMD5Password is a better choice.

I think was misled by the documentation for IUsernameMD5Password which incorrectly claims that it accepts a plaintext password just like IUsernameHashedPassword.

[jesstess]

(In [28225]) Branching to 'pb-md5-docstring-3056'

[jesstess]

(In [28226]) checkMD5Password should say it works on a digest, not plaintext. Also, the @return markup on a number of interfaces extending ICredentials is hopefully now less confusing.

refs #3056

[exarkun]

Looks good. Please merge. Thanks!

[jesstess]

(In [28247]) Merge pb-md5-docstring-3056

Author: jesstess Reviewer: exarkun Fixes: #3056

IUsernameMD5Password.checkMD5Password now correctly say it works on a digest, not plaintext. Also, the @return description on a number of interfaces extending ICredentials has been clarified.