Fix for digest authentication bug - issue #2301 in rails/rails #7240

Merged
merged 2 commits into from Aug 2, 2012

Conversation

Projects
None yet
4 participants
@steveklabnik
Member

steveklabnik commented Aug 2, 2012

This is a rebase of #2323. Tests pass.

Fixes #2301.

@schneems

This comment has been minimized.

Show comment Hide comment
@schneems

schneems Aug 2, 2012

Member

👍

Member

schneems commented Aug 2, 2012

👍

@arthurpsmith

This comment has been minimized.

Show comment Hide comment
@arthurpsmith

arthurpsmith Aug 2, 2012

Contributor

Hey, I have to try that:
👍

Contributor

arthurpsmith commented Aug 2, 2012

Hey, I have to try that:
👍

@arthurpsmith

This comment has been minimized.

Show comment Hide comment
@arthurpsmith

arthurpsmith Aug 2, 2012

Contributor

Thanks again @steveklabnik - so what happens next?

Contributor

arthurpsmith commented Aug 2, 2012

Thanks again @steveklabnik - so what happens next?

@@ -139,11 +139,12 @@ def authenticate_with_request
test "authentication request with request-uri that doesn't match credentials digest-uri" do

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 2, 2012

Member

The test description doesn't match with the content.

@rafaelfranca

rafaelfranca Aug 2, 2012

Member

The test description doesn't match with the content.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 2, 2012

Member

Now you have to explain why this change.

Before we had a failure case test. Now a success. Why this change?

Member

rafaelfranca commented Aug 2, 2012

Now you have to explain why this change.

Before we had a failure case test. Now a success. Why this change?

@arthurpsmith

This comment has been minimized.

Show comment Hide comment
@arthurpsmith

arthurpsmith Aug 2, 2012

Contributor

Rafael - read Issue #2301 - the old failure test case was the problem. It should have allowed it. I modified the test case to be more explicit that proxy URL rewriting is allowed by the Digest Auth Spec.

Contributor

arthurpsmith commented Aug 2, 2012

Rafael - read Issue #2301 - the old failure test case was the problem. It should have allowed it. I modified the test case to be more explicit that proxy URL rewriting is allowed by the Digest Auth Spec.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 2, 2012

Member

Great!. @steveklabnik could you add a changelog entry?

Member

rafaelfranca commented Aug 2, 2012

Great!. @steveklabnik could you add a changelog entry?

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Aug 2, 2012

Member

I added it and pushed. Not appearing here quite yet, I'm sure it'll just be a second. EDIT: There it is.

Member

steveklabnik commented Aug 2, 2012

I added it and pushed. Not appearing here quite yet, I'm sure it'll just be a second. EDIT: There it is.

rafaelfranca added a commit that referenced this pull request Aug 2, 2012

Merge pull request #7240 from steveklabnik/fix_2301
Fix for digest authentication bug - issue #2301 in rails/rails

@rafaelfranca rafaelfranca merged commit 6e52376 into rails:master Aug 2, 2012

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 2, 2012

Member

Done. Thanks.

Member

rafaelfranca commented Aug 2, 2012

Done. Thanks.

@arthurpsmith

This comment has been minimized.

Show comment Hide comment
@arthurpsmith

arthurpsmith Aug 2, 2012

Contributor

Oh my, it's in after all this time. Thanks all!

Contributor

arthurpsmith commented Aug 2, 2012

Oh my, it's in after all this time. Thanks all!

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Aug 2, 2012

Member

:D ❤️

Member

steveklabnik commented Aug 2, 2012

:D ❤️

@schneems

This comment has been minimized.

Show comment Hide comment
@schneems

schneems Aug 2, 2012

Member

Good work! Dreams can come true don't forget to close #2301

Member

schneems commented Aug 2, 2012

Good work! Dreams can come true don't forget to close #2301

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 2, 2012

Member

Done.

Member

rafaelfranca commented Aug 2, 2012

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment