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

Improve OIDC bearer token concept doc #37692

Merged
merged 1 commit into from
Dec 21, 2023
Merged

Improve OIDC bearer token concept doc #37692

merged 1 commit into from
Dec 21, 2023

Conversation

sberyozkin
Copy link
Member

Fixes #37463

SPA snippet currently does not work with KC 23.
CC @jedla97, we can finalize early next year if Jakub is on PTO

@sberyozkin sberyozkin requested a review from rsvoboda December 12, 2023 15:30
@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Dec 12, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 12, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@rsvoboda
Copy link
Member

Jakub added his comments to the original issue - #37463 (comment)

@sberyozkin sberyozkin force-pushed the improve_oidc_bearer_token_concept_doc branch from 39b509a to 197a170 Compare December 17, 2023 22:40
jedla97
jedla97 suggested changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@jedla97 jedla97 left a comment

Choose a reason for hiding this comment

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

Sorry I miss the push for this PR. Thanks for all the changes! Looking at the changes they look good just one comment. Also just one thing is that text If you want to protect web applications by using OIDC authorization code flow authentication, see OIDC authorization code flow authentication still point to 404 and by your comment (point 1) #37463 (comment) this should be also removed.

docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc Outdated
@@ -782,7 +818,7 @@ quarkus.oidc.public-key=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAlivFI8qB4D0y
smallrye.jwt.sign.key.location=/privateKey.pem
----

copy `privateKey.pem` from the `integration-tests/oidc-tenancy` in the `main` Quarkus repository and use a test code similar to the one in the `Wiremock` section above to generate JWT tokens. You can use your own test keys if preferred.
copy link:https://github.com/quarkusio/quarkus/tree/main/integration-tests/oidc-tenancy/src/main/resources/privateKey.pem[privateKey.pem] from the `integration-tests/oidc-tenancy` in the `main` Quarkus repository and use a test code similar to the one in the `Wiremock` section above to generate JWT tokens. You can use your own test keys if preferred.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the link. Just small note it should look something like link:{quickstarts-tree-url}/integration-tests/oidc-tenancy/src/main/resources/privateKey.pem[privateKey.pem]

Copy link
Member Author

Choose a reason for hiding this comment

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

@jedla97 Unfortunately this resource is only available in the main repository, not in quickstarts. This doc is a concept/reference doc, so it is not strictly associated with the quickstart

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry how i was jumping between guides I miss that this link to Quarkus repo. What I had tried now is this link:{quarkus-blob-url}/integration-tests/oidc-tenancy/src/main/resources/privateKey.pem[privateKey.pem] and it should work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @jedla97 Sure, let me do it

@sberyozkin sberyozkin force-pushed the improve_oidc_bearer_token_concept_doc branch from 197a170 to ea3039c Compare December 20, 2023 12:13
@sberyozkin
Copy link
Member Author

@jedla97 Thanks, I think I've fixed the link, now it should correctly point to the code flow concept/ref doc, -concept is no longer part of the file name

@rsvoboda rsvoboda requested a review from jedla97 December 20, 2023 13:56
jedla97
jedla97 approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@jedla97 jedla97 left a comment

Choose a reason for hiding this comment

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

I can confirm that 404 link is gone. For that one hardcoded link (previous comment) I don't fully mind it can stay as it. So all my concerns and problems from issue was fixed or explained. With this I can approve this PR.

rsvoboda
rsvoboda approved these changes Dec 21, 2023
View reviewed changes
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.

Didn't spot anything suspicious, approving based on the approval from Jakub.

@sberyozkin sberyozkin force-pushed the improve_oidc_bearer_token_concept_doc branch from ea3039c to 493a445 Compare December 21, 2023 16:04
@sberyozkin sberyozkin marked this pull request as ready for review December 21, 2023 16:04
@sberyozkin
Copy link
Member Author

Thanks @jedla97 @rsvoboda, did one more update as suggested by @jedla97, should be ready to go once the build is finished

jedla97
jedla97 approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@jedla97 jedla97 left a comment

Choose a reason for hiding this comment

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

Thanks for updating link. Tested it and it's work.

@sberyozkin sberyozkin merged commit 2b3320a into quarkusio:main Dec 21, 2023
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Dec 21, 2023
@sberyozkin sberyozkin deleted the improve_oidc_bearer_token_concept_doc branch December 21, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jedla97 jedla97 jedla97 approved these changes

@rsvoboda rsvoboda rsvoboda approved these changes

Assignees
No one assigned
Labels
area/docstyle issues related for manual docstyle review area/documentation kind/bugfix
Projects
Quarkus Documentation
Status: Done
Milestone
3.7.0.CR1
Development

Successfully merging this pull request may close these issues.

Docs: security-oidc-bearer-token-authentication guide
3 participants
@sberyozkin @rsvoboda @jedla97