Okay, cool. I definitely agree that IUsernamePassword.checkPassword is dumb and support deprecating it.
On Monday, April 1, 2013, Shell wrote: > It actually might be the appropriate thing already. There's a couple > of possible reasons for renaming; one is that the password might not > be hashed but the credentials object wants to insert additional logic > (exarkun's statement in IRC) anyway, but technically that's just > hashing using the identity function, so it should be OK. > > The other is that the credentials object might want to collect the > plaintext password, and perform some logic based on that *before* > collecting something which can be compared to something derived from > the plaintext password from the other end of a protocol. I don't know > of any protocols which do this off the top of my head, but I have a > suspicion that at least a couple exist, and they'd fit perfectly into > this interface. If none do, then keeping it with the same name sounds > fine to me. > > Cameron > > On 1 April 2013 22:12, Laurens Van Houtven <[email protected]> wrote: > > Why does IUsernameHashedPassword have to be renamed? It sounds like it's > the > > appropriate thing already. > > > > > > On Mon, Apr 1, 2013 at 10:55 PM, Shell <[email protected]> wrote: > >> > >> The twisted.cred.IUsernamePassword interface declares: > >> > >> * IUsernamePassword.username - "The username associated with these > >> credentials." > >> * IUsernamePassword.password - "The password associated with these > >> credentials." > >> * IUsernamePassword.checkPassword(password) - "Validate these > >> credentials against the correct password." > >> > >> The issue is that the interface (according to exarkun) allows you to > >> implement checkPassword() to do things other than the obvious > >> "password == self.password". Now, this is an issue because Twisted > >> then explicitly supports (again, according to exarkun) two different > >> uses of this interface by the credentials checker: > >> > >> * Call the checkPassword() method, passing it the correct password > >> * Just take the password out and do whatever you want with it (which > >> is necessary in any secure system) > >> > >> Now, imagine I write a version of checkPassword() in a library which > >> does something security-centric (what would this be? shouldn't it be > >> part of the checker?), assuming that it'll be used by a credentials > >> checker which calls checkPassword(). Except... then, a library user > >> uses it with a credentials checker which checks the password itself. > >> Now they're skipping over my security-centric code! > >> > >> So I have to tell my library users that they have to use my library > >> with a credentials checker which makes sure to call checkPassword(), > >> not just one which accepts the correct interface. > >> > >> IUsernamePassword's docstring claims that the stored password must be > >> reversible to plaintext to be compared with the password, which > >> implies that taking the password out and doing other things is > >> incorrect, unlike what exarkun suggests. In this case, exposing > >> password in the interface makes little sense. In addition, > >> twisted.cred.checkers.FilePasswordDB apparently ignores this docstring > >> entirely already > >> > >> ( > http://twistedmatrix.com/trac/browser/tags/releases/twisted-12.3.0/twisted/cred/checkers.py#L238 > ). > >> > >> I propose that IUsernamePassword should be split into at least two > >> interfaces: > >> > >> * IUsernamePassword, with only username and password, no methods, > >> which allows password to be used in any way > >> * Another interface, which only defines username and checkPassword() - > >> possibly just a rename of IUsernameHashedPassword, which declares a > >> similar interface > >> > >> However, this has the issue that any credential checker which can use > >> the second interface would also be able to use an IUsernamePassword > >> here if there were an adapter between the two, but support for this > >> would have to go into every credential checker which supports the > >> second interface at present. Maybe the Portal could automatically > >> search for adapters if it can't find a direct match? > >> > >> Thanks, > >> Cameron > >> > >> _______________________________________________ > >> Twisted-Python mailing list > >> [email protected] > >> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python > > > > > > > > > > -- > > cheers > > lvh > > > > _______________________________________________ > > Twisted-Python mailing list > > [email protected] > > http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python > > > -- Sent from Gmail Mobile
_______________________________________________ Twisted-Python mailing list [email protected] http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
