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

Breaks IndieAuth authorization flow #164

Open
janboddez opened this issue Jun 15, 2022 · 0 comments
Open

Breaks IndieAuth authorization flow #164

janboddez opened this issue Jun 15, 2022 · 0 comments

Comments

@janboddez
Copy link

janboddez commented Jun 15, 2022

Probably a bit niche, but this bit of code (https://github.com/wp-graphql/wp-graphql-jwt-authentication/blob/develop/src/ManageTokens.php#L345) somehow removes the Location header off of certain IndieAuth responses.

Well, this one response right here: https://github.com/indieweb/wordpress-indieauth/blob/trunk/includes/class-indieauth-authorization-endpoint.php#L273

Now, I can't say I fully understand what's going on, but that Location header "just works" again if in src/ManageTokens.php I comment out the $response->set_headers() call on line 345-355.

(Was somehow hoping that merely adding a second ("override") parameter equal to false to the $response->set_headers() call would get it to leave existing headers alone, but that doesn't seem to do much at all.)

I also noticed how just above it says, "The Access-Control-Expose-Headers aren't directly filterable for REST API responses, so this overrides them altogether."

I'm wondering if that's the case, still, as WP core seems to now set these as follows (since WP5.5, in wp-includes/rest-api/class-wp-rest-server.php):

$expose_headers = apply_filters( 'rest_exposed_cors_headers', $expose_headers );
$this->send_header( 'Access-Control-Expose-Headers', implode( ', ', $expose_headers ) );

Again, not 100% sure, but it looks like rest_exposed_cors_headers might be the filter you're looking for here. Seems it would allow one to just tack on the X-JWT-Refresh header.

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

No branches or pull requests

1 participant