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

OIDC updates #8387

Merged
merged 6 commits into from
Feb 22, 2024
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
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -61,4 +61,38 @@ public static Config get(Config config, String currentKey, String deprecatedKey)
return currentConfig;
}
}

/**
* Get a value from config, attempting to read both the keys.
* Warning is logged if either the current key is not defined, or both the keys are defined.
*
* @param config configuration instance
* @param currentKey key that should be used
* @param deprecatedKey key that should not be used
*
* @return config node of the current key if exists, of the deprecated key if it does not, missing node otherwise
*/
public static io.helidon.common.config.Config get(io.helidon.common.config.Config config,
String currentKey,
String deprecatedKey) {
io.helidon.common.config.Config deprecatedConfig = config.get(deprecatedKey);
io.helidon.common.config.Config currentConfig = config.get(currentKey);

if (deprecatedConfig.exists()) {
if (currentConfig.exists()) {
LOGGER.log(Level.WARNING, "You are using both a deprecated configuration and a current one. "
+ "Deprecated key: \"" + deprecatedConfig.key() + "\", "
+ "current key: \"" + currentConfig.key() + "\", "
+ "only the current key will be used, and deprecated will be ignored.");
return currentConfig;
} else {
LOGGER.log(Level.WARNING, "You are using a deprecated configuration key. "
+ "Deprecated key: \"" + deprecatedConfig.key() + "\", "
+ "current key: \"" + currentConfig.key() + "\".");
return deprecatedConfig;
}
} else {
return currentConfig;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, 2023 Oracle and/or its affiliates.
* Copyright (c) 2021, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -122,7 +122,8 @@ public AuthenticationResponse map(ProviderRequest authenticatedRequest, Authenti

// create a new response
AuthenticationResponse.Builder builder = AuthenticationResponse.builder()
.requestHeaders(previousResponse.requestHeaders());
.requestHeaders(previousResponse.requestHeaders())
.responseHeaders(previousResponse.responseHeaders());
previousResponse.description().ifPresent(builder::description);

if (maybeUser.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
* Copyright (c) 2023, 2024 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,7 @@
import io.helidon.common.Errors;
import io.helidon.common.config.Config;
import io.helidon.common.configurable.Resource;
import io.helidon.config.DeprecatedConfig;
import io.helidon.config.metadata.Configured;
import io.helidon.config.metadata.ConfiguredOption;
import io.helidon.security.jwt.jwk.JwkKeys;
Expand Down Expand Up @@ -119,7 +120,8 @@ public B config(Config config) {
config.get("sign-jwk.resource").map(Resource::create).ifPresent(this::signJwk);

config.get("introspect-endpoint-uri").as(URI.class).ifPresent(this::introspectEndpointUri);
config.get("validate-with-jwk").asBoolean().ifPresent(this::validateJwtWithJwk);
DeprecatedConfig.get(config, "validate-jwt-with-jwk", "validate-with-jwk")
.asBoolean().ifPresent(this::validateJwtWithJwk);
config.get("issuer").asString().ifPresent(this::issuer);
config.get("audience").asString().ifPresent(this::audience);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@
* <td>URI of an authorization endpoint used to redirect users to for logging-in.</td>
* </tr>
* <tr>
* <td>validate-with-jwk</td>
* <td>validate-jwt-with-jwk</td>
* <td>true</td>
* <td>When true - validate against jwk defined by "sign-jwk", when false
* validate JWT through OIDC Server endpoint "validation-endpoint-uri"</td>
Expand All @@ -225,7 +225,7 @@
* <tr>
* <td>introspect-endpoint-uri</td>
* <td>"introspection_endpoint" in OIDC metadata, or identity-uri/oauth2/v1/introspect</td>
* <td>When validate-with-jwk is set to "false", this is the endpoint used</td>
* <td>When validate-jwt-with-jwk is set to "false", this is the endpoint used</td>
* </tr>
* <tr>
* <td>base-scopes</td>
Expand Down Expand Up @@ -401,6 +401,7 @@ public final class OidcConfig extends TenantConfigImpl {
private final OidcCookieHandler stateCookieHandler;
private final boolean tokenSignatureValidation;
private final boolean idTokenSignatureValidation;
private final boolean accessTokenIpCheck;

private OidcConfig(Builder builder) {
super(builder);
Expand Down Expand Up @@ -433,6 +434,7 @@ private OidcConfig(Builder builder) {
this.stateCookieHandler = builder.stateCookieBuilder.build();
this.tokenSignatureValidation = builder.tokenSignatureValidation;
this.idTokenSignatureValidation = builder.idTokenSignatureValidation;
this.accessTokenIpCheck = builder.accessTokenIpCheck;

this.webClientBuilderSupplier = builder.webClientBuilderSupplier;
this.defaultTenant = LazyValue.create(() -> Tenant.create(this, this));
Expand Down Expand Up @@ -816,6 +818,15 @@ public boolean idTokenSignatureValidation() {
return idTokenSignatureValidation;
}

/**
* Whether to check IP address access token was issued for.
*
* @return whether to check IP address access token was issued for
*/
public boolean accessTokenIpCheck() {
return accessTokenIpCheck;
}

Supplier<WebClientConfig.Builder> webClientBuilderSupplier() {
return webClientBuilderSupplier;
}
Expand Down Expand Up @@ -946,6 +957,7 @@ public static class Builder extends BaseBuilder<Builder, OidcConfig> {
private boolean relativeUris = DEFAULT_RELATIVE_URIS;
private boolean tokenSignatureValidation = true;
private boolean idTokenSignatureValidation = true;
private boolean accessTokenIpCheck = true;

protected Builder() {
}
Expand Down Expand Up @@ -1070,6 +1082,7 @@ public Builder config(Config config) {

config.get("token-signature-validation").asBoolean().ifPresent(this::tokenSignatureValidation);
config.get("id-token-signature-validation").asBoolean().ifPresent(this::idTokenSignatureValidation);
config.get("access-token-ip-check").asBoolean().ifPresent(this::accessTokenIpCheck);

config.get("tenants").asList(Config.class)
.ifPresent(confList -> confList.forEach(tenantConfig -> tenantFromConfig(config, tenantConfig)));
Expand Down Expand Up @@ -1720,5 +1733,18 @@ public Builder idTokenSignatureValidation(boolean enabled) {
return this;
}

/**
* Whether to check if current IP address matches the one access token was issued for.
* This check helps with cookie replay attack prevention.
*
* @param enabled whether to check if current IP address matches the one access token was issued for
* @return updated builder instance
*/
@ConfiguredOption("true")
public Builder accessTokenIpCheck(boolean enabled) {
accessTokenIpCheck = enabled;
return this;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import io.helidon.webserver.security.SecurityHttpFeature;

import jakarta.json.Json;
import jakarta.json.JsonBuilderFactory;
import jakarta.json.JsonObject;
import jakarta.json.JsonReaderFactory;

Expand Down Expand Up @@ -147,11 +148,12 @@
*/
@Weight(800)
public final class OidcFeature implements HttpFeature {
static final JsonReaderFactory JSON_READER_FACTORY = Json.createReaderFactory(Map.of());
static final JsonBuilderFactory JSON_BUILDER_FACTORY = Json.createBuilderFactory(Map.of());
private static final System.Logger LOGGER = System.getLogger(OidcFeature.class.getName());
private static final String CODE_PARAM_NAME = "code";
private static final String STATE_PARAM_NAME = "state";
private static final String DEFAULT_REDIRECT = "/index.html";
private static final JsonReaderFactory JSON = Json.createReaderFactory(Map.of());

private final List<TenantConfigFinder> oidcConfigFinders;
private final LruCache<String, Tenant> tenants = LruCache.create();
Expand Down Expand Up @@ -391,7 +393,7 @@ private void processCodeWithTenant(String code, ServerRequest req, ServerRespons
return;
}
String stateBase64 = new String(Base64.getDecoder().decode(maybeStateCookie.get()), StandardCharsets.UTF_8);
JsonObject stateCookie = JSON.createReader(new StringReader(stateBase64)).readObject();
JsonObject stateCookie = JSON_READER_FACTORY.createReader(new StringReader(stateBase64)).readObject();
//Remove state cookie
res.headers().addCookie(stateCookieHandler.removeCookie().build());
String state = stateCookie.getString("state");
Expand Down Expand Up @@ -422,7 +424,7 @@ private void processCodeWithTenant(String code, ServerRequest req, ServerRespons
if (response.status().family() == Status.Family.SUCCESSFUL) {
try {
JsonObject jsonObject = response.as(JsonObject.class);
processJsonResponse(res, jsonObject, tenantName, stateCookie);
processJsonResponse(req, res, jsonObject, tenantName, stateCookie);
} catch (Exception e) {
processError(res, e, "Failed to read JSON from response");
}
Expand Down Expand Up @@ -478,20 +480,21 @@ private String redirectUri(ServerRequest req, String tenantName) {
return uri;
}

private String processJsonResponse(ServerResponse res,
private String processJsonResponse(ServerRequest req,
ServerResponse res,
JsonObject json,
String tenantName,
JsonObject stateCookie) {
String accessToken = json.getString("access_token");
String idToken = json.getString("id_token", null);
String refreshToken = json.getString("refresh_token", null);

Jwt accessTokenJwt = SignedJwt.parseToken(accessToken).getJwt();
Jwt idTokenJwt = SignedJwt.parseToken(idToken).getJwt();
String nonceOriginal = stateCookie.getString("nonce");
String nonceAccess = accessTokenJwt.nonce()
.orElseThrow(() -> new IllegalStateException("Nonce is required to be present in the access token"));
String nonceAccess = idTokenJwt.nonce()
.orElseThrow(() -> new IllegalStateException("Nonce is required to be present in the id token"));
if (!nonceAccess.equals(nonceOriginal)) {
throw new IllegalStateException("Original nonce and the one obtained from access token does not match");
throw new IllegalStateException("Original nonce and the one obtained from id token does not match");
}

//redirect to "originalUri"
Expand All @@ -512,12 +515,19 @@ private String processJsonResponse(ServerResponse res,

if (oidcConfig.useCookie()) {
try {
JsonObject accessTokenJson = JSON_BUILDER_FACTORY.createObjectBuilder()
.add("accessToken", accessToken)
.add("remotePeer", req.remotePeer().host())
.build();
String encodedAccessToken = Base64.getEncoder()
.encodeToString(accessTokenJson.toString().getBytes(StandardCharsets.UTF_8));

ServerResponseHeaders headers = res.headers();

OidcCookieHandler tenantCookieHandler = oidcConfig.tenantCookieHandler();

headers.addCookie(tenantCookieHandler.createCookie(tenantName).build()); //Add tenant name cookie
headers.addCookie(tokenCookieHandler.createCookie(accessToken).build()); //Add token cookie
headers.addCookie(tokenCookieHandler.createCookie(encodedAccessToken).build()); //Add token cookie
if (refreshToken != null) {
headers.addCookie(refreshTokenCookieHandler.createCookie(refreshToken).build()); //Add refresh token cookie
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.helidon.security.providers.oidc;

import java.io.StringReader;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
Expand Down Expand Up @@ -68,10 +69,10 @@
import io.helidon.webclient.api.HttpClientResponse;
import io.helidon.webclient.api.WebClient;

import jakarta.json.Json;
import jakarta.json.JsonBuilderFactory;
import jakarta.json.JsonObject;

import static io.helidon.security.providers.oidc.OidcFeature.JSON_BUILDER_FACTORY;
import static io.helidon.security.providers.oidc.OidcFeature.JSON_READER_FACTORY;
import static io.helidon.security.providers.oidc.common.spi.TenantConfigFinder.DEFAULT_TENANT_ID;

/**
Expand All @@ -81,7 +82,6 @@ class TenantAuthenticationHandler {
private static final System.Logger LOGGER = System.getLogger(TenantAuthenticationHandler.class.getName());
private static final TokenHandler PARAM_HEADER_HANDLER = TokenHandler.forHeader(OidcConfig.PARAM_HEADER_NAME);
private static final TokenHandler PARAM_ID_HEADER_HANDLER = TokenHandler.forHeader(OidcConfig.PARAM_ID_HEADER_NAME);
private static final JsonBuilderFactory JSON = Json.createBuilderFactory(Map.of());
private static final SecureRandom RANDOM = new SecureRandom();

private final boolean optional;
Expand Down Expand Up @@ -264,7 +264,23 @@ private AuthenticationResponse processAccessToken(String tenantId, ProviderReque
} else {
try {
String tokenValue = cookie.get();
return validateAccessToken(tenantId, providerRequest, tokenValue, idToken);
String decodedJson = new String(Base64.getDecoder().decode(tokenValue), StandardCharsets.UTF_8);
JsonObject jsonObject = JSON_READER_FACTORY.createReader(new StringReader(decodedJson)).readObject();
if (oidcConfig.accessTokenIpCheck()) {
Object userIp = providerRequest.env().abacAttribute("userIp").orElseThrow();
if (!jsonObject.getString("remotePeer").equals(userIp)) {
if (LOGGER.isLoggable(System.Logger.Level.DEBUG)) {
LOGGER.log(System.Logger.Level.DEBUG,
"Current peer IP does not match the one this access token was issued for");
}
return errorResponse(providerRequest,
Status.UNAUTHORIZED_401,
"peer_host_mismatch",
"Peer host access token mismatch",
tenantId);
}
}
return validateAccessToken(tenantId, providerRequest, jsonObject.getString("accessToken"), idToken);
} catch (Exception e) {
if (LOGGER.isLoggable(System.Logger.Level.DEBUG)) {
LOGGER.log(System.Logger.Level.DEBUG, "Invalid access token in cookie", e);
Expand Down Expand Up @@ -372,7 +388,7 @@ private AuthenticationResponse errorResponse(ProviderRequest providerRequest,
+ "nonce=" + nonce + "&"
+ "state=" + state;

JsonObject stateJson = JSON.createObjectBuilder()
JsonObject stateJson = JSON_BUILDER_FACTORY.createObjectBuilder()
.add("originalUri", origUri)
.add("state", state)
.add("nonce", nonce)
Expand Down Expand Up @@ -575,8 +591,7 @@ private AuthenticationResponse refreshAccessToken(ProviderRequest providerReques
Parameters.Builder form = Parameters.builder("oidc-form-params")
.add("grant_type", "refresh_token")
.add("refresh_token", refreshTokenString)
.add("client_id", tenantConfig.clientId())
.add("client_secret", tenantConfig.clientSecret());
.add("client_id", tenantConfig.clientId());

HttpClientRequest post = webClient.post()
.uri(tenant.tokenEndpointUri())
Expand All @@ -600,10 +615,18 @@ private AuthenticationResponse refreshAccessToken(ProviderRequest providerReques
return AuthenticationResponse.failed("Invalid access token", e);
}
Errors.Collector newAccessTokenCollector = jwtValidator.apply(signedAccessToken, Errors.collector());
Object remotePeer = providerRequest.env().abacAttribute("userIp").orElseThrow();

JsonObject accessTokenCookie = JSON_BUILDER_FACTORY.createObjectBuilder()
.add("accessToken", signedAccessToken.tokenContent())
.add("remotePeer", remotePeer.toString())
.build();
String base64 = Base64.getEncoder()
.encodeToString(accessTokenCookie.toString().getBytes(StandardCharsets.UTF_8));

List<String> setCookieParts = new ArrayList<>();
setCookieParts.add(oidcConfig.tokenCookieHandler()
.createCookie(accessToken)
.createCookie(base64)
.build()
.toString());
if (refreshToken != null) {
Expand Down
Loading
Loading