-
Notifications
You must be signed in to change notification settings - Fork 267
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
add logout redirect option to Customer Account Client #1871
Conversation
9de9ba2
to
a301a0e
Compare
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
8185cd7
to
ffc7016
Compare
@@ -144,7 +148,7 @@ export type CustomerAccountForDocs = { | |||
* `en`, `fr`, `cs`, `da`, `de`, `es`, `fi`, `it`, `ja`, `ko`, `nb`, `nl`, `pl`, `pt-BR`, `pt-PT`, | |||
* `sv`, `th`, `tr`, `vi`, `zh-CN`, `zh-TW`. If supplied any other language code, it will default to `en`. | |||
* */ | |||
login: (options?: LoginOptions) => Promise<Response>; | |||
login?: (options?: LoginOptions) => Promise<Response>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
login were made optional in #1682 for docs but reverted.
ffc7016
to
4078cf0
Compare
…p origin url + add test
8e6593e
to
b59217d
Compare
Making one last update to ensure all redirect links are within the app |
const redirectUri = ensureLocalRedirectUrl({ | ||
requestUrl: request.url, | ||
defaultUrl: DEFAULT_AUTH_URL, | ||
redirectUrl: authUrl, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ensure authUrl is not cross site
* @param defaultUrl - The default URL to redirect to. | ||
* @param redirectUrl - Relative or absolute URL of redirect. If the absolute URL is cross domain return undefined. | ||
* */ | ||
export function ensureLocalRedirectUrl({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wizardlyhel hopefully this util make sense
WHY are these changes introduced?
Fixes #1809
WHAT is this pull request doing?
HOW to test your changes?
in
templates/skeleton/app/routes/account_.logout.tsx
also ensure this url is included in Customer Account application setup as logout url
/collections/all
Post-merge steps
Checklist