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.

Sign up for GitHub

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

Jump to bottom

fix: update basePath config in index.js #1485

Closed
wants to merge 1 commit into from
Closed

fix: update basePath config in index.js #1485

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion code-examples/protect-page-login/expressjs/routes/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var sdk = require("@ory/client")
var ory = new sdk.FrontendApi(
new sdk.Configuration({
basePath:
process.env.ORY_SDK_URL || "https://playground.projects.oryapis.com",
"http://localhost:4000/.ory",
Copy link
Member

Choose a reason for hiding this comment

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

This API call doesn't need to go though the proxy - so it's easier / better / more flexible if it just points to the regular SDK url. I think this was fine the way it was, or did you encounter any issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did encounter an issue (on localhost) when trying to implement the logout method via createBrowserLogoutFlow. Redirecting the user to the provided logout_url, led to the error: Unable to log out because the logout token in the URL query does not match the session cookie.
My fix was to set the basePath to localhost:4000. After that change, the logout_url is working as desired

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the guide is inconsistent with the example code. or it has been changed.
I don't think this example uses the integrations package to proxy requests to itself and thus you need to spin up the proxy / tunnel externally.

Using the upstream Ory APIs would thus give infinite redirect loops since the cookie is not set correctly. So i think this change is actually correct.

Copy link
Member

@aeneasr aeneasr Aug 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why is the CI then failing? :D

The set up for this app is ory proxy -> node app iirc. The node app calling the ory APIs will not send any cookies anyways. I don't think this makes sense to loop it again through the proxy: ory proxy->node app->ory proxy->oryapis.com

We only need the proxy for the browser to send the right cookies. The node app doesn't send cookies anyways (because it's an SDK client). It can only forward those that were included in the incoming request.

Will need to look into the reason why logout_url isn't working though

Copy link
Contributor

Choose a reason for hiding this comment

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

i'll take a look

}),
)
// highlight-end
Expand Down
Loading