meejah / txtorcon

Twisted-based asynchronous Tor control protocol implementation. Includes unit-tests, examples, state-tracking code and configuration abstraction.
http://fjblvrw2jrxnhtg67qpbzi45r7ofojaoo3orzykesly2j3c2m3htapid.onion/
MIT License
250 stars 72 forks source link

Fallback to password authentication #150

Closed coffeemakr closed 8 years ago

coffeemakr commented 8 years ago

Fix for issue #149

Functional changes:

meejah commented 8 years ago

(p.s. i'm happy to take any subsequent changes as force-pushes to this branch, includes rebase/squashes etc)

codecov-io commented 8 years ago

Current coverage is 99.69%

Merging #150 into master will not affect coverage as of 9c41270

@@            master    #150   diff @@
======================================
  Files           15      15       
  Stmts         2631    2658    +27
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit           2623    2650    +27
  Partial          0       0       
  Missed           8       8       

Review entire Coverage Diff as of 9c41270

Powered by Codecov. Updated on successful CI builds.

meejah commented 8 years ago

I haven't looked at password-auth for a while, so it's possible it's changed since the PASSWORD value was tested for. The right place to look is the TorSpec repository: https://gitweb.torproject.org/torspec.git/tree/control-spec.txt and it looks like it is indeed "HASHEDPASSWORD" now.

meejah commented 8 years ago

Overall, this patch is looking great! Thanks :)

Just a couple error-cases to cover with unit-tests and we're good ;)

coffeemakr commented 8 years ago

I have removed the PASSWORD method support because it seems that it was named HASHEDPASSWORD since the beginning of protocol version 1: https://gitweb.torproject.org/torspec.git/diff/proposals/119-controlport-auth.txt?id=3e553a946f8567a44af64f35d2bc13ac68c0fd85

The brackets in "if" are removed. I don't know why I made those.

Some test will follow.

coffeemakr commented 8 years ago

should I add some more tests?

meejah commented 8 years ago

Looks great, thanks :)