fix(security): don't grant reviewer authz shortcut on CREATE by sonika-shah · Pull Request #30026 · open-metadata/OpenMetadata

sonika-shah · 2026-07-14T07:49:14Z

Fixes #29837

Problem

DefaultAuthorizer.authorize() short-circuits policy evaluation when the caller is a reviewer of the resource:

if (subjectContext.isAdmin()) return;
if (isReviewer(resourceContext, subjectContext)) return;   // <-- fires before RBAC
PolicyEvaluator.hasPermission(subjectContext, resourceContext, operationContext);

For CREATE operations the ResourceContextInterface is a CreateResourceContext, whose getEntity() returns the unsaved, caller-supplied request body. isReviewer() reads entity.getReviewers() from that in-flight entity, so a user can add themselves to the reviewers field of a create request, make isReviewer() return true, and skip RBAC entirely — creating entities (e.g. Data Products) they have no Create permission for.

Because the REST API and the MCP tool share this authorization path, both are affected. The UI's own gating masks it, but the server does not enforce it.

Scope is broader than Data Products: any reviewer-supporting entity (Glossary, GlossaryTerm, DataProduct, Metric, Tag, DataContract, …) created via the standard path is exposed.

Reproduction (before this change)

Principal: a non-admin with only DataConsumer role → effective Create on dataProduct = notAllow.

Request	Result
`POST /dataProducts` (no reviewers)	403 `operations [Create] not allowed`
`POST /dataProducts` + `reviewers:[{self}]`	201 Created ← bypass

Fix

Gate the reviewer shortcut so it only applies to already persisted entities, never a CreateResourceContext:

DefaultAuthorizer.isReviewer() — the hard-coded shortcut (the reported vector).
RuleEvaluator.isReviewer() — the SpEL condition available to custom policies, which reads the same in-flight entity. No shipped policy uses it, but guarding it too closes the same bug class as defense-in-depth.

Reviewer-based authorization continues to work for Edit/View/approval operations on existing entities. Verified the legitimate paths are unaffected: glossary-term approval is an UPDATE on a persisted term (checkUpdatedByReviewer), and inherited parent reviewers are not present on the entity at authorize-time (prepareInternal never populates reviewers; storage runs in repository.create() after authorize).

Tests

DefaultAuthorizerTest.authorizeDoesNotTreatSelfAssignedReviewerAsReviewerOnCreate — policy evaluation still runs when a user self-assigns as reviewer during CREATE.
RuleEvaluatorTest.test_isReviewer — extended so a self-assigned reviewer on a CreateResourceContext returns false.

Both fail without the corresponding guard and pass with it. Full security/policy unit suites green: DefaultAuthorizerTest (14), RuleEvaluatorTest (24), SubjectContextTest (11), ExpressionValidatorTest (35).

Greptile Summary

This PR fixes the reviewer authorization shortcut for create requests. The main changes are:

Blocks reviewer shortcut evaluation for CreateResourceContext in DefaultAuthorizer.
Applies the same create-time reviewer guard in RuleEvaluator.isReviewer().
Adds tests for self-assigned reviewers during create authorization and policy expression evaluation.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java	Adds a create-context guard before reviewer shortcut authorization.
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java	Prevents `isReviewer()` from treating create-request reviewers as persisted reviewers.
openmetadata-service/src/test/java/org/openmetadata/service/security/DefaultAuthorizerTest.java	Adds coverage that create requests with self-assigned reviewers still call policy evaluation.
openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java	Adds coverage that `isReviewer()` is false for self-assigned reviewers on create contexts.

_{Reviews (2): Last reviewed commit: "fix(security): don't grant reviewer auth..." | Re-trigger Greptile}