Skip to content

Commit 5212eed

Browse files
DarshitChanpuraDarshitChanpura
authored
[Resource Sharing] Adds post support for update sharing info API (opensearch-project#5799)
Signed-off-by: Darshit Chanpura <[email protected]>
1 parent 892e99f commit 5212eed

File tree

6 files changed

+109
-19
lines changed

6 files changed

+109
-19
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3939
- Add security provider earlier in bootstrap process ([#5749](https://github.com/opensearch-project/security/pull/5749))
4040
- [GRPC] Fix compilation errors from core protobuf version bump to 0.23.0 ([#5763](https://github.com/opensearch-project/security/pull/5763))
4141
- Modularized PrivilegesEvaluator ([#5791](https://github.com/opensearch-project/security/pull/5791))
42+
- [Resource Sharing] Adds post support for update sharing info API ([#5799](https://github.com/opensearch-project/security/pull/5799))
4243

4344
### Maintenance
4445
- Bump `org.junit.jupiter:junit-jupiter` from 5.13.4 to 5.14.1 ([#5678](https://github.com/opensearch-project/security/pull/5678), [#5764](https://github.com/opensearch-project/security/pull/5764))

‎RESOURCE_SHARING_AND_ACCESS_CONTROL.md‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,8 @@ Read documents from a plugin’s index and migrate ownership and backend role-ba
639639
}
640640
```
641641

642+
**NOTE:** Can only be invoked successfully by a [REST admin](https://docs.opensearch.org/latest/security/access-control/api/#access-control-for-the-api) or an [admin certificate user](https://docs.opensearch.org/latest/security/access-control/users-roles/#super-admin-users).
643+
642644

643645
## **4. Resource Sharing API**
644646

@@ -700,10 +702,11 @@ Creates or replaces sharing settings for a resource.
700702
}
701703
```
702704

703-
### 2. `PATCH /_plugins/_security/api/resource/share`
705+
### 2. `PATCH /_plugins/_security/api/resource/share` and `POST /_plugins/_security/api/resource/share`
704706

705707
**Description:**
706708
Updates sharing settings by **adding** or **removing** recipients at any access level. Unlike `PUT`, this is a **non-destructive** operation.
709+
Can be used alternatively. POST version supports calls from dashboards.
707710

708711
**Request Body:**
709712

@@ -750,7 +753,7 @@ Updates sharing settings by **adding** or **removing** recipients at any access
750753
}
751754
```
752755

753-
#### Allowed Patch operations:
756+
#### Allowed Patch/Post operations:
754757
- `"add"` – Adds recipients
755758
- `"revoke"` – Removes recipients
756759

@@ -851,26 +854,28 @@ NOTE:
851854

852855
## Who Can Use This?
853856

854-
| API | Permission Required | Intended User |
855-
|-------------------------------------------------|-----------------------------------|-------------------|
857+
| API | Permission Required | Intended User |
858+
|--------------------------------------------------|-----------------------------------|-------------------|
856859
| `POST /_plugins/_security/api/resources/migrate` | REST admin or Super admin | Cluster admin |
857-
| `PUT /_plugins/_security/api/resource/share` | Resource Owner | Dashboards / REST |
858-
| `PATCH /_plugins/_security/api/resource/share` | Resource Owner / share permission | Dashboards / REST |
859-
| `GET /_plugins/_security/api/resource/share` | Resource Owner / read permission | Dashboards / REST |
860-
| `GET /_plugins/_security/api/resource/types` | Dashboard access | Dashboards |
861-
| `GET /_plugins/_security/api/resource/list` | Dashboard access | Dashboards |
860+
| `PUT /_plugins/_security/api/resource/share` | Resource Owner | Dashboards / REST |
861+
| `PATCH /_plugins/_security/api/resource/share` | Resource Owner / share permission | REST |
862+
| `POST /_plugins/_security/api/resource/share` | Resource Owner / share permission | Dashboards / REST |
863+
| `GET /_plugins/_security/api/resource/share` | Resource Owner / read permission | Dashboards / REST |
864+
| `GET /_plugins/_security/api/resource/types` | Dashboard access | Dashboards |
865+
| `GET /_plugins/_security/api/resource/list` | Dashboard access | Dashboards |
862866

863867

864868
## When to Use
865869

866-
| Use Case | API |
867-
|-------------------------------------------------------------|--------------------------------------------------|
868-
| Migrating existing plugin-specific sharing configs | `POST /_plugins/_security/api/resources/migrate` |
869-
| Sharing a document with another user or role | `PUT /_plugins/_security/api/resource/share` |
870-
| Granting/revoking access without affecting others | `PATCH /_plugins/_security/api/resource/share` |
871-
| Fetching the current sharing status of a resource | `GET /_plugins/_security/api/resource/share` |
872-
| Listing resource type. Encouraged only for dashboard access | `GET /_plugins/_security/api/resource/types` |
873-
| Listing accessible resources in given resource index. | `GET /_plugins/_security/api/resource/list` |
870+
| Use Case | API |
871+
|----------------------------------------------------------------|--------------------------------------------------|
872+
| Migrating existing plugin-specific sharing configs | `POST /_plugins/_security/api/resources/migrate` |
873+
| Sharing a document with another user or role | `PUT /_plugins/_security/api/resource/share` |
874+
| Granting/revoking access without affecting others | `PATCH /_plugins/_security/api/resource/share` |
875+
| Granting/revoking access without affecting others (dashboards) | `POST /_plugins/_security/api/resource/share` |
876+
| Fetching the current sharing status of a resource | `GET /_plugins/_security/api/resource/share` |
877+
| Listing resource type. Encouraged only for dashboard access | `GET /_plugins/_security/api/resource/types` |
878+
| Listing accessible resources in given resource index. | `GET /_plugins/_security/api/resource/list` |
874879

875880

876881
## **5. Best Practices For Users & Admins**

‎sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import com.carrotsearch.randomizedtesting.RandomizedRunner;
1717
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
18+
import org.apache.hc.core5.http.ContentType;
19+
import org.apache.hc.core5.http.io.entity.StringEntity;
1820
import org.apache.http.HttpStatus;
1921
import org.junit.After;
2022
import org.junit.Before;
@@ -260,6 +262,86 @@ public void testPatchSharingInfo() {
260262
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
261263
}
262264
}
265+
266+
@Test
267+
public void testPostSharingInfo() {
268+
Map<Recipient, Set<String>> recs = new HashMap<>();
269+
Set<String> users = new HashSet<>();
270+
users.add(FULL_ACCESS_USER.getName());
271+
recs.put(Recipient.USERS, users);
272+
Recipients recipients = new Recipients(recs);
273+
274+
TestUtils.PatchSharingInfoPayloadBuilder patchSharingInfoPayloadBuilder = new TestUtils.PatchSharingInfoPayloadBuilder();
275+
patchSharingInfoPayloadBuilder.resourceId(adminResId).resourceType(RESOURCE_TYPE).share(recipients, SAMPLE_FULL_ACCESS);
276+
277+
// full-access user cannot share with itself since user doesn't have permission to share
278+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
279+
TestRestClient.HttpResponse response = client.post(
280+
SECURITY_SHARE_ENDPOINT,
281+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
282+
);
283+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
284+
}
285+
286+
// a sharing entry should be created successfully since admin has access to share API
287+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
288+
TestRestClient.HttpResponse response = client.post(
289+
SECURITY_SHARE_ENDPOINT,
290+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
291+
);
292+
response.assertStatusCode(HttpStatus.SC_OK);
293+
assertThat(response.getBody(), containsString(FULL_ACCESS_USER.getName()));
294+
}
295+
296+
// limited access user will not be able to call patch endpoint
297+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
298+
TestRestClient.HttpResponse response = client.post(
299+
SECURITY_SHARE_ENDPOINT,
300+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
301+
);
302+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
303+
}
304+
305+
// full-access user will now be able to patch and grant access to limited access user
306+
// they can also shoot themselves in the foot and remove own access
307+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
308+
// add limited user
309+
users.add(LIMITED_ACCESS_USER.getName());
310+
patchSharingInfoPayloadBuilder.share(recipients, SAMPLE_FULL_ACCESS);
311+
// remove self
312+
Set<String> revokedUsers = new HashSet<>();
313+
revokedUsers.add(FULL_ACCESS_USER.getName());
314+
recs.put(Recipient.USERS, revokedUsers);
315+
recipients = new Recipients(recs);
316+
patchSharingInfoPayloadBuilder.revoke(recipients, SAMPLE_FULL_ACCESS);
317+
318+
TestRestClient.HttpResponse response = client.post(
319+
SECURITY_SHARE_ENDPOINT,
320+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
321+
);
322+
response.assertStatusCode(HttpStatus.SC_OK);
323+
}
324+
325+
// limited access user will now be able to call get and patch endpoint, but full-access won't
326+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
327+
TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId);
328+
response.assertStatusCode(HttpStatus.SC_OK);
329+
response = client.post(
330+
SECURITY_SHARE_ENDPOINT,
331+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
332+
);
333+
response.assertStatusCode(HttpStatus.SC_OK);
334+
}
335+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
336+
TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId);
337+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
338+
response = client.post(
339+
SECURITY_SHARE_ENDPOINT,
340+
new StringEntity(patchSharingInfoPayloadBuilder.build(), ContentType.APPLICATION_JSON)
341+
);
342+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
343+
}
344+
}
263345
}
264346

265347
}

‎src/main/java/org/opensearch/security/resources/api/share/ShareRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public ActionRequestValidationException validate() {
9595
arv.addValidationError("share_with is required");
9696
throw arv;
9797
}
98-
if (method == RestRequest.Method.PATCH && add == null && revoke == null) {
98+
if ((method == RestRequest.Method.PATCH || method == RestRequest.Method.POST) && add == null && revoke == null) {
9999
arv.addValidationError("either add or revoke must be present");
100100
throw arv;
101101
}

‎src/main/java/org/opensearch/security/resources/api/share/ShareRestAction.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
import static org.opensearch.rest.RestRequest.Method.GET;
2828
import static org.opensearch.rest.RestRequest.Method.PATCH;
29+
import static org.opensearch.rest.RestRequest.Method.POST;
2930
import static org.opensearch.rest.RestRequest.Method.PUT;
3031
import static org.opensearch.security.dlic.rest.support.Utils.PLUGIN_API_RESOURCE_ROUTE_PREFIX;
3132
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
@@ -55,7 +56,7 @@ public ShareRestAction(
5556
@Override
5657
public List<Route> routes() {
5758
return addRoutesPrefix(
58-
ImmutableList.of(new Route(PUT, "/share"), new Route(GET, "/share"), new Route(PATCH, "/share")),
59+
ImmutableList.of(new Route(PUT, "/share"), new Route(GET, "/share"), new Route(PATCH, "/share"), new Route(POST, "/share")),
5960
PLUGIN_API_RESOURCE_ROUTE_PREFIX
6061
);
6162
}

‎src/main/java/org/opensearch/security/resources/api/share/ShareTransportAction.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ protected void doExecute(Task task, ShareRequest request, ActionListener<ShareRe
4545
resourceAccessHandler.getSharingInfo(request.id(), request.type(), sharingInfoListener);
4646
return;
4747
case PATCH:
48+
case POST:
4849
resourceAccessHandler.patchSharingInfo(
4950
request.id(),
5051
request.type(),

0 commit comments

Comments
 (0)