Skip to content

Pass through the from parameter when connecting an already connected Jetpack#12351

Merged
emdashcodes merged 3 commits intomasterfrom
fix/connect-from-redirect
May 15, 2019
Merged

Pass through the from parameter when connecting an already connected Jetpack#12351
emdashcodes merged 3 commits intomasterfrom
fix/connect-from-redirect

Conversation

@emdashcodes
Copy link
Copy Markdown

If a user tries to connect Jetpack when the are already connected, they are redirected back to the onboarding flow in Calypso. See #3788.

Currently, if a from parameter is supplied (like woocommerce-setup-wizard), this is lost during the redirect. We are going to start using this parameter in Calypso to show some different copy/design elements during the flow, so passing this along will help users who hit this edge case.

To Test:

@emdashcodes emdashcodes added [Status] Needs Review This PR is ready for review. Connect Flow Connection banners, buttons, ... labels May 13, 2019
@emdashcodes emdashcodes requested review from a team, jeherve, johnHackworth and tyxla May 13, 2019 18:37
@emdashcodes emdashcodes self-assigned this May 13, 2019
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented May 13, 2019

Warnings
⚠️ "Testing instructions" are missing for this PR. Please add some
⚠️ "Proposed changelog entry" is missing for this PR. Please include any meaningful changes

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 20d3e9e

@kraftbj kraftbj added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Needs Review This PR is ready for review. labels May 13, 2019
@emdashcodes
Copy link
Copy Markdown
Author

Thanks for the reviews! I've moved the line inside the connect_url_redirect block, and added a @todo for a whitelist.

@emdashcodes emdashcodes added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels May 14, 2019
Copy link
Copy Markdown
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@tyxla tyxla added this to the 7.4 milestone May 14, 2019
Copy link
Copy Markdown
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This should work. I only have a minor request until we have a whitelist of parameters.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels May 15, 2019
@jeherve
Copy link
Copy Markdown
Member

jeherve commented May 15, 2019

Unblocking for now; as @tyxla mentioned it can be addressed in a future PR.

@justinshreve You can merge whenever you're ready :)

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels May 15, 2019
jeherve added a commit that referenced this pull request May 15, 2019
Follow-up from #12351

Since one can currently pass any "from" parameter when building that URL, let's sanitize that value.
@emdashcodes emdashcodes merged commit ee71e31 into master May 15, 2019
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels May 15, 2019
@emdashcodes emdashcodes deleted the fix/connect-from-redirect branch May 15, 2019 14:33
jeherve added a commit that referenced this pull request May 21, 2019
…2380)

* Connect flow: sanitize "from" parameter when building connect url

Follow-up from #12351

Since one can currently pass any "from" parameter when building that URL, let's sanitize that value.

* Connect URL: allow the use of periods in from parameter

See #12380 (comment)

Co-Authored-By: Marin Atanasov <8436925+tyxla@users.noreply.github.com>

* Connect URL: escape full URL instead of sanitizing one parameter


Co-authored-by: Eric Binnion <ericbinnion@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Connect Flow Connection banners, buttons, ... Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants