INT-438: Sanitize logged URLs

orchestrator/careplancontributor/integration_test.go

@@ -167,7 +167,7 @@ func setupIntegrationTest(t *testing.T, notificationEndpoint *url.URL) (*url.URL
 	cpcConfig.CarePlanService.URL = carePlanServiceURL.String()
 	cpcConfig.HealthDataViewEndpointEnabled = true
 	sessionManager, _ := createTestSession()
-	cpc, err := New(cpcConfig, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	cpc, err := New(cpcConfig, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	require.NoError(t, err)
 
 	cpcServerMux := http.NewServeMux()

orchestrator/careplancontributor/service.go

@@ -34,7 +34,8 @@ func New(
 	config Config,
 	profile profile.Provider,
 	orcaPublicURL *url.URL,
-	sessionManager *user.SessionManager) (*Service, error) {
+	sessionManager *user.SessionManager,
+	strictMode bool) (*Service, error) {
 
 	fhirURL, _ := url.Parse(config.FHIR.BaseURL)
 	cpsURL, _ := url.Parse(config.CarePlanService.URL)
@@ -62,6 +63,11 @@ func New(
 		},
 		healthdataviewEndpointEnabled: config.HealthDataViewEndpointEnabled,
 	}
+	if strictMode {
+		result.urlLoggerSanitizer = coolfhir.FhirUrlLoggerSanitizer
+	} else {
+		result.urlLoggerSanitizer = coolfhir.NoopUrlLoggerSanitizer
+	}
 	pubsub.DefaultSubscribers.FhirSubscriptionNotify = result.handleNotification
 	return result, nil
 }
@@ -88,6 +94,7 @@ type Service struct {
 	workflows                     taskengine.Workflows
 	questionnaireLoader           taskengine.QuestionnaireLoader
 	healthdataviewEndpointEnabled bool
+	urlLoggerSanitizer            func(*url.URL) *url.URL
 }
 
 func (s Service) RegisterHandlers(mux *http.ServeMux) {
@@ -169,7 +176,7 @@ func (s Service) withSession(next func(response http.ResponseWriter, request *ht
 // handleProxyAppRequestToEHR handles a request from the CPC application (e.g. Frontend), forwarding it to the local EHR's FHIR API.
 func (s Service) handleProxyAppRequestToEHR(writer http.ResponseWriter, request *http.Request, session *user.SessionData) {
 	clientFactory := clients.Factories[session.FHIRLauncher](session.StringValues)
-	proxy := coolfhir.NewProxy(log.Logger, clientFactory.BaseURL, basePath+"/ehr/fhir", clientFactory.Client)
+	proxy := coolfhir.NewProxy(log.Logger, clientFactory.BaseURL, basePath+"/ehr/fhir", clientFactory.Client, coolfhir.NoopUrlLoggerSanitizer)
 
 	resourcePath := request.PathValue("rest")
 	// If the requested resource is cached in the session, directly return it. This is used to support resources that are required (e.g. by Frontend), but not provided by the EHR.
@@ -329,7 +336,7 @@ func (s Service) handleNotification(ctx context.Context, resource any) error {
 			StatusCode: http.StatusUnprocessableEntity,
 		}
 	}
-	// TODO: for now, we assume the resource URL is always in the form of <FHIR base url>/<resource type>/<resource id>
+	// TODO: for now, we assume the resource URL is always in the form of <FHIR base url>/<reso urce type>/<resource id>
 	//       Then, we can deduce the FHIR base URL from the resource URL
 	resourceUrlParts := strings.Split(resourceUrl, "/")
 	resourceUrlParts = resourceUrlParts[:len(resourceUrlParts)-2]

orchestrator/careplancontributor/service_test.go

@@ -43,7 +43,7 @@ func TestService_Proxy_NoHealthdataviewEndpointEnabledFlag_Fails(t *testing.T) {
 		FHIR: coolfhir.ClientConfig{
 			BaseURL: fhirServer.URL + "/fhir",
 		},
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -72,7 +72,7 @@ func TestService_Proxy_NoHeader_Fails(t *testing.T) {
 			BaseURL: fhirServer.URL + "/fhir",
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -111,7 +111,7 @@ func TestService_Proxy_NoCarePlanInHeader_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -160,7 +160,7 @@ func TestService_Proxy_CarePlanNotFound_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -211,7 +211,7 @@ func TestService_Proxy_CareTeamNotPresent_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -262,7 +262,7 @@ func TestService_Proxy_RequesterNotInCareTeam_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -315,7 +315,7 @@ func TestService_Proxy_Valid(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -369,7 +369,7 @@ func TestService_Proxy_ProxyReturnsError_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -420,7 +420,7 @@ func TestService_Proxy_CareTeamMemberInvalidPeriod_Fails(t *testing.T) {
 			URL: carePlanServiceURL.String(),
 		},
 		HealthDataViewEndpointEnabled: true,
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
 	service.RegisterHandlers(frontServerMux)
@@ -492,7 +492,7 @@ func TestService_HandleNotification_Invalid(t *testing.T) {
 		},
 	}, profile.TestProfile{
 		Principal: auth.TestPrincipal1,
-	}, orcaPublicURL, sessionManager)
+	}, orcaPublicURL, sessionManager, false)
 
 	frontServerMux := http.NewServeMux()
 	frontServer := httptest.NewServer(frontServerMux)
@@ -594,7 +594,7 @@ func TestService_HandleNotification_Valid(t *testing.T) {
 		},
 	}, profile.TestProfile{
 		Principal: auth.TestPrincipal2,
-	}, orcaPublicURL, sessionManager)
+	}, orcaPublicURL, sessionManager, false)
 
 	var capturedFhirBaseUrl string
 	service.cpsClientFactory = func(baseUrl *url.URL) fhirclient.Client {
@@ -672,7 +672,7 @@ func TestService_ProxyToEHR(t *testing.T) {
 	}
 	sessionManager, sessionID := createTestSession()
 
-	service, err := New(Config{}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	service, err := New(Config{}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	require.NoError(t, err)
 	// Setup: configure the service to proxy to the backing FHIR server
 	frontServerMux := http.NewServeMux()
@@ -736,7 +736,7 @@ func TestService_ProxyToCPS(t *testing.T) {
 		CarePlanService: CarePlanServiceConfig{
 			URL: carePlanServiceURL.String(),
 		},
-	}, profile.TestProfile{}, orcaPublicURL, sessionManager)
+	}, profile.TestProfile{}, orcaPublicURL, sessionManager, false)
 	require.NoError(t, err)
 	// Setup: configure the service to proxy to the upstream CarePlanService
 	frontServerMux := http.NewServeMux()

orchestrator/careplanservice/service.go

@@ -118,7 +118,7 @@ func (r FHIRHandlerRequest) bundleEntry() fhir.BundleEntry {
 type FHIRHandlerResult func(txResult *fhir.Bundle) (*fhir.BundleEntry, []any, error)
 
 func (s *Service) RegisterHandlers(mux *http.ServeMux) {
-	s.proxy = coolfhir.NewProxy(log.Logger, s.fhirURL, basePath, s.transport)
+	s.proxy = coolfhir.NewProxy(log.Logger, s.fhirURL, basePath, s.transport, coolfhir.NoopUrlLoggerSanitizer)
 	baseUrl := s.baseUrl()
 	s.profile.RegisterHTTPHandlers(basePath, baseUrl, mux)
 

orchestrator/cmd/config.go

@@ -25,6 +25,9 @@ type Config struct {
 	// CarePlanService holds the configuration for the CarePlanService.
 	CarePlanService careplanservice.Config `koanf:"careplanservice"`
 	LogLevel        zerolog.Level          `koanf:"loglevel"`
+	// StrictMode is a flag that enables strict mode. It enforces safe behavior for production environments,
+	// e.g. prevent logging of personal data.
+	StrictMode bool `koanf:"strictmode"`
 }
 
 func (c Config) Validate() error {
@@ -81,7 +84,8 @@ func LoadConfig() (*Config, error) {
 // DefaultConfig returns sensible, but not complete, default configuration values.
 func DefaultConfig() Config {
 	return Config{
-		LogLevel: zerolog.InfoLevel,
+		LogLevel:   zerolog.InfoLevel,
+		StrictMode: true,
 		Public: InterfaceConfig{
 			Address: ":8080",
 			URL:     "/",

orchestrator/cmd/server.go

@@ -42,7 +42,7 @@ func Start(config Config) error {
 			config.CarePlanContributor,
 			activeProfile,
 			config.Public.ParseURL(),
-			sessionManager)
+			sessionManager, config.StrictMode)
 		if err != nil {
 			return err
 		}

orchestrator/lib/coolfhir/logging.go

@@ -0,0 +1,29 @@
+package coolfhir
+
+import "net/url"
+
+// NoopUrlLoggerSanitizer is a URL logger sanitizer that does nothing.
+func NoopUrlLoggerSanitizer(in *url.URL) *url.URL {
+	return in
+}
+
+// FhirUrlLoggerSanitizer is a URL logger sanitizer that masks certain query parameters.
+func FhirUrlLoggerSanitizer(in *url.URL) *url.URL {
+	result := *in
+	q := url.Values{}
+	for name, values := range in.Query() {
+		for _, value := range values {
+			switch name {
+			case "_include":
+				// Don't mask
+				q.Add(name, value)
+			default:
+				// Mask others
+				q.Add(name, "****")
+			}
+
+		}
+	}
+	result.RawQuery = q.Encode()
+	return &result
+}

orchestrator/lib/coolfhir/logging_test.go

@@ -0,0 +1,61 @@
+package coolfhir
+
+import (
+	"github.com/stretchr/testify/assert"
+	"net/url"
+	"testing"
+)
+
+func TestFhirUrlLoggerSanitizer(t *testing.T) {
+	type args struct {
+		in string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "no query parameters",
+			args: args{
+				in: "http://example.com",
+			},
+			want: "http://example.com",
+		},
+		{
+			name: "no query parameters, question mark is retained",
+			args: args{
+				in: "http://example.com?",
+			},
+			want: "http://example.com?",
+		},
+		{
+			name: "_include is not sanitized",
+			args: args{
+				in: "http://example.com?_include=foo",
+			},
+			want: "http://example.com?_include=foo",
+		},
+		{
+			name: "multiple non-sanitized parameters",
+			args: args{
+				in: "http://example.com?_include=foo&_include=bar",
+			},
+			want: "http://example.com?_include=foo&_include=bar",
+		},
+		{
+			name: "sanitized parameter",
+			args: args{
+				in: "http://example.com?_id=bar",
+			},
+			want: "http://example.com?_id=%2A%2A%2A%2A",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			in, _ := url.Parse(tt.args.in)
+
+			assert.Equalf(t, tt.want, FhirUrlLoggerSanitizer(in).String(), "FhirUrlLoggerSanitizer(%v)", tt.args.in)
+		})
+	}
+}

orchestrator/lib/coolfhir/proxy.go

@@ -9,7 +9,7 @@ import (
 	"strings"
 )
 
-func NewProxy(logger zerolog.Logger, targetFHIRBaseURL *url.URL, proxyBasePath string, transport http.RoundTripper) *httputil.ReverseProxy {
+func NewProxy(logger zerolog.Logger, targetFHIRBaseURL *url.URL, proxyBasePath string, transport http.RoundTripper, urlLoggerSanitizer func(*url.URL) *url.URL) *httputil.ReverseProxy {
 	return &httputil.ReverseProxy{
 		Rewrite: func(r *httputil.ProxyRequest) {
 			r.Out.URL = targetFHIRBaseURL.JoinPath("/", strings.TrimPrefix(r.In.URL.Path, proxyBasePath))
@@ -18,8 +18,9 @@ func NewProxy(logger zerolog.Logger, targetFHIRBaseURL *url.URL, proxyBasePath s
 		},
 		Transport: sanitizingRoundTripper{
 			next: loggingRoundTripper{
-				logger: &logger,
-				next:   transport,
+				logger:             &logger,
+				next:               transport,
+				urlLoggerSanitizer: urlLoggerSanitizer,
 			},
 		},
 		ErrorHandler: func(writer http.ResponseWriter, request *http.Request, err error) {
@@ -58,8 +59,9 @@ func (s sanitizingRoundTripper) RoundTrip(request *http.Request) (*http.Response
 var _ http.RoundTripper = &loggingRoundTripper{}
 
 type loggingRoundTripper struct {
-	logger *zerolog.Logger
-	next   http.RoundTripper
+	logger             *zerolog.Logger
+	next               http.RoundTripper
+	urlLoggerSanitizer func(*url.URL) *url.URL
 }
 
 func (l loggingRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {

orchestrator/lib/coolfhir/proxy_test.go

@@ -39,7 +39,7 @@ func TestProxy(t *testing.T) {
 			}
 			return request
 		},
-	})
+	}, NoopUrlLoggerSanitizer)
 	proxyServer := httptest.NewServer(proxyServerMux)
 	proxyServerMux.HandleFunc("/localfhir/{rest...}", proxy.ServeHTTP)