jleclanche / python-bna

Python implementation of the mobile Blizzard Authenticator (TOTP)
https://eu.battle.net/support/en/article/24520
MIT License
250 stars 38 forks source link

Prints remaining time now by default, replaced --remaining with --token-only accordingly #4

Closed Tharre closed 10 years ago

Tharre commented 10 years ago

It's much saner to print the remaining time by default, as most users just want to type in bna and get the token + the time they have left to type it in.

Also the token is now always displayed with leading zeros. Legacy behaviour is available with --token-only.

jleclanche commented 10 years ago

Thanks for the PR but you'll have to give me a very good reason to try and convince me =) As far as personal preference goes, you can always just do something like alias bna="bna --remaining".

In the mean time. feel free to resubmit the leading zeroes fix as a standalone PR, that's something I overlooked.

Tharre commented 10 years ago

The reason is the very same as why the official app includes a bar that shows the remaining time. It's just annoying if you type in the key only to find out that the key changed the millisecond you hit enter. Besides that there is really no reason not to, the additional information can't hurt you =).

Sure you could add aliases, but IIRC that's painful under bash as you have to add a entry for every variation you need (i.e. -i). And after all this is something that shouldn't be a personal preference, but rather the default, as you don't want to pollute your users dotrc files.

jleclanche commented 10 years ago

The reason is the very same as why the official app includes a bar that shows the remaining time.

The official app has a very different use case though. Namely, it is expected to be usable over a long period of time without being restarted - this is covered by -i.

It's just annoying if you type in the key only to find out that the key changed the millisecond you hit enter.

That's actually extremely unlikely. The algorithm dictates for a 30 seconds gap, but the authentication server accepts a difference of up to 5 minutes, to account for the situation you're describing as well as time differences etc.

Besides that there is really no reason not to, the additional information can't hurt you =). The reason I set it to quiet by default was so that its output would be directly usable in other apps, such as other interfaces or things like xsel. Spirit of GNU and all that.

Sure you could add aliases, but IIRC that's painful under bash as you have to add a entry for every variation you need (i.e. -i).

That's not actually the case.

And after all this is something that shouldn't be a personal preference, but rather the default, as you don't want to pollute your users dotrc files.

I don't, but I also don't see the remaining time as valuable information when not in interactive mode.

Thanks for trying, and thanks for the PR. I'd be willing to change my decision if you made this a setting. If you're interested in doing this, you'd need to add a --no-remaining-time flag (in order to be able to override either behaviour) and a default_behaviour setting under the [bna] section in the config file with possible values of "plain", "verbose" or "interactive" (feel free to come up with better names). The readme would also have to be updated to mention it. It would have to default to false (current behaviour). I'd be very open to such functionality.

Tharre commented 10 years ago

Wow, the server really accepts a huge variance. I wasn't aware of that. I wrongly assumed it would only be 1 minute or so (felt like this with the official authenticator after all). With that in mind I actually agree with you, it would be really rare that someone actually needed the remaining time.

Making a settings entry just for that seems to be a little bit overkill to me, although I would implement it if you think that that's a good idea.

jleclanche commented 10 years ago

Sure, I'd love to merge it in if you wrote it. Being able to set the default output is neat I think, especially if you can set it to interactive by default for example. But it does need the ability to disable those then (--no-interactive etc)