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

Improve OIDC client reference document #39367

Merged
merged 1 commit into from
Mar 13, 2024

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Mar 12, 2024

The OIDC client reference document has some problems at the moment:

  • It does not have the generated OIDC client properties
  • Examples where OidcClient is used directly are pretty bad, they were added awhile back, the assumption was the users who chose to use OidcClient directly would write some code manually acquiring and refreshing tokens, but it is not good for new users

So this PR does the following:

  • Adds a Configuration section where OIDC client properties are listed. Also I've added another section there showing the token propagation properties (the PR refers to the old quarkus-oidc-token-propagation-reactive because the directory has not been changed - but the generated properties correctly show the quarkus.rest-client-oidc-token-propagation namespace)
  • Improve several sections showing how to use OidcClient directly, without relying on the filter
  • It also fixes a typo where a tenant-id attribute is mentioned

@rolfedh Can you please do a Doc team check on this PR, mostly, these are code examples, but there is a bit if extra text there as well

@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Mar 12, 2024
@quarkus-bot

This comment has been minimized.

Copy link

github-actions bot commented Mar 12, 2024

🙈 The PR is closed and the preview is expired.

@sberyozkin
Copy link
Member Author

sberyozkin commented Mar 12, 2024

@sheilamjones I've noticed Rolfe is away, would you be OK with doing a review. The actual text changes are quite limited, but I'm asking the Doc team to check given that it will be eventually synced to the downstream docs. May be I can just merge as the team will likely be doing the whole doc review later anyway.

@sheilamjones
Copy link
Contributor

Hi @sberyozkin, sure, I will take a look now.

@sheilamjones
Copy link
Contributor

Hi @sberyozkin, I completed a review focusing on your changes only. I just had some minor comments for your consideration and also a couple of questions. Hope this helps.
Kind regards,
Sheila

@sberyozkin
Copy link
Member Author

Hi @sheilamjones Thanks very much for the prompt review, applied all proposed changes, and I'll also do a minor clarification re the header and the scheme, thanks

@sberyozkin sberyozkin force-pushed the oidc_client_config_properties branch from e2e7802 to 68229c0 Compare March 12, 2024 23:55
@quarkus-bot
Copy link

quarkus-bot bot commented Mar 13, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 68229c0.

✅ 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.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 13, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 68229c0.

✅ 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

Let me merge it now, so that I can link to the doc snapshots showing the generated properties, thanks @sheilamjones @gastaldi.

Sheila, FYI, I only did a minor update after applying your suggestions, with You can use 'OidcClient' directly to acquire access tokens and set them in an HTTP Authorizationheader as aBearer scheme value. without focusing on the actual technical representation like I did with Authorization: Bearer - since the user would not have to set : etc...

Thanks

@sberyozkin sberyozkin merged commit 284c7dd into quarkusio:main Mar 13, 2024
22 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.10 - main milestone Mar 13, 2024
@sberyozkin sberyozkin deleted the oidc_client_config_properties branch March 13, 2024 09:37
@sberyozkin
Copy link
Member Author

These changes should be backportable I hope

@sberyozkin
Copy link
Member Author

Sorry, @sheilamjones, if you think I merged too early, let me know, I'll apply other suggestions you may have right away :-)

@gsmet gsmet modified the milestones: 3.10 - main, 3.9.0.CR2 Mar 14, 2024
Copy link
Contributor

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

LGTM

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 triage/backport-3.8
Projects
Development

Successfully merging this pull request may close these issues.

5 participants