Fixes #27536: allow clearing masked password fields in connection edit form - #32945
Conversation
…t form Show the masked sentinel as password dots (not blank) so users know a secret is stored, and convert empty-string onChange to undefined so the RJSF patch does not write replace/'' to the backend. Adds allowClear button, 3 new unit tests (100% stmt/func/line coverage on PasswordWidget), and a Playwright E2E spec covering the masked-dots UX, clear-then-save PATCH guard, and round-trip behaviour. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Hi there 👋 Thanks for your contribution! The OpenMetadata team will review the PR shortly! Once it has been labeled as Let us know if you need any help! |
Rohit0301
added
the
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
Jest test CoverageUI tests summary
|
✅ Playwright Results — workflow succeededValidated commit ✅ 4492 passed · ❌ 0 failed · 🟡 5 flaky · ⏭️ 1 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 4h 4m 33s ⏱️ Max setup 5m 17s · max shard execution 21m 33s · max shard-job elapsed before upload 24m 32s · reporting 21s 🌐 217.36 requests/attempt · 2.30 app boots/UI scenario · 38.44% common-shard skew Optimization targets still in progress:
🟡 5 flaky test(s) (passed on retry)
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
| if (ALL_ASTERISKS_REGEX.test(props.value)) { | ||
| return undefined; // Do not show the password if it is masked | ||
| } else { |
There was a problem hiding this comment.
This was needed since when people go to edit we don't have actual password, so we are not seding it back to avoid overrride to some static value, please check PR with this change before modifying it
…rvices Extends PasswordFieldClear.spec.ts to cover: - MySQL database service (authType/password): masked dots UX, no replace/'' on clear, and regression guard that untouched password generates no patch op. - Superset dashboard service (connection/password): same three scenarios. Also makes navigateToEditConnection accept a serviceType parameter (defaulting to Messaging) so all three service types share the same navigation helper. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…test In both the database and dashboard describe blocks the regression guard (saving without touching the password should produce no patch op) was running after the clearing test that had already removed the password, making the guard trivially pass. Moved it before the clearing test so it runs while the stored password is still present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review ✅ Approved 4 resolved / 4 findingsFixes password field masking in the connection edit form by displaying masked values as dots, adding a clear button, and converting empty input to ✅ 4 resolved✅ Bug: Typing into masked field appends to sentinel, corrupting saved password
✅ Quality: E2E comment contradicts the assertion it documents
✅ Quality: Password clear E2E assertion is a no-op when op is absent
✅ Quality: Regression guard runs after password already cleared, weakening it
OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
|
🚦 Removed from the merge queue —
|
2 participants
Describe your changes:
Fixes #27536
Password fields in the connection edit form were hidden (shown as empty) when the API returned a masked sentinel value (
*****). This prevented users from knowing a secret was stored, and made it impossible to clear an optional password field — clearing the field would sendreplace/''in the PATCH, overwriting the secret with an empty string instead of removing it.Changes made:
PasswordWidget.tsx: Removed theuseMemothat converted the masked sentinel toundefined; the masked value now flows to the input and displays as password dots so users can see a secret is stored. AddedallowClearbutton to Ant DesignInput.Password. AddedhandleChangethat converts''→undefined(oremptyValuefrom RJSF options) so clearing the field produces a proper JSON Patchremoveop instead ofreplace/''.PasswordWidget.test.tsx: Added 3 new tests covering masked-value display, clear→undefined conversion, and post-clear re-render.PasswordFieldClear.spec.ts(new): Playwright E2E spec with 4 scenarios — masked dots visible in edit form, PATCH does not containreplace/''after clearing, round-trip shows empty after save, and regression guard that untouched password generates no patch op.Type of change:
High-level design:
N/A — small change.
Tests:
Use cases covered
replace/''in the PATCH requestUnit tests
openmetadata-ui/src/main/resources/ui/src/components/common/Form/JSONSchema/JsonSchemaWidgets/PasswordWidget.test.tsxPasswordWidget.tsx: 100% Statements, 91.66% Branches, 100% Functions, 100% Lines (11 tests pass)Backend integration tests
Ingestion integration tests
Playwright (UI) tests
openmetadata-ui/.../ui/playwright/for UI changes.openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/PasswordFieldClear.spec.tsManual testing performed
UI screen recording / screenshots:
Not applicable — the visual change (masked dots vs empty field) is verified via Playwright assertions on
toHaveValue.Checklist:
Fixes <issue-number>: <short explanation>Fixes #27536above.