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

Add simple form base app in security-authentication-mechanisms guide #39079

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

jedla97
Copy link
Contributor

@jedla97 jedla97 commented Feb 29, 2024

Fixes #37645

Adding simple app example for form-base security which I was missing when reading the guide

@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Feb 29, 2024
@GET
@Path("hello")
@RolesAllowed("user")
public String hello(@Context SecurityContext security) {
Copy link
Member

Choose a reason for hiding this comment

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

We don't really use JAX-RS SecurityContext in our security examples, but SecurityIdentity which has more options for checks and is injected directly into the endpoint.

Also, there is nearly an identical code example for the MTLS below. We should probably have a dedicated small section describing how to access the security identity and refer to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Theoretically in this example is not necessary use SecurityContext/SecurityIdentity but just return simple string like Successfully logged in.

Or I can create that small section but where should I put it?

Copy link
Member

Choose a reason for hiding this comment

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

@jedla97 Since you agreed to remove the code example, it is ok, we can add something later

return "Hello " + security.getUserPrincipal().getName();
}

@GET
Copy link
Member

Choose a reason for hiding this comment

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

IMHO we can just drop this example, for the user facing application one would not return Strings but for example here produce HTML with Qute. This specific doc's goal is to give an overview of the authentication mechanisms only.
Perhaps it is time to introduce a dedicated doc related to the form authentication

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll remove it

This comment has been minimized.


[IMPORTANT]
====
Configuring user names, secrets, and roles in the application.properties file is appropriate only for testing scenarios. For securing a production application, it is crucial to use a database to store this information.
Copy link
Member

Choose a reason for hiding this comment

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

LDAP is also an option, it also uses DB indirectly, but may be it is worth saying database or LDAP and link to the basic authentication with JPA doc for an example.

Copy link

github-actions bot commented Feb 29, 2024

🙈 The PR is closed and the preview is expired.


[NOTE]
====
Endpoind for verification `j_security_check` and name of parameters for username (`j_username`) and password (`j_password`) can be changed by setting it's configuration properties.
Copy link
Member

Choose a reason for hiding this comment

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

A few typos, but IMHO this note is unnecessary, the configuration section follows just below and it is clear all of these properties can be changed

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

Thanks @jedla97, LGTM, I'm only proposing to try to minimize the new text as this overview of the authentication mechanisms doc should only give some basic overview of a specific mechanism as opposed to for example being a tutorial on the Form authentication.

IMHO it will make sense to create a dedicated Form authentication doc later with a Form auth tutorial, and a quickstart, as it looks like Form authentication is starting to dominate in this doc

@jedla97
Copy link
Contributor Author

jedla97 commented Feb 29, 2024

@sberyozkin I tried to keep it at minimum. I'll update all the comments just one have question.

IMHO it will make sense to create a dedicated Form authentication doc later with a Form auth tutorial, and a quickstart, as it looks like Form authentication is starting to dominate in this doc

It would be nice to have it separate, for me it was that I just missing where to start with form. As I believe that lots of small projects or small learning projects don't need complex security as using oidc for example.

@sberyozkin
Copy link
Member

sberyozkin commented Feb 29, 2024

@jedla97

It would be nice to have it separate, for me it was that I just missing where to start with form. As I believe that lots of small projects or small learning projects don't need complex security as using oidc for example.

Sure you are right, I'd only like to avoid the doc which is meant to give an overview of the available mechanisms provide a form authentication tutorial.
So lets add a few more clarifications with your PR (for ex, how to configure) but then, in a follow up, create a security-form-authentication.adoc and move most of the current Form related content there and only retain a short overview in this doc, with a link to the new one.
Does it sound reasonable ?

@jedla97
Copy link
Contributor Author

jedla97 commented Feb 29, 2024

Does it sound reasonable ?

@sberyozkin Yes it does!

So lets add a few more clarifications with your PR (for ex, how to configure) but then, in a follow up, create a security-form-authentication.adoc and move most of the current Form related content there and only retain a short overview in this doc, with a link to the new one.

Just for clarification, who should create the follow-up? I can work on this a bit, but I'm not sure I'll cover all the technical details correctly. If it should be similar to this for example I can do it.

Copy link

quarkus-bot bot commented Feb 29, 2024

Status for workflow Quarkus Documentation CI

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

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

Copy link
Member

@sberyozkin sberyozkin left a comment

Choose a reason for hiding this comment

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

Thanks @jedla97

@sberyozkin sberyozkin merged commit 962bae3 into quarkusio:main Feb 29, 2024
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Feb 29, 2024
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/bugfix
Projects
Development

Successfully merging this pull request may close these issues.

Docs: security-authentication-mechanisms Guide
2 participants