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]>
| * <p>This {@link LoginService} does not check credentials, a {@link UserIdentity} will be produced for any | ||
| * username provided in {@link #login(String, Object, Request, Function)}.</p> |
There was a problem hiding this comment.
Credentials are not checked only if the nested LoginService is null, otherwise they are checked, right?
If so, can you clarify?
| import org.eclipse.jetty.server.Session; | ||
|
|
||
| /** | ||
| * A {@link LoginService} which allows unknown users to be authenticated. |
There was a problem hiding this comment.
Can you please expand on why this LoginService would be useful with an example usage?
Basically this javadoc says: "this is a LoginService that just authenticates users without checking credentials", which seems like a security issue, so one more paragraph explaining that this class should not be used blindly, but perhaps in conjunction with something else?
| public class MultiAuthenticator extends LoginAuthenticator | ||
| { | ||
| private static final Logger LOG = LoggerFactory.getLogger(MultiAuthenticator.class); | ||
| public static final String LOGIN_PATH_PARAM = "org.eclipse.jetty.security.multi.login_path"; |
There was a problem hiding this comment.
Make it private.
Also, group together static fields, then empty line, then non-static fields.
| { | ||
| if (!loginPath.startsWith("/")) | ||
| { | ||
| LOG.warn("login path must start with /"); |
There was a problem hiding this comment.
No warn-level logs. Convert to debug.
| if (session == null) | ||
| return false; | ||
|
|
||
| synchronized (session) |
There was a problem hiding this comment.
This is unnecessary, following the discussion on #10392.
I'd remove all the synchronized blocks around the session, as I understand from @janbartel that it is going to be a different instance for every request in any case, so synchronization is useless.
| 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.
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).
Issue #5442
Introduces the
MultiAuthenticatorclass 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.