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

Add extended range support to TOTP.verify #19

Merged
merged 9 commits into from
Oct 21, 2015
Merged

Add extended range support to TOTP.verify #19

merged 9 commits into from
Oct 21, 2015

Conversation

zeevro
Copy link

@zeevro zeevro commented Oct 20, 2015

No description provided.

@@ -16,15 +16,16 @@ def __init__(self, *args, **kwargs):
self.interval = kwargs.pop('interval', 30)
super(TOTP, self).__init__(*args, **kwargs)

def at(self, for_time):
def at(self, for_time, increment=0):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename increment to counter_offset.

@@ -33,13 +34,20 @@ def now(self):
"""
return self.generate_otp(self.timecode(datetime.datetime.now()))

def verify(self, otp, for_time=None):
def verify(self, otp, for_time=None, extra_range=0):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename extra_range to valid_window.

@kislyuk
Copy link
Member

kislyuk commented Oct 20, 2015

Please add a test. You'll want a new test class in https://github.com/pyotp/pyotp/blob/master/test.py#L143. You can just look at Travis output if you don't want to figure out how to run tests locally.

@kislyuk
Copy link
Member

kislyuk commented Oct 20, 2015

Thanks for opening the PR, this is a useful change. Once the test is in place to exercise these new options, I can merge it.

"""
if for_time is None:
for_time = datetime.datetime.now()

if valid_window:
for i in xrange(-valid_window, valid_window + 1):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use range() instead of xrange(). This library is Python 3 compatible.

@kislyuk
Copy link
Member

kislyuk commented Oct 20, 2015

I'd like to make the tests pass. They seem to time out at the moment. Do the tests pass for you locally?

@zeevro
Copy link
Author

zeevro commented Oct 21, 2015

They pass for me locally in both 2.7.6 and 3.4.3.

Update: I accidentally ran the master instead of the branch with the fix.
The test tries time 0 with validity window 1, which tries to generate an OTP for time -1, and that stalls the test. I'll just change the time.

On Tue, Oct 20, 2015 at 8:32 PM, Andrey Kislyuk notifications@github.com
wrote:

I'd like to make the tests pass. They seem to time out at the moment. Do
the tests pass for you locally?


Reply to this email directly or view it on GitHub
#19 (comment).

@zeevro
Copy link
Author

zeevro commented Oct 21, 2015

There. All the tests pass and coverage is good. :)

kislyuk added a commit that referenced this pull request Oct 21, 2015
Add extended range support to TOTP.verify
@kislyuk kislyuk merged commit 498d7f7 into pyauth:master Oct 21, 2015
@kislyuk
Copy link
Member

kislyuk commented Oct 21, 2015

Excellent, thanks!

@zeevro zeevro deleted the patch-2 branch October 21, 2015 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants