Skip to content

Fixes #31070: let any authenticated user read glossary term relation types - #31146

Merged
harshach merged 3 commits into
mainfrom
harshach/relations-read-access
Aug 7, 2026
Merged

Fixes #31070: let any authenticated user read glossary term relation types#31146
harshach merged 3 commits into
mainfrom
harshach/relations-read-access

Conversation

@harshach

@harshach harshach commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes:

Fixes #31070

The Related Terms dropdown and the ontology explorer both fetch glossaryTermRelationSettings, but the read was gated behind authorizer.authorizeAdmin. Non-admins got a 403 ... is not admin, the UI silently fell back to DEFAULT_GLOSSARY_TERM_RELATION_TYPES_FALLBACK (a single entry), and the relation-type dropdown offered only "Related To". I made reads open to any authenticated principal while every write stays admin-only.

Relation types are a global vocabulary — there is no owner or resource to evaluate a policy against, and the payload carries no secrets (prepareFetchedSettings only masks email/auth configs) — so reads now get the same treatment lineageSettings already had, via a USER_READABLE_SETTINGS allowlist that replaces the ad-hoc single-name check. Creating, updating and deleting relation types (and every other setting) is unchanged and still requires admin.

Type of change:

  • Bug fix

High-level design:

N/A — small change (3 files, one authorization rule).

Tests:

Use cases covered

  • A non-admin user opening a glossary term's Related Terms tab sees every configured relation type, not just "Related To"
  • A non-admin user can GET /system/settings/glossaryTermRelationSettings and GET /system/settings/glossaryTermRelationSettings/relationTypes
  • A non-admin user is still denied POST/PUT/DELETE .../relationTypes, PUT /system/settings, and PATCH /system/settings/{name}
  • A non-admin user is still denied every other setting (searchSettings asserted explicitly, so the allowlist can't silently widen)
  • Unauthenticated requests still get 401

Unit tests

  • Added openmetadata-service/src/test/java/org/openmetadata/service/resources/system/SystemResourceSettingsAuthorizationTest.java (6 tests) — the authorizer rejects every admin check, standing in for a non-admin caller.
  • Fails without the fix: Tests run: 6, Errors: 2 — both reads throw AuthorizationException: Principal: is not admin, reproducing the reported 403. Passes with it: Tests run: 6, Failures: 0, Errors: 0.
  • Coverage (jacoco, this test alone) on the changed lines: isUserReadableSetting 1/1, listGlossaryTermRelationTypes 10/11 (uncovered line is the pre-existing relationTypes == null fallback), getSettingByName 4/6 (uncovered lines are the pre-existing auth-config rejection branch). Whole-class coverage is not meaningful here — SystemResource is ~1450 lines covered mostly by the IT suite, which doesn't run in this surefire session.

Backend integration tests

  • Added openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java (10 tests) — same read/write split over real HTTP with a DataConsumer JWT, plus the searchSettings 403 and unauthenticated 401 guards. Compiles clean; it needs the Testcontainers stack, which I did not spin up locally, so it has not been executed outside CI.

Ingestion integration tests

  • Not applicable (no ingestion changes).

Playwright (UI) tests

  • Not applicable — no UI files changed. The dropdown populates once the existing fetch stops returning 403.

Manual testing performed

No live stack run — verification was the automated RED/GREEN above. To reproduce by hand:

  1. Create a non-admin user (any role, e.g. DataConsumer) and get its JWT.
  2. curl -H "Authorization: Bearer $TOKEN" $HOST/api/v1/system/settings/glossaryTermRelationSettings → 200 with the full relationTypes array (was 403).
  3. curl -X POST -H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' -d '{"name":"test"}' $HOST/api/v1/system/settings/glossaryTermRelationSettings/relationTypes → still 403.
  4. As that user, open a glossary term → Related Terms → Add: the relation-type dropdown lists every configured type.

UI screen recording / screenshots:

Not applicable — no UI changes.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: not applicable — no schema changes.
  • For UI changes: not applicable — no UI changes.
  • I have added tests (unit / integration) and listed them above.

Note for reviewers: 1.13 carries the same gate (SystemResource.java:259-260, 285) and needs the same backport.

🤖 Generated with Claude Code

…types

The Related Terms dropdown and the ontology explorer both fetch
glossaryTermRelationSettings, but the read was gated behind
authorizer.authorizeAdmin. Non-admins got a 403, the UI fell back to its
single-entry DEFAULT_GLOSSARY_TERM_RELATION_TYPES_FALLBACK, and the
relation-type dropdown offered only "Related To".

Relation types are a global vocabulary with no owner to evaluate a policy
against and no secrets in the payload, so reads now follow the same rule
lineageSettings already gets: open to any authenticated principal, via a
USER_READABLE_SETTINGS allowlist that replaces the ad-hoc lineage check.
Creating, updating and deleting relation types — and every other setting —
stays admin-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 01:25
@harshach
harshach requested a review from a team as a code owner August 7, 2026 01:25
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added backend safe to test Add this label to run secure Github workflows on PRs labels Aug 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a permission bug in the backend System settings API that prevented non-admin users from reading glossary term relation types (used by the Related Terms UI and ontology explorer), while keeping all writes admin-only.

Changes:

  • Allow authenticated non-admin reads for glossaryTermRelationSettings (and keep the existing lineageSettings exception) via a USER_READABLE_SETTINGS allowlist in SystemResource.
  • Remove the admin gate from GET /system/settings/glossaryTermRelationSettings/relationTypes.
  • Add unit + integration tests to validate the read/write authorization split and guard against accidental allowlist widening.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
openmetadata-service/src/main/java/org/openmetadata/service/resources/system/SystemResource.java Introduces USER_READABLE_SETTINGS allowlist and opens glossary relation-type reads to authenticated users while preserving admin-only writes.
openmetadata-service/src/test/java/org/openmetadata/service/resources/system/SystemResourceSettingsAuthorizationTest.java Adds unit tests asserting non-admin can read glossary relation settings/types but cannot write or read other settings.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java Adds HTTP-level integration tests covering 200/401/403 behavior for non-admin reads/writes, plus an explicit admin-only setting check.
Suppressed comments (4)

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java:149

  • This test issues a DELETE (write) request; it should take a READ_WRITE resource lock to avoid shared glossary-term-relation settings being mutated concurrently if authorization ever regresses.
  @Test
  @ResourceLock(
      value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS,
      mode = ResourceAccessMode.READ)

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java:161

  • This test issues a PUT to /v1/system/settings (write). Use a READ_WRITE lock on the glossary term relation settings resource so an unexpected authorization regression cannot corrupt shared state under only a READ lock.
  @Test
  @ResourceLock(
      value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS,
      mode = ResourceAccessMode.READ)

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java:177

  • This test issues a PATCH (write) request; it should take a READ_WRITE lock for the same reason as other write-path tests, even when expecting 403, to prevent cross-test interference if the write ever succeeds unexpectedly.
  @Test
  @ResourceLock(
      value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS,
      mode = ResourceAccessMode.READ)

openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsPermissionsIT.java:137

  • This test issues a PUT (write) request; it should take a READ_WRITE resource lock to prevent unintended shared-state mutations from impacting other concurrent tests if a regression ever allows the write.
  @Test
  @ResourceLock(
      value = SharedResourceLocks.GLOSSARY_TERM_RELATION_SETTINGS,
      mode = ResourceAccessMode.READ)

The five tests that fire POST/PUT/DELETE/PATCH at the shared relation
settings expect a 403, but a regression in the admin gate is exactly what
they guard against — and if one lands, the write mutates settings that
concurrent READ-lock tests are asserting on, turning one clean failure into
cascading ones. READ_WRITE matches how GlossaryTermRelationSettingsIT already
splits its locks.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 01:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 24965dc029e2ae8c47fb562dfb3b4b07eddeeb95 in Playwright run 31141944897, attempt 1.

✅ 758 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking 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) 36m 33s

⏱️ Max setup 3m 12s · max shard execution 19m 14s · max shard-job elapsed before upload 22m 34s · reporting 6s

🌐 197.16 requests/attempt · 2.51 app boots/UI scenario · 6.41% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 2.51 per UI scenario (1966 boots / 783 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 161 0 0 0 0 0
✅ Shard chromium-02 149 0 0 0 0 0
✅ Shard chromium-03 155 0 0 0 0 0
🟡 Shard chromium-04 142 0 1 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0
🟡 1 flaky test(s) (passed on retry)
  • Pages/CustomProperties.spec.tsString (shard chromium-04, 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

… authorizer

The create and update tests posted a GlossaryTermRelationType without
`category`. Jersey runs @Valid bean validation before the resource method
body, so those requests were rejected with 400 and never reached
authorizeAdmin — the assertions failed on 400 vs 403, and would have passed
just as happily against a broken admin gate.

Both payloads now carry all three @NotNull fields (name, displayName,
category), and the update targets relatedTo so the body name matches the
path. Assertion messages include the response body so the next mismatch says
why on the first read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 02:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@harshach harshach added the To release Will cherry-pick this PR into the release branch label Aug 7, 2026
@harshach
harshach merged commit 3171820 into main Aug 7, 2026
193 of 195 checks passed
@harshach
harshach deleted the harshach/relations-read-access branch August 7, 2026 22:55
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 2.0 branch.

github-actions Bot pushed a commit that referenced this pull request Aug 7, 2026
…types (#31146)

* Fixes #31070: let any authenticated user read glossary term relation types

The Related Terms dropdown and the ontology explorer both fetch
glossaryTermRelationSettings, but the read was gated behind
authorizer.authorizeAdmin. Non-admins got a 403, the UI fell back to its
single-entry DEFAULT_GLOSSARY_TERM_RELATION_TYPES_FALLBACK, and the
relation-type dropdown offered only "Related To".

Relation types are a global vocabulary with no owner to evaluate a policy
against and no secrets in the payload, so reads now follow the same rule
lineageSettings already gets: open to any authenticated principal, via a
USER_READABLE_SETTINGS allowlist that replaces the ad-hoc lineage check.
Creating, updating and deleting relation types — and every other setting —
stays admin-only.

(cherry picked from commit 3171820)
@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Updates system resource authorization to allow any authenticated user to read glossary term relation types while keeping write operations admin-only. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-admin users cannot fetch full glossary term relation list when adding related terms

2 participants