Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

Don't ignore previous headers when beginning OAuth #652

Merged
merged 1 commit into from
Dec 23, 2022

Conversation

paulomarg
Copy link
Contributor

WHY are these changes introduced?

Fixes #651

As pointed out in the issue, there is currently no way of adding custom headers to a response when beginning OAuth on Node.js environments, because we overwrite all headers with the ones we need to set. We need to preserve any headers previously set by the app when adding our own.

WHAT is this pull request doing?

Changing the oauth begin process to start out from the incoming response via a new conversion method (defaulting to an empty one for non-node environments), and then adding our cookies / redirect headers on top of those rather ignoring all of them.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

@paulomarg paulomarg requested a review from a team as a code owner December 22, 2022 20:47
Copy link
Contributor

@cquemin cquemin left a comment

Choose a reason for hiding this comment

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

Nice fix!

@paulomarg paulomarg merged commit efe0770 into main Dec 23, 2022
@paulomarg paulomarg deleted the allow_extra_params_in_oauth branch December 23, 2022 13:40
@shopify-shipit shopify-shipit bot temporarily deployed to production January 5, 2023 18:47 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable a way to pass information through the OAuth Process
2 participants