Auth: create session and validate signatures perform the same checks …

…(backport v5.4) (#2673) * Auth: create session and validate signatures perform the same checks (#2664) * release notes v5.4.5 * upgrade to golang 1.21, fix tests * circle-go-version
nuts-foundation · Dec 12, 2023 · 14c5d4f · 14c5d4f
1 parent 45bba45
commit 14c5d4f
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 22 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -12,7 +12,7 @@ jobs:
   build:
     parallelism: 8
     docker:
-      - image: cimg/go:1.20
+      - image: cimg/go:1.21
     steps:
       - checkout
 
@@ -36,7 +36,7 @@ jobs:
 
   report:
     docker:
-      - image: cimg/go:1.20
+      - image: cimg/go:1.21
     steps:
       - checkout
       - attach_workspace:

diff --git a/Dockerfile b/Dockerfile
@@ -1,5 +1,5 @@
 # golang alpine
-FROM golang:1.20.6-alpine as builder
+FROM golang:1.21.5-alpine as builder
 
 ARG TARGETARCH
 ARG TARGETOS

diff --git a/auth/api/auth/v1/api.go b/auth/api/auth/v1/api.go
@@ -124,6 +124,7 @@ func (w Wrapper) VerifySignature(_ context.Context, request VerifySignatureReque
 		vpType := validationResult.VPType()
 		response.VpType = &vpType
 	} else {
+		log.Logger().Warnf("Signature verification failed, reason: %s", validationResult.Reason())
 		response.Validity = false
 	}
 	return VerifySignature200JSONResponse(response), nil

diff --git a/auth/services/selfsigned/signer.go b/auth/services/selfsigned/signer.go
@@ -226,20 +226,19 @@ func checkSessionParams(params map[string]interface{}) error {
 	if !ok {
 		return fmt.Errorf("employee should be an object")
 	}
-	_, ok = employeeMap["identifier"]
-	if !ok {
-		return fmt.Errorf("missing employee identifier")
+	identifier, _ := employeeMap["identifier"].(string)
+	if len(identifier) == 0 {
+		return fmt.Errorf("missing/invalid employee identifier")
 	}
-	_, ok = employeeMap["initials"]
-	if !ok {
-		return fmt.Errorf("missing employee initials")
+	initials, _ := employeeMap["initials"].(string)
+	if len(initials) == 0 {
+		return fmt.Errorf("missing/invalid employee initials")
 	}
-	_, ok = employeeMap["familyName"]
-	if !ok {
-		return fmt.Errorf("missing employee familyName")
+	familyName, _ := employeeMap["familyName"].(string)
+	if len(familyName) == 0 {
+		return fmt.Errorf("missing/invalid employee familyName")
 	}
 	return nil
-
 }
 
 func (v *signer) Routes(router core.EchoRouter) {

diff --git a/auth/services/selfsigned/signer_test.go b/auth/services/selfsigned/signer_test.go
@@ -109,6 +109,52 @@ func TestSessionStore_StartSigningSession(t *testing.T) {
 
 		require.Error(t, err)
 	})
+
+	t.Run("empty employee familyName", func(t *testing.T) {
+		params := map[string]interface{}{
+			"employer": employer.String(),
+			"employee": map[string]interface{}{
+				"identifier": identifier,
+				"roleName":   roleName,
+				"initials":   initials,
+				"familyName": "",
+			},
+		}
+
+		ss := NewSigner(nil, "").(*signer)
+		_, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
+		require.ErrorContains(t, err, "missing/invalid employee familyName")
+	})
+	t.Run("empty employee initials", func(t *testing.T) {
+		params := map[string]interface{}{
+			"employer": employer.String(),
+			"employee": map[string]interface{}{
+				"identifier": identifier,
+				"roleName":   roleName,
+				"initials":   "",
+				"familyName": familyName,
+			},
+		}
+
+		ss := NewSigner(nil, "").(*signer)
+		_, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
+		require.ErrorContains(t, err, "missing/invalid employee initials")
+	})
+	t.Run("empty employee identifier", func(t *testing.T) {
+		params := map[string]interface{}{
+			"employer": employer.String(),
+			"employee": map[string]interface{}{
+				"identifier": "",
+				"roleName":   roleName,
+				"initials":   initials,
+				"familyName": familyName,
+			},
+		}
+
+		ss := NewSigner(nil, "").(*signer)
+		_, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
+		require.ErrorContains(t, err, "missing/invalid employee identifier")
+	})
 }
 
 func TestSessionStore_SigningSessionStatus(t *testing.T) {
@@ -132,8 +178,8 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) {
 	t.Run("status completed returns VP on SigningSessionResult", func(t *testing.T) {
 		mockContext := newMockContext(t)
 		ss := NewSigner(mockContext.vcr, "").(*signer)
-		mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(&testVC, nil)
-		mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil)
+		mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(&testVC, nil)
+		mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil)
 
 		sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
 		require.NoError(t, err)
@@ -191,7 +237,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) {
 	t.Run("correct VC options are passed to issuer", func(t *testing.T) {
 		mockContext := newMockContext(t)
 		ss := NewSigner(mockContext.vcr, "").(*signer)
-		mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).DoAndReturn(
+		mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).DoAndReturn(
 			func(arg0 interface{}, unsignedCredential interface{}, public interface{}, publish interface{}) (*vc.VerifiableCredential, error) {
 				isPublic, ok := public.(bool)
 				isPublished, ok2 := publish.(bool)
@@ -219,7 +265,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) {
 
 				return &testVC, nil
 			})
-		mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil)
+		mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(&testVP, nil)
 
 		sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
 		require.NoError(t, err)
@@ -241,7 +287,7 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) {
 	t.Run("error on VC issuance", func(t *testing.T) {
 		mockContext := newMockContext(t)
 		ss := NewSigner(mockContext.vcr, "").(*signer)
-		mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(nil, errors.New("error"))
+		mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(nil, errors.New("error"))
 
 		sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
 		require.NoError(t, err)
@@ -256,8 +302,8 @@ func TestSessionStore_SigningSessionStatus(t *testing.T) {
 	t.Run("error on building VP", func(t *testing.T) {
 		mockContext := newMockContext(t)
 		ss := NewSigner(mockContext.vcr, "").(*signer)
-		mockContext.issuer.EXPECT().Issue(context.TODO(), gomock.Any(), false, false).Return(&testVC, nil)
-		mockContext.holder.EXPECT().BuildVP(context.TODO(), gomock.Len(1), gomock.Any(), &employer, true).Return(nil, errors.New("error"))
+		mockContext.issuer.EXPECT().Issue(context.Background(), gomock.Any(), false, false).Return(&testVC, nil)
+		mockContext.holder.EXPECT().BuildVP(context.Background(), gomock.Len(1), gomock.Any(), &employer, true).Return(nil, errors.New("error"))
 
 		sp, err := ss.StartSigningSession(contract.Contract{RawContractText: testContract}, params)
 		require.NoError(t, err)

diff --git a/docs/pages/release_notes.rst b/docs/pages/release_notes.rst
@@ -3,6 +3,16 @@
 Release notes
 #############
 
+************************
+Hazelnut update (v5.4.5)
+************************
+
+Release date: 2023-12-11
+
+- Auth: make sure create session and validate signatures perform the same checks (#2664)
+
+**Full Changelog**: https://github.com/nuts-foundation/nuts-node/compare/v5.4.4...v5.4.5
+
 ************************
 Hazelnut update (v5.4.4)
 ************************

diff --git a/http/engine_test.go b/http/engine_test.go
@@ -163,7 +163,7 @@ func TestEngine_Configure(t *testing.T) {
 			thisTLSConfig := tlsConfig.Clone()
 			thisTLSConfig.Certificates = nil
 			_, err = doHTTPSRequest(thisTLSConfig, engine.config.InterfaceConfig.Address)
-			assert.ErrorContains(t, err, "tls: bad certificate")
+			assert.ErrorContains(t, err, "tls: certificate required")
 
 			err = engine.Shutdown()
 			assert.NoError(t, err)

diff --git a/network/network_test.go b/network/network_test.go
@@ -1246,7 +1246,7 @@ func TestNetwork_checkHealth(t *testing.T) {
 			result := n.CheckHealth()
 
 			assert.Equal(t, core.HealthStatusDown, result[healthTLS].Status)
-			assert.Equal(t, "x509: certificate signed by unknown authority", result[healthTLS].Details)
+			assert.Contains(t, result[healthTLS].Details, "x509: certificate has expired or is not yet valid")
 		})
 	})