Skip to content

Commit ad45784

Browse files
committed
Fix few tests
Signed-off-by: Craig Perkins <[email protected]>
1 parent de14df5 commit ad45784

File tree

4 files changed

+32
-37
lines changed

4 files changed

+32
-37
lines changed

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import java.util.ArrayList;
1414
import java.util.Collection;
1515
import java.util.Collections;
16-
import java.util.HashMap;
1716
import java.util.HashSet;
1817
import java.util.List;
1918
import java.util.Map;
@@ -720,33 +719,19 @@ public void patchSharingInfo(
720719

721720
// Apply patch and update the document
722721
sharingInfoListener.whenComplete(sharingInfo -> {
723-
ShareWith updatedShareWith = sharingInfo.getShareWith();
724-
if (updatedShareWith == null) {
725-
updatedShareWith = new ShareWith(new HashMap<>());
726-
}
727722
if (add != null) {
728-
updatedShareWith = updatedShareWith.add(add);
723+
sharingInfo.getShareWith().add(add);
729724
}
730725
if (revoke != null) {
731-
updatedShareWith = updatedShareWith.revoke(revoke);
726+
sharingInfo.getShareWith().revoke(revoke);
732727
}
733728

734-
ShareWith cleaned = null;
735-
if (updatedShareWith != null) {
736-
ShareWith pruned = updatedShareWith.prune();
737-
if (!pruned.isPrivate()) {
738-
cleaned = pruned; // store only if something non-empty remains
739-
}
740-
}
741-
742-
ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, sharingInfo.getCreatedBy(), cleaned);
743-
744729
try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) {
745730
// update the record
746731
IndexRequest ir = client.prepareIndex(resourceSharingIndex)
747732
.setId(resourceId)
748733
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE)
749-
.setSource(updatedSharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS))
734+
.setSource(sharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS))
750735
.setOpType(DocWriteRequest.OpType.INDEX)
751736
.request();
752737

@@ -762,13 +747,13 @@ public void patchSharingInfo(
762747
updateResourceVisibility(
763748
resourceId,
764749
resourceIndex,
765-
updatedSharingInfo.getAllPrincipals(),
750+
sharingInfo.getAllPrincipals(),
766751
ActionListener.wrap((updateResponse) -> {
767752
LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex);
768-
listener.onResponse(updatedSharingInfo);
753+
listener.onResponse(sharingInfo);
769754
}, (e) -> {
770755
LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e);
771-
listener.onResponse(updatedSharingInfo);
756+
listener.onResponse(sharingInfo);
772757
})
773758
);
774759

src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ public CreatedBy getCreatedBy() {
112112
}
113113

114114
public ShareWith getShareWith() {
115+
if (shareWith == null) {
116+
// never been shared before, private access
117+
return new ShareWith(new HashMap<>());
118+
}
115119
return shareWith;
116120
}
117121

@@ -251,13 +255,25 @@ public static ResourceSharing fromXContent(XContentParser parser) throws IOExcep
251255
b.resourceId(parser.text());
252256
break;
253257
case "resource_type":
254-
b.resourceType(parser.text());
258+
if (token == XContentParser.Token.VALUE_NULL) {
259+
b.resourceType(null);
260+
} else {
261+
b.resourceType(parser.text());
262+
}
255263
break;
256264
case "parent_type":
257-
b.parentType(parser.text());
265+
if (token == XContentParser.Token.VALUE_NULL) {
266+
b.parentType(null);
267+
} else {
268+
b.parentType(parser.text());
269+
}
258270
break;
259271
case "parent_id":
260-
b.parentId(parser.text());
272+
if (token == XContentParser.Token.VALUE_NULL) {
273+
b.parentId(null);
274+
} else {
275+
b.parentId(parser.text());
276+
}
261277
break;
262278
case "created_by":
263279
b.createdBy(CreatedBy.fromXContent(parser));

src/main/java/org/opensearch/security/resources/sharing/ShareWith.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,16 +141,15 @@ public ShareWith add(ShareWith other) {
141141
if (other == null || other.isPrivate()) {
142142
return this;
143143
}
144-
Map<String, Recipients> updated = new HashMap<>(this.sharingInfo);
145144
for (var entry : other.sharingInfo.entrySet()) {
146145
String level = entry.getKey();
147146
Recipients patchRecipients = entry.getValue();
148-
updated.merge(level, patchRecipients, (orig, patchRec) -> {
147+
sharingInfo.merge(level, patchRecipients, (orig, patchRec) -> {
149148
orig.share(patchRec);
150149
return orig;
151150
});
152151
}
153-
return new ShareWith(updated);
152+
return this;
154153
}
155154

156155
/**
@@ -160,16 +159,15 @@ public ShareWith revoke(ShareWith other) {
160159
if (this.sharingInfo.isEmpty() || other == null || other.isPrivate()) {
161160
return this;
162161
}
163-
Map<String, Recipients> updated = new HashMap<>(this.sharingInfo);
164162
for (var entry : other.sharingInfo.entrySet()) {
165163
String level = entry.getKey();
166-
Recipients revokeRecipients = entry.getValue();
167-
updated.computeIfPresent(level, (lvl, orig) -> {
168-
orig.revoke(revokeRecipients);
164+
Recipients toRevoke = entry.getValue();
165+
sharingInfo.computeIfPresent(level, (lvl, orig) -> {
166+
orig.revoke(toRevoke);
169167
return orig;
170168
});
171169
}
172-
return new ShareWith(updated);
170+
return this;
173171
}
174172

175173
/** Return a normalized ShareWith with no empty buckets and no empty action-groups. */

src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.opensearch.security.auth.UserSubjectImpl;
2525
import org.opensearch.security.configuration.AdminDNs;
2626
import org.opensearch.security.privileges.PrivilegesEvaluator;
27-
import org.opensearch.security.resources.sharing.Recipient;
2827
import org.opensearch.security.resources.sharing.ResourceSharing;
2928
import org.opensearch.security.resources.sharing.ShareWith;
3029
import org.opensearch.security.securityconf.FlattenedActionGroups;
@@ -124,10 +123,7 @@ public void testHasPermission_sharedWithUserAllowed() {
124123

125124
// Document setup: shared with the user at access-level "read"
126125
ResourceSharing doc = mock(ResourceSharing.class);
127-
when(doc.isCreatedBy("bob")).thenReturn(false);
128-
when(doc.fetchAccessLevels(eq(Recipient.USERS), any())).thenReturn(Set.of("read"));
129-
when(doc.fetchAccessLevels(eq(Recipient.ROLES), any())).thenReturn(Set.of());
130-
when(doc.fetchAccessLevels(eq(Recipient.BACKEND_ROLES), any())).thenReturn(Set.of());
126+
when(doc.getAccessLevelsForUser(user)).thenReturn(Set.of("read"));
131127

132128
FlattenedActionGroups ag = mock(FlattenedActionGroups.class);
133129
when(resourcePluginInfo.flattenedForType(TYPE)).thenReturn(ag);

0 commit comments

Comments
 (0)