Bug Fix: unquote base_url as a third attempt to validate signature

[Mr-TaylorHobbs]

No description provided.

[Mr-TaylorHobbs]

@stclair Talked to Matt about this before he took off today. If we validate the signature here, we don't actually have to change the way that anything is signed now and all current cases that work shouldn't be affected because they will still fall into one of the first two validation cases.

[stclair]

@Mr-TaylorHobbs Alright. Now that config is on 1.8 in staging, is this still an issue?

[Mr-TaylorHobbs]

It is. Our initial thought of the incompatible Django versions wasn't the actual root of the problem.

[stclair]

Can you explain why this is necessary? I dislike the fork in logic. It seems to be that when a URL is signed, everything should be unquoted, and when the signature is validated, everything should be unquoted. How is it that we have a signature being created where the url is not quoted and the query string is quoted? @madisona will you have a look at this as well? I think you have been the most recent one to troubleshoot a quoted/unquoted issue.

[Mr-TaylorHobbs]

Agreed that the "more correct" way to go about this would be to make sure that everything is either unquoted, or quoted when we both sign and check the signature. Doing this, however, is much less safe for current apps because everything will have to be updated to have the correct version of the signer at the same time. Matt said his intention was to simplify this process in some way that will make all of this unnecessary and he was comfortable with the forking for now.

Right now the issue we are having is the fact that urls are signed as unquoted before they are subsequently quoted and the request sent. The query params are quoted as soon as they are stuck on the url, so they stay consistent.

On the sending end the signature is created with the following url: www.abc.com/some thing/?query=string%2Cparams, then the url is encoded (www.abc.com/some%20thing) and the request is sent out. When the request is received by the other application, it first tries to validate with what it got (www.abc.com/some%20thing/?query=string%2Cparams). When this doesn't match it then tries to unquote the entire url and match it (www.abc.com/some thing/?query=string,params). Neither of these match and the request returns a 403. This third case that the PR adds is a third chance to validate, where we only unquote the first half so that this case would match (www.abc.com/some thing/?query=string%2Cparams).

Changing the receiving end may be the "less correct" way to go about handling this issue, it is definitely the safer route in terms of breaking any production apps.

[stclair]

@Mr-TaylorHobbs Ok, thank you for the explanation. I am comfortable with that.

[khornberg]

@stclair Do we merge this or wait until next week in case fires break out?

[stclair]

@khornberg Are you guys 100% comfortable with the change?

[khornberg]

@stclair I say wait until more hands are on deck
We could also make it a major version change to prevent it from being used accidentally.

[Mr-TaylorHobbs]

@stclair I am fairly confident in this change. It should not break anything further. Are you comfortable with merging this?

[stclair]

@Mr-TaylorHobbs Yes. I do not have the option to merge on this repo for some reason. Do you?

[Mr-TaylorHobbs]

Strange, I do... I'll go ahead and merge it.