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.

Sign up for GitHub

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

Issue #20 added url constructor to JwkProviderBuilder #22

Merged
merged 5 commits into from
Apr 26, 2018
Merged

Issue #20 added url constructor to JwkProviderBuilder #22

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
15121b2
added url constructor to JwkProviderBuilder
darthvalinor Apr 6, 2018
74de690
UrlJwkProvider(String) doesn't fail on invalid domains, removed url f…
darthvalinor Apr 17, 2018
2e7e2fe
JavaDocs improved
darthvalinor Apr 23, 2018
c4a5f97
added test for invalid protocol case; some test cleanup
darthvalinor Apr 23, 2018
d07af0c
expectedException as rule
darthvalinor Apr 23, 2018
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
46 changes: 31 additions & 15 deletions src/main/java/com/auth0/jwk/JwkProviderBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
package com.auth0.jwk;

import java.net.URL;
import java.util.concurrent.TimeUnit;

import static com.auth0.jwk.UrlJwkProvider.urlForDomain;

/**
* JwkProvider builder
*/
@SuppressWarnings("WeakerAccess")
public class JwkProviderBuilder {

private final String url;
private final URL url;
private TimeUnit expiresUnit;
private long expiresIn;
private long cacheSize;
Expand All @@ -17,16 +20,16 @@ public class JwkProviderBuilder {
private boolean rateLimited;

/**
* Creates a new Builder with a URL from where to load the jwks.
* It can be a url lik 'https://samples.auth0.com' or just the Auth0 domain 'samples.auth0.com'.
* Creates a new Builder with the given URL where to load the jwks from.
*
* @param domain from where to load the jwks
* @param url to load the jwks
* @throws IllegalStateException if url is null
*/
public JwkProviderBuilder(String domain) {
if (domain == null) {
throw new IllegalStateException("Cannot build provider without domain");
public JwkProviderBuilder(URL url) {
if (url == null) {
throw new IllegalStateException("Cannot build provider without url to jwks");
}
this.url = normalizeDomain(domain);
this.url = url;
this.cached = true;
this.expiresIn = 10;
this.expiresUnit = TimeUnit.HOURS;
Expand All @@ -35,6 +38,26 @@ public JwkProviderBuilder(String domain) {
this.bucket = new BucketImpl(10, 1, TimeUnit.MINUTES);
}

/**
* Creates a new Builder with a domain where to look for the jwks.
* It can be a url link 'https://samples.auth0.com' or just a domain 'samples.auth0.com'.
Copy link
Contributor

Choose a reason for hiding this comment

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

You should mention that this method appends the default jwks path to the given string domain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

* <br><br> Use {@link #JwkProviderBuilder(URL)} if you need to pass a full URL.
*
* @param domain where jwks is published
* @throws IllegalStateException if domain is null
* @see UrlJwkProvider#UrlJwkProvider(String)
*/
public JwkProviderBuilder(String domain) {
this(buildJwkUrl(domain));
}

private static URL buildJwkUrl(String domain) {
if (domain == null) {
throw new IllegalStateException("Cannot build provider without domain");
}
return urlForDomain(domain);
}

/**
* Toggle the cache of Jwk. By default the provider will use cache.
*
Expand Down Expand Up @@ -101,11 +124,4 @@ public JwkProvider build() {
}
return urlProvider;
}

private String normalizeDomain(String domain) {
if (!domain.startsWith("http")) {
return "https://" + domain;
}
return domain;
}
}
30 changes: 19 additions & 11 deletions src/main/java/com/auth0/jwk/UrlJwkProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;

import java.io.IOException;
Expand All @@ -14,40 +13,49 @@
import java.util.List;
import java.util.Map;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Strings.isNullOrEmpty;

/**
* Jwk provider that loads them from a {@link URL}
*/
@SuppressWarnings("WeakerAccess")
public class UrlJwkProvider implements JwkProvider {

static final String WELL_KNOWN_JWKS_PATH = "/.well-known/jwks.json";
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a comment above

//visible for testing

Copy link
Contributor Author

@darthvalinor darthvalinor Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Used @VisibleForTesting from Guava instead


final URL url;

/**
* Creates a provider that loads from a given URL
* Creates a provider that loads from the given URL
* @param url to load the jwks
*/
public UrlJwkProvider(URL url) {
if (url == null) {
throw new IllegalArgumentException("A non-null url is required");
}
checkArgument(url != null, "A non-null url is required");
this.url = url;
}

/**
* Creates a provider that loads from the given domain's well-known directory
* @param domain domain where to look for the jwks.json file
* Creates a provider that loads from the given domain's well-known directory.
* <br>If the protocol (http or https) is not provided then https is used by default
* (some.domain -> "https://" + some.domain + {@value #WELL_KNOWN_JWKS_PATH}).
Copy link
Contributor

Choose a reason for hiding this comment

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

  • use {DOMAIN} instead of "some.domain". You can also say something like "for example, when -domain- is -acme.com- the jwks url that will be used is -https://acme.com/.well-known/jwks.json"
  • don't mention the constant. Use "/.well-known/jwks.json" instead, since the constant is package private anyway and users won't see it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the JavaDoc fixed and alligned with JwkProviderBuilder(String)

* <br><br> Use {@link #UrlJwkProvider(URL)} if you need to pass a full URL.
* @param domain where jwks is published
*/
public UrlJwkProvider(String domain) {
this(urlForDomain(domain));
}

private static URL urlForDomain(String domain) {
if (Strings.isNullOrEmpty(domain)) {
throw new IllegalArgumentException("A domain is required");
static URL urlForDomain(String domain) {
checkArgument(!isNullOrEmpty(domain), "A domain is required");

if (!domain.startsWith("http")) {
domain = "https://" + domain;
}

try {
final URL url = new URL(domain);
return new URL(url, "/.well-known/jwks.json");
return new URL(url, WELL_KNOWN_JWKS_PATH);
} catch (MalformedURLException e) {
throw new IllegalArgumentException("Invalid jwks uri", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

This line hasn't been tested. Telling by the docs it will throw "if no protocol is specified, or an unknown protocol is found, or spec is null."

  • being "protocol" part of the URL (first param) and
  • being "spec" the well-known constant, which is never null.

So protocol could be any of http, https, ftp, file, and jar. We are adding https if the domain doesn't begin with http, so those 2 protocols are covered. And because we're enforcing any of those 2, this should be a valid value too. I don't see how to trick our logic in a test to make it fail in this line. If you happen to find something, feel free to add it. But don't worry if you can't that's OK anyway and we should catch it here. (which again, should never be called).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

covered with a test with "httptest://..."

}
Expand Down
57 changes: 44 additions & 13 deletions src/test/java/com/auth0/jwk/JwkProviderBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,54 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.net.URL;
import java.util.concurrent.TimeUnit;

import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.notNullValue;
import static com.auth0.jwk.UrlJwkProvider.WELL_KNOWN_JWKS_PATH;
import static org.hamcrest.Matchers.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to import the required ones only

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, safer, fixed

import static org.junit.Assert.assertThat;

public class JwkProviderBuilderTest {

@Rule
public ExpectedException expectedException = ExpectedException.none();

private String domain = "samples.auth0.com";
private String normalizedDomain = "https://" + domain;

@Test
public void shouldCreateForUrl() throws Exception {
URL urlToJwks = new URL(normalizedDomain + WELL_KNOWN_JWKS_PATH);
assertThat(new JwkProviderBuilder(urlToJwks).build(), notNullValue());
}

@Test
public void shouldCreateForDomain() throws Exception {
assertThat(new JwkProviderBuilder("samples.auth0.com").build(), notNullValue());
assertThat(new JwkProviderBuilder(domain).build(), notNullValue());
}

@Test
public void shouldCreateForHttpUrl() throws Exception {
assertThat(new JwkProviderBuilder("https://samples.auth0.com").build(), notNullValue());
public void shouldCreateForNormalizedDomain() throws Exception {
assertThat(new JwkProviderBuilder(normalizedDomain).build(), notNullValue());
}

@Test
public void shouldFailWhenNoUrlIsProvided() throws Exception {
expectedException.expect(IllegalStateException.class);
expectedException.expectMessage("Cannot build provider without url to jwks");
new JwkProviderBuilder((URL) null).build();
}

@Test
public void shouldFailWhenNoDomainIsProvided() throws Exception {
expectedException.expect(IllegalStateException.class);
expectedException.expectMessage("Cannot build provider without domain");
new JwkProviderBuilder(null).build();
new JwkProviderBuilder((String) null).build();
}

@Test
public void shouldCreateCachedProvider() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.rateLimited(false)
.cached(true)
.build();
Expand All @@ -45,7 +62,7 @@ public void shouldCreateCachedProvider() throws Exception {

@Test
public void shouldCreateCachedProviderWithCustomValues() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.rateLimited(false)
.cached(10, 24, TimeUnit.HOURS)
.build();
Expand All @@ -56,7 +73,7 @@ public void shouldCreateCachedProviderWithCustomValues() throws Exception {

@Test
public void shouldCreateRateLimitedProvider() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.cached(false)
.rateLimited(true)
.build();
Expand All @@ -67,7 +84,7 @@ public void shouldCreateRateLimitedProvider() throws Exception {

@Test
public void shouldCreateRateLimitedProviderWithCustomValues() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.cached(false)
.rateLimited(10, 24, TimeUnit.HOURS)
.build();
Expand All @@ -78,7 +95,7 @@ public void shouldCreateRateLimitedProviderWithCustomValues() throws Exception {

@Test
public void shouldCreateCachedAndRateLimitedProvider() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.cached(true)
.rateLimited(true)
.build();
Expand All @@ -91,7 +108,7 @@ public void shouldCreateCachedAndRateLimitedProvider() throws Exception {

@Test
public void shouldCreateCachedAndRateLimitedProviderWithCustomValues() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com")
JwkProvider provider = new JwkProviderBuilder(domain)
.cached(10, 24, TimeUnit.HOURS)
.rateLimited(10, 24, TimeUnit.HOURS)
.build();
Expand All @@ -104,11 +121,25 @@ public void shouldCreateCachedAndRateLimitedProviderWithCustomValues() throws Ex

@Test
public void shouldCreateCachedAndRateLimitedProviderByDefault() throws Exception {
JwkProvider provider = new JwkProviderBuilder("samples.auth0.com").build();
JwkProvider provider = new JwkProviderBuilder(domain).build();
assertThat(provider, notNullValue());
assertThat(provider, instanceOf(GuavaCachedJwkProvider.class));
JwkProvider baseProvider = ((GuavaCachedJwkProvider) provider).getBaseProvider();
assertThat(baseProvider, instanceOf(RateLimitedJwkProvider.class));
assertThat(((RateLimitedJwkProvider) baseProvider).getBaseProvider(), instanceOf(UrlJwkProvider.class));
}

@Test
public void shouldSupportUrlToJwksDomainWithSubPath() throws Exception {
String urlToJwksWithSubPath = normalizedDomain + "/sub/path" + WELL_KNOWN_JWKS_PATH;
URL url = new URL(urlToJwksWithSubPath);
JwkProvider provider = new JwkProviderBuilder(url)
.rateLimited(false)
.cached(false)
.build();
assertThat(provider, notNullValue());
assertThat(provider, instanceOf(UrlJwkProvider.class));
UrlJwkProvider urlJwkProvider = (UrlJwkProvider) provider;
assertThat(urlJwkProvider.url.toString(), equalTo(urlToJwksWithSubPath));
}
}
Loading