Skip to content
Closed
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
15 changes: 12 additions & 3 deletions docs/src/main/asciidoc/security-oidc-expanded-configuration.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ See the <<multi-tenancy>> section for more details.
|Property name |Default |Description

|quarkus.oidc.auth-server-url ||OIDC base URL
|quarkus.oidc.discovery-path |.well-known/openid-configuration|Discovery path
|quarkus.oidc.discovery-enabled |true|Enable discovery
|quarkus.oidc.authorization-path ||Authorization path
|quarkus.oidc.token-path ||Token path
Expand All @@ -88,7 +89,10 @@ See the <<multi-tenancy>> section for more details.
|quarkus.oidc.registration-path ||OIDC client registration path
|====

`quarkus.oidc.auth-server-url` is a key base OIDC URL property. By default, Quarkus OIDC adds a `.well-known/openid-configuration` path segment to this URL and discovers the OIDC provider metadata.
`quarkus.oidc.auth-server-url` is a key base OIDC URL property.

By default, Quarkus OIDC adds a `.well-known/openid-configuration` path segment to this URL and discovers the OIDC provider metadata.
The discovery path can be customized with `quarkus.oidc.discovery-path`, for example, it can be set to `.well-known/oauth-authorization-server`.

You can disable metadata discovery with `quarkus.oidc.discovery-enabled=false` when the provider does not support it (most OAuth2 providers do not), when you would like to optimize start up time by skipping a remote discovery call and configure individual OIDC provider endpoint URLs instead.

Expand Down Expand Up @@ -552,7 +556,8 @@ The common cookie properties impact both the authorization code flow session and
|quarkus.oidc.authentication.cookie-path |'/'| The cookie path
|quarkus.oidc.authentication.cookie-domain || The cookie domain
|quarkus.oidc.authentication.cookie-path-header || The cookie path header
|quarkus.oidc.authentication.cookie-same-site |lax| The cookie SameSite status
|quarkus.oidc.authentication.cookie-same-site |lax| The session cookie SameSite status
|quarkus.oidc.authentication.state-cookie-same-site |lax| The state cookie SameSite status
|quarkus.oidc.authentication.cookie-suffix || The cookie suffix
|quarkus.oidc.authentication.cookie-force-secure |false| The cookie force secure
|====
Expand All @@ -562,7 +567,11 @@ You may also want to restrict it when the specific endpoint paths should only be

`quarkus.oidc.authentication.cookie-path-header` can be used to dynamically set the required cookie path - this property should be used with care and only when it fits your deployment requirements.

`quarkus.oidc.authentication.cookie-same-site` defines a `Same-Site` attribute as `lax` by default, since setting it to `strict` proved to be breaking some deployments. However, setting it to `strict` is RECOMMENDED when it is known to work in your deployment, for example, when the OIDC provider is hosted in the same domain as the application, etc.
`quarkus.oidc.authentication.cookie-same-site` defines a `Same-Site` attribute for a session cookie as `lax` by default, since setting it to `strict` proved to be breaking some deployments. However, setting it to `strict` is RECOMMENDED when it is known to work in your deployment, for example, when the OIDC provider is hosted in the same domain as the application, etc.

Similarly, `quarkus.oidc.authentication.state-cookie-same-site` defines a `Same-Site` attribute for a state cookie as `lax` by default, but setting it to `strict` is RECOMMENDED when it is known to work in your deployment.

Ideally both the session and state cookies have the same `Same-Site` attribute value, however having the same `Same-Site` `strict` value for both cookies also proved to be problematic in some cases.

`quarkus.oidc.authentication.cookie-suffix` can be used to customize the state and session cookie names. For example, by setting `%test.quarkus.oidc.authentication.cookie-suffix=test` you can have the session cookie name qualified with the `_test` suffix in the test profile only.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,10 @@ private static Uni<OidcConfigurationMetadata> discoverRegistrationUri(WebClient
Map<OidcEndpoint.Type, List<OidcResponseFilter>> oidcResponseFilters,
String authServerUrl, io.vertx.mutiny.core.Vertx vertx, OidcClientRegistrationConfig oidcConfig) {
final long connectionDelayInMillisecs = OidcCommonUtils.getConnectionDelayInMillis(oidcConfig);
final String discoveryUri = OidcCommonUtils.getDiscoveryUri(authServerUrl, oidcConfig.discoveryPath());
return OidcCommonUtils
.discoverMetadata(client, oidcRequestFilters, new OidcRequestContextProperties(),
oidcResponseFilters, authServerUrl,
oidcResponseFilters, discoveryUri,
connectionDelayInMillisecs, vertx,
oidcConfig.useBlockingDnsLookup())
.onItem().transform(json -> new OidcConfigurationMetadata(json.getString("registration_endpoint")));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ private String checkClient(HealthCheckResponseBuilder builder, String clientId,
} else if (oidcClientConfig.discoveryEnabled().orElse(true) && oidcClientConfig.authServerUrl().isPresent()) {
try {
String authServerUriString = OidcCommonUtils.getAuthServerUrl(oidcClientConfig);
String discoveryUri = getDiscoveryUri(authServerUriString);
String discoveryUri = getDiscoveryUri(authServerUriString, oidcClientConfig.discoveryPath());
status = checkHealth(oidcClientImpl, discoveryUri).await().indefinitely();
} catch (Exception e) {
status = ERROR_STATUS + ": " + e.getMessage();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,9 @@ private static Uni<OidcConfigurationMetadata> discoverTokenUris(WebClient client
final long connectionDelayInMillisecs = OidcCommonUtils.getConnectionDelayInMillis(oidcConfig);
OidcRequestContextProperties contextProps = new OidcRequestContextProperties(
Map.of(CLIENT_ID_ATTRIBUTE, oidcConfig.id().orElse(DEFAULT_OIDC_CLIENT_ID)));
final String discoveryUrl = OidcCommonUtils.getDiscoveryUri(authServerUrl, oidcConfig.discoveryPath());
return OidcCommonUtils.discoverMetadata(client, oidcRequestFilters, contextProps, oidcResponseFilters,
authServerUrl, connectionDelayInMillisecs, vertx, oidcConfig.useBlockingDnsLookup())
discoveryUrl, connectionDelayInMillisecs, vertx, oidcConfig.useBlockingDnsLookup())
.onItem().transform(json -> new OidcConfigurationMetadata(json.getString("token_endpoint"),
json.getString("revocation_endpoint")));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ final class OidcClientConfigImpl implements OidcClientConfig {
enum ConfigMappingMethods {
ID,
AUTH_SERVER_URL,
DISCOVERY_PATH,
DISCOVERY_ENABLED,
REGISTRATION_PATH,
CONNECTION_DELAY,
Expand Down Expand Up @@ -433,6 +434,12 @@ public Optional<String> authServerUrl() {
return Optional.empty();
}

@Override
public String discoveryPath() {
invocationsRecorder.put(ConfigMappingMethods.DISCOVERY_PATH, true);
return null;
}

@Override
public Optional<Boolean> discoveryEnabled() {
invocationsRecorder.put(ConfigMappingMethods.DISCOVERY_ENABLED, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public OidcCommonConfig() {

protected OidcCommonConfig(io.quarkus.oidc.common.runtime.config.OidcCommonConfig mapping) {
this.authServerUrl = mapping.authServerUrl();
this.discoveryPath = mapping.discoveryPath();
this.discoveryEnabled = mapping.discoveryEnabled();
this.registrationPath = mapping.registrationPath();
this.connectionDelay = mapping.connectionDelay();
Expand Down Expand Up @@ -46,6 +47,14 @@ protected OidcCommonConfig(io.quarkus.oidc.common.runtime.config.OidcCommonConfi
@Deprecated(since = "3.18", forRemoval = true)
public Optional<Boolean> discoveryEnabled = Optional.empty();

/**
* The relative path of the OIDC discovery endpoint.
*
* @deprecated use {@link #discoveryPath()} method instead
*/
@Deprecated(forRemoval = true)
Copy link
Member

Choose a reason for hiding this comment

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

personally, I introduce them as private and thus avoid the deprecation, but that is just FYI, it makes no practical difference. IMO we should cut them soon anyway (Quarkus 4?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @michalvavrik Yeah, sounds like the right time

Copy link
Member Author

Choose a reason for hiding this comment

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

@michalvavrik Ouch, I messed up with the squash and used the wrong base commit, I'll have to create a new commit, reverting squash did not go according to plan

public String discoveryPath;

/**
* The relative path or absolute URL of the OIDC dynamic client registration endpoint.
* Set if {@link #discoveryEnabled} is `false` or a discovered token endpoint path must be customized.
Expand Down Expand Up @@ -138,6 +147,11 @@ public Optional<Boolean> discoveryEnabled() {
return discoveryEnabled;
}

@Override
public String discoveryPath() {
return discoveryPath;
}

@Override
public Optional<String> registrationPath() {
return registrationPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,8 @@ private static boolean isValidOidcClientRedirectRequest(OidcClientRedirectExcept
public static Uni<JsonObject> discoverMetadata(WebClient client,
Map<OidcEndpoint.Type, List<OidcRequestFilter>> requestFilters,
OidcRequestContextProperties contextProperties, Map<OidcEndpoint.Type, List<OidcResponseFilter>> responseFilters,
String authServerUrl,
String discoveryUrl,
long connectionDelayInMillisecs, Vertx vertx, boolean blockingDnsLookup) {
final String discoveryUrl = getDiscoveryUri(authServerUrl);
final OidcRequestContextProperties requestProps = requestFilters.isEmpty() ? null
: getDiscoveryRequestProps(contextProperties, discoveryUrl);

Expand Down Expand Up @@ -695,8 +694,8 @@ public static Buffer getResponseBuffer(OidcRequestContextProperties requestProps
return updatedResponseBody == null ? buffer : updatedResponseBody;
}

public static String getDiscoveryUri(String authServerUrl) {
return authServerUrl + OidcConstants.WELL_KNOWN_CONFIGURATION;
public static String getDiscoveryUri(String authServerUrl, String discoveryPath) {
return authServerUrl + prependSlash(discoveryPath != null ? discoveryPath : OidcConstants.WELL_KNOWN_CONFIGURATION);
Copy link
Member

Choose a reason for hiding this comment

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

If you are going to make another push, you could apply "prependSlash" only on discoveryPath, not the constant. If not, let's keep it as is.

Copy link
Member Author

Choose a reason for hiding this comment

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

@michalvavrik hey, it would not add it if the path already starts with / so I'd probably avoid another push

Copy link
Member

Choose a reason for hiding this comment

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

@michalvavrik hey, it would not add it if the path already starts with / so I'd probably avoid another push

cool, just please check that non-related code, because I think this PR contains changes it shouldn't. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

state cookie

}

private static byte[] getFileContent(Path path) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,33 @@
@ConfigGroup
public interface OidcCommonConfig {
/**
* The base URL of the OpenID Connect (OIDC) server, for example, `https://host:port/auth`.
* Do not set this property if you use 'quarkus-oidc' and the public key verification ({@link #publicKey})
* or certificate chain verification only ({@link #certificateChain}) is required.
* The OIDC discovery endpoint is called by default by appending a `.well-known/openid-configuration` path to this URL.
* The base URL of an OpenID Connect (OIDC) server, for example, `https://host:port/auth`.
* Do not set this property if you use the public key verification ({@link #publicKey})
* or certificate chain verification only ({@link #certificateChain}).
* <p>
* By default, when an OIDC configuration metadata discovery is enabled with the {@link #discoveryEnabled()} property,
* it is retrieved from a well known provider endpoint with its URL calculated by appending a value
* of the {@link #discoveryPath()} path such as `.well-known/openid-configuration` to this URL.
* <p>
* For Keycloak, use `https://host:port/realms/{realm}`, replacing `{realm}` with the Keycloak realm name.
*/
Optional<String> authServerUrl();

/**
* Discovery of the OIDC endpoints.
* Enable discovery of the OIDC endpoints.
* If not enabled, you must configure the OIDC endpoint URLs individually.
*/
@ConfigDocDefault("true")
Optional<Boolean> discoveryEnabled();

/**
* The relative path or absolute URL of the OIDC dynamic client registration endpoint.
* The relative path of the OIDC discovery endpoint.
*/
@WithDefault(".well-known/openid-configuration")
String discoveryPath();

/**
* The relative path of the OIDC dynamic client registration endpoint.
* Set if {@link #discoveryEnabled} is `false` or a discovered token endpoint path must be customized.
*/
Optional<String> registrationPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ private record ProxyImpl(Optional<String> host, int port, Optional<String> usern
protected static class OidcCommonConfigImpl implements OidcCommonConfig {

private final Optional<String> authServerUrl;
private final String discoveryPath;
private final Optional<Boolean> discoveryEnabled;
private final Optional<String> registrationPath;
private final Optional<Duration> connectionDelay;
Expand All @@ -39,6 +40,7 @@ protected static class OidcCommonConfigImpl implements OidcCommonConfig {

protected OidcCommonConfigImpl(OidcCommonConfigBuilder<?> builder) {
this.authServerUrl = builder.authServerUrl;
this.discoveryPath = builder.discoveryPath;
this.discoveryEnabled = builder.discoveryEnabled;
this.registrationPath = builder.registrationPath;
this.connectionDelay = builder.connectionDelay;
Expand All @@ -57,6 +59,11 @@ public Optional<String> authServerUrl() {
return authServerUrl;
}

@Override
public String discoveryPath() {
return discoveryPath;
}

@Override
public Optional<Boolean> discoveryEnabled() {
return discoveryEnabled;
Expand Down Expand Up @@ -109,6 +116,7 @@ public Tls tls() {
}

private Optional<String> authServerUrl;
private String discoveryPath;
private Optional<Boolean> discoveryEnabled;
private Optional<String> registrationPath;
private Optional<Duration> connectionDelay;
Expand All @@ -126,6 +134,7 @@ public Tls tls() {

protected OidcCommonConfigBuilder(OidcCommonConfig oidcCommonConfig) {
this.authServerUrl = oidcCommonConfig.authServerUrl();
this.discoveryPath = oidcCommonConfig.discoveryPath();
this.discoveryEnabled = oidcCommonConfig.discoveryEnabled();
this.registrationPath = oidcCommonConfig.registrationPath();
this.connectionDelay = oidcCommonConfig.connectionDelay();
Expand Down Expand Up @@ -153,6 +162,15 @@ public T authServerUrl(String authServerUrl) {
return getBuilder();
}

/**
* @param discoveryPath {@link OidcCommonConfig#discoveryPath()}
* @return T builder
*/
public T discoveryPath(String discoveryPath) {
this.discoveryPath = discoveryPath;
return getBuilder();
}

/**
* @param discoveryEnabled {@link OidcCommonConfig#discoveryEnabled()}
* @return T builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,13 @@ public io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite co
: io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite.valueOf(cookieSameSite.toString());
}

@Override
public io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite stateCookieSameSite() {
return stateCookieSameSite == null ? null
: io.quarkus.oidc.runtime.OidcTenantConfig.Authentication.CookieSameSite
.valueOf(stateCookieSameSite.toString());
}

@Override
public boolean allowMultipleCodeFlows() {
return allowMultipleCodeFlows;
Expand Down Expand Up @@ -1713,6 +1720,11 @@ public enum ResponseMode {
*/
public CookieSameSite cookieSameSite = CookieSameSite.LAX;

/**
* SameSite attribute for the state cookie.
*/
public CookieSameSite stateCookieSameSite = CookieSameSite.LAX;

/**
* If a state cookie is present, a `state` query parameter must also be present and both the state
* cookie name suffix and state cookie value must match the value of the `state` query parameter when
Expand Down Expand Up @@ -2137,6 +2149,7 @@ private void addConfigMappingValues(io.quarkus.oidc.runtime.OidcTenantConfig.Aut
cookiePathHeader = mapping.cookiePathHeader();
cookieDomain = mapping.cookieDomain();
cookieSameSite = CookieSameSite.valueOf(mapping.cookieSameSite().toString());
stateCookieSameSite = CookieSameSite.valueOf(mapping.stateCookieSameSite().toString());
allowMultipleCodeFlows = mapping.allowMultipleCodeFlows();
failOnMissingStateParam = mapping.failOnMissingStateParam();
failOnUnresolvedKid = mapping.failOnUnresolvedKid();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@
import io.smallrye.mutiny.Uni;
import io.vertx.core.MultiMap;
import io.vertx.core.http.Cookie;
import io.vertx.core.http.CookieSameSite;
import io.vertx.core.http.HttpHeaders;
import io.vertx.core.http.impl.ServerCookie;
import io.vertx.core.json.JsonArray;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.RoutingContext;
Expand Down Expand Up @@ -1299,9 +1301,12 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext
cookieValue += (COOKIE_DELIM + encodeExtraStateValue(extraStateValue, configContext));
}
String stateCookieNameSuffix = configContext.oidcConfig().authentication().allowMultipleCodeFlows() ? "_" + uuid : "";
OidcUtils.createCookie(context, configContext.oidcConfig(),
ServerCookie stateCookie = OidcUtils.createCookie(context, configContext.oidcConfig(),
getStateCookieName(configContext.oidcConfig()) + stateCookieNameSuffix, cookieValue,
configContext.oidcConfig().authentication().stateCookieAge().toSeconds());
stateCookie
.setSameSite(CookieSameSite.valueOf(configContext.oidcConfig().authentication().stateCookieSameSite().name()));

return uuid;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,12 @@ String directive() {
@WithDefault("lax")
CookieSameSite cookieSameSite();

/**
* SameSite attribute for the state cookie.
*/
@WithDefault("lax")
CookieSameSite stateCookieSameSite();

/**
* If a state cookie is present, a `state` query parameter must also be present and both the state
* cookie name suffix and state cookie value must match the value of the `state` query parameter when
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,18 @@ static void removeCookie(RoutingContext context, ServerCookie cookie, OidcTenant
if (auth.cookieDomain().isPresent()) {
cookie.setDomain(auth.cookieDomain().get());
}

CookieSameSite sameSite = null;
if (cookie.getName().startsWith(SESSION_COOKIE_NAME)) {
sameSite = CookieSameSite.valueOf(oidcConfig.authentication().cookieSameSite().name());
} else if (cookie.getName().startsWith(STATE_COOKIE_NAME)) {
sameSite = CookieSameSite.valueOf(oidcConfig.authentication().stateCookieSameSite().name());
}
if (CookieSameSite.NONE == sameSite) {
// browsers may want it if the same site is none
cookie.setSameSite(sameSite);
cookie.setSecure(oidcConfig.authentication().cookieForceSecure() || context.request().isSSL());
}
}
}

Expand Down
Loading
Loading