From 0e5a3b7bab56ead382412bc8d5dd58ee3e906922 Mon Sep 17 00:00:00 2001 From: Sun Cao Date: Thu, 15 Oct 2026 16:49:03 -0700 Subject: [PATCH 1/2] Introduce USE_RECIPIENT for reading other users' recipients. --- .../java/io/opensharing/auth/Privilege.java | 4 +- .../recipient/RecipientService.java | 44 ++++++++---- .../opensharing/recipient/RecipientStore.java | 14 ++++ .../recipient/RecipientAdminService.java | 25 ++++--- .../src/main/resources/local-catalog.yml | 4 +- .../recipient/RecipientReadPrivilegeTest.java | 67 +++++++++++++++++++ 6 files changed, 135 insertions(+), 23 deletions(-) create mode 100644 server/opensharing-server/src/test/java/io/opensharing/recipient/RecipientReadPrivilegeTest.java diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/auth/Privilege.java b/server/opensharing-server-core/src/main/java/io/opensharing/auth/Privilege.java index a1088bbe..ad772eb3 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/auth/Privilege.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/auth/Privilege.java @@ -5,5 +5,7 @@ public enum Privilege { CREATE_SHARE, CREATE_RECIPIENT, /** Lists and reads every share, not only the caller's own. */ - USE_SHARE + USE_SHARE, + /** Lists and reads every recipient, not only the caller's own. */ + USE_RECIPIENT } diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java index 27e02509..55862364 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientService.java @@ -2,10 +2,12 @@ import io.opensharing.ObjectNames; import io.opensharing.auth.AuthContext; +import io.opensharing.auth.Ownership; import io.opensharing.auth.Privilege; import io.opensharing.auth.TokenHashes; import io.opensharing.auth.UserContext; import io.opensharing.catalog.CatalogConnector; +import io.opensharing.exception.CatalogAuthorizationException; import io.opensharing.http.ApiException; import io.opensharing.http.ListResponse; import io.opensharing.http.Pagination; @@ -19,9 +21,10 @@ /** * Provider recipient CRUD and token issuance for a caller the host has authenticated. Creating a - * recipient needs CREATE_RECIPIENT from the catalog, and only the owner may change it. Only the - * owner and callers with USE_RECIPIENT, who may read anyone's recipients, are meant to list and get - * recipients. {@link #activate} is public: the activation code itself is the credential. + * recipient needs CREATE_RECIPIENT from the catalog, and only the owner may change it. The owner + * and callers with USE_RECIPIENT from the catalog may read a recipient; listing without + * USE_RECIPIENT returns only the caller's own recipients. {@link #activate} is public: the + * activation code itself is the credential. * *

{@code activationBaseUrl} is the absolute URL recipient activation links are built under, such * as {@code https://host/api/1.0/opensharing/activations}; the host knows its own address. @@ -72,18 +75,26 @@ public RecipientResponse create( return toResponse(recipient, activationBaseUrl); } - /** Lists one page of recipients by name. */ + /** + * Lists one page of recipients by name: every recipient with USE_RECIPIENT, otherwise the + * caller's own. + */ public ListResponse list( - Integer maxResults, String pageToken, String activationBaseUrl) { - // TODO: introduce USE_RECIPIENT and list only the caller's recipients unless they hold it. - return Pagination.page( - maxResults, pageToken, recipients::list, r -> toResponse(r, activationBaseUrl)); + UserContext user, Integer maxResults, String pageToken, String activationBaseUrl) { + Pagination.Slice slice = + holds(user, Privilege.USE_RECIPIENT) + ? recipients::list + : (offset, limit) -> recipients.listOwnedBy(user.userId(), offset, limit); + return Pagination.page(maxResults, pageToken, slice, r -> toResponse(r, activationBaseUrl)); } - /** Gets a recipient by name in any case. */ - public RecipientResponse get(String recipient, String activationBaseUrl) { - // TODO: introduce USE_RECIPIENT and get only the caller's recipient unless they hold it. - return toResponse(recipients.getOrThrow(recipient), activationBaseUrl); + /** Gets a recipient by name in any case. Owner or USE_RECIPIENT. */ + public RecipientResponse get(UserContext user, String recipient, String activationBaseUrl) { + RecipientEntity entity = recipients.getOrThrow(recipient); + if (!Ownership.owns(user, entity.getOwnerId())) { + catalog.authorize(AuthContext.of(user), Privilege.USE_RECIPIENT); + } + return toResponse(entity, activationBaseUrl); } /** Updates the fields set in {@code request}. Owner only. */ @@ -141,6 +152,15 @@ private static Instant expiresAt(Long days, Instant now) { return now.plus(Duration.ofDays(days)); } + private boolean holds(UserContext user, Privilege privilege) { + try { + catalog.authorize(AuthContext.of(user), privilege); + return true; + } catch (CatalogAuthorizationException e) { + return false; + } + } + private RecipientResponse toResponse(RecipientEntity recipient, String activationBaseUrl) { String code = recipients.findActivationCode(recipient); return RecipientResponse.from(recipient, code == null ? null : activationBaseUrl + "/" + code); diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java index 050c46a7..ffd5e38c 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java @@ -104,6 +104,20 @@ public List list(int offset, int limit) { .getResultList()); } + /** {@code SELECT * FROM os_recipients WHERE owner_id = ? ORDER BY name ASC LIMIT ? OFFSET ?}. */ + public List listOwnedBy(String ownerId, int offset, int limit) { + return tx.inTransaction( + true, + em -> + em.createQuery( + "select r from RecipientEntity r where r.ownerId = :ownerId order by r.name", + RecipientEntity.class) + .setParameter("ownerId", ownerId) + .setFirstResult(offset) + .setMaxResults(limit) + .getResultList()); + } + /** Deletes a recipient, its tokens and its permissions. Only the owner may delete it. */ public void delete(String name, UserContext user) { tx.inTransaction( diff --git a/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java b/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java index ae26c3c5..629d16a2 100644 --- a/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java +++ b/server/opensharing-server/src/main/java/io/opensharing/recipient/RecipientAdminService.java @@ -17,8 +17,8 @@ /** * Provider HTTP API for recipient CRUD, behind {@link ProviderTokenDecorator}. Only the owner may - * change a recipient. Only the owner and callers with USE_RECIPIENT, who may read anyone's - * recipients, are meant to list and get recipients. + * change a recipient. The owner and callers with USE_RECIPIENT may read it; listing without + * USE_RECIPIENT returns only the caller's own recipients. */ @Blocking @ProducesJson @@ -46,17 +46,26 @@ public RecipientResponse create(ServiceRequestContext ctx, CreateRecipientReques return recipients.create(ProviderTokenDecorator.user(ctx), request, activationBaseUrl); } - /** {@code GET /recipients}: lists one page of recipients by name. */ + /** + * {@code GET /recipients}: lists one page of recipients by name: every recipient with + * USE_RECIPIENT, otherwise the caller's own. + */ @Get("/recipients") public ListResponse list( - @Param @Nullable Integer maxResults, @Param @Nullable String pageToken) { - return recipients.list(maxResults, pageToken, activationBaseUrl); + ServiceRequestContext ctx, + @Param @Nullable Integer maxResults, + @Param @Nullable String pageToken) { + return recipients.list( + ProviderTokenDecorator.user(ctx), maxResults, pageToken, activationBaseUrl); } - /** {@code GET /recipients/{recipient}}: gets a recipient by name in any case. */ + /** + * {@code GET /recipients/{recipient}}: gets a recipient by name in any case. Owner or + * USE_RECIPIENT. + */ @Get("/recipients/{recipient}") - public RecipientResponse get(@Param String recipient) { - return recipients.get(recipient, activationBaseUrl); + public RecipientResponse get(ServiceRequestContext ctx, @Param String recipient) { + return recipients.get(ProviderTokenDecorator.user(ctx), recipient, activationBaseUrl); } /** {@code PATCH /recipients/{recipient}}: updates the fields set in the body. Owner only. */ diff --git a/server/opensharing-server/src/main/resources/local-catalog.yml b/server/opensharing-server/src/main/resources/local-catalog.yml index 28d73a21..4c01c06f 100644 --- a/server/opensharing-server/src/main/resources/local-catalog.yml +++ b/server/opensharing-server/src/main/resources/local-catalog.yml @@ -10,11 +10,11 @@ principals: - bearerToken: alice-token userId: catalog-alice-id userName: alice - privileges: [CREATE_SHARE, CREATE_RECIPIENT, USE_SHARE] + privileges: [CREATE_SHARE, CREATE_RECIPIENT, USE_SHARE, USE_RECIPIENT] - bearerToken: bob-token userId: catalog-bob-id userName: bob - privileges: [CREATE_SHARE, CREATE_RECIPIENT, USE_SHARE] + privileges: [CREATE_SHARE, CREATE_RECIPIENT, USE_SHARE, USE_RECIPIENT] - bearerToken: carol-token userId: catalog-carol-id userName: carol diff --git a/server/opensharing-server/src/test/java/io/opensharing/recipient/RecipientReadPrivilegeTest.java b/server/opensharing-server/src/test/java/io/opensharing/recipient/RecipientReadPrivilegeTest.java new file mode 100644 index 00000000..c73bb29a --- /dev/null +++ b/server/opensharing-server/src/test/java/io/opensharing/recipient/RecipientReadPrivilegeTest.java @@ -0,0 +1,67 @@ +package io.opensharing.recipient; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.opensharing.TestServer; +import io.opensharing.TestServer.Response; +import io.opensharing.http.ErrorCodes; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** In the local catalog, alice and bob hold USE_RECIPIENT and carol does not. */ +class RecipientReadPrivilegeTest { + + private static final String RECIPIENTS = "/api/1.0/opensharing/provider/recipients"; + + private static TestServer server; + + @BeforeAll + static void startServer() { + server = TestServer.start(); + create("alice-token", "alice_recipient"); + create("carol-token", "carol_recipient"); + } + + @AfterAll + static void stopServer() { + server.close(); + } + + @Test + void listsOnlyOwnRecipientsWithoutUseRecipient() { + assertEquals(List.of("carol_recipient"), names(server.get(RECIPIENTS, "carol-token"))); + assertEquals( + List.of("alice_recipient", "carol_recipient"), names(server.get(RECIPIENTS, "bob-token"))); + } + + @Test + void readsOthersRecipientsOnlyWithUseRecipient() { + assertEquals(200, server.get(RECIPIENTS + "/carol_recipient", "carol-token").status()); + assertEquals(200, server.get(RECIPIENTS + "/carol_recipient", "bob-token").status()); + + Response denied = server.get(RECIPIENTS + "/alice_recipient", "carol-token"); + assertEquals(403, denied.status()); + assertEquals(ErrorCodes.PERMISSION_DENIED, denied.errorCode()); + } + + private static void create(String token, String name) { + Response created = + server.post( + RECIPIENTS, + token, + "{\"name\":\"" + + name + + "\",\"authenticationType\":\"TOKEN\",\"tokenExpirationDays\":30}"); + assertEquals(201, created.status()); + } + + private static List names(Response response) { + assertEquals(200, response.status()); + List names = new ArrayList<>(); + response.body().path("items").forEach(item -> names.add(item.path("name").asText())); + return names; + } +} From 09b066da8d5a69719d6a4ae749e86f961054719f Mon Sep 17 00:00:00 2001 From: Sun Cao Date: Mon, 19 Oct 2026 17:01:57 -0700 Subject: [PATCH 2/2] Describe store queries in prose rather than SQL. --- .../src/main/java/io/opensharing/recipient/RecipientStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java index ffd5e38c..9af115db 100644 --- a/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java +++ b/server/opensharing-server-core/src/main/java/io/opensharing/recipient/RecipientStore.java @@ -104,7 +104,7 @@ public List list(int offset, int limit) { .getResultList()); } - /** {@code SELECT * FROM os_recipients WHERE owner_id = ? ORDER BY name ASC LIMIT ? OFFSET ?}. */ + /** Recipients owned by {@code ownerId}, ordered by name. */ public List listOwnedBy(String ownerId, int offset, int limit) { return tx.inTransaction( true,