Skip to content
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

feat: add support for authenticated datafile access #378

Merged
merged 7 commits into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,29 @@ public static Optimizely newDefaultInstance(String sdkKey) {
* @param fallback Fallback datafile string used by the ProjectConfigManager to be immediately available.
*/
public static Optimizely newDefaultInstance(String sdkKey, String fallback) {
String datafileAccessToken = PropertyUtils.get(HttpProjectConfigManager.CONFIG_DATAFILE_AUTH_TOKEN);
return newDefaultInstance(sdkKey, fallback, datafileAccessToken);
}

/**
* Returns a new Optimizely instance with authenticated datafile support.
*
* @param sdkKey SDK key used to build the ProjectConfigManager.
* @param fallback Fallback datafile string used by the ProjectConfigManager to be immediately available.
* @param datafileAccessToken Token for authenticated datafile access.
*/
public static Optimizely newDefaultInstance(String sdkKey, String fallback, String datafileAccessToken) {
NotificationCenter notificationCenter = new NotificationCenter();

HttpProjectConfigManager.Builder builder = HttpProjectConfigManager.builder()
.withDatafile(fallback)
.withNotificationCenter(notificationCenter)
.withSdkKey(sdkKey);

if (datafileAccessToken != null) {
builder.withDatafileAccessToken(datafileAccessToken);
}

return newDefaultInstance(builder.build(), notificationCenter);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.optimizely.ab.HttpClientUtils;
import com.optimizely.ab.OptimizelyHttpClient;
import com.optimizely.ab.annotations.VisibleForTesting;
import com.optimizely.ab.config.parser.ConfigParseException;
import com.optimizely.ab.internal.PropertyUtils;
import com.optimizely.ab.notification.NotificationCenter;
Expand All @@ -44,6 +45,7 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager {
public static final String CONFIG_BLOCKING_DURATION = "http.project.config.manager.blocking.duration";
public static final String CONFIG_BLOCKING_UNIT = "http.project.config.manager.blocking.unit";
public static final String CONFIG_SDK_KEY = "http.project.config.manager.sdk.key";
public static final String CONFIG_DATAFILE_AUTH_TOKEN = "http.project.config.manager.datafile.auth.token";

public static final long DEFAULT_POLLING_DURATION = 5;
public static final TimeUnit DEFAULT_POLLING_UNIT = TimeUnit.MINUTES;
Expand All @@ -54,12 +56,21 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager {

private final OptimizelyHttpClient httpClient;
private final URI uri;
private final String datafileAccessToken;
private String datafileLastModified;

private HttpProjectConfigManager(long period, TimeUnit timeUnit, OptimizelyHttpClient httpClient, String url, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit, NotificationCenter notificationCenter) {
private HttpProjectConfigManager(long period,
TimeUnit timeUnit,
OptimizelyHttpClient httpClient,
String url,
String datafileAccessToken,
long blockingTimeoutPeriod,
TimeUnit blockingTimeoutUnit,
NotificationCenter notificationCenter) {
super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit, notificationCenter);
this.httpClient = httpClient;
this.uri = URI.create(url);
this.datafileAccessToken = datafileAccessToken;
}

public URI getUri() {
Expand Down Expand Up @@ -104,11 +115,7 @@ static ProjectConfig parseProjectConfig(String datafile) throws ConfigParseExcep

@Override
protected ProjectConfig poll() {
HttpGet httpGet = new HttpGet(uri);

if (datafileLastModified != null) {
httpGet.setHeader(HttpHeaders.IF_MODIFIED_SINCE, datafileLastModified);
}
HttpGet httpGet = createHttpRequest();

logger.debug("Fetching datafile from: {}", httpGet.getURI());
try {
Expand All @@ -125,14 +132,31 @@ protected ProjectConfig poll() {
return null;
}

@VisibleForTesting
HttpGet createHttpRequest() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should createHttpRequest be private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Private must be right one. This default access will be useful for header testing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then let's use the VisibleForTesting annotation.

HttpGet httpGet = new HttpGet(uri);

if (datafileAccessToken != null) {
httpGet.setHeader(HttpHeaders.AUTHORIZATION, "Bearer " + datafileAccessToken);
}

if (datafileLastModified != null) {
httpGet.setHeader(HttpHeaders.IF_MODIFIED_SINCE, datafileLastModified);
}

return httpGet;
}

public static Builder builder() {
return new Builder();
}

public static class Builder {
private String datafile;
private String url;
private String datafileAccessToken = null;
private String format = "https://cdn.optimizely.com/datafiles/%s.json";
private String authFormat = "https://config.optimizely.com/datafiles/auth/%s.json";
private OptimizelyHttpClient httpClient;
private NotificationCenter notificationCenter;

Expand All @@ -153,6 +177,11 @@ public Builder withSdkKey(String sdkKey) {
return this;
}

public Builder withDatafileAccessToken(String token) {
this.datafileAccessToken = token;
return this;
}

public Builder withUrl(String url) {
this.url = url;
return this;
Expand Down Expand Up @@ -261,14 +290,26 @@ public HttpProjectConfigManager build(boolean defer) {
throw new NullPointerException("sdkKey cannot be null");
}

url = String.format(format, sdkKey);
if (datafileAccessToken == null) {
url = String.format(format, sdkKey);
} else {
url = String.format(authFormat, sdkKey);
}
}

if (notificationCenter == null) {
notificationCenter = new NotificationCenter();
}

HttpProjectConfigManager httpProjectManager = new HttpProjectConfigManager(period, timeUnit, httpClient, url, blockingTimeoutPeriod, blockingTimeoutUnit, notificationCenter);
HttpProjectConfigManager httpProjectManager = new HttpProjectConfigManager(
period,
timeUnit,
httpClient,
url,
datafileAccessToken,
blockingTimeoutPeriod,
blockingTimeoutUnit,
notificationCenter);

if (datafile != null) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,13 @@ public void newDefaultInstanceWithFallback() throws Exception {
assertTrue(optimizely.isValid());
}

@Test
public void newDefaultInstanceWithDatafileAccessToken() throws Exception {
String datafileString = Resources.toString(Resources.getResource("valid-project-config-v4.json"), Charsets.UTF_8);
optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token");
assertTrue(optimizely.isValid());
}

@Test
public void newDefaultInstanceWithProjectConfig() throws Exception {
optimizely = OptimizelyFactory.newDefaultInstance(() -> null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.junit.Assert.*;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.*;

@RunWith(MockitoJUnitRunner.class)
public class HttpProjectConfigManagerTest {
Expand Down Expand Up @@ -125,6 +123,58 @@ public void testHttpGetByCustomUrl() throws Exception {
assertEquals(new URI(expected), actual);
}

@Test
public void testHttpGetBySdkKeyForAuthDatafile() throws Exception {
projectConfigManager = builder()
.withOptimizelyHttpClient(mockHttpClient)
.withSdkKey("sdk-key")
.withDatafileAccessToken("auth-token")
.build();

URI actual = projectConfigManager.getUri();
assertEquals(new URI("https://config.optimizely.com/datafiles/auth/sdk-key.json"), actual);
}

@Test
public void testHttpGetByCustomUrlForAuthDatafile() throws Exception {
String expected = "https://custom.optimizely.com/custom-location.json";

projectConfigManager = builder()
.withOptimizelyHttpClient(mockHttpClient)
.withUrl(expected)
.withSdkKey("sdk-key")
.withDatafileAccessToken("auth-token")
.build();

URI actual = projectConfigManager.getUri();
assertEquals(new URI(expected), actual);
}

@Test
public void testCreateHttpRequest() throws Exception {
projectConfigManager = builder()
.withOptimizelyHttpClient(mockHttpClient)
.withSdkKey("sdk-key")
.build();

HttpGet request = projectConfigManager.createHttpRequest();
assertEquals(request.getURI().toString(), "https://cdn.optimizely.com/datafiles/sdk-key.json");
assertEquals(request.getHeaders("Authorization").length, 0);
}

@Test
public void testCreateHttpRequestForAuthDatafile() throws Exception {
projectConfigManager = builder()
.withOptimizelyHttpClient(mockHttpClient)
.withSdkKey("sdk-key")
.withDatafileAccessToken("auth-token")
.build();

HttpGet request = projectConfigManager.createHttpRequest();
assertEquals(request.getURI().toString(), "https://config.optimizely.com/datafiles/auth/sdk-key.json");
assertEquals(request.getHeaders("Authorization")[0].getValue(), "Bearer auth-token");
}

@Test
public void testPoll() throws Exception {
projectConfigManager = builder()
Expand Down