Skip to content

Commit b707d73

Browse files
RyanHolstiendavid-leifker
authored andcommitted
fix(policy): fix for default view self policy (#19297)
1 parent 3f224a8 commit b707d73

4 files changed

Lines changed: 283 additions & 4 deletions

File tree

metadata-service/auth-impl/src/test/java/com/datahub/authorization/DataHubAuthorizerTest.java

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public class DataHubAuthorizerTest {
6969

7070
private static final Urn USER_WITH_ADMIN_ROLE =
7171
UrnUtils.getUrn("urn:li:corpuser:user-with-admin");
72+
// A user with no configured policies, exercising only the default self policies.
73+
private static final Urn SELF_USER = UrnUtils.getUrn("urn:li:corpuser:selfUser");
7274
private static final Urn USER_WITH_DOMAIN_ACCESS =
7375
UrnUtils.getUrn("urn:li:corpuser:domainAccessUser");
7476
private static final Urn USER_WITH_CONTAINER_ACCESS =
@@ -598,6 +600,43 @@ public void testAuthorizeNotGranted() throws Exception {
598600
assertEquals(_dataHubAuthorizer.authorize(request).getType(), AuthorizationResult.Type.DENY);
599601
}
600602

603+
@Test
604+
public void testDefaultSelfPolicyAllowsReadOnSelf() throws Exception {
605+
606+
// No configured policy grants these; they come from the "View Self" default policy.
607+
for (String privilege : ImmutableList.of("VIEW_ENTITY_PAGE", "GET_ENTITY_PRIVILEGE")) {
608+
AuthorizationRequest request =
609+
new AuthorizationRequest(
610+
SELF_USER.toString(),
611+
privilege,
612+
Optional.of(selfEntitySpec()),
613+
Collections.emptyList());
614+
615+
assertEquals(
616+
_dataHubAuthorizer.authorize(request).getType(),
617+
AuthorizationResult.Type.ALLOW,
618+
String.format("Expected the View Self default policy to grant %s on self", privilege));
619+
}
620+
}
621+
622+
@Test
623+
public void testDefaultSelfPolicyDeniesWriteOnSelf() throws Exception {
624+
625+
for (String privilege : ImmutableList.of("EDIT_ENTITY", "DELETE_ENTITY")) {
626+
AuthorizationRequest request =
627+
new AuthorizationRequest(
628+
SELF_USER.toString(),
629+
privilege,
630+
Optional.of(selfEntitySpec()),
631+
Collections.emptyList());
632+
633+
assertEquals(
634+
_dataHubAuthorizer.authorize(request).getType(),
635+
AuthorizationResult.Type.DENY,
636+
String.format("Expected no default policy to grant %s on self", privilege));
637+
}
638+
}
639+
601640
@Test
602641
public void testAllowAllMode() throws Exception {
603642

@@ -1214,6 +1253,10 @@ private Map<Urn, EntityResponse> createEntityBatchResponse(
12141253
return batchResponse;
12151254
}
12161255

1256+
private EntitySpec selfEntitySpec() {
1257+
return new EntitySpec(CORP_USER_ENTITY_NAME, SELF_USER.toString());
1258+
}
1259+
12171260
private AuthorizerContext createAuthorizerContext(
12181261
final OperationContext systemOpContext, final SystemEntityClient entityClient) {
12191262
return new AuthorizerContext(

metadata-utils/src/main/java/com/linkedin/metadata/authorization/PoliciesConfig.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
package com.linkedin.metadata.authorization;
22

3-
import static com.linkedin.metadata.authorization.ApiGroup.ENTITY;
4-
import static com.linkedin.metadata.authorization.ApiOperation.READ;
53
import static com.linkedin.metadata.authorization.Disjunctive.DENY_ACCESS;
64

75
import com.google.common.collect.ImmutableList;
@@ -1416,8 +1414,7 @@ public static List<DataHubPolicyInfo> getDefaultPolicies(Urn actorUrn) {
14161414
.setDescription("View self entity page.")
14171415
.setActors(new DataHubActorFilter().setUsers(new UrnArray(actorUrn)))
14181416
.setPrivileges(
1419-
PoliciesConfig.API_PRIVILEGE_MAP.get(ENTITY).get(READ).stream()
1420-
.flatMap(Collection::stream)
1417+
Stream.of(VIEW_ENTITY_PAGE_PRIVILEGE, GET_ENTITY_PRIVILEGE)
14211418
.map(PoliciesConfig.Privilege::getType)
14221419
.collect(Collectors.toCollection(StringArray::new)))
14231420
.setType(PoliciesConfig.METADATA_POLICY_TYPE)
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package com.linkedin.metadata.authorization;
2+
3+
import static com.linkedin.metadata.authorization.ApiGroup.ENTITY;
4+
import static com.linkedin.metadata.authorization.PoliciesConfig.GET_ENTITY_PRIVILEGE;
5+
import static com.linkedin.metadata.authorization.PoliciesConfig.VIEW_ENTITY_PAGE_PRIVILEGE;
6+
import static org.testng.Assert.assertEquals;
7+
import static org.testng.Assert.assertFalse;
8+
import static org.testng.Assert.assertTrue;
9+
10+
import com.linkedin.common.urn.Urn;
11+
import com.linkedin.common.urn.UrnUtils;
12+
import com.linkedin.policy.DataHubPolicyInfo;
13+
import java.util.List;
14+
import java.util.Set;
15+
import java.util.stream.Collectors;
16+
import org.testng.annotations.Test;
17+
18+
/**
19+
* The default policies are granted to every actor on their own entity without an administrator
20+
* opting in, so any privilege they carry beyond read access is a self-service escalation: edit
21+
* access on your own corpuser entity is enough to grant yourself the Admin role.
22+
*/
23+
public class PoliciesConfigDefaultPoliciesTest {
24+
25+
private static final Urn ACTOR_URN = UrnUtils.getUrn("urn:li:corpuser:test");
26+
27+
@Test
28+
public void testDefaultPoliciesGrantOnlyEntityReadPrivileges() {
29+
assertEquals(
30+
grantedPrivilegeTypes(),
31+
Set.of(VIEW_ENTITY_PAGE_PRIVILEGE.getType(), GET_ENTITY_PRIVILEGE.getType()),
32+
"Default self policies must remain read-only");
33+
}
34+
35+
@Test
36+
public void testDefaultPoliciesDoNotSatisfyMutatingEntityOperations() {
37+
final Set<String> granted = grantedPrivilegeTypes();
38+
39+
assertTrue(
40+
satisfies(granted, ApiOperation.READ),
41+
"Default self policies must still allow read on self");
42+
43+
for (ApiOperation operation :
44+
List.of(
45+
ApiOperation.CREATE, ApiOperation.UPDATE, ApiOperation.DELETE, ApiOperation.EXECUTE)) {
46+
assertFalse(
47+
satisfies(granted, operation),
48+
String.format("Default self policies must not authorize ENTITY %s on self", operation));
49+
}
50+
}
51+
52+
private static Set<String> grantedPrivilegeTypes() {
53+
return PoliciesConfig.getDefaultPolicies(ACTOR_URN).stream()
54+
.map(DataHubPolicyInfo::getPrivileges)
55+
.flatMap(List::stream)
56+
.collect(Collectors.toSet());
57+
}
58+
59+
/**
60+
* Mirrors {@code AuthUtil.isAPIAuthorized}: an operation is authorized once the actor holds every
61+
* privilege of at least one of the disjoint conjunctives required for it.
62+
*/
63+
private static boolean satisfies(Set<String> granted, ApiOperation operation) {
64+
final Disjunctive<Conjunctive<PoliciesConfig.Privilege>> required =
65+
PoliciesConfig.API_PRIVILEGE_MAP.get(ENTITY).get(operation);
66+
return required.stream()
67+
.anyMatch(
68+
conjunctive ->
69+
!conjunctive.isEmpty()
70+
&& granted.containsAll(
71+
conjunctive.stream()
72+
.map(PoliciesConfig.Privilege::getType)
73+
.collect(Collectors.toSet())));
74+
}
75+
}
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
"""
2+
E2E authorization smoke tests for a non-admin acting on their own corpuser entity.
3+
4+
Self-service profile editing is allowed by an explicit self short-circuit in ``CorpUserType``,
5+
not by any policy. The generic write APIs have no such short-circuit and must keep denying
6+
writes to your own user entity - otherwise any user could patch their own roleMembership and
7+
grant themselves the Admin role.
8+
"""
9+
10+
import logging
11+
import uuid
12+
13+
import pytest
14+
15+
from datahub.metadata.schema_classes import RoleMembershipClass
16+
from tests.consistency_utils import wait_for_writes_to_sync
17+
from tests.privileges.utils import create_user, remove_user
18+
from tests.utils import (
19+
get_frontend_session,
20+
get_frontend_url,
21+
login_as,
22+
with_test_retry,
23+
)
24+
25+
logger = logging.getLogger(__name__)
26+
27+
pytestmark = pytest.mark.no_cypress_suite1
28+
29+
_UNIQUE = uuid.uuid4().hex[:8]
30+
TEST_USER_EMAIL = f"self.auth.test.{_UNIQUE}@smoke.datahub.test"
31+
TEST_USER_URN = f"urn:li:corpuser:{TEST_USER_EMAIL}"
32+
OTHER_USER_EMAIL = f"self.auth.other.{_UNIQUE}@smoke.datahub.test"
33+
OTHER_USER_URN = f"urn:li:corpuser:{OTHER_USER_EMAIL}"
34+
TEST_USER_PASSWORD = "user"
35+
36+
ADMIN_ROLE_URN = "urn:li:dataHubRole:Admin"
37+
38+
UPDATE_CORP_USER_PROPERTIES_MUTATION = """
39+
mutation updateCorpUserProperties($urn: String!, $input: CorpUserUpdateInput!) {
40+
updateCorpUserProperties(urn: $urn, input: $input) {
41+
urn
42+
editableProperties {
43+
aboutMe
44+
title
45+
}
46+
}
47+
}
48+
"""
49+
50+
PATCH_ENTITY_MUTATION = """
51+
mutation patchEntity($input: PatchEntityInput!) {
52+
patchEntity(input: $input) {
53+
urn
54+
success
55+
error
56+
}
57+
}
58+
"""
59+
60+
61+
@pytest.fixture(scope="module", autouse=True)
62+
def self_auth_setup(auth_session):
63+
admin_session = get_frontend_session()
64+
admin_session = create_user(admin_session, TEST_USER_EMAIL, TEST_USER_PASSWORD)
65+
admin_session = create_user(admin_session, OTHER_USER_EMAIL, TEST_USER_PASSWORD)
66+
67+
yield
68+
69+
remove_user(admin_session, TEST_USER_URN)
70+
remove_user(admin_session, OTHER_USER_URN)
71+
72+
73+
def _is_graphql_auth_denied(res: dict) -> bool:
74+
errors = res.get("errors") or []
75+
if not errors:
76+
return False
77+
code = errors[0].get("extensions", {}).get("code")
78+
return code in (401, 403)
79+
80+
81+
@with_test_retry(max_attempts=10)
82+
def _post_graphql_as_user(email: str, password: str, payload: dict) -> dict:
83+
user_session = login_as(email, password)
84+
response = user_session.post(f"{get_frontend_url()}/api/v2/graphql", json=payload)
85+
response.raise_for_status()
86+
return response.json()
87+
88+
89+
def _update_profile_payload(target_urn: str, about_me: str) -> dict:
90+
return {
91+
"query": UPDATE_CORP_USER_PROPERTIES_MUTATION,
92+
"variables": {
93+
"urn": target_urn,
94+
"input": {"aboutMe": about_me, "title": "Analyst"},
95+
},
96+
}
97+
98+
99+
def test_user_can_edit_own_profile():
100+
"""A user with no granted privileges can still edit their own profile."""
101+
about_me = f"Self edit {_UNIQUE}"
102+
res = _post_graphql_as_user(
103+
TEST_USER_EMAIL,
104+
TEST_USER_PASSWORD,
105+
_update_profile_payload(TEST_USER_URN, about_me),
106+
)
107+
108+
assert not _is_graphql_auth_denied(res), res
109+
editable = ((res.get("data") or {}).get("updateCorpUserProperties") or {}).get(
110+
"editableProperties"
111+
) or {}
112+
assert editable.get("aboutMe") == about_me, res
113+
assert editable.get("title") == "Analyst", res
114+
115+
116+
def test_user_cannot_edit_another_users_profile():
117+
"""The self short-circuit must not extend to other users' profiles."""
118+
res = _post_graphql_as_user(
119+
TEST_USER_EMAIL,
120+
TEST_USER_PASSWORD,
121+
_update_profile_payload(OTHER_USER_URN, f"Cross edit {_UNIQUE}"),
122+
)
123+
124+
assert _is_graphql_auth_denied(res), res
125+
126+
127+
def test_user_cannot_patch_own_role_membership(graph_client):
128+
"""A user must not be able to grant themselves the Admin role on their own entity.
129+
130+
patchEntity is gated on EDIT_ENTITY, which the default self policy must never grant.
131+
"""
132+
payload = {
133+
"query": PATCH_ENTITY_MUTATION,
134+
"variables": {
135+
"input": {
136+
"urn": TEST_USER_URN,
137+
"entityType": "corpuser",
138+
"aspectName": "roleMembership",
139+
"patch": [
140+
{
141+
"op": "ADD",
142+
"path": f"/roles/{ADMIN_ROLE_URN}",
143+
"value": ADMIN_ROLE_URN,
144+
}
145+
],
146+
"arrayPrimaryKeys": [{"arrayField": "roles", "keys": []}],
147+
"forceGenericPatch": True,
148+
}
149+
},
150+
}
151+
res = _post_graphql_as_user(TEST_USER_EMAIL, TEST_USER_PASSWORD, payload)
152+
153+
# PatchEntityResolver reports authorization failures in the payload rather than as a
154+
# GraphQL error, so check the reason too - success=False alone would also be satisfied
155+
# by an unrelated failure.
156+
result = (res.get("data") or {}).get("patchEntity") or {}
157+
assert result.get("success") is False, res
158+
assert "unauthorized" in (result.get("error") or "").lower(), res
159+
160+
wait_for_writes_to_sync(mcp_only=True)
161+
role_membership = graph_client.get_aspect(TEST_USER_URN, RoleMembershipClass)
162+
assert role_membership is None or ADMIN_ROLE_URN not in role_membership.roles, (
163+
f"Test user was granted {ADMIN_ROLE_URN}: {role_membership}"
164+
)

0 commit comments

Comments
 (0)