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

Create security cors doc #36541

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Conversation

sberyozkin
Copy link
Member

Fixes #36509.

This PR promotes the CORS related section as an independent security concept, linked to from both HTTP reference and Security overview docs

@sberyozkin sberyozkin marked this pull request as draft October 17, 2023 21:36
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Oct 17, 2023
@github-actions
Copy link

github-actions bot commented Oct 17, 2023

🙈 The PR is closed and the preview is expired.

@sberyozkin sberyozkin force-pushed the create-security-cors-doc branch from 9807857 to d7b4dd8 Compare October 18, 2023 13:30
@sberyozkin sberyozkin marked this pull request as ready for review October 18, 2023 13:30
@quarkus-bot

This comment has been minimized.

@rsvoboda
Copy link
Member

rsvoboda commented Oct 20, 2023

@sberyozkin do you know why the preview deploy failed?

Asking because want to check how the guide is presented on /guides page

Copy link
Member

@rsvoboda rsvoboda left a comment

Choose a reason for hiding this comment

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

References will need to be updated in more places:

git grep -l "#cors-filter"

docs/src/main/asciidoc/openapi-swaggerui.adoc
docs/src/main/asciidoc/resteasy-reactive.adoc
docs/src/main/asciidoc/resteasy.adoc
docs/src/main/asciidoc/security-csrf-prevention.adoc
docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc
docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc
docs/src/main/asciidoc/security-overview.adoc

@rsvoboda rsvoboda added the triage/qe? Issue could use a quality focused review to further harden it label Oct 20, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 20, 2023

/cc @mjurc (qe)

@sberyozkin
Copy link
Member Author

Sorry, @rsvoboda, I was under impression only one doc was referring to the HTTP CORS section, thanks for reminding all docs have to be checked (those links would still work since HTTP section is still there, the only problem, users will need to click on another link from there, so yes, let me handle these links)

@sberyozkin sberyozkin force-pushed the create-security-cors-doc branch 2 times, most recently from c96a457 to 758e8e6 Compare October 20, 2023 10:07
@sberyozkin
Copy link
Member Author

@rsvoboda This is done, note http-reference.adoc#cors-filter will now be a more precise check if some old references are still there, all is clear now

@sberyozkin
Copy link
Member Author

Not sure though why surge is failing to publish

@rsvoboda
Copy link
Member

rsvoboda commented Oct 20, 2023

https://quarkus-pr-main-36541-preview.surge.sh/version/main/guides/ reports Unavailable

First line of the guide is shown on /guides page. This CORS guide has quite long first sentence, that's why it would be good to check it.

@gastaldi @gsmet did you notice troubles with surge.sh recently?

@gastaldi
Copy link
Contributor

@rsvoboda not recently. I had some trouble in the past when the payload is bigger than the limit supported in surge.sh, which I reported in #35660

@sberyozkin sberyozkin force-pushed the create-security-cors-doc branch from 758e8e6 to e1cf334 Compare October 23, 2023 08:59
@sberyozkin
Copy link
Member Author

@rsvoboda

This CORS guide has quite long first sentence, that's why it would be good to check it.

It was immediately starting from a link to the Mozilla docs, so I removed it, may be it will help for surge to succeed

@quarkus-bot
Copy link

quarkus-bot bot commented Oct 23, 2023

✔️ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@sberyozkin
Copy link
Member Author

@rsvoboda surge did not publish again, not sure why, shall we merge it if you are otherwise happy ? I'd not mind the Doc team having a quick look though

@rsvoboda
Copy link
Member

@sunayna15 can you have somebody from the team to look into this? We may merge it soon, docs feedback can be addressed in separate pull request.

@rsvoboda rsvoboda merged commit e59fe0a into quarkusio:main Oct 24, 2023
22 checks passed
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Oct 24, 2023
@quarkus-bot quarkus-bot bot added this to the 3.6 - main milestone Oct 24, 2023
@MichalMaler
Copy link
Contributor

@rsvoboda @sberyozkin @sunayna15 Let me have a look on this content. Will create a new PR if needed.

@sberyozkin sberyozkin deleted the create-security-cors-doc branch October 24, 2023 10:13
@sberyozkin
Copy link
Member Author

Thanks @rsvoboda @MichalMaler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docstyle issues related for manual docstyle review area/documentation kind/enhancement New feature or request triage/qe? Issue could use a quality focused review to further harden it
Projects
Development

Successfully merging this pull request may close these issues.

Move HTTP CORS section to its own document
4 participants