Saml metadata xml upload

[Rohit0301]

Describe your changes:

Fixes #26354

I worked on ... because ...

Screen.Recording.2026-03-30.at.6.42.56.PM.mov

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

---

Summary by Gitar

• SAML metadata XML upload feature:
  • Added file upload UI component with drag-and-drop support for SAML IdP metadata XML files
  • Auto-fills entityId, ssoLoginUrl, and idpX509Certificate fields from parsed metadata
  • Displays success/error status cards with option to re-upload
• Parsing logic:
  • Implemented parseSamlMetadataXml utility supporting Azure AD, Okta, Auth0, Google Workspace metadata formats
  • Enforces required X.509 certificate; prefers use="signing" certificates
• Tests & localization:
  • Added 373-line test suite covering valid/invalid metadata, binding preferences, edge cases
  • Added E2E tests for upload flow, success/error states, file re-upload
  • Updated 18 language files with UI strings

_{This will update automatically on new commits.}

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	64.84% (58852/90753)	44.43% (30906/69549)	47.66% (9338/19589)

[github-actions]

🟡 Playwright Results — all passed (22 flaky)

✅ 3446 passed · ❌ 0 failed · 🟡 22 flaky · ⏭️ 223 skipped

Shard	Passed	Failed	Flaky	Skipped
🟡 Shard 1	450	0	5	2
🟡 Shard 2	614	0	4	32
🟡 Shard 3	618	0	4	27
🟡 Shard 4	620	0	4	47
🟡 Shard 5	585	0	2	67
🟡 Shard 6	559	0	3	48

🟡 22 flaky test(s) (passed on retry)

• Flow/Tour.spec.ts › Tour should work from help section (shard 1, 1 retry)
• Flow/Tour.spec.ts › Tour should work from welcome screen (shard 1, 1 retry)
• Flow/Tour.spec.ts › Tour should work from URL directly (shard 1, 1 retry)
• Pages/Bots.spec.ts › Bots Page should work properly (shard 1, 1 retry)
• Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
• Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
• Features/DataQuality/TestCaseImportExportE2eFlow.spec.ts › Admin: Complete export-import-validate flow (shard 2, 1 retry)
• Features/DataQuality/TestCaseIncidentPermissions.spec.ts › User with only VIEW cannot PATCH incidents (shard 2, 1 retry)
• Features/DataQuality/TestCaseResultPermissions.spec.ts › User with TEST_CASE.EDIT_ALL can see edit action on test case (shard 2, 1 retry)
• Features/IncidentManager.spec.ts › Complete Incident lifecycle with table owner (shard 3, 1 retry)
• Features/Permissions/EntityPermissions.spec.ts › Metric deny common operations permissions (shard 3, 1 retry)
• Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
• Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
• Pages/Customproperties-part2.spec.ts › entityReferenceList shows item count, scrollable list, no expand toggle (shard 4, 1 retry)
• Pages/Domains.spec.ts › Rename domain with data products attached at domain and subdomain levels (shard 4, 1 retry)
• Pages/Domains.spec.ts › Multiple consecutive domain renames preserve all associations (shard 4, 1 retry)
• Pages/Entity.spec.ts › User as Owner with unsorted list (shard 4, 1 retry)
• Pages/EntityDataConsumer.spec.ts › Tier Add, Update and Remove (shard 5, 1 retry)
• Pages/EntityDataSteward.spec.ts › User as Owner Add, Update and Remove (shard 5, 1 retry)
• Pages/Glossary.spec.ts › Column dropdown drag-and-drop functionality for Glossary Terms table (shard 6, 1 retry)
• Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
• VersionPages/EntityVersionPages.spec.ts › Directory (shard 6, 1 retry)

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[gitar-bot]

Code Review ⚠️ Changes requested 3 resolved / 7 findings

Adds SAML metadata XML upload functionality with UI improvements and error handling refinements. However, flex layout regression allows icon collapse in tight layouts, thread-safe logging issues in concurrent runs, and unhandled NumberFormatException in app run ID parsing must be addressed before approval.

⚠️ Edge Case: Thread prefix bindings are global; concurrent runs misroute logs

The threadPrefixBindings in AppRunLogAppender is a global static list. When startCapture is called, it replaces any existing binding for a given prefix with the new buffer (line 165-166). If two app runs with the same thread prefixes start concurrently (e.g., two SearchIndexApp runs, or any apps matching the searchindex name check), the second call overwrites the first run's bindings, causing the first run's worker thread logs to be routed to the second run's buffer.

While SearchIndexApp uses a reindex lock to prevent truly concurrent runs, there's a window during startup/shutdown overlap, and the design doesn't protect against other apps whose names happen to contain 'searchindex' (see getThreadPrefixesForApp line 255 in OmAppJobListener).

Suggested fix

Key the thread prefix bindings by both prefix AND appRunId (or buffer identity), so concurrent runs don't overwrite each other's bindings. Alternatively, make getThreadPrefixesForApp use exact app name matching instead of `.contains("searchindex")`.

💡 Bug: flex-shrink allows icon to collapse in tight layouts

📄 openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx:121

The CSS class was changed from flex-shrink-0 (prevents shrinking) to flex-shrink (allows shrinking with flex-shrink: 1). Since this is applied to a fixed-size w-6 h-6 icon container, allowing it to shrink could cause the status icon to get visually squished when the parent flex container is space-constrained (e.g., long file names). The previous flex-shrink-0 was likely intentional to protect the icon's dimensions.

Suggested fix

Revert to `flex-shrink-0` to prevent the icon from shrinking:
'flex-shrink-0 flex items-center justify-center rounded-full w-6 h-6',

💡 Edge Case: Long.parseLong(appRunId) can throw uncaught NumberFormatException

In AppRunLogAppender.startCapture (line 158), Long.parseLong(appRunId) is called without a try-catch. While the current caller in OmAppJobListener always passes String.valueOf(runRecord.getTimestamp()) (guaranteed numeric), this is a public static method that could be called from other contexts. An invalid appRunId would cause an unhandled exception that propagates up to the Quartz job listener, potentially disrupting the app run itself.

Suggested fix

Wrap in try-catch or add input validation:
try {
  long runTimestamp = Long.parseLong(appRunId);
} catch (NumberFormatException e) {
  LOG.warn("Invalid appRunId '{}', using current time", appRunId);
  runTimestamp = System.currentTimeMillis();
}

💡 Quality: JSON escapeJson misses control chars below 0x20

The hand-rolled escapeJson in AppRunLogAppender (line 108-118) handles \, ", ``, \r, and ` `, but does not escape other control characters (e.g. `\b`, `\f`, or characters with codepoints < 0x20). If a log message contains such characters, the resulting JSON line in the log file will be malformed and unparseable by downstream log consumers.

Suggested fix

Add a sweep for remaining control characters:
.replace("\b", "\b")
.replace("\f", "\f")
Or better, use a library JSON encoder (e.g. Jackson's JsonStringEncoder) to handle all edge cases.

✅ 3 resolved

✅ Edge Case: FileReader.onerror not handled in metadata upload

📄 openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx:325-339
The handleMetadataFileUpload callback at line 325 creates a FileReader and sets reader.onload, but does not set reader.onerror. If the file read fails (e.g., file becomes unavailable, permission issues), the user gets no feedback — the upload silently does nothing. An onerror handler should set the status to 'error' so the user sees the error status card.

✅ Edge Case: Upload status not reset when switching away from SAML

📄 openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx:184-188 📄 openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx:973-987
The metadataUploadStatus and metadataUploadFileName states are never reset when the user switches providers or re-enters edit mode. If a user uploads a SAML XML, switches to another provider, then switches back to SAML, the old status card will still be displayed instead of the fresh upload drop zone. The state should be reset when the provider changes.

✅ Quality: Grammar: "a XML" should be "an XML" in English locale

📄 openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json:1391
The English locale string or-drag-and-drop-a-xml-file-here reads "or drag and drop a XML file here". Since "XML" is pronounced starting with a vowel sound ("ex-em-el"), the correct article is "an". This also affects the i18n key name itself which propagated to all locale files.

🤖 Prompt for agents

Code Review: Adds SAML metadata XML upload functionality with UI improvements and error handling refinements. However, flex layout regression allows icon collapse in tight layouts, thread-safe logging issues in concurrent runs, and unhandled NumberFormatException in app run ID parsing must be addressed before approval.

1. 💡 Bug: flex-shrink allows icon to collapse in tight layouts
   Files: openmetadata-ui/src/main/resources/ui/src/components/SettingsSso/SSOConfigurationForm/SSOConfigurationForm.tsx:121

   The CSS class was changed from `flex-shrink-0` (prevents shrinking) to `flex-shrink` (allows shrinking with `flex-shrink: 1`). Since this is applied to a fixed-size `w-6 h-6` icon container, allowing it to shrink could cause the status icon to get visually squished when the parent flex container is space-constrained (e.g., long file names). The previous `flex-shrink-0` was likely intentional to protect the icon's dimensions.

   Suggested fix:
   Revert to `flex-shrink-0` to prevent the icon from shrinking:
   'flex-shrink-0 flex items-center justify-center rounded-full w-6 h-6',

2. ⚠️ Edge Case: Thread prefix bindings are global; concurrent runs misroute logs

   The `threadPrefixBindings` in `AppRunLogAppender` is a global static list. When `startCapture` is called, it replaces any existing binding for a given prefix with the new buffer (line 165-166). If two app runs with the same thread prefixes start concurrently (e.g., two SearchIndexApp runs, or any apps matching the `searchindex` name check), the second call overwrites the first run's bindings, causing the first run's worker thread logs to be routed to the second run's buffer.
   
   While SearchIndexApp uses a reindex lock to prevent truly concurrent runs, there's a window during startup/shutdown overlap, and the design doesn't protect against other apps whose names happen to contain 'searchindex' (see `getThreadPrefixesForApp` line 255 in OmAppJobListener).

   Suggested fix:
   Key the thread prefix bindings by both prefix AND appRunId (or buffer identity), so concurrent runs don't overwrite each other's bindings. Alternatively, make getThreadPrefixesForApp use exact app name matching instead of `.contains("searchindex")`.

3. 💡 Edge Case: Long.parseLong(appRunId) can throw uncaught NumberFormatException

   In `AppRunLogAppender.startCapture` (line 158), `Long.parseLong(appRunId)` is called without a try-catch. While the current caller in `OmAppJobListener` always passes `String.valueOf(runRecord.getTimestamp())` (guaranteed numeric), this is a public static method that could be called from other contexts. An invalid appRunId would cause an unhandled exception that propagates up to the Quartz job listener, potentially disrupting the app run itself.

   Suggested fix:
   Wrap in try-catch or add input validation:
   try {
     long runTimestamp = Long.parseLong(appRunId);
   } catch (NumberFormatException e) {
     LOG.warn("Invalid appRunId '{}', using current time", appRunId);
     runTimestamp = System.currentTimeMillis();
   }

4. 💡 Quality: JSON escapeJson misses control chars below 0x20

   The hand-rolled `escapeJson` in `AppRunLogAppender` (line 108-118) handles `\`, `"`, `
   `, `\r`, and `	`, but does not escape other control characters (e.g. `\b`, `\f`, or characters with codepoints < 0x20). If a log message contains such characters, the resulting JSON line in the log file will be malformed and unparseable by downstream log consumers.

   Suggested fix:
   Add a sweep for remaining control characters:
   .replace("\b", "\b")
   .replace("\f", "\f")
   Or better, use a library JSON encoder (e.g. Jackson's JsonStringEncoder) to handle all edge cases.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[ShaileshParmar11]

Could you please clarify the reason for making this change?

Also, wherever this value is being used, do we have proper checks in place to handle empty cases and prevent any unexpected behavior?

[Rohit0301]

When user upload a valid xml then we populates the data in the correct fields, but if user uploads an invalid XML then we need to clear those fields. Also we have validations for those fields, so when we make them empty, validations checks will run

[github-actions]

Failed to cherry-pick changes to the 1.12.5 branch.
Please cherry-pick the changes manually.
You can find more details here.