From 0e8c57adca2677ab69778090a82f9472961a15a2 Mon Sep 17 00:00:00 2001 From: Jason Kulatunga Date: Fri, 20 Sep 2024 09:56:23 -0400 Subject: [PATCH 1/2] adding support for a resource type allow list -- restrict the Resource types we query for and return to the user. --- Dockerfile | 4 +-- clients/internal/base/base_client.go | 18 +++++++--- clients/internal/base/fhir401_client.go | 10 +++++- clients/internal/dynamic_client.go | 13 ++++++-- clients/models/mock/mock_source_client.go | 40 +++++++++++++++-------- clients/models/source_client.go | 3 +- clients/models/source_client_options.go | 8 +++++ 7 files changed, 71 insertions(+), 25 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1f70336cf..d864ecaf6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,8 +3,8 @@ FROM golang:1.21 WORKDIR /go/src/github.com/fastenhealth/fasten-sources COPY . . -RUN go mod tidy && go mod vendor && go build -o /usr/bin/oauth_cli ./testutils/oauth_cli.go +RUN go mod tidy && go mod vendor && go build -o /usr/bin/test-smart-client ./tools/test-smart-client/main.go -CMD ["/usr/bin/oauth_cli"] +CMD ["/usr/bin/test-smart-client"] EXPOSE 9999 diff --git a/clients/internal/base/base_client.go b/clients/internal/base/base_client.go index 78ca61850..b07ea3916 100644 --- a/clients/internal/base/base_client.go +++ b/clients/internal/base/base_client.go @@ -34,8 +34,8 @@ type SourceClientBase struct { EndpointDefinition *definitionsModels.LighthouseSourceDefinition Headers map[string]string - UsCoreResources []string - FhirVersion string + ResourceTypesUsCore []string + FhirVersion string SourceClientOptions *models.SourceClientOptions @@ -69,7 +69,7 @@ func NewBaseClient(env pkg.FastenLighthouseEnvType, ctx context.Context, globalL Headers: map[string]string{}, // https://build.fhir.org/ig/HL7/US-Core/ - UsCoreResources: []string{ + ResourceTypesUsCore: []string{ "AllergyIntolerance", //"Binary", "CarePlan", @@ -113,8 +113,16 @@ func NewBaseClient(env pkg.FastenLighthouseEnvType, ctx context.Context, globalL return client, nil } -func (c *SourceClientBase) GetUsCoreResources() []string { - return c.UsCoreResources +func (c *SourceClientBase) GetResourceTypesUsCore() []string { + return c.ResourceTypesUsCore +} + +func (c *SourceClientBase) GetResourceTypesAllowList() []string { + if c.SourceClientOptions == nil { + return []string{} + } else { + return c.SourceClientOptions.ResourceTypesAllowList + } } func (c *SourceClientBase) GetSourceCredential() models.SourceCredential { diff --git a/clients/internal/base/fhir401_client.go b/clients/internal/base/fhir401_client.go index d1e38d794..b7a770960 100644 --- a/clients/internal/base/fhir401_client.go +++ b/clients/internal/base/fhir401_client.go @@ -198,7 +198,10 @@ func (c *SourceClientFHIR401) SyncAllByResourceName(db models.DatabaseRepository } //process any pending resources - lookupResourceReferences, syncErrors = c.ProcessPendingResources(db, &summary, lookupResourceReferences, syncErrors) + //if we have a populated whitelist, skip this -- we don't care about references to other resources, just the ones we queried. This is a naiive solution to prevent requesting unnecessary records. This should be improved. + if len(c.GetResourceTypesAllowList()) == 0 { + lookupResourceReferences, syncErrors = c.ProcessPendingResources(db, &summary, lookupResourceReferences, syncErrors) + } checkpointErrorData := map[string]interface{}{} if len(syncErrors) > 0 { @@ -469,6 +472,11 @@ func (c *SourceClientFHIR401) ProcessResource(db models.DatabaseRepository, reso c.Logger.Warnf("Skipping contained resource missing id: (%s/%s#%s index: %d)", currentResourceType, *currentResourceId, containedResourceType, cndx) continue } + if len(c.GetResourceTypesAllowList()) > 0 && !lo.Contains(c.GetResourceTypesAllowList(), containedResourceType) { + c.Logger.Warnf("Skipping contained resource not in allow list: (%s/%s#%s index: %d)", currentResourceType, *currentResourceId, containedResourceType, cndx) + continue + } + normalizedContainedResourceId := normalizeContainedResourceId(currentResourceType, *currentResourceId, *containedResourceId) //generate a unique id for this contained resource by base64 url encoding this string diff --git a/clients/internal/dynamic_client.go b/clients/internal/dynamic_client.go index 6aff1f90e..96d47494b 100644 --- a/clients/internal/dynamic_client.go +++ b/clients/internal/dynamic_client.go @@ -68,9 +68,16 @@ func (c dynamicSourceClient) SyncAll(db models.DatabaseRepository) (models.Upser //Operation-PatientEverything is not supported - https://build.fhir.org/operation-patient-everything.html //Manually processing individual resources - supportedResources := c.GetUsCoreResources() - if c.EndpointDefinition.ClientSupportedResources != nil { - supportedResources = append(supportedResources, c.EndpointDefinition.ClientSupportedResources...) + var supportedResources []string + if len(c.GetResourceTypesAllowList()) > 0 { + supportedResources = c.GetResourceTypesAllowList() + supportedResources = append(supportedResources, "Patient") //always ensure the patient is included + } else { + //no override provided, attempt to sync all resources + supportedResources = c.GetResourceTypesUsCore() + if c.EndpointDefinition.ClientSupportedResources != nil { + supportedResources = append(supportedResources, c.EndpointDefinition.ClientSupportedResources...) + } } return c.SyncAllByResourceName(db, supportedResources) } else if len(c.EndpointDefinition.CustomOpPatientEverything) > 0 { diff --git a/clients/models/mock/mock_source_client.go b/clients/models/mock/mock_source_client.go index afb467289..da1d99b05 100644 --- a/clients/models/mock/mock_source_client.go +++ b/clients/models/mock/mock_source_client.go @@ -82,32 +82,46 @@ func (mr *MockSourceClientMockRecorder) GetResourceBundle(relativeResourcePath i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResourceBundle", reflect.TypeOf((*MockSourceClient)(nil).GetResourceBundle), relativeResourcePath) } -// GetSourceCredential mocks base method. -func (m *MockSourceClient) GetSourceCredential() models.SourceCredential { +// GetResourceTypesAllowList mocks base method. +func (m *MockSourceClient) GetResourceTypesAllowList() []string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetSourceCredential") - ret0, _ := ret[0].(models.SourceCredential) + ret := m.ctrl.Call(m, "GetResourceTypesAllowList") + ret0, _ := ret[0].([]string) return ret0 } -// GetSourceCredential indicates an expected call of GetSourceCredential. -func (mr *MockSourceClientMockRecorder) GetSourceCredential() *gomock.Call { +// GetResourceTypesAllowList indicates an expected call of GetResourceTypesAllowList. +func (mr *MockSourceClientMockRecorder) GetResourceTypesAllowList() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSourceCredential", reflect.TypeOf((*MockSourceClient)(nil).GetSourceCredential)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResourceTypesAllowList", reflect.TypeOf((*MockSourceClient)(nil).GetResourceTypesAllowList)) } -// GetUsCoreResources mocks base method. -func (m *MockSourceClient) GetUsCoreResources() []string { +// GetResourceTypesUsCore mocks base method. +func (m *MockSourceClient) GetResourceTypesUsCore() []string { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetUsCoreResources") + ret := m.ctrl.Call(m, "GetResourceTypesUsCore") ret0, _ := ret[0].([]string) return ret0 } -// GetUsCoreResources indicates an expected call of GetUsCoreResources. -func (mr *MockSourceClientMockRecorder) GetUsCoreResources() *gomock.Call { +// GetResourceTypesUsCore indicates an expected call of GetResourceTypesUsCore. +func (mr *MockSourceClientMockRecorder) GetResourceTypesUsCore() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetResourceTypesUsCore", reflect.TypeOf((*MockSourceClient)(nil).GetResourceTypesUsCore)) +} + +// GetSourceCredential mocks base method. +func (m *MockSourceClient) GetSourceCredential() models.SourceCredential { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSourceCredential") + ret0, _ := ret[0].(models.SourceCredential) + return ret0 +} + +// GetSourceCredential indicates an expected call of GetSourceCredential. +func (mr *MockSourceClientMockRecorder) GetSourceCredential() *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUsCoreResources", reflect.TypeOf((*MockSourceClient)(nil).GetUsCoreResources)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSourceCredential", reflect.TypeOf((*MockSourceClient)(nil).GetSourceCredential)) } // RefreshAccessToken mocks base method. diff --git a/clients/models/source_client.go b/clients/models/source_client.go index 066c304cd..366151d61 100644 --- a/clients/models/source_client.go +++ b/clients/models/source_client.go @@ -7,7 +7,8 @@ import ( //go:generate mockgen -source=source_client.go -destination=mock/mock_source_client.go type SourceClient interface { - GetUsCoreResources() []string + GetResourceTypesUsCore() []string + GetResourceTypesAllowList() []string GetRequest(resourceSubpath string, decodeModelPtr interface{}) (string, error) GetResourceBundle(relativeResourcePath string) (interface{}, error) SyncAll(db DatabaseRepository) (UpsertSummary, error) diff --git a/clients/models/source_client_options.go b/clients/models/source_client_options.go index 4c602daf3..78c04eb25 100644 --- a/clients/models/source_client_options.go +++ b/clients/models/source_client_options.go @@ -17,6 +17,8 @@ type SourceClientOptions struct { RedirectURL string Scopes []string + ResourceTypesAllowList []string //list of resource types that are allowed to be fetched from this source. Default empty (USCDI Core List) + SourceClientRefreshOptions []func(*SourceClientRefreshOptions) Context context.Context @@ -65,6 +67,12 @@ func WithScopes(scopes []string) func(*SourceClientOptions) { } } +func WithResourceTypeAllowList(resourceTypeAllowList []string) func(*SourceClientOptions) { + return func(s *SourceClientOptions) { + s.ResourceTypesAllowList = resourceTypeAllowList + } +} + func WithSourceClientRefreshOptions(options ...func(*SourceClientRefreshOptions)) func(*SourceClientOptions) { return func(s *SourceClientOptions) { s.SourceClientRefreshOptions = options From 572b3c821d0341db820e8055d4395ef7752e4b6e Mon Sep 17 00:00:00 2001 From: Jason Kulatunga Date: Fri, 20 Sep 2024 11:09:13 -0400 Subject: [PATCH 2/2] make sure manual client correctly implements the sourceclient interface. --- clients/internal/dynamic_client_test.go | 14 ++++++++++++++ clients/internal/fasten/client_test.go | 14 ++++++++++++++ clients/internal/manual/client.go | 8 ++++++++ clients/internal/manual/client_test.go | 8 ++++++++ 4 files changed, 44 insertions(+) create mode 100644 clients/internal/dynamic_client_test.go create mode 100644 clients/internal/fasten/client_test.go diff --git a/clients/internal/dynamic_client_test.go b/clients/internal/dynamic_client_test.go new file mode 100644 index 000000000..10ea53fdb --- /dev/null +++ b/clients/internal/dynamic_client_test.go @@ -0,0 +1,14 @@ +package internal + +import ( + "github.com/fastenhealth/fasten-sources/clients/models" + "github.com/stretchr/testify/require" + "testing" +) + +func TestGetDynamicSourceClient_ImplementsInterface(t *testing.T) { + t.Parallel() + + //assert + require.Implements(t, (*models.SourceClient)(nil), &dynamicSourceClient{}, "should implement the models.SourceClient interface") +} diff --git a/clients/internal/fasten/client_test.go b/clients/internal/fasten/client_test.go new file mode 100644 index 000000000..07c7d8012 --- /dev/null +++ b/clients/internal/fasten/client_test.go @@ -0,0 +1,14 @@ +package fasten + +import ( + "github.com/fastenhealth/fasten-sources/clients/models" + "github.com/stretchr/testify/require" + "testing" +) + +func TestGetSourceClientFasten_ImplementsInterface(t *testing.T) { + t.Parallel() + + //assert + require.Implements(t, (*models.SourceClient)(nil), &FastenClient{}, "should implement the models.SourceClient interface") +} diff --git a/clients/internal/manual/client.go b/clients/internal/manual/client.go index e070f72c3..6551c1854 100644 --- a/clients/internal/manual/client.go +++ b/clients/internal/manual/client.go @@ -45,6 +45,14 @@ func (m ManualClient) GetUsCoreResources() []string { panic("implement me") } +func (m ManualClient) GetResourceTypesUsCore() []string { + return []string{} +} + +func (m ManualClient) GetResourceTypesAllowList() []string { + return []string{} +} + func (m ManualClient) SyncAllByResourceName(db models.DatabaseRepository, resourceNames []string) (models.UpsertSummary, error) { //TODO implement me panic("implement me") diff --git a/clients/internal/manual/client_test.go b/clients/internal/manual/client_test.go index 793e5afc8..267104e4b 100644 --- a/clients/internal/manual/client_test.go +++ b/clients/internal/manual/client_test.go @@ -2,6 +2,7 @@ package manual import ( "context" + "github.com/fastenhealth/fasten-sources/clients/models" mock_models "github.com/fastenhealth/fasten-sources/clients/models/mock" "github.com/fastenhealth/fasten-sources/pkg" "github.com/golang/mock/gomock" @@ -11,6 +12,13 @@ import ( "testing" ) +func TestGetSourceClientManual_ImplementsInterface(t *testing.T) { + t.Parallel() + + //assert + require.Implements(t, (*models.SourceClient)(nil), &ManualClient{}, "should implement the models.SourceClient interface") +} + func TestGetSourceClientManual_ExtractPatientId_Bundle(t *testing.T) { t.Parallel() //setup