Add more auth options, including digest support #338

Merged
merged 3 commits into from Jan 16, 2013

Projects

None yet

3 participants

@nylen
Member
nylen commented Oct 9, 2012

Add the capability to specify auth username and password in request
options or via Request.prototype.auth, as well as whether they should be
sent immediately as Basic authorization (set this to false when using
digest authentication).

@mikeal
Member
mikeal commented Oct 14, 2012

i really want to merge this, but i'm hesitant to take it without a test.

@nylen
Member
nylen commented Nov 13, 2012

How do you propose that I test it? I have a similar patch to superagent that tests this feature using express and http-auth (https://github.com/nylen/superagent/blob/digest-auth/test/node/digest-auth.js). Would that be OK? How do I declare those dependencies?

@loicbar
loicbar commented Jan 13, 2013

I actually need this feature. Would be great if we can merge this (after testing)

@mikeal
Member
mikeal commented Jan 13, 2013

check out test-oauth.js to see how you might test authentication.

@nylen
Member
nylen commented Jan 15, 2013

That approach won't work here, since we need to spin up a server that requires digest auth. (Then, we could also test basic auth in a similar fashion).

Mind if I pull in express (or just connect) and http-auth as dev dependencies?

@mikeal
Member
mikeal commented Jan 16, 2013

i'm not taking all of express and http-auth just for 5 lines that do digest auth.

nylen added some commits Oct 9, 2012
@nylen nylen Add more auth options, including digest support
Add the capability to specify auth username and password in request
options or via Request.prototype.auth, as well as whether they should be
sent immediately as Basic authorization (set this to false when using
digest authentication).

Conflicts:
	main.js
e2d7d4f
@nylen nylen Add tests for basic and digest auth d0d536c
@nylen nylen Document new auth options 85fd359
@nylen
Member
nylen commented Jan 16, 2013

Added tests and docs, and updated to merge cleanly with HEAD. Let me know if there are any problems.

@mikeal
Member
mikeal commented Jan 16, 2013

ok, this is looking good now.

i'm merging to master but it's going to sit there a little while until the next release to flush out any bugs.

@mikeal mikeal merged commit 8d35203 into request:master Jan 16, 2013
@mikeal
Member
mikeal commented Feb 19, 2013

the time has come! this was just pushed in a release.

@nylen nylen deleted the nylen:digest-auth branch Apr 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment