-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Support one-time authorization code for form-based authentication mechanism #46974
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
base: main
Are you sure you want to change the base?
Support one-time authorization code for form-based authentication mechanism #46974
Conversation
This comment has been minimized.
This comment has been minimized.
🎊 PR Preview 0061772 has been successfully built and deployed to https://quarkus-pr-main-46974-preview.surge.sh/version/main/guides/
|
This comment has been minimized.
This comment has been minimized.
Update: Windows failure seems like a bug (though don't see how it is related to changes here), I'll find it and fix it. |
Oh, it is failing because during merging I removed the fix I introduced to Quarkus main branch already, my bad, I'll drop it. |
Thanks @michalvavrik, I'll look a bit later. |
7c7b6b6
to
6db6efa
Compare
Thanks as well, no hurry, I expect this will take a longer discussion. But I think it is easier this way because it is less code to review and once this is in, JDBC part will be easy. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...spi/src/main/java/io/quarkus/security/spi/runtime/OneTimeAuthZCodeAuthenticationRequest.java
Outdated
Show resolved
Hide resolved
...ns/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/FormAuthRuntimeConfig.java
Outdated
Show resolved
Hide resolved
@michalvavrik Thanks, I've started commenting and we can continue in the next few days, I suggest to set it Draft for now as I guess there will be a few more iterations |
As the PR is draft now, I'll be pushing changes continuously. I'll inform you with a PR comment once the PR is adapted to what we agreed in here and via email. Please ignore pushes until then. |
fff3113
to
dbfe323
Compare
dbfe323
to
c94c5f1
Compare
@sberyozkin your turn :-) |
c94c5f1
to
55cf344
Compare
[source,properties] | ||
---- | ||
quarkus.http.auth.form.authentication-token.enabled=true | ||
quarkus.http.auth.form.authentication-token.request-redirect-path=/authentication-token-form <1> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path can be made a default value, quarkus.http.auth.form.authentication-token.enabled=true
should be enough for such a case. Also propose to shorten the property to authentication-token.redirect-path
* This interface should be implemented as {@link jakarta.enterprise.context.ApplicationScoped} | ||
* or {@link jakarta.inject.Singleton} CDI bean. | ||
*/ | ||
public interface OneTimeAuthenticationTokenSender { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be correct to name it FormAuthenticationTokenSender
, with the one-time aspect highlighted in the docs ? Form authentication is the only built in mechanism that this interface can work with
import io.smallrye.mutiny.Uni; | ||
import io.vertx.ext.web.RoutingContext; | ||
|
||
final class OneTimeAuthTokenRequestHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
final class OneTimeAuthTokenRequestHandler { | |
final class FormAuthenticationTokenRequestHandler { |
or, FormAuthenticationTokenHandler
/** | ||
* Event fired when Quarkus received one-time authentication token request. | ||
*/ | ||
ONE_TIME_AUTH_TOKEN_REQUESTED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose to propose to replace ONE_TIME
(and lower cases onetime
) everywhere with Form
(using the right case), in this case it might be AUTHENTICATION_TOKEN
(since it in the FormEventType
), not sure _REQUESTED
is necessary
Hi @michalvavrik, apologies for delaying reviewing it. IMHO, it indeed looks better with the cookie. I have a question, as it is not quite obvious to me from the PR.
This may already be done this way, please clarify, but the way I imagine it should is as follows, if Form 2FA is enabled:
Do you agree ? Sorry if it is how it is already implemented Thanks |
[[two-factor-auth]] | ||
==== Two-factor authentication | ||
|
||
The form-based authentication mechanism supports two-factor authentication (2FA) with a one-time authentication token second-factor option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The form-based authentication mechanism supports two-factor authentication (2FA) with a one-time authentication token second-factor option. | |
The form-based authentication mechanism supports two-factor authentication (2FA) with a one-time authentication token as a second-factor option. |
---- | ||
<1> Once the one-time authentication token has been generated, redirect to a page with an authentication token form. | ||
|
||
When user submit a username and password to the `/j_security_check` POST location, they will get relocated to the `/authentication-token-form` page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When user submit a username and password to the `/j_security_check` POST location, they will get relocated to the `/authentication-token-form` page. | |
After users post their username and password to `/j_security_check`, they are redirected to `/authentication-token-form`. |
|
||
The form-based authentication mechanism supports two-factor authentication (2FA) with a one-time authentication token second-factor option. | ||
|
||
.Enable two-factor authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Enable two-factor authentication | |
* To enable 2FA, set the following properties: | |
+ |
quarkus.http.auth.form.authentication-token.enabled=true | ||
quarkus.http.auth.form.authentication-token.request-redirect-path=/authentication-token-form <1> | ||
---- | ||
<1> Once the one-time authentication token has been generated, redirect to a page with an authentication token form. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<1> Once the one-time authentication token has been generated, redirect to a page with an authentication token form. | |
<1> After generating the one-time authentication token, redirect to the authentication token form. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello Michal! Not sure if we need to use the <1> here. If not, I would just use the "+" above that sentence to glue and align it to that snippet above, and we should be good. It's a way to describe the aftermath of our earlier actions.
<1> Once the one-time authentication token has been generated, redirect to a page with an authentication token form. | ||
|
||
When user submit a username and password to the `/j_security_check` POST location, they will get relocated to the `/authentication-token-form` page. | ||
The `/authentication-token-form` page should allow users to submit the one-time authentication token sent to them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `/authentication-token-form` page should allow users to submit the one-time authentication token sent to them. | |
The `/authentication-token-form` page allows them to submit the one-time authentication token. |
When user submit a username and password to the `/j_security_check` POST location, they will get relocated to the `/authentication-token-form` page. | ||
The `/authentication-token-form` page should allow users to submit the one-time authentication token sent to them. | ||
|
||
.Example form for authentication with a one-time authentication token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Example form for authentication with a one-time authentication token | |
.An example of an HTML form that submits a one-time authentication token |
|
||
Once Quarkus has generated the one-time authentication token, you need to deliver the token to a user by declaring a CDI bean that implements the `io.quarkus.vertx.http.security.token.OneTimeAuthenticationTokenSender` interface. | ||
|
||
.Example one-time authentication token sender |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.Example one-time authentication token sender | |
.An example of a CDI bean that sends the one-time authentication token by email |
} | ||
---- | ||
<1> Use the Quarkus Mailer extension to send the email with the authentication token. | ||
See the xref:mailer-reference.adoc[Quarkus Mailer Reference documentation] for more information about the mailer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the xref:mailer-reference.adoc[Quarkus Mailer Reference documentation] for more information about the mailer. | |
For more information about the mailer, see the xref:mailer-reference.adoc[Quarkus Mailer reference documentation]. |
Hey, @michalvavrik ! I just spotted this awesome PR and provided a few suggestions. Use them if you like it, or feel free to tweak them or drop them completely :) |
Uh oh!
There was an error while loading. Please reload this page.