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

Add docs for the CSRF prevention plugin #1060

Merged
merged 1 commit into from
May 17, 2022
Merged

Add docs for the CSRF prevention plugin #1060

merged 1 commit into from
May 17, 2022

Conversation

SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented May 16, 2022

Fixes #1039

The plugin was added in #1006

Docs are imported from Apollo Server’s own docs and adapted.

@SimonSapin SimonSapin requested a review from StephenBarlow as a code owner May 16, 2022 11:11
@netlify
Copy link

netlify bot commented May 16, 2022

Deploy Preview for apollo-router-docs ready!

Name Link
🔨 Latest commit 7ac52bc
🔍 Latest deploy log https://app.netlify.com/sites/apollo-router-docs/deploys/6283c74089f1ed0009cc1c41
😎 Deploy Preview https://deploy-preview-1060--apollo-router-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link
Contributor

@SimonSapin your pull request is missing a changelog!


Note that all HTTP header names are case-insensitive.

> CSRF prevention is only applied to requests that will execute GraphQL operations, not to requests that would load [landing pages](../api/plugin/landing-pages) or run [health checks](../monitoring/health-checks).)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This probably needs change, at least for fixing the links.

Does the router have a landing page, health checks, or anything else that’s not GraphQL requests? Do these go through plugins?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we have other routes. Looking at this code, for health check, landing page and graphql endpoint the request first goes through CORS, then plugins. While for plugin specific endpoint, the request goes through directly:

.route(
&graphql_endpoint,
get({
let display_landing_page = configuration.server.landing_page;
move |host: Host,
service: Extension<BufferedService>,
http_request: Request<Body>| {
handle_get(host, service, http_request, display_landing_page)
}
})
.post(handle_post),
)
.layer(
TraceLayer::new_for_http()
.make_span_with(PropagatingMakeSpan::new())
.on_response(|resp: &Response<_>, _duration: Duration, span: &Span| {
if resp.status() >= StatusCode::BAD_REQUEST {
span.record(
"otel.status_code",
&opentelemetry::trace::StatusCode::Error.as_str(),
);
} else {
span.record(
"otel.status_code",
&opentelemetry::trace::StatusCode::Ok.as_str(),
);
}
}),
)
.route("/.well-known/apollo/server-health", get(health_check))
.layer(Extension(boxed_service))
.layer(cors);

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’ve removed the note for now and opened #1081 to follow up.

@abernix abernix linked an issue May 17, 2022 that may be closed by this pull request
@abernix
Copy link
Member

abernix commented May 17, 2022

This closes #1039, right?

@SimonSapin
Copy link
Contributor Author

Oh indeed. I’ve added the magic keyword in the PR description.

Copy link
Contributor

@o0Ignition0o o0Ignition0o left a comment

Choose a reason for hiding this comment

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

This looks great, congratulations! 🎉

@SimonSapin SimonSapin enabled auto-merge (squash) May 17, 2022 16:04
@SimonSapin SimonSapin merged commit c6dce05 into main May 17, 2022
@SimonSapin SimonSapin deleted the simon/csrf-docs branch May 17, 2022 16:21
@BrynCooke BrynCooke added this to the v0.9.1 milestone May 20, 2022
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.

CSRF plugin: Update documentation
6 participants