Skip to content

Commit

Permalink
[FFM-8688] - Handle accountID as an optional field in the JWT token (#…
Browse files Browse the repository at this point in the history 
…157)

[FFM-8688] - Handle accountID as an optional field in the JWT token

What
Adding new unit tests + harden the existing JWT parsing code to handle information which may be optional when an ff-proxy is in use.
If the info is not available the header will not be added. Also fixed env header to fall back to UUID when not present.

Why
The proxy doesn’t send accountID in the JWT token on auth, so we should make sure the code handles this correctly without bailing out when a proxy is in use.

Testing
New unit tests written + manual
  • Loading branch information
@andybharness
andybharness authored Jul 18, 2023
1 parent 5a5140d commit 14d3516
Show file tree
Hide file tree
Showing 7 changed files with 440 additions and 42 deletions.
4 changes: 2 additions & 2 deletions examples/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>io.harness.featureflags</groupId>
<artifactId>examples</artifactId>
<version>1.2.4</version>
<version>1.2.5</version>

<properties>
<maven.compiler.source>8</maven.compiler.source>
Expand All @@ -33,7 +33,7 @@
<dependency>
<groupId>io.harness</groupId>
<artifactId>ff-java-server-sdk</artifactId>
<version>1.2.4</version>
<version>1.2.5</version>
</dependency>

<dependency>
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>io.harness</groupId>
<artifactId>ff-java-server-sdk</artifactId>
<version>1.2.4</version>
<version>1.2.5</version>
<packaging>jar</packaging>
<name>Harness Feature Flag Java Server SDK</name>
<description>Harness Feature Flag Java Server SDK</description>
Expand Down
97 changes: 66 additions & 31 deletions src/main/java/io/harness/cf/client/connector/HarnessConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class HarnessConnector implements Connector, AutoCloseable {
private final HarnessConfig options;

private String token;
private String environment;
private String environmentUuid;
private String cluster;
private String environmentIdentifier;
private String accountID;
Expand Down Expand Up @@ -196,36 +196,50 @@ protected void processToken(@NonNull final String token) {

Claim claim = gson.fromJson(decoded, Claim.class);
log.debug("Claims successfully parsed from decoded payload");
environment = claim.getEnvironment();
environmentUuid = claim.getEnvironment();
cluster = claim.getClusterIdentifier();
accountID = claim.getAccountID();
environmentIdentifier = claim.getEnvironmentIdentifier();
accountID = emptyToNull(claim.getAccountID());
environmentIdentifier = getEnvOrUuidEnv(claim.getEnvironmentIdentifier(), environmentUuid);

api.getApiClient().addDefaultHeader("Harness-EnvironmentID", environmentIdentifier);
api.getApiClient().addDefaultHeader("Harness-AccountID", accountID);
metricsApi.getApiClient().addDefaultHeader("Harness-EnvironmentID", environmentIdentifier);
metricsApi.getApiClient().addDefaultHeader("Harness-AccountID", accountID);
if (environmentIdentifier != null) {
api.getApiClient().addDefaultHeader("Harness-EnvironmentID", environmentIdentifier);
metricsApi.getApiClient().addDefaultHeader("Harness-EnvironmentID", environmentIdentifier);
}

if (accountID != null) {
api.getApiClient().addDefaultHeader("Harness-AccountID", accountID);
metricsApi.getApiClient().addDefaultHeader("Harness-AccountID", accountID);
}

log.info(
"Token successfully processed, environment {}, cluster {}, account {}, environmentIdentifier {}",
environment,
environmentUuid,
cluster,
accountID,
environmentIdentifier);
}

private String getEnvOrUuidEnv(String env, String envUuid) {
String envToReturn = emptyToNull(env);
return (envToReturn == null) ? emptyToNull(envUuid) : envToReturn;
}

private String emptyToNull(String jsonValue) {
return (jsonValue != null && !jsonValue.trim().isEmpty()) ? jsonValue : null;
}

@Override
public List<FeatureConfig> getFlags() throws ConnectorException {
final String requestId = UUID.randomUUID().toString();
MDC.put(REQUEST_ID_KEY, requestId);
log.info("Fetching flags on env {} and cluster {}", this.environment, this.cluster);
log.info("Fetching flags on env {} and cluster {}", this.environmentUuid, this.cluster);
List<FeatureConfig> featureConfig = new ArrayList<>();
try {
featureConfig = api.getFeatureConfig(environment, cluster);
featureConfig = api.getFeatureConfig(environmentUuid, cluster);
log.info(
"Total configurations fetched: {} on env {} and cluster {}",
featureConfig.size(),
this.environment,
this.environmentUuid,
this.cluster);
if (log.isTraceEnabled()) {
log.trace("Got the following features: " + featureConfig);
Expand All @@ -234,7 +248,7 @@ public List<FeatureConfig> getFlags() throws ConnectorException {
} catch (ApiException e) {
log.error(
"Exception was raised while fetching the flags on env {} and cluster {}",
this.environment,
this.environmentUuid,
this.cluster,
e);
throw new ConnectorException(e.getMessage(), e.getCode(), e.getMessage());
Expand All @@ -248,21 +262,21 @@ public FeatureConfig getFlag(@NonNull final String identifier) throws ConnectorE
final String requestId = UUID.randomUUID().toString();
MDC.put(REQUEST_ID_KEY, requestId);
log.debug(
"Fetch flag {} from env {} and cluster {}", identifier, this.environment, this.cluster);
"Fetch flag {} from env {} and cluster {}", identifier, this.environmentUuid, this.cluster);
try {
FeatureConfig featureConfigByIdentifier =
api.getFeatureConfigByIdentifier(identifier, environment, cluster);
api.getFeatureConfigByIdentifier(identifier, environmentUuid, cluster);
log.debug(
"Flag {} successfully fetched from env {} and cluster {}",
identifier,
this.environment,
this.environmentUuid,
this.cluster);
return featureConfigByIdentifier;
} catch (ApiException e) {
log.error(
"Exception was raised while fetching the flag {} on env {} and cluster {}",
identifier,
this.environment,
this.environmentUuid,
this.cluster,
e);
throw new ConnectorException(e.getMessage(), e.getCode(), e.getMessage());
Expand All @@ -276,20 +290,22 @@ public List<Segment> getSegments() throws ConnectorException {
final String requestId = UUID.randomUUID().toString();
MDC.put(REQUEST_ID_KEY, requestId);
log.debug(
"Fetching target groups on environment {} and cluster {}", this.environment, this.cluster);
"Fetching target groups on environment {} and cluster {}",
this.environmentUuid,
this.cluster);
List<Segment> allSegments = new ArrayList<>();
try {
allSegments = api.getAllSegments(environment, cluster);
allSegments = api.getAllSegments(environmentUuid, cluster);
log.debug(
"Total target groups fetched: {} on env {} and cluster {}",
allSegments.size(),
this.environment,
this.environmentUuid,
this.cluster);
return allSegments;
} catch (ApiException e) {
log.error(
"Exception was raised while fetching the target groups on env {} and cluster {} : httpCode={} message={}",
this.environment,
this.environmentUuid,
this.cluster,
e.getCode(),
e.getMessage(),
Expand All @@ -307,21 +323,22 @@ public Segment getSegment(@NonNull final String identifier) throws ConnectorExce
log.debug(
"Fetching the target group {} on environment {} and cluster {}",
identifier,
this.environment,
this.environmentUuid,
this.cluster);
try {
Segment segmentByIdentifier = api.getSegmentByIdentifier(identifier, environment, cluster);
Segment segmentByIdentifier =
api.getSegmentByIdentifier(identifier, environmentUuid, cluster);
log.debug(
"Segment {} successfully fetched from env {} and cluster {}",
identifier,
this.environment,
this.environmentUuid,
this.cluster);
return segmentByIdentifier;
} catch (ApiException e) {
log.error(
"Exception was raised while fetching the target group {} on env {} and cluster {}",
identifier,
this.environment,
this.environmentUuid,
this.cluster,
e);
throw new ConnectorException(e.getMessage(), e.getCode(), e.getMessage());
Expand All @@ -334,17 +351,18 @@ public Segment getSegment(@NonNull final String identifier) throws ConnectorExce
public void postMetrics(@NonNull final Metrics metrics) throws ConnectorException {
final String requestId = UUID.randomUUID().toString();
MDC.put(REQUEST_ID_KEY, requestId);
log.debug("Uploading metrics on environment {} and cluster {}", this.environment, this.cluster);
log.debug(
"Uploading metrics on environment {} and cluster {}", this.environmentUuid, this.cluster);
try {
metricsApi.postMetrics(environment, cluster, metrics);
metricsApi.postMetrics(environmentUuid, cluster, metrics);
log.debug(
"Metrics uploaded successfully on environment {} and cluster {}",
this.environment,
this.environmentUuid,
this.cluster);
} catch (ApiException e) {
log.error(
"Exception was raised while uploading metrics on env {} and cluster {}",
this.environment,
this.environmentUuid,
this.cluster,
e);
throw new ConnectorException(e.getMessage(), e.getCode(), e.getMessage());
Expand All @@ -366,8 +384,14 @@ public Service stream(@NonNull final Updater updater) throws ConnectorException
map.put("Authorization", "Bearer " + token);
map.put("API-Key", apiKey);
map.put("Harness-SDK-Info", HARNESS_SDK_INFO);
map.put("Harness-EnvironmentID", environmentIdentifier);
map.put("Harness-AccountID", accountID);

if (environmentIdentifier != null) {
map.put("Harness-EnvironmentID", environmentIdentifier);
}

if (accountID != null) {
map.put("Harness-AccountID", accountID);
}

log.info("Initialize new EventSource instance");
eventSource =
Expand Down Expand Up @@ -431,4 +455,15 @@ private static boolean isNullOrEmpty(String string) {
options,
retryBackOffDelay);
}

HarnessConnector(
@NonNull String apiKey,
@NonNull HarnessConfig options,
ClientApi clientApi,
MetricsApi metricsApi) {
this.apiKey = apiKey;
this.options = options;
this.api = clientApi;
this.metricsApi = metricsApi;
}
}
67 changes: 67 additions & 0 deletions src/test/java/io/harness/cf/client/api/CfClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;

class CfClientTest {
Expand Down Expand Up @@ -511,6 +512,72 @@ void shouldRetryThenReAuthenticateWhen403IsReturnedOnGetAllSegments() throws Exc
}
}

@ParameterizedTest
@NullSource()
@ValueSource(strings = {"dummyAccId", "", " ", "\t", "\n", "\r"})
void shouldTestVariousJwtAccountIDs(String nextAccountId) throws Exception {
BaseConfig config =
BaseConfig.builder()
.pollIntervalInSeconds(1)
.analyticsEnabled(false)
.streamEnabled(true)
.debug(false)
.build();

JwtMissingFieldsAuthDispatcher webserverDispatcher =
new JwtMissingFieldsAuthDispatcher("devEnv", nextAccountId);

try (MockWebServer mockSvr = new MockWebServer()) {
mockSvr.setDispatcher(webserverDispatcher);
mockSvr.start();

try (CfClient client =
new CfClient(
makeConnectorWithMinimalRetryBackOff(mockSvr.getHostName(), mockSvr.getPort()),
config)) {

client.waitForInitialization();
webserverDispatcher.waitForAllEndpointsToBeCalled(15);
webserverDispatcher.getErrors().forEach(Throwable::printStackTrace);

assertTrue(webserverDispatcher.getErrors().isEmpty());
}
}
}

@ParameterizedTest
@NullSource()
@ValueSource(strings = {"dummyAccId", "", " ", "\t", "\n", "\r"})
void shouldTestVariousJwtEnvironmentIdentifiers(String nextEnvId) throws Exception {
BaseConfig config =
BaseConfig.builder()
.pollIntervalInSeconds(1)
.analyticsEnabled(false)
.streamEnabled(true)
.debug(false)
.build();

JwtMissingFieldsAuthDispatcher webserverDispatcher =
new JwtMissingFieldsAuthDispatcher(nextEnvId, "dummyAccount");

try (MockWebServer mockSvr = new MockWebServer()) {
mockSvr.setDispatcher(webserverDispatcher);
mockSvr.start();

try (CfClient client =
new CfClient(
makeConnectorWithMinimalRetryBackOff(mockSvr.getHostName(), mockSvr.getPort()),
config)) {

client.waitForInitialization();
webserverDispatcher.waitForAllEndpointsToBeCalled(15);
webserverDispatcher.getErrors().forEach(Throwable::printStackTrace);

assertTrue(webserverDispatcher.getErrors().isEmpty());
}
}
}

static class DummyCache implements Cache {

@Override
Expand Down
36 changes: 30 additions & 6 deletions src/test/java/io/harness/cf/client/api/dispatchers/CannedResponses.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ public static MockResponse makeAuthResponse(int httpCode) {
return makeMockJsonResponse(httpCode, "{\"authToken\": \"" + makeDummyJwtToken() + "\"}");
}

public static MockResponse makeAuthResponse(
int httpCode, String envUuid, String env, String accountId) {
return makeMockJsonResponse(
httpCode, "{\"authToken\": \"" + makeDummyJwtToken(envUuid, env, accountId) + "\"}");
}

public static MockResponse makeMockStreamResponse(int httpCode, Event... events) {

final StringBuilder builder = new StringBuilder();
Expand Down Expand Up @@ -72,17 +78,35 @@ public static CannedResponses.Event makeFlagPatchEvent(String identifier, int ve
}

public static String makeDummyJwtToken() {
return makeDummyJwtToken(
"00000000-0000-0000-0000-000000000000", "Production", "aaaaa_BBBBB-cccccccccc");
}

public static String makeDummyJwtToken(String envUuid, String env, String accountID) {
final String header = "{\"alg\":\"HS256\",\"typ\":\"JWT\"}";
final String payload =
"{\"environment\":\"00000000-0000-0000-0000-000000000000\","
+ "\"environmentIdentifier\":\"Production\","
+ "\"project\":\"00000000-0000-0000-0000-000000000000\","
String payload = "{";

if (envUuid != null) {
payload += "\"environment\":\"" + envUuid + "\",";
}

if (env != null) {
payload += "\"environmentIdentifier\":\"" + env + "\",";
}

if (accountID != null) {
payload += "\"accountID\":\"" + accountID + "\",";
}

payload +=
"\"project\":\"00000000-0000-0000-0000-000000000000\","
+ "\"projectIdentifier\":\"dev\","
+ "\"accountID\":\"aaaaa_BBBBB-cccccccccc\","
+ "\"organization\":\"00000000-0000-0000-0000-000000000000\","
+ "\"organizationIdentifier\":\"default\","
+ "\"clusterIdentifier\":\"1\","
+ "\"key_type\":\"Server\"}";
+ "\"key_type\":\"Server\""
+ "}";

final byte[] hmac256 = new byte[32];
return Base64.getEncoder().encodeToString(header.getBytes(StandardCharsets.UTF_8))
+ "."
Expand Down
Loading

0 comments on commit 14d3516

Please sign in to comment.