IGNITE-19073 Add authentication to thin client protocol #1918
Conversation
Flaugh24
changed the title
Flaugh24
changed the title
modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/Extension.java
Outdated
Show resolved
Hide resolved
| private final ClientHandlerMetricSource metrics = new ClientHandlerMetricSource(); | ||
|
|
||
| private long idleTimeout = 5000; | ||
|
|
||
| TestServer(@Nullable TestSslConfig testSslConfig) { | ||
| TestServer(@Nullable TestSslConfig testSslConfig, AuthenticationConfiguration authenticationConfiguration) { |
There was a problem hiding this comment.
Can we make authenticationConfiguration nullable? It is not convenient to inject it in all tests where we don't actually need it.
There was a problem hiding this comment.
Done
Here and in another TestServer, it is better to use the Builder pattern. But I'd rather do it as a special ticket.
...ient-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
Show resolved
Hide resolved
...ient-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
Outdated
Show resolved
Hide resolved
...ient-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
Outdated
Show resolved
Hide resolved
...ient-handler/src/main/java/org/apache/ignite/client/handler/ClientInboundMessageHandler.java
Outdated
Show resolved
Hide resolved
| return e; | ||
| } | ||
| } | ||
| throw new IllegalArgumentException("Unknown extension key: " + key); |
There was a problem hiding this comment.
The point of handshake extensions is to be flexible and optional. We should skip unknown extensions instead of crashing.
There was a problem hiding this comment.
Agree, we should probably add UNKNOWN extension variant.
There was a problem hiding this comment.
For what? We can return null here and check where the method is called.
There was a problem hiding this comment.
It's probably more explicit and thus more obvious that extension can be unknown and we need to check for this result rather then checking for null, but I'm not insisting as you for sure have more expertise in Java than I.
ptupitsyn
changed the title
| }); | ||
| }).join(); | ||
| } | ||
|
|
* Pass credentials in client handshake extensions, authenticate when required. * Disconnect clients when authentication configuration changes. Co-authored-by: Ivan Gagarkin <igagarkin@gridgain.com>
https://issues.apache.org/jira/browse/IGNITE-19073