Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add Authenticate storefront app proxy #422

Merged
merged 28 commits into from
Sep 11, 2023

Conversation

byrichardpowell
Copy link
Contributor

@byrichardpowell byrichardpowell commented Aug 25, 2023

WHY are these changes introduced?

Parters need an easy way to authenticate app proxy requests

WHAT is this pull request doing?

  1. Introduce authenticate.public.appProxy(request)
  2. Deprecate authenticate.public(request)
  3. Add authenticate.public.checkout(request) to replace authenticate.public(request)

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have used yarn changeset to create a draft changelog entry (do NOT update the CHANGELOG.md files manually)
  • I have added/updated tests for this change
  • I have documented new APIs/updated the documentation for modified APIs (for public APIs)

@byrichardpowell byrichardpowell requested a review from a team as a code owner August 25, 2023 19:07
Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

Some early thoughts.

@byrichardpowell byrichardpowell force-pushed the authenticate-storefront-app-proxy branch 2 times, most recently from ecb5dfe to 6ab2ea7 Compare September 1, 2023 14:41
Copy link
Contributor

@paulomarg paulomarg left a comment

Choose a reason for hiding this comment

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

Just had some nits, but happy with this!

byrichardpowell and others added 23 commits September 11, 2023 13:43
…older.

The intent is:

1. Make it obvious what test-helpers we have by making them visible in the file system
2. Make it clear where they live (__tests__ is for test files
…all 3 places that we return the admin API client
1. Test a valid request for appProxy
2. Make test for invalid signature more explicit
3. Add comment why we have a suprising type
4. Add TSDoc comments to the authenticate public API
1. Removed .only from two tests (oops)
2. Wrap example titles in <caption>
3. Make it clear in the tests the Request is cross origin
4. app Proxy requests no longer check for a valid session, since that does not seem to be a requirement
…roxy/types.ts

Co-authored-by: Paulo Margarido <64600052+paulomarg@users.noreply.github.com>
@byrichardpowell byrichardpowell merged commit 786b5ae into main Sep 11, 2023
13 checks passed
@byrichardpowell byrichardpowell deleted the authenticate-storefront-app-proxy branch September 11, 2023 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants