don't look for device data provenance without a JWT

If a request uses a service secret, then there can be no extraction of
provenance information, as that's only carried in a JWT, so don't try.

While failed attempts weren't hurting anything, they were adding noise to the
logs.

BACK-2995

Author: ewollesen

Makefile

@@ -112,7 +112,7 @@ format-write-changed:
 imports: goimports
 	@echo "goimports -d -e -local 'github.com/tidepool-org/platform'"
 	@cd $(ROOT_DIRECTORY) && \
-		O=`find . -not -path './.gvm_local/*' -not -path './vendor/*' -not -path '**/test/mock.go' -name '*.go' -type f -exec goimports -d -e -local 'github.com/tidepool-org/platform' {} \; 2>&1` && \
+		O=`find . -not -path './.gvm_local/*' -not -path './vendor/*' -not -path '**/test/mock.go' -not -name '**_gen.go' -name '*.go' -type f -exec goimports -d -e -local 'github.com/tidepool-org/platform' {} \; 2>&1` && \
 		[ -z "$${O}" ] || (echo "$${O}" && exit 1)
 
 imports-write: goimports

data/service/api/v1/datasets_data_create.go

@@ -10,7 +10,6 @@ import (
 	"github.com/ant0ine/go-json-rest/rest"
 	"github.com/golang-jwt/jwt/v4"
 
-	"github.com/tidepool-org/platform/auth"
 	"github.com/tidepool-org/platform/data"
 	dataNormalizer "github.com/tidepool-org/platform/data/normalizer"
 	dataService "github.com/tidepool-org/platform/data/service"
@@ -46,10 +45,10 @@ func DataSetsDataCreate(dataServiceContext dataService.Context) {
 		return
 	}
 
-	var details request.AuthDetails
-	if details = request.GetAuthDetails(ctx); !details.IsService() {
+	var authDetails request.AuthDetails
+	if authDetails = request.GetAuthDetails(ctx); !authDetails.IsService() {
 		var permissions permission.Permissions
-		permissions, err = dataServiceContext.PermissionClient().GetUserPermissions(ctx, details.UserID(), *dataSet.UserID)
+		permissions, err = dataServiceContext.PermissionClient().GetUserPermissions(ctx, authDetails.UserID(), *dataSet.UserID)
 		if err != nil {
 			if request.IsErrorUnauthorized(err) {
 				dataServiceContext.RespondWithError(service.ErrorUnauthorized())
@@ -106,7 +105,7 @@ func DataSetsDataCreate(dataServiceContext dataService.Context) {
 	for _, datum := range datumArray {
 		datum.SetUserID(dataSet.UserID)
 		datum.SetDataSetID(dataSet.UploadID)
-		datum.SetProvenance(CollectProvenanceInfo(ctx, req, details))
+		datum.SetProvenance(CollectProvenanceInfo(ctx, req, authDetails))
 	}
 
 	if deduplicator, getErr := dataServiceContext.DataDeduplicatorFactory().Get(dataSet); getErr != nil {
@@ -136,28 +135,24 @@ func DataSetsDataCreate(dataServiceContext dataService.Context) {
 // CollectProvenanceInfo from a request and its auth details.
 //
 // All work is done as a best effort right now.
-func CollectProvenanceInfo(ctx context.Context, req *rest.Request, details request.AuthDetails) *data.Provenance {
+func CollectProvenanceInfo(ctx context.Context, req *rest.Request, authDetails request.AuthDetails) *data.Provenance {
 	lgr := log.LoggerFromContext(ctx)
 	provenance := &data.Provenance{}
 
-	token := req.Header.Get(auth.TidepoolSessionTokenHeaderKey)
-	if token == "" {
-		token = req.Header.Get(auth.TidepoolAuthorizationHeaderKey)
-		if token != "" {
-			if strings.EqualFold(token[:len("bearer ")], "bearer ") {
-				token = token[len("bearer "):]
-			}
-		}
+	token := authDetails.Token()
+	if strings.HasPrefix(strings.ToLower(token), "bearer ") {
+		token = token[len("bearer "):]
 	}
+
 	if token != "" {
 		claims := &TokenClaims{}
 		if _, _, err := jwt.NewParser().ParseUnverified(token, claims); err != nil {
 			lgr.WithError(err).Warn("Unable to parse access token for provenance")
 		} else {
 			provenance.ClientID = claims.ClientID
 		}
-	} else {
-		lgr.Warn("Unable to find access token for provenance")
+	} else if !authDetails.IsService() {
+		lgr.Warn("Unable to read ClientID: The request's access token is blank")
 	}
 
 	if xff := SelectXFF(req.Header); xff != "" {
@@ -170,7 +165,7 @@ func CollectProvenanceInfo(ctx context.Context, req *rest.Request, details reque
 		}
 	}
 
-	if userID := details.UserID(); userID == "" {
+	if userID := authDetails.UserID(); userID == "" {
 		lgr.Warnf("Unable to read the request's userID for provenance: userID is empty")
 	} else {
 		provenance.ByUserID = userID

data/service/api/v1/datasets_data_create_provenance_test.go

@@ -8,18 +8,20 @@ import (
 	"strings"
 
 	"github.com/ant0ine/go-json-rest/rest"
+	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	v1 "github.com/tidepool-org/platform/data/service/api/v1"
+	"github.com/tidepool-org/platform/data/service/api/v1/mocks"
 	"github.com/tidepool-org/platform/log"
 	"github.com/tidepool-org/platform/log/null"
 	"github.com/tidepool-org/platform/request"
 )
 
+//go:generate mockgen -destination mocks/mocklogger_test_gen.go -package mocks github.com/tidepool-org/platform/log Logger
+
 var _ = Describe("collectProvenanceInfo", func() {
-	// logger, err := logJson.NewLogger(os.Stderr, log.DefaultLevelRanks(), log.DefaultLevel())
-	// Expect(err).ShouldNot(HaveOccurred())
 	logger := null.NewLogger()
 	ctx := log.NewContextWithLogger(context.Background(), logger)
 
@@ -50,6 +52,36 @@ var _ = Describe("collectProvenanceInfo", func() {
 	It("handles a missing ClientID", func() {
 		req, details := newTestReqAndDetails("", "bar", "192.0.2.1")
 		prov := v1.CollectProvenanceInfo(ctx, req, details)
+		Expect(prov).ToNot(BeNil())
+		Expect(prov.ByUserID).To(Equal("bar"))
+		Expect(prov.SourceIP).To(Equal("192.0.2.1"))
+		Expect(prov.ClientID).To(Equal(""))
+	})
+
+	It("logs missing ClientIDs when expected, but not found", func() {
+		ctrl := gomock.NewController(GinkgoT())
+		defer ctrl.Finish()
+		mockLogger := mocks.NewMockLogger(ctrl)
+		mockLogger.EXPECT().Warn("Unable to read ClientID: The request's access token is blank")
+		ctx := log.NewContextWithLogger(context.Background(), mockLogger)
+		req, _ := newTestReqAndDetails("", "", "192.0.2.1")
+		details := request.NewAuthDetails(request.MethodSessionToken, "bar", "")
+		prov := v1.CollectProvenanceInfo(ctx, req, details)
+		Expect(prov).ToNot(BeNil())
+		Expect(prov.ByUserID).To(Equal("bar"))
+		Expect(prov.SourceIP).To(Equal("192.0.2.1"))
+		Expect(prov.ClientID).To(Equal(""))
+	})
+
+	It("doesn't log missing ClientIDs for service secret authenticated requests", func() {
+		ctrl := gomock.NewController(GinkgoT())
+		defer ctrl.Finish()
+		mockLogger := mocks.NewMockLogger(ctrl)
+		ctx := log.NewContextWithLogger(context.Background(), mockLogger)
+		req, _ := newTestReqAndDetails("", "", "192.0.2.1")
+		details := request.NewAuthDetails(request.MethodServiceSecret, "bar", "")
+		prov := v1.CollectProvenanceInfo(ctx, req, details)
+		Expect(prov).ToNot(BeNil())
 		Expect(prov.ByUserID).To(Equal("bar"))
 		Expect(prov.SourceIP).To(Equal("192.0.2.1"))
 		Expect(prov.ClientID).To(Equal(""))
@@ -58,18 +90,27 @@ var _ = Describe("collectProvenanceInfo", func() {
 
 func newTestReqAndDetails(clientID, userID, sourceIP string) (*rest.Request, request.AuthDetails) {
 	remoteAddr := ""
+	headers := http.Header{}
 	if sourceIP != "" {
 		remoteAddr = sourceIP + ":1234"
+		headers.Set("X-Forwarded-For", sourceIP)
+	}
+	token := ""
+	if clientID != "" {
+		token = newTestToken(clientID)
+		headers.Set("X-Tidepool-Session-Token", token)
 	}
 	req := &rest.Request{
 		Request: &http.Request{
 			RemoteAddr: remoteAddr,
-			Header: http.Header{
-				"X-Tidepool-Session-Token": {newTestToken(clientID)},
-				"X-Forwarded-For":          {sourceIP}},
+			Header:     headers,
 		},
 	}
-	details := request.NewAuthDetails("", userID, "token")
+	method := request.MethodSessionToken
+	if userID == "" {
+		method = request.MethodServiceSecret
+	}
+	details := request.NewAuthDetails(method, userID, token)
 	return req, details
 }