Make security.adoc the main Quarkus Security document #10937
Conversation
sberyozkin
requested review from
gastaldi,
emmanuelbernard,
stuartwdouglas,
pedroigor,
gsmet and
loicmathieu
sberyozkin
force-pushed
the
security-doc
branch
2 times, most recently
from
8647c93 to
9dc4035
Compare
|
Hi @gastaldi, @pedroigor thanks for the review so far :-) I'll wait and see if Guillaume , Stuart , Loic and Emmanuel have more comments, re some possible fixes to do with some section placements, etc, cheers |
|
Hi All, I've marked it for |
|
|
||
| == HttpAuthenticationMechanism Customization | ||
|
|
||
| One can customize `HttpAuthenticationMechanism` by registering an `ApplicationScoped` implementation bean. |
There was a problem hiding this comment.
Why must it be an ApplicationScoped ? Isn't a Singleton working ?
For customization via CDI bean we usually just say something like by providing a CDI bean that implements ... then in the example uses Singleton if possible (no proxy generation) or ApplicationScoped.
There was a problem hiding this comment.
@loicmathieu I've really no idea, I've only seen ApplicationScoped. Would it work if I say an ApplicationScoped or Singleton ?
There was a problem hiding this comment.
In fact, better check with @stuartwdouglas if @Singleton will works here. If yes better to suggest people to use this scope.
There was a problem hiding this comment.
@loicmathieu Hi, yeah, I've just updated the line to One can customize HttpAuthenticationMechanism by registering a CDI implementation bean. to avoid mentioning ApplicationScoped, though it is still in the listed example.
There was a problem hiding this comment.
Good for me. It's a valid implementation so it should not be important
There was a problem hiding this comment.
@loicmathieu Thanks, if you'd like, please open an issue to investigate running authenticators as Singletons, suppose would be good if it worked for the perf reasons
|
Hi @loicmathieu I've resolved 4 comments which I think I have addressed, please reopen if you think more work is needed. I've only left the one related to I'm on PTO from Monday, travelling, so I'm signing off later this evening and won't really have time to look at this issue before Monday. Thanks all |
|
Fixed the build issue introduced with my prev commit. I'm holding the merge for now as Guillaume can spot some doc issues immediately :-) and it is a pretty big refactoring. But I'll be away from Monday, so I'd appreciate if someone could merge it before CR1/early next week. Cheers |
|
I'm working on reviewing this and fixing the typos. Will push an update as soon as I'm done. |
gsmet
added
the
triage/waiting-for-ci
|
Merged! Very nice work @sberyozkin , this is a great improvement! |
Fixes #10694
Fixes #10856
Fixes #10427
Fixes #10361
This PR attempts to re-purpose
security.adocinto the main entry page about everything related toQuarkus Security. Right now Googling forQuarkus Securitylinks tosecurity.adocso it would be good if the users who land on this page can have an immediate high-level picture of what Quarkus Security offers.Some help from @stuartwdouglas and other colleagues will most likely needed to complete this PR and improve the language where needed.
Note this PR does not aim to change the layout of the docs/etc, the immediate task is to have a central page about the security which does a brief overview of all the security features and links to the specific docs/guides. Further overall doc optimizations can follow independently.
So in this PR:
security.adocprovides a brief overview of various sec features and links further. It would be good to have a small diagram in itsArchitrecturesection but we can add it later.security-authorization.adoc(where more can be added as needed)security-built-in-authentication.adocsecurity-customization.adocsecurity-testing.adocSome sections can be moved around but IMHO it is not a bad start toward consolidating the security documentation.
I'll be off for the next 3 weeks, I'll take care of all the review comments when I get back but it this PR will have been merged by then then it would be great too :-)