-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Issue #5442 - add MultiAuthenticator to support multiple authentication options #12393
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: jetty-12.1.x
Are you sure you want to change the base?
Issue #5442 - add MultiAuthenticator to support multiple authentication options #12393
Conversation
…entication options Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Lachlan Roberts <[email protected]>
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
...tty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AnyUserLoginService.java
Show resolved
Hide resolved
...ore/jetty-security/src/main/java/org/eclipse/jetty/security/DefaultAuthenticatorFactory.java
Show resolved
Hide resolved
Signed-off-by: Lachlan Roberts <[email protected]>
...ore/jetty-security/src/main/java/org/eclipse/jetty/security/DefaultAuthenticatorFactory.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
Signed-off-by: Lachlan Roberts <[email protected]>
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AnyUserLoginService.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AnyUserLoginService.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
assertThat(response.getContentAsString(), containsString("<h1>Multi Login Page</h1>")); | ||
assertThat(response.getContentAsString(), containsString("/login/openid")); | ||
assertThat(response.getContentAsString(), containsString("/login/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.
I would like to see:
- for this test, the access to a protected resource should be tried for both authentications.
- a new test for failed authentication, protected resources are not accessible
- a test where one authentication succeeds but the other fails -- can I still access the resource? Basically I would like to know if "multi" has "and" semantic (all authentications but must successful), or "or" semantic (one successful authentication is enough).
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 first two points are covered by the main test case.
a test where one authentication succeeds but the other fails -- can I still access the resource? Basically I would like to know if "multi" has "and" semantic (all authentications but must successful), or "or" semantic (one successful authentication is enough).
It doesn't work like this, once an authentication has succeeded you cannot attempt another authentication, because you are locked into that authenticator until you log out. So it has OR semantics.
So if I login with OPENID, then try to login as FORM, I will not be redirected to the form login page, and if I submit a form request to login it will go straight through the OpenIdAuthenticator
and be delivered to the application.
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts let's wake this up and get it merged |
…5442-multipleAuthentication
…ibute Signed-off-by: Lachlan Roberts <[email protected]>
…5442-multipleAuthentication
@sbordet please review this one again so it can be merged |
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.
Please fix the small nits I reported.
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Outdated
Show resolved
Hide resolved
jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/MultiAuthenticator.java
Show resolved
Hide resolved
tests/test-integration/src/test/java/org/eclipse/jetty/test/MultiAuthenticatorTest.java
Show resolved
Hide resolved
Signed-off-by: Lachlan Roberts <[email protected]>
…5442-multipleAuthentication
Signed-off-by: Lachlan Roberts <[email protected]>
…5442-multipleAuthentication
Issue #5442
Introduces the
MultiAuthenticator
class which can be used to support multiple authentication options simultaneously for the same webapp.For example you could have an app with the options to login with FORM, OpenID or Ethereum.