Skip to content

Fixes #27091 : Fix resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource #27092

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.

Sign up for GitHub

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

Open
RajdeepKushwaha5 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixes #27091 : Fix resource leaks in SamlSettingsHolder, SamlValidator, and IndexResource #27092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 24 additions & 10 deletions ...tadata-service/src/main/java/org/openmetadata/service/resources/system/IndexResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.charset.StandardCharsets;
import java.util.stream.Collectors;
import lombok.extern.slf4j.Slf4j;
import org.openmetadata.service.OpenMetadataApplicationConfig;
Expand All @@ -18,11 +19,17 @@ public class IndexResource {
private String indexHtml;

public IndexResource() {
InputStream inputStream = getClass().getResourceAsStream("/assets/index.html");
indexHtml =
new BufferedReader(new InputStreamReader(inputStream))
.lines()
.collect(Collectors.joining("\n"));
try (InputStream inputStream = getClass().getResourceAsStream("/assets/index.html")) {
if (inputStream == null) {
throw new IllegalStateException("Resource /assets/index.html not found on classpath");
}
try (BufferedReader reader =
new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
indexHtml = reader.lines().collect(Collectors.joining("\n"));
}
} catch (java.io.IOException e) {
throw new IllegalStateException("Failed to read /assets/index.html", e);
}
}

public void initialize(OpenMetadataApplicationConfig config) {
Expand All @@ -32,11 +39,18 @@ public void initialize(OpenMetadataApplicationConfig config) {
public static String getIndexFile(String basePath) {
LOG.info("IndexResource.getIndexFile called with basePath: [{}]", basePath);

InputStream inputStream = IndexResource.class.getResourceAsStream("/assets/index.html");
String indexHtml =
new BufferedReader(new InputStreamReader(inputStream))
.lines()
.collect(Collectors.joining("\n"));
String indexHtml;
try (InputStream inputStream = IndexResource.class.getResourceAsStream("/assets/index.html")) {
if (inputStream == null) {
throw new IllegalStateException("Resource /assets/index.html not found on classpath");
}
try (BufferedReader reader =
new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8))) {
indexHtml = reader.lines().collect(Collectors.joining("\n"));
}
} catch (java.io.IOException e) {
throw new IllegalStateException("Failed to read /assets/index.html", e);
}

String result = indexHtml.replace("${basePath}", basePath);
String basePathLine =
Expand Down
109 changes: 58 additions & 51 deletions ...service/src/main/java/org/openmetadata/service/security/auth/validator/SamlValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,46 +294,50 @@ private FieldError validateIdpConnectivity(SamlSSOClientConfig samlConfig) {

URL url = new URL(urlWithParams);
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
conn.setRequestMethod("GET");
conn.setConnectTimeout(5000);
conn.setReadTimeout(5000);
conn.setInstanceFollowRedirects(false);
int responseCode = conn.getResponseCode();
LOG.debug("IdP response code to SAML request: {}", responseCode);

// Analyze response
if (responseCode == 404) {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO Login URL not found (HTTP 404). The URL '" + ssoUrl + "' does not exist.");
} else if (responseCode == 405) {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL doesn't accept GET requests (HTTP 405). Please check the SSO URL configuration.");
} else if (responseCode >= 200 && responseCode < 400) {
// 200 or 302 means the IdP accepted our SAML request
return null; // Success - SSO Login URL validated
} else if (responseCode >= 400 && responseCode < 500) {
// 400-499 could mean wrong URL or IdP rejecting the request
// Read a bit of the response to check for specific errors
String responseSnippet = readResponseSnippet(conn);
if (responseSnippet.toLowerCase().contains("saml")
|| responseSnippet.toLowerCase().contains("invalid")) {
// Warning case - treat as success
LOG.warn(
"SSO URL responded with client error (HTTP {}). This might be due to the test SAML request format.",
responseCode);
return null;
try {
conn.setRequestMethod("GET");
conn.setConnectTimeout(5000);
conn.setReadTimeout(5000);
conn.setInstanceFollowRedirects(false);
int responseCode = conn.getResponseCode();
LOG.debug("IdP response code to SAML request: {}", responseCode);

// Analyze response
if (responseCode == 404) {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO Login URL not found (HTTP 404). The URL '" + ssoUrl + "' does not exist.");
} else if (responseCode == 405) {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL doesn't accept GET requests (HTTP 405). Please check the SSO URL configuration.");
} else if (responseCode >= 200 && responseCode < 400) {
// 200 or 302 means the IdP accepted our SAML request
return null; // Success - SSO Login URL validated
} else if (responseCode >= 400 && responseCode < 500) {
// 400-499 could mean wrong URL or IdP rejecting the request
// Read a bit of the response to check for specific errors
String responseSnippet = readResponseSnippet(conn);
if (responseSnippet.toLowerCase().contains("saml")
|| responseSnippet.toLowerCase().contains("invalid")) {
// Warning case - treat as success
LOG.warn(
"SSO URL responded with client error (HTTP {}). This might be due to the test SAML request format.",
responseCode);
return null;
}
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL returned error (HTTP "
+ responseCode
+ "). Please verify the URL is correct.");
} else {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL is not accessible (HTTP " + responseCode + ")");
}
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL returned error (HTTP "
+ responseCode
+ "). Please verify the URL is correct.");
} else {
return ValidationErrorBuilder.createFieldError(
ValidationErrorBuilder.FieldPaths.SAML_IDP_SSO_URL,
"SSO URL is not accessible (HTTP " + responseCode + ")");
} finally {
conn.disconnect();
}
} catch (Exception e) {
LOG.warn("SSO URL validation failed", e);
Expand Down Expand Up @@ -376,10 +380,13 @@ private String createTestSamlRequest(SamlSSOClientConfig samlConfig) {
java.io.ByteArrayOutputStream bytesOut = new java.io.ByteArrayOutputStream();
java.util.zip.Deflater deflater =
new java.util.zip.Deflater(java.util.zip.Deflater.DEFLATED, true);
java.util.zip.DeflaterOutputStream deflaterStream =
new java.util.zip.DeflaterOutputStream(bytesOut, deflater);
deflaterStream.write(samlRequestXml.getBytes(StandardCharsets.UTF_8));
deflaterStream.finish();
try (java.util.zip.DeflaterOutputStream deflaterStream =
new java.util.zip.DeflaterOutputStream(bytesOut, deflater)) {
deflaterStream.write(samlRequestXml.getBytes(StandardCharsets.UTF_8));
deflaterStream.finish();
} finally {
deflater.end();
}

// Base64 encode
String base64Request = Base64.getEncoder().encodeToString(bytesOut.toByteArray());
Expand All @@ -395,15 +402,15 @@ private String createTestSamlRequest(SamlSSOClientConfig samlConfig) {

private String readResponseSnippet(HttpURLConnection conn) {
try {
java.io.InputStream inputStream = conn.getErrorStream();
if (inputStream == null) {
inputStream = conn.getInputStream();
}
java.io.InputStream errorStream = conn.getErrorStream();
java.io.InputStream inputStream = (errorStream != null) ? errorStream : conn.getInputStream();
if (inputStream != null) {
byte[] buffer = new byte[500];
int bytesRead = inputStream.read(buffer);
if (bytesRead > 0) {
return new String(buffer, 0, bytesRead, StandardCharsets.UTF_8);
try (inputStream) {
byte[] buffer = new byte[500];
int bytesRead = inputStream.read(buffer);
Comment thread
RajdeepKushwaha5 marked this conversation as resolved.
Comment thread
RajdeepKushwaha5 marked this conversation as resolved.
if (bytesRead > 0) {
return new String(buffer, 0, bytesRead, StandardCharsets.UTF_8);
}
}
}
} catch (Exception e) {
Expand Down
6 changes: 3 additions & 3 deletions ...data-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ public void initDefaultSettings(OpenMetadataApplicationConfig catalogApplication
&& !CommonUtil.nullOrEmpty(securityConfig.getKeyStorePassword())
&& !CommonUtil.nullOrEmpty(securityConfig.getKeyStoreAlias())) {
KeyStore keyStore = KeyStore.getInstance("JKS");
keyStore.load(
new FileInputStream(securityConfig.getKeyStoreFilePath()),
securityConfig.getKeyStorePassword().toCharArray());
try (FileInputStream fis = new FileInputStream(securityConfig.getKeyStoreFilePath())) {
keyStore.load(fis, securityConfig.getKeyStorePassword().toCharArray());
}
samlData.put(SettingsBuilder.KEYSTORE_KEY, keyStore);
samlData.put(SettingsBuilder.KEYSTORE_ALIAS, securityConfig.getKeyStoreAlias());
samlData.put(SettingsBuilder.KEYSTORE_KEY_PASSWORD, securityConfig.getKeyStorePassword());
Expand Down
Loading