Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue/247 duplicate resources #250

Merged
merged 17 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ public interface AuthorizationRule<R extends Resource>
*/
Optional<String> reasonDeleteAllowed(Identity identity, R oldResource);


/**
* Override this method for non default behavior. Default: Not allowed.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ private Optional<String> newResourceOk(Connection connection, StructureDefinitio
{
errors.add("StructureDefinition.version not defined");
}
if (!newResource.hasBaseDefinition())
{
errors.add("StructureDefinition.baseDefinition not defined");
}

if (!hasValidReadAccessTag(connection, newResource))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
Expand All @@ -15,6 +16,7 @@
import org.hl7.fhir.r4.model.ActivityDefinition;
import org.hl7.fhir.r4.model.CanonicalType;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.Identifier;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Resource;
import org.hl7.fhir.r4.model.StringType;
Expand All @@ -34,6 +36,10 @@
import dev.dsf.fhir.dao.TaskDao;
import dev.dsf.fhir.dao.provider.DaoProvider;
import dev.dsf.fhir.help.ParameterConverter;
import dev.dsf.fhir.search.PageAndCount;
import dev.dsf.fhir.search.PartialResult;
import dev.dsf.fhir.search.SearchQuery;
import dev.dsf.fhir.search.SearchQueryParameterError;
import dev.dsf.fhir.service.ReferenceResolver;
import dev.dsf.fhir.service.ResourceReference;

Expand Down Expand Up @@ -88,10 +94,20 @@ public Optional<String> reasonCreateAllowed(Connection connection, Identity iden
Optional<String> errors = draftTaskOk(connection, identity, newResource);
if (errors.isEmpty())
{
logger.info("Create of Task authorized for local organization identity '{}', Task.status draft",
identity.getName());
if (!draftTaskExists(connection, newResource))
{
logger.info(
"Create of Task authorized for local organization identity '{}', Task.status draft",
identity.getName());

return Optional.of("Local identity, Task.status draft");
}
else
{
logger.warn("Create of Task unauthorized, unique resource already exists");

return Optional.of("Local identity, Task.status draft");
return Optional.empty();
}
}
else
{
Expand Down Expand Up @@ -284,9 +300,16 @@ private Optional<String> draftTaskOk(Connection connection, Identity identity, T
{
List<String> errors = new ArrayList<>();

if (newResource.getIdentifier().stream().noneMatch(i -> NAMING_SYSTEM_TASK_IDENTIFIER.equals(i.getSystem())))
if (newResource.getIdentifier().stream().filter(i -> NAMING_SYSTEM_TASK_IDENTIFIER.equals(i.getSystem()))
.count() != 1)
{
errors.add("Task.identifier[" + NAMING_SYSTEM_TASK_IDENTIFIER + "] not defined or more than once");
}
else if (newResource.getIdentifier().stream().filter(i -> NAMING_SYSTEM_TASK_IDENTIFIER.equals(i.getSystem()))
.findFirst().filter(Identifier::hasValueElement).map(Identifier::getValueElement)
.filter(StringType::hasValue).isEmpty())
{
errors.add("Task.identifier[" + NAMING_SYSTEM_TASK_IDENTIFIER + "] missing");
errors.add("Task.identifier[" + NAMING_SYSTEM_TASK_IDENTIFIER + "] value not defined");
}

if (newResource.hasRequester())
Expand Down Expand Up @@ -399,6 +422,49 @@ private Stream<String> getMessageNames(Task newResource)
.filter(s -> !s.isBlank());
}

/**
* A draft {@link Task} with identifier (system {@link #NAMING_SYSTEM_TASK_IDENTIFIER}) may not exist
*
* @param connection
* not <code>null</code>
* @param newResource
* not <code>null</code>
* @return <code>true</code> if the given draft Task is unique
*/
private boolean draftTaskExists(Connection connection, Task newResource)
{
SearchQuery<Task> query = getDao().createSearchQueryWithoutUserFilter(PageAndCount.exists())
.configureParameters(Map.of("identifier",
List.of(NAMING_SYSTEM_TASK_IDENTIFIER + "|" + getDraftTaskIdentifierValue(newResource))));

List<SearchQueryParameterError> uQp = query.getUnsupportedQueryParameters();
if (!uQp.isEmpty())
{
logger.warn("Unable to search for Task: Unsupported query parameters: {}", uQp);

throw new IllegalStateException("Unable to search for Task: Unsupported query parameters");
}

try
{
PartialResult<Task> result = getDao().searchWithTransaction(connection, query);
return result.getTotal() >= 1;
}
catch (SQLException e)
{
logger.debug("Unable to search for Task", e);
logger.warn("Unable to search for Task: {} - {}", e.getClass().getName(), e.getMessage());

throw new RuntimeException("Unable to search for Task", e);
}
}

private String getDraftTaskIdentifierValue(Task newResource)
{
return newResource.getIdentifier().stream().filter(i -> NAMING_SYSTEM_TASK_IDENTIFIER.equals(i.getSystem()))
.findFirst().map(Identifier::getValue).get();
}

private boolean taskAllowedForRequesterAndRecipient(Connection connection, Identity requester, Task newResource)
{
Optional<Identity> recipientOpt = organizationProvider.getLocalOrganizationAsIdentity();
Expand Down Expand Up @@ -549,24 +615,24 @@ public Optional<String> reasonReadAllowed(Connection connection, Identity identi

if (identity.hasDsfRole(FhirServerRole.READ))
{
if (isCurrentIdentityPartOfReferencedOrganization(connection, identity, "Task.requester",
existingResource.getRequester()))
if (identity.isLocalIdentity() && isCurrentIdentityPartOfReferencedOrganization(connection, identity,
"Task.restriction.recipient", existingResource.getRestriction().getRecipientFirstRep()))
{
logger.info(
"Read of Task/{}/_history/{} authorized for identity '{}', Task.requester reference could be resolved and current identity part of referenced organization",
"Read of Task/{}/_history/{} authorized for identity '{}', Task.restriction.recipient reference could be resolved and current identity part of referenced organization",
resourceId, resourceVersion, identity.getName());

return Optional.of("Task.requester resolved and identity part of referenced organization");
return Optional
.of("Task.restriction.recipient resolved and local identity part of referenced organization");
}
else if (identity.isLocalIdentity() && isCurrentIdentityPartOfReferencedOrganization(connection, identity,
"Task.restriction.recipient", existingResource.getRestriction().getRecipientFirstRep()))
else if (isCurrentIdentityPartOfReferencedOrganization(connection, identity, "Task.requester",
existingResource.getRequester()))
{
logger.info(
"Read of Task/{}/_history/{} authorized for identity '{}', Task.restriction.recipient reference could be resolved and current identity part of referenced organization",
"Read of Task/{}/_history/{} authorized for identity '{}', Task.requester reference could be resolved and current identity part of referenced organization",
resourceId, resourceVersion, identity.getName());

return Optional
.of("Task.restriction.recipient resolved and local identity part of referenced organization");
return Optional.of("Task.requester resolved and identity part of referenced organization");
}
else
{
Expand Down Expand Up @@ -605,11 +671,23 @@ public Optional<String> reasonUpdateAllowed(Connection connection, Identity iden
Optional<String> errors = draftTaskOk(connection, identity, newResource);
if (errors.isEmpty())
{
logger.info("Update of Task/{}/_history/{} ({} -> {}) authorized for local identity '{}'",
oldResourceId, oldResourceVersion, TaskStatus.DRAFT.toCode(), TaskStatus.DRAFT.toCode(),
identity.getName());
if (draftTaskIdentifierSame(oldResource, newResource))
{
logger.info("Update of Task/{}/_history/{} ({} -> {}) authorized for local identity '{}'",
oldResourceId, oldResourceVersion, TaskStatus.DRAFT.toCode(),
TaskStatus.DRAFT.toCode(), identity.getName());

return Optional.of("Local identity, old Task.status draft, new Task.status draft");
return Optional.of("Local identity, old Task.status draft, new Task.status draft");
}
else
{
logger.warn(
"Update of Task/{}/_history/{} ({} -> {}) unauthorized for identity '{}' - identifier modified",
oldResourceId, oldResourceVersion, TaskStatus.DRAFT.toCode(),
TaskStatus.DRAFT.toCode(), identity.getName());

return Optional.empty();
}
}
else
{
Expand Down Expand Up @@ -790,6 +868,11 @@ else if (TaskStatus.INPROGRESS.equals(oldResource.getStatus())
}
}

private boolean draftTaskIdentifierSame(Task oldResource, Task newResource)
{
return Objects.equals(getDraftTaskIdentifierValue(oldResource), getDraftTaskIdentifierValue(newResource));
}

private Optional<String> reasonNotSame(Task oldResource, Task newResource)
{
List<String> errors = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import jakarta.ws.rs.core.Response.Status;
import jakarta.ws.rs.core.Response.Status.Family;

class AbstractCommandList
abstract class AbstractCommandList
{
private static final Logger audit = LoggerFactory.getLogger("dsf-audit-logger");

Expand Down Expand Up @@ -78,22 +78,22 @@ protected void auditLogAbbort(Command command)
{
if (command instanceof DeleteCommand)
{
audit.info("Delete of {} for identity '{}' via bundle at index {} abborted", command.getResourceTypeName(),
audit.info("Delete of {} for identity '{}' via bundle at index {} aborted", command.getResourceTypeName(),
command.getIdentity().getName(), command.getIndex());
}
else if (command instanceof CreateCommand)
{
audit.info("Create of {} for identity '{}' via bundle at index {} abborted", command.getResourceTypeName(),
audit.info("Create of {} for identity '{}' via bundle at index {} aborted", command.getResourceTypeName(),
command.getIdentity().getName(), command.getIndex());
}
else if (command instanceof UpdateCommand)
{
audit.info("Update of {} for identity '{}' via bundle at index {} abborted", command.getResourceTypeName(),
audit.info("Update of {} for identity '{}' via bundle at index {} aborted", command.getResourceTypeName(),
command.getIdentity().getName(), command.getIndex());
}
else if (command instanceof ReadCommand r)
{
audit.info("{} of {} for identity '{}' via bundle at index {} abborted", r.isSearch() ? "Search" : "Read",
audit.info("{} of {} for identity '{}' via bundle at index {} aborted", r.isSearch() ? "Search" : "Read",
command.getResourceTypeName(), command.getIdentity().getName(), command.getIndex());
}
}
Expand All @@ -119,7 +119,7 @@ protected BundleEntryComponent toEntry(Exception exception)
if (!(exception instanceof WebApplicationException w)
|| !(w.getResponse().getEntity() instanceof OperationOutcome))
{
exception = exceptionHandler.internalServerErrorBundleBatch(exception);
exception = internalServerError(exception);
}

Response httpResponse = ((WebApplicationException) exception).getResponse();
Expand All @@ -129,4 +129,6 @@ protected BundleEntryComponent toEntry(Exception exception)

return entry;
}

protected abstract Exception internalServerError(Exception exception);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,23 @@
import org.hl7.fhir.r4.model.Resource;

import dev.dsf.common.auth.conf.Identity;
import jakarta.ws.rs.WebApplicationException;

public interface AuthorizationHelper
{
void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource);
void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource)
throws WebApplicationException;

void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource);
void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource)
throws WebApplicationException;

void checkUpdateAllowed(int index, Connection connection, Identity identity, Resource oldResource,
Resource newResource);
Resource newResource) throws WebApplicationException;

void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource);
void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource)
throws WebApplicationException;

void checkSearchAllowed(int index, Identity identity, String resourceTypeName);
void checkSearchAllowed(int index, Identity identity, String resourceTypeName) throws WebApplicationException;

void filterIncludeResults(int index, Connection connection, Identity identity, Bundle multipleResult);
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ private WebApplicationException forbidden(String operation, Identity identity) t

@Override
public void checkCreateAllowed(int index, Connection connection, Identity identity, Resource newResource)
throws WebApplicationException
{
final String resourceTypeName = getResourceTypeName(newResource);

Expand All @@ -77,6 +78,7 @@ private String getResourceTypeName(Resource resource)

@Override
public void checkReadAllowed(int index, Connection connection, Identity identity, Resource existingResource)
throws WebApplicationException
{
final String resourceTypeName = getResourceTypeName(existingResource);
final String resourceId = existingResource.getIdElement().getIdPart();
Expand All @@ -98,7 +100,7 @@ public void checkReadAllowed(int index, Connection connection, Identity identity

@Override
public void checkUpdateAllowed(int index, Connection connection, Identity identity, Resource oldResource,
Resource newResource)
Resource newResource) throws WebApplicationException
{
final String resourceTypeName = getResourceTypeName(oldResource);
final String resourceId = oldResource.getIdElement().getIdPart();
Expand All @@ -121,13 +123,14 @@ public void checkUpdateAllowed(int index, Connection connection, Identity identi

@Override
public void checkDeleteAllowed(int index, Connection connection, Identity identity, Resource oldResource)
throws WebApplicationException
{
final String resourceTypeName = getResourceTypeName(oldResource);
final String resourceId = oldResource.getIdElement().getIdPart();
final long resourceVersion = oldResource.getIdElement().getVersionIdPartAsLong();

Optional<AuthorizationRule<Resource>> optRule = getAuthorizationRule(oldResource.getClass());
optRule.flatMap(rule -> rule.reasonDeleteAllowed(identity, oldResource)).ifPresentOrElse(reason ->
optRule.flatMap(rule -> rule.reasonDeleteAllowed(connection, identity, oldResource)).ifPresentOrElse(reason ->
{
audit.info("Delete of {}/{}/_history/{} allowed for identity '{}' via bundle at index {}, reason: {}",
resourceTypeName, resourceId, resourceVersion, identity.getName(), index, reason);
Expand All @@ -140,7 +143,7 @@ public void checkDeleteAllowed(int index, Connection connection, Identity identi
}

@Override
public void checkSearchAllowed(int index, Identity identity, String resourceTypeName)
public void checkSearchAllowed(int index, Identity identity, String resourceTypeName) throws WebApplicationException
{
Optional<AuthorizationRule<Resource>> optRule = getAuthorizationRule(resourceTypeName);
optRule.flatMap(rule -> rule.reasonSearchAllowed(identity)).ifPresentOrElse(reason ->
Expand Down
Loading
Loading