diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index 862a5a1ef8..4a2ac46681 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -10,7 +10,6 @@ import java.io.IOException; import java.time.Duration; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -276,11 +275,25 @@ public static String putSharingInfoPayload( """.formatted(resourceId, resourceType, accessLevel, recipient.getName(), entity); } + public static String putGeneralAccessPayload(String resourceId, String resourceType, String accessLevel) { + return """ + { + "resource_id": "%s", + "resource_type": "%s", + "share_with": { + "general_access": "%s" + } + } + """.formatted(resourceId, resourceType, accessLevel); + } + public static class PatchSharingInfoPayloadBuilder { private String resourceId; private String resourceType; private final Map share = new HashMap<>(); private final Map revoke = new HashMap<>(); + private boolean generalAccessPresent; + private String generalAccess; public PatchSharingInfoPayloadBuilder resourceId(String resourceId) { this.resourceId = resourceId; @@ -300,49 +313,42 @@ public void share(Recipients recipients, String accessLevel) { public void revoke(Recipients recipients, String accessLevel) { Recipients existing = revoke.getOrDefault(accessLevel, new Recipients(new HashMap<>())); - // intentionally share() is called here since we are building a shareWith object, this final object will be used to remove - // access - // think of it as currentShareWith.removeAll(revokeShareWith) existing.share(recipients); revoke.put(accessLevel, existing); } - private String buildJsonString(Map input) { - - List output = new ArrayList<>(); - for (Map.Entry entry : input.entrySet()) { - try { - XContentBuilder builder = XContentFactory.jsonBuilder(); - entry.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS); - String recipJson = builder.toString(); - output.add(""" - "%s" : %s - """.formatted(entry.getKey(), recipJson)); - } catch (IOException e) { - throw new RuntimeException(e); - } - - } - - return String.join(",", output); - + public void revokeGeneralAccess() { + this.generalAccessPresent = true; + this.generalAccess = null; } public String build() { - String allShares = buildJsonString(share); - String allRevokes = buildJsonString(revoke); + String generalAccessField = generalAccessPresent + ? (generalAccess != null ? "\"general_access\": \"" + generalAccess + "\"," : "\"general_access\": null,") + : ""; return """ { "resource_id": "%s", "resource_type": "%s", - "add": { - %s - }, - "revoke": { - %s - } + %s + "add": %s, + "revoke": %s } - """.formatted(resourceId, resourceType, allShares, allRevokes); + """.formatted(resourceId, resourceType, generalAccessField, buildSection(share), buildSection(revoke)); + } + + // Produces e.g.: {"sample_read_only":{"users":["alice"]}} + private String buildSection(Map named) { + try { + XContentBuilder b = XContentFactory.jsonBuilder().startObject(); + for (Map.Entry entry : named.entrySet()) { + b.field(entry.getKey()); + entry.getValue().toXContent(b, ToXContent.EMPTY_PARAMS); + } + return b.endObject().toString(); + } catch (IOException e) { + throw new RuntimeException(e); + } } } @@ -610,6 +616,22 @@ public TestRestClient.HttpResponse shareResource( } } + public TestRestClient.HttpResponse shareResourceGenerally(String resourceId, TestSecurityConfig.User user, String accessLevel) { + try (TestRestClient client = cluster.getRestClient(user)) { + return client.putJson(SECURITY_SHARE_ENDPOINT, putGeneralAccessPayload(resourceId, RESOURCE_TYPE, accessLevel)); + } + } + + public TestRestClient.HttpResponse revokeGeneralAccess(String resourceId, TestSecurityConfig.User user) { + PatchSharingInfoPayloadBuilder patchBuilder = new PatchSharingInfoPayloadBuilder(); + patchBuilder.resourceType(RESOURCE_TYPE); + patchBuilder.resourceId(resourceId); + patchBuilder.revokeGeneralAccess(); + try (TestRestClient client = cluster.getRestClient(user)) { + return client.patch(SECURITY_SHARE_ENDPOINT, patchBuilder.build()); + } + } + public TestRestClient.HttpResponse shareResourceGroup( String resourceId, TestSecurityConfig.User user, @@ -728,5 +750,17 @@ public void awaitSharingEntry(String resourceId, String expectedString) { }); } } + + public void awaitResourceVisibility(String resourceId, String expectedPrincipal) { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + Awaitility.await("Wait for all_shared_principals to contain " + expectedPrincipal + " for resource " + resourceId) + .pollInterval(Duration.ofMillis(500)) + .untilAsserted(() -> { + TestRestClient.HttpResponse response = client.get(RESOURCE_INDEX_NAME + "/_doc/" + resourceId); + response.assertStatusCode(200); + assertThat(response.getBody(), containsString(expectedPrincipal)); + }); + } + } } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java index 26dc6ca2f5..952aff3277 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java @@ -17,24 +17,28 @@ import org.junit.runner.RunWith; import org.opensearch.sample.resource.TestUtils; -import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; import static org.opensearch.sample.resource.TestUtils.FULL_ACCESS_USER; import static org.opensearch.sample.resource.TestUtils.LIMITED_ACCESS_USER; +import static org.opensearch.sample.resource.TestUtils.NO_ACCESS_USER; import static org.opensearch.sample.resource.TestUtils.SAMPLE_FULL_ACCESS; import static org.opensearch.sample.resource.TestUtils.SAMPLE_READ_ONLY; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_READ_WRITE; import static org.opensearch.sample.resource.TestUtils.newCluster; +import static org.opensearch.security.api.AbstractApiIntegrationTest.badRequest; import static org.opensearch.security.api.AbstractApiIntegrationTest.forbidden; import static org.opensearch.security.api.AbstractApiIntegrationTest.ok; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; /** - * Test resource access to a publicly shared resource at different access-levels. - * All tests are against USER_ADMIN's resource created during setup. + * Tests for general_access (public) sharing at a specific access level. + * Verifies that general_access grants everyone the specified level, + * while named recipients can hold higher levels independently. */ @RunWith(RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) @@ -49,7 +53,7 @@ public class PubliclySharedDocTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(resourceId); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); } @After @@ -57,52 +61,162 @@ public void cleanup() { api.wipeOutResourceEntries(); } - private void assertNoAccessBeforeSharing(TestSecurityConfig.User user) throws Exception { - forbidden(() -> api.getResource(resourceId, user)); - forbidden(() -> api.updateResource(resourceId, user, "sampleUpdateAdmin")); - forbidden(() -> api.deleteResource(resourceId, user)); + @Test + public void generalAccess_readOnly_grantsEveryoneReadAccess() throws Exception { + // no access before sharing + forbidden(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + forbidden(() -> api.getResource(resourceId, LIMITED_ACCESS_USER)); + forbidden(() -> api.getResource(resourceId, NO_ACCESS_USER)); - forbidden(() -> api.shareResource(resourceId, user, user, SAMPLE_FULL_ACCESS)); - forbidden(() -> api.revokeResource(resourceId, user, user, SAMPLE_FULL_ACCESS)); - } + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); - private void assertReadOnly() throws Exception { + // everyone can read TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); assertThat(response.getBody(), containsString("sample")); - forbidden(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "sampleUpdateAdmin")); - forbidden(() -> api.deleteResource(resourceId, FULL_ACCESS_USER)); - forbidden(() -> api.shareResource(resourceId, FULL_ACCESS_USER, FULL_ACCESS_USER, SAMPLE_FULL_ACCESS)); - forbidden(() -> api.revokeResource(resourceId, FULL_ACCESS_USER, FULL_ACCESS_USER, SAMPLE_FULL_ACCESS)); + response = ok(() -> api.getResource(resourceId, LIMITED_ACCESS_USER)); + assertThat(response.getBody(), containsString("sample")); + + // but no one can write + forbidden(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "updated")); + forbidden(() -> api.deleteResource(resourceId, FULL_ACCESS_USER)); } - private void assertFullAccess() throws Exception { + @Test + public void generalAccess_readWrite_grantsEveryoneFullContentAccess() throws Exception { + forbidden(() -> api.getResource(resourceId, LIMITED_ACCESS_USER)); + + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_WRITE)); + TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, LIMITED_ACCESS_USER)); assertThat(response.getBody(), containsString("sample")); - ok(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "sampleUpdateAdmin")); - ok(() -> api.shareResource(resourceId, LIMITED_ACCESS_USER, TestUtils.LIMITED_ACCESS_USER, SAMPLE_FULL_ACCESS)); - ok(() -> api.revokeResource(resourceId, LIMITED_ACCESS_USER, USER_ADMIN, SAMPLE_FULL_ACCESS)); + + ok(() -> api.updateResource(resourceId, LIMITED_ACCESS_USER, "updated")); ok(() -> api.deleteResource(resourceId, LIMITED_ACCESS_USER)); } @Test - public void readOnly() throws Exception { - assertNoAccessBeforeSharing(FULL_ACCESS_USER); - // 1. share at read-only for full-access user and at full-access for limited perms user - ok(() -> api.shareResource(resourceId, USER_ADMIN, new TestSecurityConfig.User("*"), SAMPLE_READ_ONLY)); + public void generalAccess_readOnly_namedRecipientCanWrite() throws Exception { + // share publicly at read-only, but grant FULL_ACCESS_USER write access explicitly + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + ok(() -> api.shareResource(resourceId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_FULL_ACCESS)); - // 2. check read-only access for full-access user - assertReadOnly(); + // everyone can read + TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, LIMITED_ACCESS_USER)); + assertThat(response.getBody(), containsString("sample")); + + // only FULL_ACCESS_USER can write + ok(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "updated")); + forbidden(() -> api.updateResource(resourceId, LIMITED_ACCESS_USER, "updated")); + forbidden(() -> api.deleteResource(resourceId, LIMITED_ACCESS_USER)); + } + + @Test + public void revokeGeneralAccess_removesPublicAccess() throws Exception { + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + + // confirm access granted + ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + + // revoke general access + ok(() -> api.revokeGeneralAccess(resourceId, USER_ADMIN)); + + // access should be gone + forbidden(() -> api.getResource(resourceId, FULL_ACCESS_USER)); } @Test - public void fullAccess() throws Exception { - assertNoAccessBeforeSharing(LIMITED_ACCESS_USER); - // 1. share at read-only for full-access user and at full-access for limited perms user - ok(() -> api.shareResource(resourceId, USER_ADMIN, new TestSecurityConfig.User("*"), SAMPLE_FULL_ACCESS)); + public void generalAccess_doesNotLeakSharingInfo() throws Exception { + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + + // a user with only general read access cannot view or modify sharing info + forbidden(() -> api.shareResource(resourceId, FULL_ACCESS_USER, FULL_ACCESS_USER, SAMPLE_FULL_ACCESS)); + forbidden(() -> api.revokeGeneralAccess(resourceId, FULL_ACCESS_USER)); + } - // 2. check read-only access for full-access user - assertFullAccess(); + @Test + public void generalAccess_fullAccess_doesNotGrantSharePermission() throws Exception { + // setting general_access to a level that includes share permission must be rejected at the API + badRequest(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_FULL_ACCESS)); } + @Test + public void generalAccess_upgradeLevel_replacesExistingGeneralAccess() throws Exception { + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + forbidden(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "updated")); + + // upgrade general access to read_write (full content access, no share permission) + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_WRITE)); + + TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + assertThat(response.getBody(), containsString("sample")); + ok(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "updated")); + } + + @Test + public void generalAccess_resourceAppearsInListAndSearch() throws Exception { + // before sharing: list and search return 200 with empty results + TestRestClient.HttpResponse listResponse = ok(() -> api.listResources(FULL_ACCESS_USER)); + assertThat(listResponse.getBody(), not(containsString("sample"))); + + TestRestClient.HttpResponse searchResponse = ok(() -> api.searchResources(FULL_ACCESS_USER)); + assertThat(searchResponse.getBody(), not(containsString("sample"))); + + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + api.awaitResourceVisibility(resourceId, "public"); + + // resource should now appear in list + listResponse = ok(() -> api.listResources(FULL_ACCESS_USER)); + assertThat(listResponse.getBody(), containsString("sample")); + + // resource should appear in search (GET and POST) + searchResponse = ok(() -> api.searchResources(FULL_ACCESS_USER)); + assertThat(searchResponse.getBody(), containsString("sample")); + + searchResponse = ok(() -> api.searchResources(TestUtils.ApiHelper.searchAllPayload(), FULL_ACCESS_USER)); + TestUtils.ApiHelper.assertSearchResponse(searchResponse, 1, "sample"); + + searchResponse = ok(() -> api.searchResources(TestUtils.ApiHelper.searchByNamePayload("sample"), FULL_ACCESS_USER)); + TestUtils.ApiHelper.assertSearchResponse(searchResponse, 1, "sample"); + } + + @Test + public void revokeGeneralAccess_resourceDisappearsFromListAndSearch() throws Exception { + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + api.awaitResourceVisibility(resourceId, "public"); + + // confirm visible + TestRestClient.HttpResponse listResponse = ok(() -> api.listResources(FULL_ACCESS_USER)); + assertThat(listResponse.getBody(), containsString("sample")); + + ok(() -> api.revokeGeneralAccess(resourceId, USER_ADMIN)); + api.awaitResourceVisibility(resourceId, "user:admin"); // user:* sentinel removed, only creator remains + + // should no longer appear + listResponse = ok(() -> api.listResources(FULL_ACCESS_USER)); + assertThat(listResponse.getBody(), not(containsString("sample"))); + + TestRestClient.HttpResponse searchResponse = ok(() -> api.searchResources(FULL_ACCESS_USER)); + assertThat(searchResponse.getBody(), not(containsString("sample"))); + } + + @Test + public void generalAccess_sharingInfoResponse_containsGeneralAccessField() throws Exception { + ok(() -> api.shareResourceGenerally(resourceId, USER_ADMIN, SAMPLE_READ_ONLY)); + + try (var client = cluster.getRestClient(USER_ADMIN)) { + TestRestClient.HttpResponse response = ok( + () -> client.get( + TestUtils.SECURITY_SHARE_ENDPOINT + + "?resource_id=" + + resourceId + + "&resource_type=" + + org.opensearch.sample.utils.Constants.RESOURCE_TYPE + ) + ); + assertThat(response.getBody(), containsString("general_access")); + assertThat(response.getBody(), containsString(SAMPLE_READ_ONLY)); + assertThat(response.getBody(), not(containsString("\"users\""))); + } + } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index adbcbd1eb9..23193e0726 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -12,7 +12,6 @@ package org.opensearch.security.resources; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -228,6 +227,8 @@ public void patchSharingInfo( @NonNull String resourceType, @Nullable ShareWith add, @Nullable ShareWith revoke, + boolean generalAccessPresent, + @Nullable String generalAccess, ActionListener listener ) { final UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent( @@ -264,19 +265,27 @@ public void patchSharingInfo( revoke ); - this.resourceSharingIndexHandler.patchSharingInfo(resourceId, resourceIndex, add, revoke, ActionListener.wrap(sharingInfo -> { - LOGGER.debug("Successfully patched sharing info for resource {} with add: {}, revoke: {}", resourceId, add, revoke); - listener.onResponse(sharingInfo); - }, e -> { - LOGGER.error( - "Failed to patched sharing info for resource {} with add: {}, revoke: {} : {}", - resourceId, - add, - revoke, - e.getMessage() - ); - listener.onFailure(e); - })); + this.resourceSharingIndexHandler.patchSharingInfo( + resourceId, + resourceIndex, + add, + revoke, + generalAccessPresent, + generalAccess, + ActionListener.wrap(sharingInfo -> { + LOGGER.debug("Successfully patched sharing info for resource {} with add: {}, revoke: {}", resourceId, add, revoke); + listener.onResponse(sharingInfo); + }, e -> { + LOGGER.error( + "Failed to patched sharing info for resource {} with add: {}, revoke: {} : {}", + resourceId, + add, + revoke, + e.getMessage() + ); + listener.onFailure(e); + }) + ); } @@ -396,14 +405,10 @@ private void loadAllResourceSharingRecords(String resourceType, ActionListener getFlatPrincipals(User user) { // 1) collect all entities we’ll match against share_with arrays // for users: - Set users = new HashSet<>(); - users.add(user.getName()); - users.add("*"); // for matching against publicly shared resource - // return flattened principals to build the bool query return Stream.concat( - // users - users.stream().map(u -> "user:" + u), + // users, plus bare "public" sentinel for publicly shared resources + Stream.concat(Stream.of("user:" + user.getName(), "public"), Stream.empty()), // then roles and backend_roles Stream.concat(user.getSecurityRoles().stream().map(r -> "role:" + r), user.getRoles().stream().map(b -> "backend:" + b)) ).collect(Collectors.toSet()); diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java index 86ac188b37..13fd2e3e80 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java @@ -35,7 +35,7 @@ public static IndexToRuleMap resourceRestrictions( ) { List principals = new ArrayList<>(); - principals.add("user:*"); // Convention for publicly visible + principals.add("public"); // matches resources shared via general_access principals.add("user:" + user.getName()); // owner // Security roles (OpenSearch Security roles) diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 77ea71eafc..f29eb63a87 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -649,9 +649,11 @@ public void share(String resourceId, String resourceIndex, ShareWith shareWith, } for (String accessLevel : shareWith.accessLevels()) { Recipients target = shareWith.atAccessLevel(accessLevel); - sharingInfo.share(accessLevel, target); } + if (shareWith.getGeneralAccess() != null) { + sharingInfo.setGeneralAccess(shareWith.getGeneralAccess()); + } String resourceSharingIndex = getSharingIndex(resourceIndex); try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { @@ -707,6 +709,8 @@ public void patchSharingInfo( String resourceIndex, ShareWith add, ShareWith revoke, + boolean generalAccessPresent, + String generalAccess, ActionListener listener ) { @@ -719,10 +723,13 @@ public void patchSharingInfo( // Apply patch and update the document sharingInfoListener.whenComplete(sharingInfo -> { if (add != null) { - sharingInfo.getShareWith().add(add); + sharingInfo.applyAdd(add); } if (revoke != null) { - sharingInfo.getShareWith().revoke(revoke); + sharingInfo.applyRevoke(revoke); + } + if (generalAccessPresent) { + sharingInfo.setGeneralAccess(generalAccess); } try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { @@ -1173,10 +1180,8 @@ public boolean canUserShare(User user, boolean isAdmin, ResourceSharing resource if (isAdmin || resourceSharingRecord.isCreatedBy(user.getName())) return true; - if (resourceSharingRecord.isSharedWithEveryone()) return true; - var sw = resourceSharingRecord.getShareWith(); - if (sw == null || sw.getSharingInfo().isEmpty()) return false; + if (sw == null) return false; Set users = Set.of(user.getName()); Set roles = new HashSet<>(user.getSecurityRoles()); diff --git a/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java b/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java index 29435e0547..e29c8cd38e 100644 --- a/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java +++ b/src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java @@ -43,6 +43,10 @@ public class ShareRequest extends ActionRequest implements DocRequest { private final ShareWith add; @JsonProperty("revoke") private final ShareWith revoke; + /** True if general_access was explicitly present in the patch request (even if null). */ + private final boolean generalAccessPresent; + @JsonProperty("general_access") + private final String generalAccess; private final RestRequest.Method method; @@ -56,6 +60,8 @@ private ShareRequest(Builder builder) { this.shareWith = builder.shareWith; this.add = builder.add; this.revoke = builder.revoke; + this.generalAccessPresent = builder.generalAccessPresent; + this.generalAccess = builder.generalAccess; this.method = builder.method; } @@ -68,6 +74,8 @@ public ShareRequest(StreamInput in) throws IOException { this.shareWith = in.readOptionalWriteable(ShareWith::new); this.add = in.readOptionalWriteable(ShareWith::new); this.revoke = in.readOptionalWriteable(ShareWith::new); + this.generalAccessPresent = in.readBoolean(); + this.generalAccess = in.readOptionalString(); } @Override @@ -79,6 +87,8 @@ public void writeTo(StreamOutput out) throws IOException { out.writeOptionalWriteable(shareWith); out.writeOptionalWriteable(add); out.writeOptionalWriteable(revoke); + out.writeBoolean(generalAccessPresent); + out.writeOptionalString(generalAccess); } @Override @@ -98,8 +108,11 @@ public ActionRequestValidationException validate() { arv.addValidationError("share_with is required"); throw arv; } - if ((method == RestRequest.Method.PATCH || method == RestRequest.Method.POST) && add == null && revoke == null) { - arv.addValidationError("either add or revoke must be present"); + if ((method == RestRequest.Method.PATCH || method == RestRequest.Method.POST) + && add == null + && revoke == null + && !generalAccessPresent) { + arv.addValidationError("at least one of add, revoke, or general_access must be present"); throw arv; } return null; @@ -117,6 +130,14 @@ public ShareWith getRevoke() { return revoke; } + public boolean isGeneralAccessPresent() { + return generalAccessPresent; + } + + public String getGeneralAccess() { + return generalAccess; + } + public RestRequest.Method getMethod() { return method; } @@ -156,6 +177,8 @@ public static class Builder { ShareWith shareWith; ShareWith add; ShareWith revoke; + boolean generalAccessPresent; + String generalAccess; RestRequest.Method method; public void resourceId(String resourceId) { @@ -247,12 +270,39 @@ public void parseContent(XContentParser xContentParser, ResourcePluginInfo resou case "revoke": this.revoke(ShareWith.fromXContent(parser, accessLevelValidator)); break; + case "general_access": + this.generalAccessPresent = true; + this.generalAccess = parser.currentToken() == XContentParser.Token.VALUE_NULL ? null : parser.text(); + break; default: parser.skipChildren(); } } } } + rejectSharePermissionOnGeneralAccess(resourcePluginInfo); + } + + /** + * Rejects any general_access value whose resolved actions include share permission. + * general_access is content access only — it must never grant policy-control (share) permission. + */ + private void rejectSharePermissionOnGeneralAccess(ResourcePluginInfo resourcePluginInfo) { + if (resourceType == null) return; + var actionGroups = resourcePluginInfo.flattenedForType(resourceType); + // check top-level general_access (PATCH) and share_with.general_access (PUT) + String putLevel = shareWith != null ? shareWith.getGeneralAccess() : null; + for (String level : new String[] { generalAccess, putLevel }) { + if (level == null) continue; + if (actionGroups.resolve(Set.of(level)).contains(ShareAction.NAME)) { + throw new IllegalArgumentException( + "general_access level ['" + + level + + "'] includes share permission, which is not allowed. " + + "general_access controls content access only." + ); + } + } } } diff --git a/src/main/java/org/opensearch/security/resources/api/share/ShareTransportAction.java b/src/main/java/org/opensearch/security/resources/api/share/ShareTransportAction.java index f60a61414a..016d9216b7 100644 --- a/src/main/java/org/opensearch/security/resources/api/share/ShareTransportAction.java +++ b/src/main/java/org/opensearch/security/resources/api/share/ShareTransportAction.java @@ -51,6 +51,8 @@ protected void doExecute(Task task, ShareRequest request, ActionListener recipients.contains("*")); - } - public boolean isSharedWithAny(Recipient recipientType, Set targets) { return !Collections.disjoint(recipients.getOrDefault(recipientType, Collections.emptySet()), targets); } @@ -143,15 +139,7 @@ public static Recipients fromXContent( // Validate element value if validator provided // For "users" field, allow wildcard; for others, don't if (elementValidator != null) { - // Check if wildcard should be allowed based on recipient type - boolean allowWildcard = (recipient == Recipient.USERS); - if (allowWildcard) { - // Use wildcard-enabled validator for users - RequestContentValidator.principalValidator(true).validate(fieldName, value); - } else { - // Use standard validator for roles and backend_roles - elementValidator.validate(fieldName, value); - } + elementValidator.validate(fieldName, value); } values.add(value); diff --git a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java index 77bf9e88d5..9c051c8252 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java @@ -30,11 +30,11 @@ /** * Represents a resource sharing configuration that manages access control for OpenSearch resources. - * This class holds information about shared resources including their source, creator, and sharing permissions. + * This class holds information about shared resources including their creator and sharing permissions. * The class maintains information about: *
    - *
  • The source index where the resource is defined
    • *
  • The unique identifier of the resource
    • + *
  • The type of the resource
    • *
  • The creator's information
    • *
  • The sharing permissions and recipients
    • *
@@ -129,15 +129,26 @@ public void share(String accessLevel, Recipients target) { return; } Recipients sharedWith = shareWith.atAccessLevel(accessLevel); - // sharedWith will be null when sharing at a new access-level if (sharedWith == null) { - // update the ShareWith object shareWith = shareWith.updateSharingInfo(accessLevel, target); } else { sharedWith.share(target); } } + public void setGeneralAccess(String generalAccess) { + ShareWith current = getShareWith(); + shareWith = new ShareWith(current.getSharingInfo(), generalAccess); + } + + public void applyAdd(ShareWith add) { + shareWith = getShareWith().add(add); + } + + public void applyRevoke(ShareWith revoke) { + shareWith = getShareWith().revoke(revoke); + } + public void revoke(String accessLevel, Recipients target) { if (shareWith == null) { log.warn("Cannot revoke access as resource {} is not shared with anyone", this.resourceId); @@ -302,9 +313,9 @@ public boolean isCreatedBy(String userName) { } /** - * Checks if the given resource is shared with everyone, i.e. the entity list is "*" + * Checks if the given resource is shared with everyone via general access. * - * @return True if the resource is shared with everyone, false otherwise. + * @return True if the resource has general access set (i.e. publicly accessible), false otherwise. */ public boolean isSharedWithEveryone() { return this.shareWith != null && this.shareWith.isPublic(); @@ -345,16 +356,15 @@ public boolean isSharedWithEntity(Recipient recipientType, Set targets, * @return a {@link Set} of access level identifiers granted to the user, never {@code null}. */ public Set getAccessLevelsForUser(User user) { - Set userRoles = new HashSet<>(user.getSecurityRoles()); - Set userBackendRoles = new HashSet<>(user.getRoles()); + Set accessLevels = new HashSet<>(); - userRoles.add("*"); - userBackendRoles.add("*"); + if (shareWith != null && shareWith.getGeneralAccess() != null) { + accessLevels.add(shareWith.getGeneralAccess()); + } - Set accessLevels = new HashSet<>(); - accessLevels.addAll(fetchAccessLevels(Recipient.USERS, Set.of(user.getName(), "*"))); - accessLevels.addAll(fetchAccessLevels(Recipient.ROLES, userRoles)); - accessLevels.addAll(fetchAccessLevels(Recipient.BACKEND_ROLES, userBackendRoles)); + accessLevels.addAll(fetchAccessLevels(Recipient.USERS, Set.of(user.getName()))); + accessLevels.addAll(fetchAccessLevels(Recipient.ROLES, user.getSecurityRoles())); + accessLevels.addAll(fetchAccessLevels(Recipient.BACKEND_ROLES, user.getRoles())); return accessLevels; } @@ -362,7 +372,7 @@ public Set getAccessLevelsForUser(User user) { * Fetches all access-levels where at-least 1 recipient matches the given set of targets * @param recipientType the type of recipient to be matched against * @param entities targets to look for - * @return set of access-levels which contain given nay of the targets + * @return set of access-levels which contain any of the targets */ public Set fetchAccessLevels(Recipient recipientType, Set entities) { if (shareWith == null) { @@ -373,14 +383,9 @@ public Set fetchAccessLevels(Recipient recipientType, Set entiti String accessLevel = entry.getKey(); Recipients recipients = entry.getValue(); - Set sharingRecipients = new HashSet<>(recipients.getRecipients().getOrDefault(recipientType, Set.of())); + Set sharingRecipients = recipients.getRecipients().getOrDefault(recipientType, Set.of()); - // if there’s a wildcard (i.e. the document is shared publicly at this access-level), or at least one entity in common, add the - // level to a final list of groups - boolean matchesWildcard = sharingRecipients.contains("*"); - boolean intersects = !Collections.disjoint(sharingRecipients, entities); - - if (matchesWildcard || intersects) { + if (!Collections.disjoint(sharingRecipients, entities)) { matchingGroups.add(accessLevel); } } @@ -403,6 +408,9 @@ public List getAllPrincipals() { // Add shared recipients if (shareWith != null) { + if (shareWith.isPublic()) { + principals.add("public"); + } // shared with at any access level for (Recipients recipients : shareWith.getSharingInfo().values()) { Map> recipientMap = recipients.getRecipients(); diff --git a/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java b/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java index 69bf635ba7..0cbfd51a03 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java @@ -26,13 +26,15 @@ /** * This class contains information about whom a resource is shared with and what is the action-group associated with it. + * Optionally includes a {@code general_access} level that grants baseline access to all authenticated users. * *

Example usage: * 

  * "share_with": {
- *   "default": {
- *     "users": [],
- *     "roles": [],
+ *   "general_access": "read_only",
+ *   "read_write": {
+ *     "users": ["alice"],
+ *     "roles": ["editors"],
  *     "backend_roles": []
  *   }
  * }
@@ -48,21 +50,37 @@ public class ShareWith implements ToXContentFragment, NamedWriteable {
      */
     private final Map sharingInfo;
 
+    /**
+     * The access level granted to everyone (general/public access).
+     * e.g. "read" means anyone can read; named recipients may hold higher levels.
+     * Null means the resource is not publicly accessible.
+     */
+    private final String generalAccess;
+
     public ShareWith(Map sharingInfo) {
+        this(sharingInfo, null);
+    }
+
+    public ShareWith(Map sharingInfo, String generalAccess) {
         this.sharingInfo = sharingInfo;
+        this.generalAccess = generalAccess;
     }
 
     public ShareWith(StreamInput in) throws IOException {
         this.sharingInfo = in.readMap(StreamInput::readString, Recipients::new);
+        this.generalAccess = in.readOptionalString();
     }
 
     public boolean isPublic() {
-        // TODO Contemplate following google doc model of link sharing which has single access level when link sharing is enabled
-        return sharingInfo.values().stream().anyMatch(Recipients::isPublic);
+        return generalAccess != null;
+    }
+
+    public String getGeneralAccess() {
+        return generalAccess;
     }
 
     public boolean isPrivate() {
-        return sharingInfo == null || sharingInfo.isEmpty();
+        return generalAccess == null && (sharingInfo == null || sharingInfo.isEmpty());
     }
 
     public Set accessLevels() {
@@ -88,11 +106,14 @@ public ShareWith updateSharingInfo(String accessLevel, Recipients target) {
     @Override
     public XContentBuilder toXContent(XContentBuilder b, Params params) throws IOException {
         b.startObject();
+        if (generalAccess != null) {
+            b.field("general_access", generalAccess);
+        }
         for (Map.Entry e : this.sharingInfo.entrySet()) {
             Recipients norm = pruneRecipients(e.getValue());
             if (norm == null) continue; // skip empty level
             b.field(e.getKey());
-            norm.toXContent(b, params); // TODO ensure this skips empty arrays too
+            norm.toXContent(b, params);
         }
         return b.endObject();
     }
@@ -117,15 +138,23 @@ public static ShareWith fromXContent(XContentParser parser, RequestContentValida
             parser.nextToken();
         }
 
+        String generalAccess = null;
+
         XContentParser.Token token;
         while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
             // Each field in the object represents a SharedWithActionGroup
             if (token == XContentParser.Token.FIELD_NAME) {
-                String accessLevel = parser.currentName();
+                String fieldName = parser.currentName();
+
+                if ("general_access".equals(fieldName)) {
+                    parser.nextToken();
+                    generalAccess = parser.textOrNull();
+                    continue;
+                }
 
                 // Validate access level if validator is provided
                 if (accessLevelValidator != null) {
-                    accessLevelValidator.validate("access_level", accessLevel);
+                    accessLevelValidator.validate("access_level", fieldName);
                 }
 
                 parser.nextToken();
@@ -135,11 +164,11 @@ public static ShareWith fromXContent(XContentParser parser, RequestContentValida
                     RequestContentValidator.ARRAY_SIZE_VALIDATOR,
                     RequestContentValidator.principalValidator(false)
                 );
-                sharingInfo.put(accessLevel, recipients);
+                sharingInfo.put(fieldName, recipients);
             }
         }
 
-        return new ShareWith(sharingInfo);
+        return new ShareWith(sharingInfo, generalAccess);
     }
 
     @Override
@@ -150,6 +179,7 @@ public String getWriteableName() {
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeMap(sharingInfo, StreamOutput::writeString, (o, sw) -> sw.writeTo(o));
+        out.writeOptionalString(generalAccess);
     }
 
     @Override
@@ -161,7 +191,7 @@ public String toString() {
      * Returns a new ShareWith by merging this and another ShareWith (adding recipients).
      */
     public ShareWith add(ShareWith other) {
-        if (other == null || other.isPrivate()) {
+        if (other == null) {
             return this;
         }
         for (var entry : other.sharingInfo.entrySet()) {
@@ -172,14 +202,16 @@ public ShareWith add(ShareWith other) {
                 return orig;
             });
         }
-        return this;
+        // generalAccess in the patch overwrites the current value
+        String newGeneralAccess = other.generalAccess != null ? other.generalAccess : this.generalAccess;
+        return new ShareWith(this.sharingInfo, newGeneralAccess);
     }
 
     /**
      * Returns a new ShareWith by revoking recipients based on another ShareWith.
      */
     public ShareWith revoke(ShareWith other) {
-        if (this.sharingInfo.isEmpty() || other == null || other.isPrivate()) {
+        if (other == null || sharingInfo.isEmpty()) {
             return this;
         }
         for (var entry : other.sharingInfo.entrySet()) {
@@ -190,7 +222,9 @@ public ShareWith revoke(ShareWith other) {
                 return orig;
             });
         }
-        return this;
+        // clear generalAccess if the revoke patch specifies the same level
+        String newGeneralAccess = other.generalAccess != null && other.generalAccess.equals(this.generalAccess) ? null : this.generalAccess;
+        return new ShareWith(this.sharingInfo, newGeneralAccess);
     }
 
     /** Return a normalized ShareWith with no empty buckets and no empty action-groups. */
@@ -202,7 +236,7 @@ public ShareWith prune() {
                 cleaned.put(e.getKey(), prunedRecipients);
             }
         }
-        return new ShareWith(cleaned);
+        return new ShareWith(cleaned, generalAccess);
     }
 
     private static Recipients pruneRecipients(Recipients r) {
diff --git a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTests.java b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTests.java
index 8fdf5dd584..b6b42e1f5a 100644
--- a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTests.java
+++ b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTests.java
@@ -280,13 +280,13 @@ public void testPatchSharingInfoSuccess() {
 
         ResourceSharing doc = mock(ResourceSharing.class);
         doAnswer(inv -> {
-            ActionListener l = inv.getArgument(4);
+            ActionListener l = inv.getArgument(6);
             l.onResponse(doc);
             return null;
-        }).when(sharingIndexHandler).patchSharingInfo(eq(RESOURCE_ID), eq(INDEX), eq(add), eq(revoke), any());
+        }).when(sharingIndexHandler).patchSharingInfo(eq(RESOURCE_ID), eq(INDEX), eq(add), eq(revoke), eq(false), eq(null), any());
 
         ActionListener listener = mock(ActionListener.class);
-        handler.patchSharingInfo(RESOURCE_ID, TYPE, add, revoke, listener);
+        handler.patchSharingInfo(RESOURCE_ID, TYPE, add, revoke, false, null, listener);
 
         verify(listener).onResponse(doc);
     }
@@ -295,7 +295,7 @@ public void testPatchSharingInfoSuccess() {
     public void testPatchSharingInfoFailsIfNoUser() {
         ShareWith x = new ShareWith(ImmutableMap.of());
         ActionListener listener = mock(ActionListener.class);
-        handler.patchSharingInfo(RESOURCE_ID, TYPE, x, x, listener);
+        handler.patchSharingInfo(RESOURCE_ID, TYPE, x, x, false, null, listener);
 
         verify(listener).onFailure(any(OpenSearchStatusException.class));
     }
diff --git a/src/test/java/org/opensearch/security/resources/sharing/ResourceSharingTests.java b/src/test/java/org/opensearch/security/resources/sharing/ResourceSharingTests.java
index 1b505252f8..470863b632 100644
--- a/src/test/java/org/opensearch/security/resources/sharing/ResourceSharingTests.java
+++ b/src/test/java/org/opensearch/security/resources/sharing/ResourceSharingTests.java
@@ -195,11 +195,11 @@ public void isSharedWithEntity_handlesNullsAndDelegates() {
     }
 
     @Test
-    public void fetchAccessLevels_returnsLevelsWithWildcardOrIntersection() {
+    public void fetchAccessLevels_returnsLevelsWithIntersection() {
         CreatedBy cb = mockCreatedBy("o");
 
-        // level:read has users {"u1", "*"} -> wildcard = match
-        Recipients rRead = mockRecipients(Map.of(Recipient.USERS, new HashSet<>(Set.of("u1", "*"))));
+        // level:read has users {"u1", "u2"} -> intersection with {"u2"} = match
+        Recipients rRead = mockRecipients(Map.of(Recipient.USERS, new HashSet<>(Set.of("u1", "u2"))));
         // level:write has roles {"roleA"} -> no match for {"roleB"} unless overlap
         Recipients rWrite = mockRecipients(Map.of(Recipient.ROLES, new HashSet<>(Set.of("roleA"))));
         // level:admin has backend {"br1","br2"} -> intersection with {"br2"} = match
@@ -214,10 +214,13 @@ public void fetchAccessLevels_returnsLevelsWithWildcardOrIntersection() {
 
         ResourceSharing rs = ResourceSharing.builder().resourceId("r").createdBy(cb).shareWith(sw).build();
 
-        // users, looking for anything in {"any"} -> wildcard on read makes it match
-        Set levelsUsers = rs.fetchAccessLevels(Recipient.USERS, Set.of("any"));
+        // users, looking for {"u2"} -> intersection with read makes it match
+        Set levelsUsers = rs.fetchAccessLevels(Recipient.USERS, Set.of("u2"));
         assertEquals(Set.of("read"), levelsUsers);
 
+        // users, looking for {"unknown"} -> no match
+        assertTrue(rs.fetchAccessLevels(Recipient.USERS, Set.of("unknown")).isEmpty());
+
         // roles, looking for {"roleB"} -> no match; {"roleA"} -> match write
         assertTrue(rs.fetchAccessLevels(Recipient.ROLES, Set.of("roleB")).isEmpty());
         assertEquals(Set.of("write"), rs.fetchAccessLevels(Recipient.ROLES, Set.of("roleA")));
@@ -259,6 +262,53 @@ public void getAllPrincipals_includesCreatorAndAllRecipientsAcrossLevels() {
         assertEquals(new HashSet<>(principals).size(), principals.size());
     }
 
+    @Test
+    public void getAccessLevelsForUser_includesNamedRecipientsAndGeneralAccess() {
+        CreatedBy cb = mockCreatedBy("owner");
+
+        Recipients readRecipients = mockRecipients(Map.of(Recipient.USERS, new HashSet<>(Set.of("alice"))));
+        Recipients writeRecipients = mockRecipients(Map.of(Recipient.ROLES, new HashSet<>(Set.of("editor"))));
+
+        Map info = new HashMap<>();
+        info.put("read", readRecipients);
+        info.put("write", writeRecipients);
+
+        ShareWith sw = mock(ShareWith.class);
+        when(sw.getSharingInfo()).thenReturn(info);
+        when(sw.getGeneralAccess()).thenReturn("read");
+        info.forEach((level, r) -> when(sw.atAccessLevel(level)).thenReturn(r));
+
+        org.opensearch.security.user.User user = new org.opensearch.security.user.User("bob").withSecurityRoles(Set.of("editor"));
+
+        ResourceSharing rs = ResourceSharing.builder().resourceId("r").createdBy(cb).shareWith(sw).build();
+        Set levels = rs.getAccessLevelsForUser(user);
+
+        // general_access contributes "read"; role "editor" contributes "write"
+        assertEquals(Set.of("read", "write"), levels);
+    }
+
+    @Test
+    public void getAccessLevelsForUser_noGeneralAccess_onlyNamedRecipients() {
+        CreatedBy cb = mockCreatedBy("owner");
+
+        Recipients readRecipients = mockRecipients(Map.of(Recipient.USERS, new HashSet<>(Set.of("alice"))));
+
+        Map info = new HashMap<>();
+        info.put("read", readRecipients);
+
+        ShareWith sw = mock(ShareWith.class);
+        when(sw.getSharingInfo()).thenReturn(info);
+        when(sw.getGeneralAccess()).thenReturn(null);
+        when(sw.atAccessLevel("read")).thenReturn(readRecipients);
+
+        org.opensearch.security.user.User user = new org.opensearch.security.user.User("alice");
+
+        ResourceSharing rs = ResourceSharing.builder().resourceId("r").createdBy(cb).shareWith(sw).build();
+        Set levels = rs.getAccessLevelsForUser(user);
+
+        assertEquals(Set.of("read"), levels);
+    }
+
     @Test
     public void getAllPrincipals_handlesNullShareWith() {
         ResourceSharing rs = ResourceSharing.builder().resourceId("r").createdBy(mockCreatedBy("owner")).build();