Skip to content

Commit 13c7a3b

Browse files
VerdentVerdent
authored
OIDC updates (helidon-io#8387)
OIDC updates Signed-off-by: David Kral <david.k.kral@oracle.com>
1 parent fbd5a2f commit 13c7a3b

10 files changed

Lines changed: 262 additions & 76 deletions

File tree

‎config/config/src/main/java/io/helidon/config/DeprecatedConfig.java‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2022 Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2024 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -61,4 +61,38 @@ public static Config get(Config config, String currentKey, String deprecatedKey)
6161
return currentConfig;
6262
}
6363
}
64+
65+
/**
66+
* Get a value from config, attempting to read both the keys.
67+
* Warning is logged if either the current key is not defined, or both the keys are defined.
68+
*
69+
* @param config configuration instance
70+
* @param currentKey key that should be used
71+
* @param deprecatedKey key that should not be used
72+
*
73+
* @return config node of the current key if exists, of the deprecated key if it does not, missing node otherwise
74+
*/
75+
public static io.helidon.common.config.Config get(io.helidon.common.config.Config config,
76+
String currentKey,
77+
String deprecatedKey) {
78+
io.helidon.common.config.Config deprecatedConfig = config.get(deprecatedKey);
79+
io.helidon.common.config.Config currentConfig = config.get(currentKey);
80+
81+
if (deprecatedConfig.exists()) {
82+
if (currentConfig.exists()) {
83+
LOGGER.log(Level.WARNING, "You are using both a deprecated configuration and a current one. "
84+
+ "Deprecated key: \"" + deprecatedConfig.key() + "\", "
85+
+ "current key: \"" + currentConfig.key() + "\", "
86+
+ "only the current key will be used, and deprecated will be ignored.");
87+
return currentConfig;
88+
} else {
89+
LOGGER.log(Level.WARNING, "You are using a deprecated configuration key. "
90+
+ "Deprecated key: \"" + deprecatedConfig.key() + "\", "
91+
+ "current key: \"" + currentConfig.key() + "\".");
92+
return deprecatedConfig;
93+
}
94+
} else {
95+
return currentConfig;
96+
}
97+
}
6498
}

‎security/providers/idcs-mapper/src/main/java/io/helidon/security/providers/idcs/mapper/IdcsRoleMapperProviderBase.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2023 Oracle and/or its affiliates.
2+
* Copyright (c) 2021, 2024 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -122,7 +122,8 @@ public AuthenticationResponse map(ProviderRequest authenticatedRequest, Authenti
122122

123123
// create a new response
124124
AuthenticationResponse.Builder builder = AuthenticationResponse.builder()
125-
.requestHeaders(previousResponse.requestHeaders());
125+
.requestHeaders(previousResponse.requestHeaders())
126+
.responseHeaders(previousResponse.responseHeaders());
126127
previousResponse.description().ifPresent(builder::description);
127128

128129
if (maybeUser.isPresent()) {

‎security/providers/oidc-common/src/main/java/io/helidon/security/providers/oidc/common/BaseBuilder.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2023 Oracle and/or its affiliates.
2+
* Copyright (c) 2023, 2024 Oracle and/or its affiliates.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
import io.helidon.common.Errors;
2525
import io.helidon.common.config.Config;
2626
import io.helidon.common.configurable.Resource;
27+
import io.helidon.config.DeprecatedConfig;
2728
import io.helidon.config.metadata.Configured;
2829
import io.helidon.config.metadata.ConfiguredOption;
2930
import io.helidon.security.jwt.jwk.JwkKeys;
@@ -119,7 +120,8 @@ public B config(Config config) {
119120
config.get("sign-jwk.resource").map(Resource::create).ifPresent(this::signJwk);
120121

121122
config.get("introspect-endpoint-uri").as(URI.class).ifPresent(this::introspectEndpointUri);
122-
config.get("validate-with-jwk").asBoolean().ifPresent(this::validateJwtWithJwk);
123+
DeprecatedConfig.get(config, "validate-jwt-with-jwk", "validate-with-jwk")
124+
.asBoolean().ifPresent(this::validateJwtWithJwk);
123125
config.get("issuer").asString().ifPresent(this::issuer);
124126
config.get("audience").asString().ifPresent(this::audience);
125127

‎security/providers/oidc-common/src/main/java/io/helidon/security/providers/oidc/common/OidcConfig.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@
210210
* <td>URI of an authorization endpoint used to redirect users to for logging-in.</td>
211211
* </tr>
212212
* <tr>
213-
* <td>validate-with-jwk</td>
213+
* <td>validate-jwt-with-jwk</td>
214214
* <td>true</td>
215215
* <td>When true - validate against jwk defined by "sign-jwk", when false
216216
* validate JWT through OIDC Server endpoint "validation-endpoint-uri"</td>
@@ -225,7 +225,7 @@
225225
* <tr>
226226
* <td>introspect-endpoint-uri</td>
227227
* <td>"introspection_endpoint" in OIDC metadata, or identity-uri/oauth2/v1/introspect</td>
228-
* <td>When validate-with-jwk is set to "false", this is the endpoint used</td>
228+
* <td>When validate-jwt-with-jwk is set to "false", this is the endpoint used</td>
229229
* </tr>
230230
* <tr>
231231
* <td>base-scopes</td>
@@ -401,6 +401,7 @@ public final class OidcConfig extends TenantConfigImpl {
401401
private final OidcCookieHandler stateCookieHandler;
402402
private final boolean tokenSignatureValidation;
403403
private final boolean idTokenSignatureValidation;
404+
private final boolean accessTokenIpCheck;
404405

405406
private OidcConfig(Builder builder) {
406407
super(builder);
@@ -433,6 +434,7 @@ private OidcConfig(Builder builder) {
433434
this.stateCookieHandler = builder.stateCookieBuilder.build();
434435
this.tokenSignatureValidation = builder.tokenSignatureValidation;
435436
this.idTokenSignatureValidation = builder.idTokenSignatureValidation;
437+
this.accessTokenIpCheck = builder.accessTokenIpCheck;
436438

437439
this.webClientBuilderSupplier = builder.webClientBuilderSupplier;
438440
this.defaultTenant = LazyValue.create(() -> Tenant.create(this, this));
@@ -816,6 +818,15 @@ public boolean idTokenSignatureValidation() {
816818
return idTokenSignatureValidation;
817819
}
818820

821+
/**
822+
* Whether to check IP address access token was issued for.
823+
*
824+
* @return whether to check IP address access token was issued for
825+
*/
826+
public boolean accessTokenIpCheck() {
827+
return accessTokenIpCheck;
828+
}
829+
819830
Supplier<WebClientConfig.Builder> webClientBuilderSupplier() {
820831
return webClientBuilderSupplier;
821832
}
@@ -946,6 +957,7 @@ public static class Builder extends BaseBuilder<Builder, OidcConfig> {
946957
private boolean relativeUris = DEFAULT_RELATIVE_URIS;
947958
private boolean tokenSignatureValidation = true;
948959
private boolean idTokenSignatureValidation = true;
960+
private boolean accessTokenIpCheck = true;
949961

950962
protected Builder() {
951963
}
@@ -1070,6 +1082,7 @@ public Builder config(Config config) {
10701082

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

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

1736+
/**
1737+
* Whether to check if current IP address matches the one access token was issued for.
1738+
* This check helps with cookie replay attack prevention.
1739+
*
1740+
* @param enabled whether to check if current IP address matches the one access token was issued for
1741+
* @return updated builder instance
1742+
*/
1743+
@ConfiguredOption("true")
1744+
public Builder accessTokenIpCheck(boolean enabled) {
1745+
accessTokenIpCheck = enabled;
1746+
return this;
1747+
}
1748+
17231749
}
17241750
}

‎security/providers/oidc/src/main/java/io/helidon/security/providers/oidc/OidcFeature.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import io.helidon.webserver.security.SecurityHttpFeature;
6868

6969
import jakarta.json.Json;
70+
import jakarta.json.JsonBuilderFactory;
7071
import jakarta.json.JsonObject;
7172
import jakarta.json.JsonReaderFactory;
7273

@@ -147,11 +148,12 @@
147148
*/
148149
@Weight(800)
149150
public final class OidcFeature implements HttpFeature {
151+
static final JsonReaderFactory JSON_READER_FACTORY = Json.createReaderFactory(Map.of());
152+
static final JsonBuilderFactory JSON_BUILDER_FACTORY = Json.createBuilderFactory(Map.of());
150153
private static final System.Logger LOGGER = System.getLogger(OidcFeature.class.getName());
151154
private static final String CODE_PARAM_NAME = "code";
152155
private static final String STATE_PARAM_NAME = "state";
153156
private static final String DEFAULT_REDIRECT = "/index.html";
154-
private static final JsonReaderFactory JSON = Json.createReaderFactory(Map.of());
155157

156158
private final List<TenantConfigFinder> oidcConfigFinders;
157159
private final LruCache<String, Tenant> tenants = LruCache.create();
@@ -391,7 +393,7 @@ private void processCodeWithTenant(String code, ServerRequest req, ServerRespons
391393
return;
392394
}
393395
String stateBase64 = new String(Base64.getDecoder().decode(maybeStateCookie.get()), StandardCharsets.UTF_8);
394-
JsonObject stateCookie = JSON.createReader(new StringReader(stateBase64)).readObject();
396+
JsonObject stateCookie = JSON_READER_FACTORY.createReader(new StringReader(stateBase64)).readObject();
395397
//Remove state cookie
396398
res.headers().addCookie(stateCookieHandler.removeCookie().build());
397399
String state = stateCookie.getString("state");
@@ -422,7 +424,7 @@ private void processCodeWithTenant(String code, ServerRequest req, ServerRespons
422424
if (response.status().family() == Status.Family.SUCCESSFUL) {
423425
try {
424426
JsonObject jsonObject = response.as(JsonObject.class);
425-
processJsonResponse(res, jsonObject, tenantName, stateCookie);
427+
processJsonResponse(req, res, jsonObject, tenantName, stateCookie);
426428
} catch (Exception e) {
427429
processError(res, e, "Failed to read JSON from response");
428430
}
@@ -478,20 +480,21 @@ private String redirectUri(ServerRequest req, String tenantName) {
478480
return uri;
479481
}
480482

481-
private String processJsonResponse(ServerResponse res,
483+
private String processJsonResponse(ServerRequest req,
484+
ServerResponse res,
482485
JsonObject json,
483486
String tenantName,
484487
JsonObject stateCookie) {
485488
String accessToken = json.getString("access_token");
486489
String idToken = json.getString("id_token", null);
487490
String refreshToken = json.getString("refresh_token", null);
488491

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

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

513516
if (oidcConfig.useCookie()) {
514517
try {
518+
JsonObject accessTokenJson = JSON_BUILDER_FACTORY.createObjectBuilder()
519+
.add("accessToken", accessToken)
520+
.add("remotePeer", req.remotePeer().host())
521+
.build();
522+
String encodedAccessToken = Base64.getEncoder()
523+
.encodeToString(accessTokenJson.toString().getBytes(StandardCharsets.UTF_8));
524+
515525
ServerResponseHeaders headers = res.headers();
516526

517527
OidcCookieHandler tenantCookieHandler = oidcConfig.tenantCookieHandler();
518528

519529
headers.addCookie(tenantCookieHandler.createCookie(tenantName).build()); //Add tenant name cookie
520-
headers.addCookie(tokenCookieHandler.createCookie(accessToken).build()); //Add token cookie
530+
headers.addCookie(tokenCookieHandler.createCookie(encodedAccessToken).build()); //Add token cookie
521531
if (refreshToken != null) {
522532
headers.addCookie(refreshTokenCookieHandler.createCookie(refreshToken).build()); //Add refresh token cookie
523533
}

‎security/providers/oidc/src/main/java/io/helidon/security/providers/oidc/TenantAuthenticationHandler.java‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package io.helidon.security.providers.oidc;
1818

19+
import java.io.StringReader;
1920
import java.net.URLEncoder;
2021
import java.nio.charset.StandardCharsets;
2122
import java.security.SecureRandom;
@@ -68,10 +69,10 @@
6869
import io.helidon.webclient.api.HttpClientResponse;
6970
import io.helidon.webclient.api.WebClient;
7071

71-
import jakarta.json.Json;
72-
import jakarta.json.JsonBuilderFactory;
7372
import jakarta.json.JsonObject;
7473

74+
import static io.helidon.security.providers.oidc.OidcFeature.JSON_BUILDER_FACTORY;
75+
import static io.helidon.security.providers.oidc.OidcFeature.JSON_READER_FACTORY;
7576
import static io.helidon.security.providers.oidc.common.spi.TenantConfigFinder.DEFAULT_TENANT_ID;
7677

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

8787
private final boolean optional;
@@ -264,7 +264,23 @@ private AuthenticationResponse processAccessToken(String tenantId, ProviderReque
264264
} else {
265265
try {
266266
String tokenValue = cookie.get();
267-
return validateAccessToken(tenantId, providerRequest, tokenValue, idToken);
267+
String decodedJson = new String(Base64.getDecoder().decode(tokenValue), StandardCharsets.UTF_8);
268+
JsonObject jsonObject = JSON_READER_FACTORY.createReader(new StringReader(decodedJson)).readObject();
269+
if (oidcConfig.accessTokenIpCheck()) {
270+
Object userIp = providerRequest.env().abacAttribute("userIp").orElseThrow();
271+
if (!jsonObject.getString("remotePeer").equals(userIp)) {
272+
if (LOGGER.isLoggable(System.Logger.Level.DEBUG)) {
273+
LOGGER.log(System.Logger.Level.DEBUG,
274+
"Current peer IP does not match the one this access token was issued for");
275+
}
276+
return errorResponse(providerRequest,
277+
Status.UNAUTHORIZED_401,
278+
"peer_host_mismatch",
279+
"Peer host access token mismatch",
280+
tenantId);
281+
}
282+
}
283+
return validateAccessToken(tenantId, providerRequest, jsonObject.getString("accessToken"), idToken);
268284
} catch (Exception e) {
269285
if (LOGGER.isLoggable(System.Logger.Level.DEBUG)) {
270286
LOGGER.log(System.Logger.Level.DEBUG, "Invalid access token in cookie", e);
@@ -372,7 +388,7 @@ private AuthenticationResponse errorResponse(ProviderRequest providerRequest,
372388
+ "nonce=" + nonce + "&"
373389
+ "state=" + state;
374390

375-
JsonObject stateJson = JSON.createObjectBuilder()
391+
JsonObject stateJson = JSON_BUILDER_FACTORY.createObjectBuilder()
376392
.add("originalUri", origUri)
377393
.add("state", state)
378394
.add("nonce", nonce)
@@ -575,8 +591,7 @@ private AuthenticationResponse refreshAccessToken(ProviderRequest providerReques
575591
Parameters.Builder form = Parameters.builder("oidc-form-params")
576592
.add("grant_type", "refresh_token")
577593
.add("refresh_token", refreshTokenString)
578-
.add("client_id", tenantConfig.clientId())
579-
.add("client_secret", tenantConfig.clientSecret());
594+
.add("client_id", tenantConfig.clientId());
580595

581596
HttpClientRequest post = webClient.post()
582597
.uri(tenant.tokenEndpointUri())
@@ -600,10 +615,18 @@ private AuthenticationResponse refreshAccessToken(ProviderRequest providerReques
600615
return AuthenticationResponse.failed("Invalid access token", e);
601616
}
602617
Errors.Collector newAccessTokenCollector = jwtValidator.apply(signedAccessToken, Errors.collector());
618+
Object remotePeer = providerRequest.env().abacAttribute("userIp").orElseThrow();
619+
620+
JsonObject accessTokenCookie = JSON_BUILDER_FACTORY.createObjectBuilder()
621+
.add("accessToken", signedAccessToken.tokenContent())
622+
.add("remotePeer", remotePeer.toString())
623+
.build();
624+
String base64 = Base64.getEncoder()
625+
.encodeToString(accessTokenCookie.toString().getBytes(StandardCharsets.UTF_8));
603626

604627
List<String> setCookieParts = new ArrayList<>();
605628
setCookieParts.add(oidcConfig.tokenCookieHandler()
606-
.createCookie(accessToken)
629+
.createCookie(base64)
607630
.build()
608631
.toString());
609632
if (refreshToken != null) {

0 commit comments

Comments
 (0)