[Twisted-Python] Re: +AFs-Twisted-Python+AF0- Re: +AFs-Twisted-commits+AF0- CVS: Twisted/twisted/python otp.py,NONE,1.1
def stringToLong(s): """ Convert bytes to long integer """
...
def stringToDWords(s): """ Convert bytes to a list of four 32-bits words """
...
def longToString(l): """ Convert long to digest """
...
Was there a good reason you reimplemented the struct module in Python?
Well, struct seems almost like an overkill, but could probably replace most usage of these routines. If I should need a good reason for keeping them, it would be my uncertainity of its (structs) endianism/portability since the codes need to be. It just looks like a module only used in interfacing.
This is an only excuse since I didn't know of struct. :) Thanks for the tip
// phed
On Thu, Sep 06, 2001 at 03:09:35AM +0200, Benjamin Bruheim wrote:
This is an only excuse since I didn't know of struct. :) Thanks for the tip
To continue the trend of beating on poor Benjamin here, a few comments about twisted.python.otp... :-)
It seems oddly placed in twisted.python, maybe twisted.protocols would be better? I know it's not a wire protocol, but it is an RFC implementation.
It uses the same vocabulary as twisted.python.authenticator, which is hopefully being obseleted soon (by twisted.internet.passport) Is that just a coincidence? If not, it shouldn't; those classes have nothing to do with each other, and the parallels are very misleading. Otherwise, leave the names as they are, since the other class will be removed soon.
I applaud your effort to do testing, but could you please use the same testing style as the rest of Twisted, as in the twisted/test directory? :-) Maybe that should be in the coding standard...
To continue the trend of beating on poor Benjamin here, a few comments about twisted.python.otp... :-)
Oh, I can take it. It kinda stores up my shares for _my_ nasty replies later on ;)
It seems oddly placed in twisted.python, maybe twisted.protocols would be better? I know it's not a wire protocol, but it is an RFC implementation.
Well, it is selfcontained and is divided into two classes: one with service routines, and one which is a selfupdating password container. I can't say that I understand what these should do in protocols, since they have nothing in common with any of the other modules in protocols. It can also be used in a calculator as well, which is the most popular usage of OTP :)
It uses the same vocabulary as twisted.python.authenticator, which is hopefully being obseleted soon (by twisted.internet.passport) Is that just a coincidence? If not, it shouldn't; those classes have nothing to do with each other, and the parallels are very misleading. Otherwise, leave the names as they are, since the other class will be removed soon.
I must admit that I used Authenticator as inspiration for finding names, but it does not work the same way at all. I think the parameters and docstrings should make that obvious. I haven't looked into it yet, but I expect that some names may change if I add SASL-compliance.
I applaud your effort to do testing, but could you please use the same testing style as the rest of Twisted, as in the twisted/test directory? :-) Maybe that should be in the coding standard...
Hm, I kinda followed the "python-standard" there, but whatever fits :)
Some other issues: SMTP seems to use SASL authentication and I could write some routines for OTP that can be used in SASL since it supports the OTP authentication scheme. And there's one big flaw in OTP as well: When the sequence has run out (default is 1000 password issued) the sequence has to be reset, this is not done automatically since this needs to be implemented in each protocol; or by an util which has to be run on the serverside on a secure connection.
On Thu, Sep 06, 2001 at 08:01:40PM +0200, Benjamin Bruheim wrote:
To continue the trend of beating on poor Benjamin here, a few comments about twisted.python.otp... :-)
Oh, I can take it. It kinda stores up my shares for _my_ nasty replies later on ;)
I'm looking forward to it... ;-)
It seems oddly placed in twisted.python, maybe twisted.protocols would be better? I know it's not a wire protocol, but it is an RFC implementation.
Well, it is selfcontained and is divided into two classes: one with service routines, and one which is a selfupdating password container. I can't say that I understand what these should do in protocols, since they have nothing in common with any of the other modules in protocols. It can also be used in a calculator as well, which is the most popular usage of OTP :)
Hmm. A calculator? I must admit I'm a little confused by that, but okay...
Some other issues: SMTP seems to use SASL authentication and I could write some routines for OTP that can be used in SASL since it supports the OTP authentication scheme. And there's one big flaw in OTP as well: When the sequence has run out (default is 1000 password issued) the sequence has to be reset, this is not done automatically since this needs to be implemented in each protocol; or by an util which has to be run on the serverside on a secure connection.
This is the reason I felt it should be in protocols. It requires specific support from various protocols, and it's not generally useful in (most?) non-networked applications...
in common with any of the other modules in protocols. It can also be used in a calculator as well, which is the most popular usage of OTP :)
Hmm. A calculator? I must admit I'm a little confused by that, but okay...
A password calculator :)
reset, this is not done automatically since this needs to be implemented in each protocol; or by an util which has to be run on the serverside on a secure connection.
This is the reason I felt it should be in protocols. It requires specific support from various protocols, and it's not generally useful in (most?) non-networked applications...
In its current state there is no protocol specific issues. All it does is to return strings which the protocols may use. The reason I don't feel it shouldn't be in protocols is that it does not depend on anything in twisted for operation. Nor is it protocolbound, actually the logic place to use it is in passport.Identity. And generate the object otp.OTP each time the password is set. But if you wish, I can move it.
// phed
On Thu, Sep 06, 2001 at 11:20:07PM +0200, Benjamin Bruheim wrote:
In its current state there is no protocol specific issues. All it does is to return strings which the protocols may use. The reason I don't feel it shouldn't be in protocols is that it does not depend on anything in twisted for operation. Nor is it protocolbound, actually the logic place to use it is in passport.Identity. And generate the object otp.OTP each time the password is set. But if you wish, I can move it.
I'll leave it up to your judgement, just take what I said as a suggestion :-). The ultimate critera for where a particular thing should go would be where the majority of people would look for it in the codebase; and since I'm not someone who'd likely be looking for an OTP implementation, I don't know where I'd look for it.
-
Benjamin Bruheim -
Glyph Lefkowitz