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

Recompose security-openid-connect-web-authentication.adoc to Diataxis framework #30518

Merged
merged 1 commit into from Feb 2, 2023

Conversation

sheilamjones
Copy link
Contributor

@sheilamjones sheilamjones commented Jan 20, 2023

Recompose the guide USING OPENID CONNECT (OIDC) TO PROTECT WEB APPLICATIONS USING AUTHORIZATION CODE FLOW (security-openid-connect-web-authentication.adoc) into the Diataxis framework.

The focus of this PR is to move and restructure the content into the topics. Further iterations are needed to further recomposition and style/editing changes

The following updates were made:

  • Moved the overview, OIDC diagram, and all sections under ==Reference guide into a new file called xref:security-oidc-code-flow-authentication-concept.adoc
  • Moved all elements in the ==Quickstart section into a new file called xref:security-oidc-code-flow-authentication-tutorial.adoc
  • Updated and fixed links to point to new topics and moved content-
  • Completed minor style changes and identified further enhancements needed
  • Fixed existing links to the content that was moved.

Closes: https://issues.redhat.com/browse/QDOCS-85
(and subtasks https://issues.redhat.com/browse/QDOCS-109 and https://issues.redhat.com/browse/QDOCS-110)

@quarkus-bot quarkus-bot bot added this to To do in Quarkus Documentation Jan 20, 2023
@sheilamjones sheilamjones marked this pull request as draft January 20, 2023 21:38
@sberyozkin
Copy link
Member

sberyozkin commented Jan 20, 2023

Good evening @sheilamjones Thanks for this PR, I'd consider naming it a security-protect-web-authentication-tutorial, simpler name, only code flow is supported in any case but we can discuss it next week, FYI, I've seen https://github.com/quarkusio/quarkus/pull/30456/files, but haven't started reviewing it as I guess there will be some links updated to link to these new files, etc.

Enjoy the weekend

@sberyozkin
Copy link
Member

Hi Sheila, should this PR be combined with another PR you opened ? I'd like to see how both new files are linked to, you probably need to rebase once @michelle-purcell 's PR is merged, as the OIDC intro section will be in the new auth mechanisms doc.
Please also simplify/shorten the name of the new file as proposed in the prev comment, the shorter the final URL segment the better IMHO
Thanks

@sheilamjones sheilamjones marked this pull request as ready for review January 24, 2023 13:01
@sheilamjones sheilamjones marked this pull request as draft January 24, 2023 13:22
@sheilamjones sheilamjones marked this pull request as ready for review January 24, 2023 18:28
@sheilamjones
Copy link
Contributor Author

@sberyozkin: Thanks for your comments above. I have renamed the new tutorial file to shorten it per your suggestions. In this PR, the Quickstart content from the original file (security-openid-connect-web-authentication.adoc) file has been removed from there and recomposed into this new tutorial file. I also rebased to pull in Michelle's latest changes.
Note, further styling needs to be done in the next iteration of updates.
There is a separate PR (https://github.com/quarkusio/quarkus/pull/30456/files) to handle the recomposing of the conceptual and reference sections of (security-openid-connect-web-authentication.adoc)

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! Nice work.

Copy link
Contributor

@michelle-purcell michelle-purcell left a comment

Choose a reason for hiding this comment

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

@sheilamjones - Nice job 👍 Just a few very minor comments.
Some might be slightly outside the scope of this iteration.

@github-actions
Copy link

github-actions bot commented Jan 26, 2023

🙈 The PR is closed and the preview is expired.

@sberyozkin
Copy link
Member

Hi @sheilamjones

Thanks for the updates,

https://github.com/quarkusio/quarkus/pull/30456/files

My question is, should #30456 and this PR become a single PR ? Both PR affect the same document, and it is not easy to see how this single security-openid-connect-web-authentication is impacted

@sheilamjones
Copy link
Contributor Author

Thanks @sberyozkin. I shall close PR #30456 and move those changes under this single PR.

@sheilamjones sheilamjones changed the title Quickstart content recomposed as a tutorial Recompose security-openid-connect-web-authentication.adoc to Diataxis framework Jan 26, 2023

Congratulations!
You have learned how to set up and use the OIDC authorization code flow mechanism to protect and test application HTTP endpoints.
After you have completed this tutorial, explore some of the other security mechanisms in Quarkus.
Copy link
Member

Choose a reason for hiding this comment

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

I think the line 249 is not necessary, and the link to the bearer token concept should be in References, as well as the security concept one.
As far as the next steps in scope of this tutorial is concerned, it should be the web-app reference (concept) and OIDC multi-tenency

@sberyozkin
Copy link
Member

Good evening @sheilamjones :-), thanks for this PR, a lot of work has been done here which is very appreciated.
IMHO we need to optimize a few things here though, lets discuss it in the next day or so.
Cheers

@sberyozkin
Copy link
Member

sberyozkin commented Jan 27, 2023

By the way, Sheila, your idea to have authorization code reflected somehow in the doc names is nice, I thought about it this morning, the current base name security-openid-connect-web-authentication-*.adoc does not reflect it, and I guess web-authentication part in this name can cause some confusion. But FYI, security-openid-connect.adoc does not reflect it either it is about a bearer authentication.
Lets agree on the restructuring aspects first and then if you don't mind, perhaps we should indeed follow your lead and rename these new docs, perhaps security-openid-connect-web-authentication-*.adoc -> security-openid-connect-web-authentication-*.adoc -> security-openid-connect-authorization-code-*.adoc or something like that

@sberyozkin
Copy link
Member

Made a bad typo, it was your lead, I typed our :-), not sure how I managed to type it but fixed now :-)

@sberyozkin
Copy link
Member

@sheilamjones Excellent work, thanks, we might tweak a few more things going forward, but it does look good now, cheers

Quarkus Documentation automation moved this from To do to Reviewer approved Feb 2, 2023
@sberyozkin sberyozkin merged commit 32c93e6 into quarkusio:main Feb 2, 2023
Quarkus Documentation automation moved this from Reviewer approved to Done Feb 2, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants