Skip to content

Commit

Permalink
[ELY-2034] Update some JSON config options for OpenID Connect to more…
Browse files Browse the repository at this point in the history
… closely match the specification
  • Loading branch information
fjuma committed Sep 10, 2021
1 parent 91a7250 commit 125217c
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ interface ElytronMessages extends BasicLogger {
@Message(id = 23002, value = "Unexpected error sending request to OIDC provider")
OidcException unexpectedErrorSendingRequestToOidcProvider(@Cause Exception cause);

@Message(id = 23003, value = "Either issuer-url or auth-server-url needs to be configured")
IllegalArgumentException issuerUrlOrAuthServerUrlNeedsToBeConfigured();
@Message(id = 23003, value = "Either provider-url or auth-server-url needs to be configured")
IllegalArgumentException providerUrlOrAuthServerUrlNeedsToBeConfigured();

@LogMessage
@Message(id = 23004, value = "Loaded OpenID provider metadata from '%s'")
Expand Down Expand Up @@ -124,14 +124,14 @@ interface ElytronMessages extends BasicLogger {
@Message(id = 23022, value = "Must set 'realm' in config")
RuntimeException keycloakRealmMissing();

@Message(id = 23023, value = "Must set 'resource' in config")
RuntimeException resourceMissing();
@Message(id = 23023, value = "Must set 'resource' or 'client-id'")
RuntimeException resourceOrClientIdMustBeSet();

@Message(id = 23024, value = "For bearer auth, you must set the 'realm-public-key' or one of 'auth-server-url' and 'issuer-url'")
@Message(id = 23024, value = "For bearer auth, you must set the 'realm-public-key' or one of 'auth-server-url' and 'provider-url'")
IllegalArgumentException invalidConfigurationForBearerAuth();

@Message(id = 23025, value = "Must set 'auth-server-url' or 'issuer-url'")
RuntimeException authServerUrlOrIssuerUrlMustBeSet();
@Message(id = 23025, value = "Must set 'auth-server-url' or 'provider-url'")
RuntimeException authServerUrlOrProviderUrlMustBeSet();

@LogMessage(level = WARN)
@Message(id = 23026, value = "Client '%s' does not have a secret configured")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public enum RelativeUrlsUsed {
protected String realm;
protected PublicKeyLocator publicKeyLocator;
protected String authServerBaseUrl;
protected String issuerUrl;
protected String providerUrl;
protected String authUrl;
protected String tokenUrl;
protected String logoutUrl;
Expand All @@ -80,7 +80,8 @@ public enum RelativeUrlsUsed {
protected String jwksUrl;
protected String principalAttribute = "sub";

protected String resourceName;
protected String resource;
protected String clientId;
protected boolean bearerOnly;
protected boolean autodetectBearerOnly;
protected boolean enableBasicAuth;
Expand Down Expand Up @@ -127,11 +128,19 @@ public OidcClientConfiguration() {
}

public boolean isConfigured() {
return getResourceName() != null && getPublicKeyLocator() != null && (isBearerOnly() || (getAuthServerBaseUrl() != null || getIssuerUrl() != null));
return getResourceName() != null && getPublicKeyLocator() != null && (isBearerOnly() || (getAuthServerBaseUrl() != null || getProviderUrl() != null));
}

public String getResourceName() {
return resourceName;
return resource != null ? resource : clientId;
}

public String getResource() {
return resource;
}

public String getClientId() {
return clientId;
}

public String getRealm() {
Expand All @@ -154,16 +163,16 @@ public String getAuthServerBaseUrl() {
return authServerBaseUrl;
}

public void setIssuerUrl(String issuerUrl) {
this.issuerUrl = issuerUrl;
public void setProviderUrl(String providerUrl) {
this.providerUrl = providerUrl;
}


public void setAuthServerBaseUrl(OidcJsonConfiguration config) {
this.authServerBaseUrl = config.getAuthServerUrl();
if (authServerBaseUrl == null) return;
authUrl = null;
issuerUrl = null;
providerUrl = null;
tokenUrl = null;
logoutUrl = null;
accountUrl = null;
Expand Down Expand Up @@ -194,15 +203,15 @@ protected void resolveUrls() {
OidcProviderMetadata config = getOidcProviderMetadata(discoveryUrl);

authUrl = config.getAuthorizationEndpoint();
if (issuerUrl == null) {
issuerUrl = config.getIssuer();
if (providerUrl == null) {
providerUrl = config.getIssuer();
}
tokenUrl = config.getTokenEndpoint();
logoutUrl = config.getLogoutEndpoint();
jwksUrl = config.getJwksUri();
if (authServerBaseUrl != null) {
// keycloak-specific properties
accountUrl = getUrl(issuerUrl, ACCOUNT_PATH);
accountUrl = getUrl(providerUrl, ACCOUNT_PATH);
registerNodeUrl = getUrl(authServerBaseUrl, KEYCLOAK_REALMS_PATH + getRealm(), CLIENTS_MANAGEMENT_REGISTER_NODE_PATH);
unregisterNodeUrl = getUrl(authServerBaseUrl, KEYCLOAK_REALMS_PATH + getRealm(), CLIENTS_MANAGEMENT_UNREGISTER_NODE_PATH);
}
Expand Down Expand Up @@ -230,14 +239,14 @@ protected OidcProviderMetadata getOidcProviderMetadata(String discoveryUrl) thro
}

private String getDiscoveryUrl() {
if (issuerUrl != null) {
if (providerUrl != null) {
// generic OpenID provider configuration found
return getUrl(issuerUrl, DISCOVERY_PATH);
return getUrl(providerUrl, DISCOVERY_PATH);
} else if (authServerBaseUrl != null) {
// keycloak-specific OpenID provider configuration found
return getUrl(authServerBaseUrl, KEYCLOAK_REALMS_PATH + getRealm(), DISCOVERY_PATH);
} else {
throw log.issuerUrlOrAuthServerUrlNeedsToBeConfigured();
throw log.providerUrlOrAuthServerUrlNeedsToBeConfigured();
}
}

Expand All @@ -259,11 +268,11 @@ public RelativeUrlsUsed getRelativeUrls() {
return relativeUrls;
}

public String getIssuerUrl() {
if (issuerUrl == null) {
public String getProviderUrl() {
if (providerUrl == null) {
resolveUrls();
}
return issuerUrl;
return providerUrl;
}

public String getAuthUrl() {
Expand Down Expand Up @@ -301,8 +310,12 @@ public String getJwksUrl() {
return jwksUrl;
}

public void setResourceName(String resourceName) {
this.resourceName = resourceName;
public void setResource(String resource) {
this.resource = resource;
}

public void setClientId(String clientId) {
this.clientId = clientId;
}

public boolean isBearerOnly() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,12 @@ protected OidcClientConfiguration internalBuild(final OidcJsonConfiguration oidc
oidcClientConfiguration.setRealm(oidcJsonConfiguration.getRealm());
}
String resource = oidcJsonConfiguration.getResource();
if (resource == null) throw log.resourceMissing();
oidcClientConfiguration.setResourceName(resource);
String clientId = oidcJsonConfiguration.getClientId();
if (resource == null && clientId == null) {
throw log.resourceOrClientIdMustBeSet();
}
oidcClientConfiguration.setResource(resource);
oidcClientConfiguration.setClientId(clientId);

String realmKeyPem = oidcJsonConfiguration.getRealmKey();
if (realmKeyPem != null) {
Expand Down Expand Up @@ -133,15 +137,15 @@ protected OidcClientConfiguration internalBuild(final OidcJsonConfiguration oidc
oidcClientConfiguration.setVerifyTokenAudience(oidcJsonConfiguration.isVerifyTokenAudience());

if (realmKeyPem == null && oidcJsonConfiguration.isBearerOnly()
&& (oidcJsonConfiguration.getAuthServerUrl() == null || oidcJsonConfiguration.getIssuerUrl() == null)) {
&& (oidcJsonConfiguration.getAuthServerUrl() == null || oidcJsonConfiguration.getProviderUrl() == null)) {
throw log.invalidConfigurationForBearerAuth();
}
if ((oidcJsonConfiguration.getAuthServerUrl() == null && oidcJsonConfiguration.getIssuerUrl() == null) && (!oidcClientConfiguration.isBearerOnly() || realmKeyPem == null)) {
throw log.authServerUrlOrIssuerUrlMustBeSet();
if ((oidcJsonConfiguration.getAuthServerUrl() == null && oidcJsonConfiguration.getProviderUrl() == null) && (!oidcClientConfiguration.isBearerOnly() || realmKeyPem == null)) {
throw log.authServerUrlOrProviderUrlMustBeSet();
}
oidcClientConfiguration.setClient(createHttpClientProducer(oidcJsonConfiguration));
oidcClientConfiguration.setAuthServerBaseUrl(oidcJsonConfiguration);
oidcClientConfiguration.setIssuerUrl(oidcJsonConfiguration.getIssuerUrl());
oidcClientConfiguration.setProviderUrl(oidcJsonConfiguration.getProviderUrl());
if (oidcJsonConfiguration.getTurnOffChangeSessionIdOnLogin() != null) {
oidcClientConfiguration.setTurnOffChangeSessionIdOnLogin(oidcJsonConfiguration.getTurnOffChangeSessionIdOnLogin());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public OidcClientConfiguration resolveDeployment(OidcHttpFacade facade) {
}

if (oidcClientConfig == null) return null;
if (oidcClientConfig.getAuthServerBaseUrl() == null && oidcClientConfig.getIssuerUrl() == null) return oidcClientConfig;
if (oidcClientConfig.getAuthServerBaseUrl() == null && oidcClientConfig.getProviderUrl() == null) return oidcClientConfig;

OidcClientConfiguration resolvedDeployment = resolveUrls(oidcClientConfig, facade);
if (resolvedDeployment.getPublicKeyLocator() == null) {
Expand All @@ -96,8 +96,8 @@ protected OidcClientConfiguration resolveUrls(OidcClientConfiguration deployment
if (oidcClientConfig.getAuthServerBaseUrl() != null) {
delegate.setAuthServerBaseUrl(getAuthServerBaseUrl(facade, this.oidcClientConfig.getAuthServerBaseUrl()));
}
if (oidcClientConfig.getIssuerUrl() != null) {
delegate.setIssuerUrl(oidcClientConfig.getIssuerUrl());
if (oidcClientConfig.getProviderUrl() != null) {
delegate.setProviderUrl(oidcClientConfig.getProviderUrl());
}
return delegate;
}
Expand All @@ -120,8 +120,8 @@ public void setAuthServerBaseUrl(String authServerBaseUrl) {
resolveUrls();
}

public void setIssuerUrl(String issuerUrl) {
this.issuerUrl = issuerUrl;
public void setProviderUrl(String providerUrl) {
this.providerUrl = providerUrl;
resolveUrls();
}

Expand Down Expand Up @@ -160,6 +160,16 @@ public String getJwksUrl() {
return (this.jwksUrl != null) ? this.jwksUrl : delegate.getJwksUrl();
}

@Override
public String getResource() {
return delegate.getResource();
}

@Override
public String getClientId() {
return delegate.getClientId();
}

@Override
public String getResourceName() {
return delegate.getResourceName();
Expand All @@ -186,8 +196,13 @@ public PublicKeyLocator getPublicKeyLocator() {
}

@Override
public void setResourceName(String resourceName) {
delegate.setResourceName(resourceName);
public void setResource(String resourceName) {
delegate.setResource(resourceName);
}

@Override
public void setClientId(String clientId) {
delegate.setClientId(clientId);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ public class OidcJsonConfiguration {
protected String authServerUrl;
@JsonProperty("ssl-required")
protected String sslRequired;
@JsonProperty("issuer-url")
protected String issuerUrl;
@JsonProperty("provider-url")
protected String providerUrl;
@JsonProperty("client-id")
protected String clientId;

/**
* The Proxy url to use for requests to the auth-server, configurable via the adapter config property {@code proxy-url}.
Expand Down Expand Up @@ -347,12 +349,12 @@ public void setAuthServerUrl(String authServerUrl) {
this.authServerUrl = authServerUrl;
}

public String getIssuerUrl() {
return issuerUrl;
public String getProviderUrl() {
return providerUrl;
}

public void setIssuerUrl(String issuerUrl) {
this.issuerUrl = issuerUrl;
public void setProviderUrl(String providerUrl) {
this.providerUrl = providerUrl;
}

public int getConfidentialPort() {
Expand All @@ -371,6 +373,18 @@ public void setResource(String resource) {
this.resource = resource;
}

public String getClientId() {
return clientId;
}

public void setClientId(String clientId) {
this.clientId = clientId;
}

public String getResourceName() {
return resource != null ? resource : clientId;
}

public boolean isUseResourceRoleMappings() {
return useResourceRoleMappings;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public static Builder builder(OidcClientConfiguration clientConfiguration) {
public static class Builder {
private OidcClientConfiguration clientConfiguration;
private String expectedIssuer;
private String clientID;
private String clientId;
private String expectedJwsAlgorithm;
private PublicKeyLocator publicKeyLocator;
private SecretKey clientSecretKey;
Expand All @@ -133,12 +133,12 @@ public static class Builder {
* @throws IllegalArgumentException if a required builder parameter is missing or invalid
*/
public TokenValidator build() throws IllegalArgumentException {
expectedIssuer = clientConfiguration.getIssuerUrl();
expectedIssuer = clientConfiguration.getProviderUrl();
if (expectedIssuer == null || expectedIssuer.length() == 0) {
throw log.noExpectedIssuerGiven();
}
clientID = clientConfiguration.getResourceName();
if (clientID == null || clientID.length() == 0) {
clientId = clientConfiguration.getResourceName();
if (clientId == null || clientId.length() == 0) {
throw log.noClientIDGiven();
}
expectedJwsAlgorithm = clientConfiguration.getJwsSignatureAlgorithm();
Expand All @@ -156,10 +156,10 @@ public TokenValidator build() throws IllegalArgumentException {

jwtConsumerBuilder = new JwtConsumerBuilder()
.setExpectedIssuer(expectedIssuer)
.setExpectedAudience(clientID)
.setExpectedAudience(clientId)
.setJwsAlgorithmConstraints(
new AlgorithmConstraints(AlgorithmConstraints.ConstraintType.PERMIT, expectedJwsAlgorithm))
.registerValidator(new AzpValidator(clientID))
.registerValidator(new AzpValidator(clientId))
.setRequireExpirationTime();

return new TokenValidator(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ public void testSucessfulAuthenticationWithAuthServerUrl() throws Exception {
}

@Test
public void testSucessfulAuthenticationWithIssuerUrl() throws Exception {
performAuthentication(getOidcConfigurationInputStreamWithIssuerUrl(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
public void testSucessfulAuthenticationWithProviderUrl() throws Exception {
performAuthentication(getOidcConfigurationInputStreamWithProviderUrl(), KeycloakConfiguration.ALICE, KeycloakConfiguration.ALICE_PASSWORD,
true, HttpStatus.SC_MOVED_TEMPORARILY, getClientUrl(), CLIENT_PAGE_TEXT);
}

Expand Down Expand Up @@ -274,11 +274,11 @@ private InputStream getOidcConfigurationInputStream(String clientSecret, String
return new ByteArrayInputStream(oidcConfig.getBytes(StandardCharsets.UTF_8));
}

private InputStream getOidcConfigurationInputStreamWithIssuerUrl() {
private InputStream getOidcConfigurationInputStreamWithProviderUrl() {
String oidcConfig = "{\n" +
" \"resource\" : \"" + CLIENT_ID + "\",\n" +
" \"public-client\" : \"false\",\n" +
" \"issuer-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"ssl-required\" : \"EXTERNAL\",\n" +
" \"credentials\" : {\n" +
" \"secret\" : \"" + CLIENT_SECRET + "\"\n" +
Expand All @@ -290,7 +290,7 @@ private InputStream getOidcConfigurationInputStreamWithIssuerUrl() {
private InputStream getOidcConfigurationMissingRequiredOption() {
String oidcConfig = "{\n" +
" \"public-client\" : \"false\",\n" +
" \"issuer-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"provider-url\" : \"" + KEYCLOAK_CONTAINER.getAuthServerUrl() + "/realms/" + TEST_REALM + "\",\n" +
" \"ssl-required\" : \"EXTERNAL\",\n" +
" \"credentials\" : {\n" +
" \"secret\" : \"" + CLIENT_SECRET + "\"\n" +
Expand Down

0 comments on commit 125217c

Please sign in to comment.