Skip to content

Commit 5988f53

Browse files
committed
JVMCBC-1689 Omit redundant authorization ID from OAUTHBEARER payload
Motivation ---------- Now that MB-68594 is resolved, the SDK no longer needs to redundantly specify the authorization ID when using a JWT to authenticate. Modifications ------------- Remove code that extracted the "sub" field from the JWT, since we no longer need to pass it to the server redundantly. Let subclasses of InitialResponseSaslClient decide which bits of information to fetch from the callback. This lets us specify a null username, while avoiding the exception that would result from trying to fetch a null username from the callback. Propagate the authorization ID from the SaslClientFactory to OauthBearerSaslClient. This feature is currently unused, (the whole point of this change is to avoid having to explicitly pass an authorization ID) but this parallels the other SaslClient implementations. When authorization ID is absent (currently always!), omit the "a=<authorization-id>" component from the SASL OAUTHBEARER payload. Change-Id: I606f8297cc5bc6547f7ef5b1adb276eaa41333b0 Reviewed-on: https://review.couchbase.org/c/couchbase-jvm-clients/+/234802 Tested-by: Build Bot <[email protected]> Reviewed-by: Michael Reiche <[email protected]>
1 parent e6979c6 commit 5988f53

File tree

5 files changed

+26
-32
lines changed

5 files changed

+26
-32
lines changed

core-io/src/main/java/com/couchbase/client/core/env/JwtAuthenticator.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,25 +110,18 @@ public String toString() {
110110
private final Jwt jwt;
111111
private final String encodedJwt;
112112
private final String authHeaderValue;
113-
private final String username;
114113

115114
private JwtAuthenticator(String encodedJwt) {
116115
this.encodedJwt = encodedJwt.trim();
117116
this.authHeaderValue = "Bearer " + this.encodedJwt;
118117
this.jwt = Jwt.parse(this.encodedJwt);
119-
120-
String fieldName = "sub";
121-
this.username = jwt.payload.path(fieldName).textValue();
122-
if (this.username == null) {
123-
throw new IllegalArgumentException("Missing '" + fieldName + "' in JWT payload");
124-
}
125118
}
126119

127120
@Override
128121
public void authKeyValueConnection(EndpointContext ctx, ChannelPipeline pipeline) {
129122
pipeline.addLast(new SaslAuthenticationHandler(
130123
ctx,
131-
username,
124+
null, // username not required
132125
encodedJwt,
133126
supportedMechanisms
134127
));

core-io/src/main/java/com/couchbase/client/core/io/netty/kv/SaslAuthenticationHandler.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.couchbase.client.core.json.Mapper;
3939
import com.couchbase.client.core.msg.kv.BaseKeyValueRequest;
4040
import com.couchbase.client.core.util.Bytes;
41+
import org.jspecify.annotations.Nullable;
4142

4243
import javax.security.auth.callback.Callback;
4344
import javax.security.auth.callback.CallbackHandler;
@@ -102,7 +103,7 @@ public class SaslAuthenticationHandler extends ChannelDuplexHandler implements C
102103
* Holds the timeout for the full feature negotiation phase.
103104
*/
104105
private final Duration timeout;
105-
private final String username;
106+
private final @Nullable String username;
106107
private final String password;
107108
private final Set<SaslMechanism> allowedMechanisms;
108109
private final EndpointContext endpointContext;
@@ -129,7 +130,7 @@ public class SaslAuthenticationHandler extends ChannelDuplexHandler implements C
129130
*/
130131
private int roundtripsToGo;
131132

132-
public SaslAuthenticationHandler(final EndpointContext endpointContext, final String username,
133+
public SaslAuthenticationHandler(final EndpointContext endpointContext, final @Nullable String username,
133134
final String password, final Set<SaslMechanism> allowedSaslMechanisms) {
134135
this.endpointContext = endpointContext;
135136
this.username = username;

core-io/src/main/java/com/couchbase/client/core/io/netty/kv/sasl/CouchbaseSaslClientFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public SaslClient createSaslClient(final String[] mechanisms, final String autho
5656

5757
List<String> mechanismList = Arrays.asList(mechanisms);
5858
if (mechanismList.contains(SaslMechanism.OAUTHBEARER.mech())) {
59-
return new OauthBearerSaslClient(cbh);
59+
return new OauthBearerSaslClient(authorizationId, cbh);
6060
}
6161

6262
return Sasl.createSaslClient(mechanisms, authorizationId, protocol, serverName, props, cbh);

core-io/src/main/java/com/couchbase/client/core/io/netty/kv/sasl/InitialResponseSaslClient.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,31 +17,31 @@
1717
package com.couchbase.client.core.io.netty.kv.sasl;
1818

1919
import com.couchbase.client.core.annotation.Stability;
20+
import org.jspecify.annotations.NullMarked;
2021
import org.jspecify.annotations.Nullable;
2122

2223
import javax.security.auth.callback.CallbackHandler;
2324
import javax.security.sasl.SaslClient;
2425
import javax.security.sasl.SaslException;
2526

26-
import static com.couchbase.client.core.io.netty.kv.sasl.CallbackHelper.getPassword;
27-
import static com.couchbase.client.core.io.netty.kv.sasl.CallbackHelper.getUsername;
2827
import static java.util.Objects.requireNonNull;
2928

3029
/**
3130
* Base class for mechanisms that have a single step: the initial response.
3231
*/
3332
@Stability.Internal
33+
@NullMarked
3434
abstract class InitialResponseSaslClient implements SaslClient {
3535
private final CallbackHandler callbackHandler;
36-
private final String authorizationId;
36+
protected final @Nullable String authorizationId;
3737

3838
private boolean complete;
3939

4040
public InitialResponseSaslClient(
4141
@Nullable String authorizationId,
4242
CallbackHandler callbackHandler
4343
) {
44-
this.authorizationId = authorizationId == null ? "" : authorizationId;
44+
this.authorizationId = authorizationId;
4545
this.callbackHandler = requireNonNull(callbackHandler);
4646
}
4747

@@ -57,16 +57,10 @@ public byte[] evaluateChallenge(byte[] challenge) throws SaslException {
5757
}
5858

5959
complete = true;
60-
String username = getUsername(callbackHandler);
61-
String password = getPassword(callbackHandler);
62-
return getInitialResponse(authorizationId, username, password);
60+
return getInitialResponse(callbackHandler);
6361
}
6462

65-
abstract byte[] getInitialResponse(
66-
String authorizationId,
67-
String username,
68-
String password
69-
);
63+
abstract byte[] getInitialResponse(CallbackHandler callbackHandler) throws SaslException;
7064

7165
@Override
7266
public boolean isComplete() {
@@ -86,7 +80,7 @@ public byte[] wrap(byte[] outgoing, int offset, int len) throws SaslException {
8680
}
8781

8882
@Override
89-
public Object getNegotiatedProperty(String propName) {
83+
public @Nullable Object getNegotiatedProperty(String propName) {
9084
requireComplete("getNegotiatedProperty");
9185
return null;
9286
}

core-io/src/main/java/com/couchbase/client/core/io/netty/kv/sasl/OauthBearerSaslClient.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,23 @@
1818

1919
import com.couchbase.client.core.annotation.Stability;
2020
import com.couchbase.client.core.env.SaslMechanism;
21+
import org.jspecify.annotations.NullMarked;
22+
import org.jspecify.annotations.Nullable;
2123

2224
import javax.security.auth.callback.CallbackHandler;
25+
import javax.security.sasl.SaslException;
2326

27+
import static com.couchbase.client.core.io.netty.kv.sasl.CallbackHelper.getPassword;
2428
import static java.nio.charset.StandardCharsets.UTF_8;
2529

2630
@Stability.Internal
31+
@NullMarked
2732
public class OauthBearerSaslClient extends InitialResponseSaslClient {
28-
public OauthBearerSaslClient(CallbackHandler callbackHandler) {
29-
super(null, callbackHandler);
33+
public OauthBearerSaslClient(
34+
@Nullable String authorizationId,
35+
CallbackHandler callbackHandler
36+
) {
37+
super(authorizationId, callbackHandler);
3038
}
3139

3240
@Override
@@ -35,11 +43,9 @@ public String getMechanismName() {
3543
}
3644

3745
@Override
38-
protected byte[] getInitialResponse(
39-
String authorizationId,
40-
String username,
41-
String password
42-
) {
43-
return ("n,a=" + username + ",\1auth=Bearer " + password + "\1\1").getBytes(UTF_8);
46+
protected byte[] getInitialResponse(CallbackHandler callbackHandler) throws SaslException {
47+
String authorizationComponent = authorizationId == null ? "" : "a=" + authorizationId;
48+
String token = getPassword(callbackHandler);
49+
return ("n," + authorizationComponent + ",\1auth=Bearer " + token + "\1\1").getBytes(UTF_8);
4450
}
4551
}

0 commit comments

Comments
 (0)