Skip to content

Commit

Permalink
OIDC updates (helidon-io#8387)
Browse files Browse the repository at this point in the history
OIDC updates

Signed-off-by: David Kral <david.k.kral@oracle.com>
  • Loading branch information
Verdent authored and hrstoyanov committed Feb 23, 2024
1 parent 0532bc0 commit f6a74b4
Show file tree
Hide file tree
Showing 10 changed files with 262 additions and 76 deletions.
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

0 comments on commit f6a74b4

Please sign in to comment.