Skip to content

JPO: Remove sanitize key on registration URL#8501

Merged
oskosk merged 1 commit intomasterfrom
fix/register-sanitize-key-removal-jpo
Jan 11, 2018
Merged

JPO: Remove sanitize key on registration URL#8501
oskosk merged 1 commit intomasterfrom
fix/register-sanitize-key-removal-jpo

Conversation

@tyxla
Copy link
Copy Markdown
Member

@tyxla tyxla commented Jan 11, 2018

Seems like in #8449 we introduced sanitize_key over the registration URL onboarding token. This was a mistake, because it caused (only) fresh Jetpack sites onboarding tokens to be passed to Calypso as lowercase instead of uppercase 🤦‍♂️ And then, when comparing the lowercase tokens to the original ones, comparison failed and signature was different, thus we received the "The request is not signed correctly." error.

So, we don't need the sanitize_key and we can remove it safely.

To test:

Kudos to @oskosk for finding this issue, and for helping diagnose it.

@tyxla tyxla added General [Status] Needs Review This PR is ready for review. labels Jan 11, 2018
@tyxla tyxla self-assigned this Jan 11, 2018
@tyxla tyxla requested review from AnnaMag, ockham and oskosk January 11, 2018 15:02
@tyxla tyxla requested a review from a team as a code owner January 11, 2018 15:02
Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@oskosk oskosk merged commit 7d6b617 into master Jan 11, 2018
@oskosk oskosk deleted the fix/register-sanitize-key-removal-jpo branch January 11, 2018 15:55
@jeherve jeherve removed the [Status] Needs Review This PR is ready for review. label Jan 11, 2018
@oskosk oskosk added this to the 5.8 milestone Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants