diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java index 7925377f5c..37530e0f2c 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/SampleExtensionPlugin.java @@ -51,11 +51,11 @@ import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceRestAction; import org.opensearch.security.sampleextension.actions.update.UpdateSampleResourceTransportAction; import org.opensearch.security.sampleextension.resource.SampleResource; -import org.opensearch.security.sampleextension.resource.SampleResourceFactory; +import org.opensearch.security.sampleextension.resource.SampleResourceParser; import org.opensearch.security.sampleextension.resource.SampleResourceSharingServiceProvider; import org.opensearch.security.spi.DefaultResourceSharingService; import org.opensearch.security.spi.Resource; -import org.opensearch.security.spi.ResourceFactory; +import org.opensearch.security.spi.ResourceParser; import org.opensearch.security.spi.ResourceSharingExtension; import org.opensearch.security.spi.ResourceSharingService; import org.opensearch.threadpool.ThreadPool; @@ -92,7 +92,7 @@ public Collection createComponents( if (SampleResourceSharingServiceProvider.getInstance().get() == null) { System.out.println("Using DefaultResourceSharingService"); SampleResourceSharingServiceProvider.getInstance() - .set(new DefaultResourceSharingService<>(client, RESOURCE_INDEX_NAME, new SampleResourceFactory())); + .set(new DefaultResourceSharingService<>(client, RESOURCE_INDEX_NAME, new SampleResourceParser(), xContentRegistry)); } System.out.println( "SampleResourceSharingServiceProvider.getInstance(): " + SampleResourceSharingServiceProvider.getInstance().get() @@ -145,8 +145,8 @@ public String getResourceIndex() { } @Override - public ResourceFactory getResourceFactory() { - return new SampleResourceFactory(); + public ResourceParser getResourceParser() { + return new SampleResourceParser(); } @SuppressWarnings("unchecked") diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java index 942449c8bb..3a8e04629e 100644 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResource.java @@ -1,14 +1,13 @@ package org.opensearch.security.sampleextension.resource; import java.io.IOException; -import java.util.Map; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.spi.Resource; -public class SampleResource extends Resource { +public class SampleResource implements Resource { private String name; @@ -22,12 +21,6 @@ public static SampleResource from(StreamInput in) throws IOException { return new SampleResource(in); } - @Override - public void fromSource(String resourceId, Map sourceAsMap) { - super.fromSource(resourceId, sourceAsMap); - this.name = (String) sourceAsMap.get("name"); - } - @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { return builder.startObject().field("name", name).endObject(); diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java deleted file mode 100644 index b902f38fb3..0000000000 --- a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceFactory.java +++ /dev/null @@ -1,10 +0,0 @@ -package org.opensearch.security.sampleextension.resource; - -import org.opensearch.security.spi.ResourceFactory; - -public class SampleResourceFactory implements ResourceFactory { - @Override - public SampleResource createResource() { - return new SampleResource(); - } -} diff --git a/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceParser.java b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceParser.java new file mode 100644 index 0000000000..6ecd4e2e73 --- /dev/null +++ b/sample-extension-plugin/src/main/java/org/opensearch/security/sampleextension/resource/SampleResourceParser.java @@ -0,0 +1,29 @@ +package org.opensearch.security.sampleextension.resource; + +import java.io.IOException; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.core.xcontent.XContentParserUtils; +import org.opensearch.security.spi.ResourceParser; + +public class SampleResourceParser implements ResourceParser { + + @Override + public SampleResource parse(XContentParser parser, String id) throws IOException { + SampleResource resource = new SampleResource(); + XContentParserUtils.ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser); + + while (!parser.nextToken().equals(XContentParser.Token.END_OBJECT)) { + String fieldName = parser.currentName(); + parser.nextToken(); + switch (fieldName) { + case "name": + resource.setName(parser.text()); + break; + default: + XContentParserUtils.throwUnknownToken(parser.currentToken(), parser.getTokenLocation()); + } + } + return resource; + } +} diff --git a/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java b/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java index f2c7ed2ed1..34167d3b69 100644 --- a/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java +++ b/spi/src/main/java/org/opensearch/security/spi/DefaultResourceSharingService.java @@ -1,5 +1,6 @@ package org.opensearch.security.spi; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -10,7 +11,12 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.XContentHelper; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; @@ -18,12 +24,19 @@ public class DefaultResourceSharingService implements ResourceSharingService { private final Client client; private final String resourceIndex; - private final ResourceFactory resourceFactory; + private final ResourceParser resourceParser; + private final NamedXContentRegistry xContentRegistry; - public DefaultResourceSharingService(Client client, String resourceIndex, ResourceFactory resourceFactory) { + public DefaultResourceSharingService( + Client client, + String resourceIndex, + ResourceParser resourceParser, + NamedXContentRegistry xContentRegistry + ) { this.client = client; this.resourceIndex = resourceIndex; - this.resourceFactory = resourceFactory; + this.resourceParser = resourceParser; + this.xContentRegistry = xContentRegistry; } @SuppressWarnings("unchecked") @@ -40,9 +53,18 @@ public void listResources(ActionListener> listResourceListener) { public void onResponse(SearchResponse searchResponse) { List resources = new ArrayList<>(); for (SearchHit hit : searchResponse.getHits().getHits()) { - T resource = resourceFactory.createResource(); - resource.fromSource(hit.getId(), hit.getSourceAsMap()); - resources.add(resource); + try { + XContentParser parser = XContentHelper.createParser( + xContentRegistry, + LoggingDeprecationHandler.INSTANCE, + hit.getSourceRef(), + XContentType.JSON + ); + T resource = resourceParser.parse(parser, hit.getId()); + resources.add(resource); + } catch (IOException e) { + throw new OpenSearchException("Caught exception while loading resources: " + e.getMessage()); + } } listResourceListener.onResponse(resources); } @@ -66,9 +88,18 @@ public void getResource(String resourceId, ActionListener getResourceListener ActionListener getListener = new ActionListener() { @Override public void onResponse(GetResponse getResponse) { - T resource = resourceFactory.createResource(); - resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap()); - getResourceListener.onResponse(resource); + try { + XContentParser parser = XContentHelper.createParser( + xContentRegistry, + LoggingDeprecationHandler.INSTANCE, + getResponse.getSourceAsBytesRef(), + XContentType.JSON + ); + T resource = resourceParser.parse(parser, getResponse.getId()); + getResourceListener.onResponse(resource); + } catch (IOException e) { + throw new OpenSearchException("Caught exception while loading resources: " + e.getMessage()); + } } @Override diff --git a/spi/src/main/java/org/opensearch/security/spi/Resource.java b/spi/src/main/java/org/opensearch/security/spi/Resource.java index 2ef8795874..b7df3ae131 100644 --- a/spi/src/main/java/org/opensearch/security/spi/Resource.java +++ b/spi/src/main/java/org/opensearch/security/spi/Resource.java @@ -1,18 +1,6 @@ package org.opensearch.security.spi; -import java.util.Map; - import org.opensearch.core.common.io.stream.NamedWriteable; -import org.opensearch.core.xcontent.ToXContentFragment; - -public abstract class Resource implements NamedWriteable, ToXContentFragment { - protected String resourceId; - - public String getResourceId() { - return resourceId; - } +import org.opensearch.core.xcontent.ToXContentObject; - public void fromSource(String resourceId, Map sourceAsMap) { - this.resourceId = resourceId; - } -} +public interface Resource extends NamedWriteable, ToXContentObject {} diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceDocVersion.java b/spi/src/main/java/org/opensearch/security/spi/ResourceDocVersion.java new file mode 100644 index 0000000000..80aabd8011 --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceDocVersion.java @@ -0,0 +1,94 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.security.spi; + +import java.io.IOException; +import java.util.Locale; + +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.common.io.stream.Writeable; + +/** + * Structure to represent resource document version. + */ +public class ResourceDocVersion implements Comparable, Writeable { + private final long primaryTerm; + private final long seqNo; + private final long version; + + public ResourceDocVersion(long primaryTerm, long seqNo, long version) { + this.primaryTerm = primaryTerm; + this.seqNo = seqNo; + this.version = version; + } + + public ResourceDocVersion(StreamInput in) throws IOException { + this.primaryTerm = in.readLong(); + this.seqNo = in.readLong(); + this.version = in.readLong(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeLong(this.primaryTerm); + out.writeLong(this.seqNo); + out.writeLong(this.version); + } + + public long getPrimaryTerm() { + return primaryTerm; + } + + public long getSeqNo() { + return seqNo; + } + + public long getVersion() { + return version; + } + + /** + * Compare two doc versions. Refer to https://github.com/elastic/elasticsearch/issues/10708 + * + * @param v the doc version to compare. + * @return -1 if this < v, 0 if this == v, otherwise 1; + */ + @Override + public int compareTo(ResourceDocVersion v) { + if (v == null) { + return 1; + } + if (this.seqNo < v.seqNo) { + return -1; + } + if (this.seqNo > v.seqNo) { + return 1; + } + if (this.primaryTerm < v.primaryTerm) { + return -1; + } + if (this.primaryTerm > v.primaryTerm) { + return 1; + } + return 0; + } + + @Override + public String toString() { + return String.format( + Locale.getDefault(), + "{_version: %s, _primary_term: %s, _seq_no: %s}", + this.version, + this.primaryTerm, + this.seqNo + ); + } +} diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java b/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java deleted file mode 100644 index ab86c7a819..0000000000 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceFactory.java +++ /dev/null @@ -1,5 +0,0 @@ -package org.opensearch.security.spi; - -public interface ResourceFactory { - T createResource(); -} diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceParser.java b/spi/src/main/java/org/opensearch/security/spi/ResourceParser.java new file mode 100644 index 0000000000..0e4a3ac0bd --- /dev/null +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceParser.java @@ -0,0 +1,9 @@ +package org.opensearch.security.spi; + +import java.io.IOException; + +import org.opensearch.core.xcontent.XContentParser; + +public interface ResourceParser { + T parse(XContentParser xContentParser, String id) throws IOException; +} diff --git a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java index 85cb73dcb5..1b92843d3e 100644 --- a/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java +++ b/spi/src/main/java/org/opensearch/security/spi/ResourceSharingExtension.java @@ -23,9 +23,9 @@ public interface ResourceSharingExtension { String getResourceIndex(); /** - * @return The class corresponding to this resource + * @return returns a parser for this resource */ - ResourceFactory getResourceFactory(); + ResourceParser getResourceParser(); void assignResourceSharingService(ResourceSharingService service); } diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 8aa1eb0e02..47f83c2a7f 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1081,7 +1081,8 @@ public Collection createComponents( ResourceSharingService resourceSharingService = new SecurityResourceSharingService<>( localClient, extension.getResourceIndex(), - extension.getResourceFactory() + extension.getResourceParser(), + xContentRegistry ); extension.assignResourceSharingService(resourceSharingService); } diff --git a/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java b/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java index 990360c0f9..c44ea872ba 100644 --- a/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java +++ b/src/main/java/org/opensearch/security/resource/SecurityResourceSharingService.java @@ -11,6 +11,7 @@ package org.opensearch.security.resource; +import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -25,14 +26,19 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.XContentHelper; +import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.QueryBuilders; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.security.rest.resource.ShareWith; import org.opensearch.security.spi.Resource; -import org.opensearch.security.spi.ResourceFactory; +import org.opensearch.security.spi.ResourceParser; import org.opensearch.security.spi.ResourceSharingService; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.WildcardMatcher; @@ -43,12 +49,19 @@ public class SecurityResourceSharingService implements ResourceSharingService { private final Client client; private final String resourceIndex; - private final ResourceFactory resourceFactory; + private final ResourceParser resourceParser; + private final NamedXContentRegistry xContentRegistry; - public SecurityResourceSharingService(Client client, String resourceIndex, ResourceFactory resourceFactory) { + public SecurityResourceSharingService( + Client client, + String resourceIndex, + ResourceParser resourceParser, + NamedXContentRegistry xContentRegistry + ) { this.client = client; this.resourceIndex = resourceIndex; - this.resourceFactory = resourceFactory; + this.resourceParser = resourceParser; + this.xContentRegistry = xContentRegistry; } private boolean hasPermissionsFor(User authenticatedUser, ResourceSharingEntry sharedWith) { @@ -131,10 +144,18 @@ public void onResponse(MultiGetResponse response) { if (singleResponse != null && !singleResponse.isFailed()) { GetResponse singleGetResponse = singleResponse.getResponse(); if (singleGetResponse.isExists() && !singleGetResponse.isSourceEmpty()) { - // TODO Is there a better way to create this instance of a generic w/o using reflection - T resource = resourceFactory.createResource(); - resource.fromSource(singleGetResponse.getId(), singleGetResponse.getSourceAsMap()); - resources.add(resource); + try { + XContentParser parser = XContentHelper.createParser( + xContentRegistry, + LoggingDeprecationHandler.INSTANCE, + singleGetResponse.getSourceAsBytesRef(), + XContentType.JSON + ); + T resource = resourceParser.parse(parser, singleGetResponse.getId()); + resources.add(resource); + } catch (IOException e) { + throw new OpenSearchException("Caught exception while loading resources: " + e.getMessage()); + } } else { // does not exist or empty source continue; @@ -214,9 +235,18 @@ private void finishGetResourceIfUserIsAllowed(String resourceId, ActionListener< ActionListener getListener = new ActionListener() { @Override public void onResponse(GetResponse getResponse) { - T resource = resourceFactory.createResource(); - resource.fromSource(getResponse.getId(), getResponse.getSourceAsMap()); - getResourceListener.onResponse(resource); + try { + XContentParser parser = XContentHelper.createParser( + xContentRegistry, + LoggingDeprecationHandler.INSTANCE, + getResponse.getSourceAsBytesRef(), + XContentType.JSON + ); + T resource = resourceParser.parse(parser, getResponse.getId()); + getResourceListener.onResponse(resource); + } catch (IOException e) { + throw new OpenSearchException("Caught exception while loading resources: " + e.getMessage()); + } } @Override