Skip to content

Commit eae5500

Browse files
mohityadav766claude
andcommitted
Fixes 28619: Accept Okta SAML certificates whose CN is the org name
SamlValidator required the IdP certificate's CN to equal the Entity ID host. Okta Entity IDs are always http://www.okta.com/{appId} — a format validateBasicSamlConfig itself enforces — so the host is always the literal www.okta.com, while Okta issues signing certificates with CN=<org short name>. The branch was unsatisfiable, so every legitimate Okta configuration was rejected on save. The only passing value was a CN of literally *.okta.com, which Okta never issues and which was the sole case the existing test covered. The check had no security value: the certificate is a trust anchor taken from the IdP's own metadata and handed to OneLogin's IDP_X509CERT_PROPERTY_KEY, and SAML verifies signatures against its public key without PKIX name validation, so the CN is never consulted. The Auth0, Azure and non-Azure branches encoded the same assumption and broke on Auth0 custom domains and on any change to Microsoft's certificate naming, so all four collapse into the warn-and-continue path that already existed for generic providers. A CN differing from the Entity ID host logs at INFO — it is the expected shape for these providers, so warning would train operators to ignore warnings. Parsing and the validity window are still enforced. Also scopes bean validation on PUT /system/security/config to the active provider. @Valid cascaded into every nested block present, so an instance carrying a partial ldapConfiguration could not save a SAML configuration and a GET response was not accepted by PUT unchanged. Validation now runs against a copy with inactive providers' blocks nulled, and IllegalArgumentException is rethrown ahead of the blanket catch so an invalid configuration answers 400 rather than 500. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 42d1535 commit eae5500

7 files changed

Lines changed: 637 additions & 141 deletions

File tree

Lines changed: 347 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,347 @@
1+
/*
2+
* Copyright 2021 Collate
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
14+
package org.openmetadata.it.tests;
15+
16+
import static org.junit.jupiter.api.Assertions.assertEquals;
17+
import static org.junit.jupiter.api.Assertions.assertNotNull;
18+
import static org.junit.jupiter.api.Assertions.assertThrows;
19+
import static org.junit.jupiter.api.Assertions.assertTrue;
20+
21+
import com.fasterxml.jackson.databind.JsonNode;
22+
import com.fasterxml.jackson.databind.ObjectMapper;
23+
import com.fasterxml.jackson.databind.node.ObjectNode;
24+
import java.util.ArrayList;
25+
import java.util.Arrays;
26+
import java.util.HashSet;
27+
import java.util.Set;
28+
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.parallel.Execution;
30+
import org.junit.jupiter.api.parallel.ExecutionMode;
31+
import org.junit.jupiter.api.parallel.Isolated;
32+
import org.openmetadata.catalog.security.client.SamlSSOClientConfig;
33+
import org.openmetadata.catalog.type.IdentityProviderConfig;
34+
import org.openmetadata.catalog.type.ServiceProviderConfig;
35+
import org.openmetadata.it.util.SdkClients;
36+
import org.openmetadata.schema.api.security.AuthenticationConfiguration;
37+
import org.openmetadata.schema.api.security.AuthorizerConfiguration;
38+
import org.openmetadata.schema.api.security.ClientType;
39+
import org.openmetadata.schema.api.security.ResponseType;
40+
import org.openmetadata.schema.configuration.SecurityConfiguration;
41+
import org.openmetadata.schema.services.connections.metadata.AuthProvider;
42+
import org.openmetadata.sdk.exceptions.InvalidRequestException;
43+
import org.openmetadata.sdk.network.HttpMethod;
44+
import org.openmetadata.sdk.network.RequestOptions;
45+
46+
/**
47+
* Integration tests for validating SAML security configurations through
48+
* {@code /v1/system/security/validate} — the endpoint the SSO settings UI calls after a user
49+
* uploads their IdP's {@code metadata.xml}.
50+
*
51+
* <p>These cover the handoff the UI cannot: {@code parseSamlMetadataXml} runs client-side and only
52+
* fills the form, so every provider-specific rule is enforced here on save. Issue #28619 shipped
53+
* because no test posted a realistically-shaped provider payload — Okta signs with
54+
* {@code CN=<org short name>} against an Entity ID of {@code http://www.okta.com/{appId}}, and
55+
* requiring those to be equal rejected every valid Okta configuration.
56+
*
57+
* <p>The validate endpoint is side-effect free, so those tests never mutate the running server's
58+
* security configuration. The two {@code PUT} tests write back only a configuration they just read,
59+
* leave the active provider untouched, and restore the original.
60+
*/
61+
@Execution(ExecutionMode.SAME_THREAD)
62+
@Isolated
63+
public class SamlSecurityConfigIT {
64+
65+
private static final ObjectMapper MAPPER = new ObjectMapper();
66+
67+
private static final String SECURITY_CONFIG_PATH = "/v1/system/security/config";
68+
private static final String SECURITY_VALIDATE_PATH = "/v1/system/security/validate";
69+
private static final String IDP_CERT_FIELD =
70+
"authenticationConfiguration.samlConfiguration.idp.idpX509Certificate";
71+
72+
/** Okta's Entity ID is always this shape, so its host is always the literal {@code www.okta.com}. */
73+
private static final String OKTA_ENTITY_ID = "http://www.okta.com/exk1a2b3c4d5";
74+
75+
/**
76+
* SSO login URLs are reached over the network by the validator. {@code .invalid} is reserved by
77+
* RFC 2606 and never resolves, which keeps these tests hermetic — the validator treats an
78+
* unreachable IdP as a warning, not an error.
79+
*/
80+
private static final String OKTA_SSO_URL =
81+
"https://example-org.okta.invalid/app/openmetadata/exk1a2b3c4d5/sso/saml";
82+
83+
/** Self-signed, {@code CN=example-org, O=Okta}, valid 2026-09 to 2036-09. */
84+
private static final String OKTA_ORG_CERT =
85+
"""
86+
-----BEGIN CERTIFICATE-----
87+
MIIDyzCCArOgAwIBAgIUIZV2P9D5F0TyWYkhelWTeh0aQEowDQYJKoZIhvcNAQEL
88+
BQAwdTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM
89+
DVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBE9rdGExFDASBgNVBAsMC1NTT1Byb3Zp
90+
ZGVyMRQwEgYDVQQDDAtleGFtcGxlLW9yZzAeFw0yNjA5MTYyMDQ1MjNaFw0zNjA5
91+
MTMyMDQ1MjNaMHUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYw
92+
FAYDVQQHDA1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQKDARPa3RhMRQwEgYDVQQLDAtT
93+
U09Qcm92aWRlcjEUMBIGA1UEAwwLZXhhbXBsZS1vcmcwggEiMA0GCSqGSIb3DQEB
94+
AQUAA4IBDwAwggEKAoIBAQDVQvEms8hTsAV8LUUx8uTQhxER/Hf3+yx3LPjMD+KY
95+
+4YpRHhQk3IY6r3DOQvT/PsUUWuqbRyYmGWiXkDyPsQAGUd3HW63fAcX+0TfBYEz
96+
75pt9U/QYhjJQAvmBmxqrak25bImKEZln7/+65zBS8Ohs9TYFBAT1lfaCBifhh0A
97+
80FkQXl8/5thnZAaNaFK3YIJfd3PBpMt2uGYOxo6xJbKS9NSGZLc3gRPRqcLhNx9
98+
01n8CxwFrCwqbY3urJ2rejPIaCZ+iVaJhFzuM1i1sB3H7/9+U6Wns1Is7a+OZLY8
99+
l41nDougHYvwwQaWA9xZ5lHPIiTdE9UWlHS53pfkp2qdAgMBAAGjUzBRMB0GA1Ud
100+
DgQWBBT5mtJZvkkmbFeAdkQt1ka2gyfakTAfBgNVHSMEGDAWgBT5mtJZvkkmbFeA
101+
dkQt1ka2gyfakTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCI
102+
/vPj2Z3lk9lvlnSsoXCpSxGfkxR3XSy/Pkm4CTPLN0XXhVKjyPTb2kdqM8AXjrID
103+
qehxhNunicS3+w3bwK4XZ4gTxpXnOflkXE+B38/1+rz9QQwiIx8q0B3thspazkm3
104+
aawU4aQEwSWygI8sXuxkShru5hHcwXWRA5F5s5kpiFK50FLOqWbOukLMSFoVNu1D
105+
Y8or/But9KUsAfHy294vbQAOj4kI0SYqBvhhlpJxXq1vMFtBiiwKeNIxE0mW240w
106+
b0/XIHowzxARt4SAZS+0jpaHqBESyanV8iVNqx8heeW+odH2cBkE0ic6t6/s3SrD
107+
V7z+p03MrlFxMur28rIH
108+
-----END CERTIFICATE-----
109+
""";
110+
111+
/** Same subject as {@link #OKTA_ORG_CERT}, but expired in 2021. */
112+
private static final String EXPIRED_CERT =
113+
"""
114+
-----BEGIN CERTIFICATE-----
115+
MIIDyzCCArOgAwIBAgIUWp9vJboRFKCPwhcH8zGg2pxfZuEwDQYJKoZIhvcNAQEL
116+
BQAwdTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcM
117+
DVNhbiBGcmFuY2lzY28xDTALBgNVBAoMBE9rdGExFDASBgNVBAsMC1NTT1Byb3Zp
118+
ZGVyMRQwEgYDVQQDDAtleGFtcGxlLW9yZzAeFw0yMDAxMDEwMDAwMDBaFw0yMTAx
119+
MDEwMDAwMDBaMHUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRYw
120+
FAYDVQQHDA1TYW4gRnJhbmNpc2NvMQ0wCwYDVQQKDARPa3RhMRQwEgYDVQQLDAtT
121+
U09Qcm92aWRlcjEUMBIGA1UEAwwLZXhhbXBsZS1vcmcwggEiMA0GCSqGSIb3DQEB
122+
AQUAA4IBDwAwggEKAoIBAQC+bJPEDwiBsdu6dkAGwgG9R+VeUM7x7UWKrIwoxenk
123+
knNlVPedm5nJF7LFuiY/vQ3tIOxfCyF4icgLBWS6NPBnRI0BkX2xpvbFB445j3By
124+
arePF8d93fGdJ8V8X5YvnLNcrTcrr9U53JRMYJu1x55ta9KvWYL3U4vHjXglRJPR
125+
rtsA/fko3VzbtAzqHSw4EIlYnIDrGntO4Df+sDa8iD716ZxuSScO6O5cBKjo2+UC
126+
QPbX72h8/MUSBQI3zOUtOLudg1wAtAIa11FDuxpth8sN3Wi8Sc+SDobL7t5CbVT1
127+
QbnPGd3p4aVxmMZFA9hsNtGNNP4Uz9LxPLKIeuzkfQvpAgMBAAGjUzBRMB0GA1Ud
128+
DgQWBBQnxmEpagXDyyiu1UNh8eydPt6JUDAfBgNVHSMEGDAWgBQnxmEpagXDyyiu
129+
1UNh8eydPt6JUDAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQAR
130+
tTQCxp6DDu6nA/JcE2ZRrpgHDmKypXl8VIaIhzXog/wS+l4ZLbJSEJxq5Z+EY6gw
131+
0+63tQqEx3atcVS1REzfrpZtcnLBzl8rEK25YWKNx//U5kav6EnH63s0uRXdJG34
132+
VCVe13AmOcsPByyfHnQ8U+3/x6qAu6Bhnl9z7A8GbhWP6FJmJ44rnIdEOSJJ6mSX
133+
cJVPw1lHyx5msihCABQ+LSfzV/F1QJ8B95wJ54cIVFIuapmGZmOjk++wEk1Os6C7
134+
e3wdc9f/xhF/BzDTTRUPQUGCCkpwe/xozjp5OE9Jr8P8sYXp0/qBxsCPzg2nMLtD
135+
M+GwtCm/xL8aQHKZkTY3
136+
-----END CERTIFICATE-----
137+
""";
138+
139+
/**
140+
* Issue #28619: the payload the SSO form produces after a real Okta {@code metadata.xml} upload
141+
* must validate cleanly.
142+
*/
143+
@Test
144+
void validateSecurityConfig_acceptsOktaOrgNameCertificate() throws Exception {
145+
JsonNode response = validate(samlConfig(OKTA_ENTITY_ID, OKTA_SSO_URL, OKTA_ORG_CERT));
146+
147+
assertNoErrorOnField(response, IDP_CERT_FIELD);
148+
assertEquals("success", response.get("status").asText(), "Validation response: " + response);
149+
}
150+
151+
@Test
152+
void validateSecurityConfig_acceptsAuth0CustomDomainCertificate() throws Exception {
153+
JsonNode response =
154+
validate(
155+
samlConfig(
156+
"urn:dev-tenant.us.auth0.com",
157+
"https://login.example.invalid/samlp/abcdefgh12345",
158+
OKTA_ORG_CERT));
159+
160+
assertNoErrorOnField(response, IDP_CERT_FIELD);
161+
assertEquals("success", response.get("status").asText(), "Validation response: " + response);
162+
}
163+
164+
@Test
165+
void validateSecurityConfig_stillRejectsExpiredCertificate() throws Exception {
166+
JsonNode response = validate(samlConfig(OKTA_ENTITY_ID, OKTA_SSO_URL, EXPIRED_CERT));
167+
168+
assertEquals("failed", response.get("status").asText(), "Validation response: " + response);
169+
assertTrue(
170+
fieldError(response, IDP_CERT_FIELD).contains("expired"),
171+
"Expected an expiry error on the IdP certificate but got: " + response);
172+
}
173+
174+
@Test
175+
void validateSecurityConfig_stillRejectsMalformedCertificate() throws Exception {
176+
JsonNode response =
177+
validate(
178+
samlConfig(
179+
OKTA_ENTITY_ID,
180+
OKTA_SSO_URL,
181+
"-----BEGIN CERTIFICATE-----not-a-certificate-----END CERTIFICATE-----"));
182+
183+
assertEquals("failed", response.get("status").asText(), "Validation response: " + response);
184+
assertNotNull(fieldError(response, IDP_CERT_FIELD));
185+
}
186+
187+
/**
188+
* The operator escape hatch for a blocked SSO save is to write the configuration through
189+
* {@code PUT}, which skips provider validation. That only works if the object {@code GET} returns
190+
* is accepted by {@code PUT} unchanged.
191+
*/
192+
@Test
193+
void securityConfig_getPutRoundTripIsAccepted() throws Exception {
194+
String current = getSecurityConfig();
195+
assertNotNull(current);
196+
197+
String result = putSecurityConfig(current);
198+
199+
assertNotNull(result, "GET /security/config must return a body that PUT accepts unchanged");
200+
}
201+
202+
/**
203+
* An instance that previously touched LDAP keeps a partially-filled {@code ldapConfiguration}
204+
* block in its stored configuration. That block is irrelevant while another provider is active,
205+
* but it must not make the configuration unwritable.
206+
*/
207+
@Test
208+
void securityConfig_putIgnoresIncompleteBlockOfInactiveProvider() throws Exception {
209+
String original = getSecurityConfig();
210+
211+
try {
212+
ObjectNode config = (ObjectNode) MAPPER.readTree(original);
213+
ObjectNode authConfig = (ObjectNode) config.get("authenticationConfiguration");
214+
ObjectNode partialLdap = MAPPER.createObjectNode();
215+
partialLdap.put("dnAdminPrincipal", "cn=admin,dc=example,dc=com");
216+
partialLdap.put("userBaseDN", "ou=people,dc=example,dc=com");
217+
authConfig.set("ldapConfiguration", partialLdap);
218+
219+
String result = putSecurityConfig(MAPPER.writeValueAsString(config));
220+
221+
assertNotNull(
222+
result,
223+
"PUT must not reject a configuration over required fields of a provider that is not active");
224+
} finally {
225+
putSecurityConfig(original);
226+
}
227+
}
228+
229+
/** Scoping validation to the active provider must not stop enforcing that provider's own block. */
230+
@Test
231+
void securityConfig_putStillRejectsIncompleteBlockOfActiveProvider() throws Exception {
232+
String original = getSecurityConfig();
233+
234+
ObjectNode config = (ObjectNode) MAPPER.readTree(original);
235+
ObjectNode authConfig = (ObjectNode) config.get("authenticationConfiguration");
236+
authConfig.put("provider", "ldap");
237+
ObjectNode partialLdap = MAPPER.createObjectNode();
238+
partialLdap.put("dnAdminPrincipal", "cn=admin,dc=example,dc=com");
239+
authConfig.set("ldapConfiguration", partialLdap);
240+
241+
InvalidRequestException exception =
242+
assertThrows(
243+
InvalidRequestException.class,
244+
() -> putSecurityConfig(MAPPER.writeValueAsString(config)),
245+
"An incomplete block of the active provider must still be rejected as a client error");
246+
247+
assertTrue(
248+
exception.getMessage().contains("ldapConfiguration"),
249+
"Expected the error to name the offending LDAP fields but got: " + exception.getMessage());
250+
}
251+
252+
private String getSecurityConfig() throws Exception {
253+
return SdkClients.adminClient()
254+
.getHttpClient()
255+
.executeForString(
256+
HttpMethod.GET, SECURITY_CONFIG_PATH, null, RequestOptions.builder().build());
257+
}
258+
259+
private String putSecurityConfig(String body) throws Exception {
260+
return SdkClients.adminClient()
261+
.getHttpClient()
262+
.executeForString(
263+
HttpMethod.PUT, SECURITY_CONFIG_PATH, body, RequestOptions.builder().build());
264+
}
265+
266+
private JsonNode validate(SecurityConfiguration config) throws Exception {
267+
String response =
268+
SdkClients.adminClient()
269+
.getHttpClient()
270+
.executeForString(
271+
HttpMethod.POST,
272+
SECURITY_VALIDATE_PATH,
273+
MAPPER.writeValueAsString(config),
274+
RequestOptions.builder().build());
275+
276+
assertNotNull(response);
277+
return MAPPER.readTree(response);
278+
}
279+
280+
private void assertNoErrorOnField(JsonNode response, String field) {
281+
assertTrue(
282+
fieldError(response, field) == null,
283+
"Expected no validation error on '" + field + "' but got: " + fieldError(response, field));
284+
}
285+
286+
private String fieldError(JsonNode response, String field) {
287+
JsonNode errors = response.get("errors");
288+
if (errors == null || !errors.isArray()) {
289+
return null;
290+
}
291+
for (JsonNode error : errors) {
292+
if (field.equals(error.path("field").asText())) {
293+
return error.path("error").asText();
294+
}
295+
}
296+
return null;
297+
}
298+
299+
/**
300+
* Mirrors what the SSO form submits after {@code parseSamlMetadataXml} fills the IdP fields from
301+
* an uploaded {@code metadata.xml}.
302+
*/
303+
private SecurityConfiguration samlConfig(String entityId, String ssoLoginUrl, String idpCert) {
304+
SamlSSOClientConfig samlConfig =
305+
new SamlSSOClientConfig()
306+
.withIdp(
307+
new IdentityProviderConfig()
308+
.withEntityId(entityId)
309+
.withSsoLoginUrl(ssoLoginUrl)
310+
.withIdpX509Certificate(idpCert)
311+
.withNameId("urn:oasis:names:tc:SAML:2.0:nameid-format:emailAddress"))
312+
.withSp(
313+
new ServiceProviderConfig()
314+
.withEntityId("http://localhost:8585/api/v1/saml/metadata")
315+
.withAcs("http://localhost:8585/api/v1/saml/acs")
316+
.withCallback("http://localhost:8585/saml/callback"));
317+
318+
return new SecurityConfiguration()
319+
.withAuthenticationConfiguration(
320+
new AuthenticationConfiguration()
321+
.withClientType(ClientType.PUBLIC)
322+
.withProvider(AuthProvider.SAML)
323+
.withResponseType(ResponseType.ID_TOKEN)
324+
.withProviderName("SAML")
325+
.withPublicKeyUrls(Arrays.asList("http://localhost:8585/api/v1/system/config/jwks"))
326+
.withTokenValidationAlgorithm(
327+
AuthenticationConfiguration.TokenValidationAlgorithm.RS_256)
328+
.withAuthority("http://localhost:8585")
329+
.withClientId("open-metadata")
330+
.withCallbackUrl("http://localhost:8585/callback")
331+
.withJwtPrincipalClaims(Arrays.asList("email", "preferred_username", "sub"))
332+
.withJwtPrincipalClaimsMapping(new ArrayList<>())
333+
.withEnableSelfSignup(true)
334+
.withSamlConfiguration(samlConfig))
335+
.withAuthorizerConfiguration(
336+
new AuthorizerConfiguration()
337+
.withClassName("org.openmetadata.service.security.DefaultAuthorizer")
338+
.withContainerRequestFilter("org.openmetadata.service.security.JwtFilter")
339+
.withAdminPrincipals(Set.of("admin"))
340+
.withAllowedEmailRegistrationDomains(Set.of("all"))
341+
.withPrincipalDomain("open-metadata.org")
342+
.withAllowedDomains(new HashSet<>())
343+
.withEnforcePrincipalDomain(false)
344+
.withEnableSecureSocketConnection(false)
345+
.withUseRolesFromProvider(false));
346+
}
347+
}

0 commit comments

Comments
 (0)