SQL: Fix locking with MS SQL Server

discovery/store.go

@@ -233,12 +233,8 @@ func (s *sqlStore) search(serviceID string, query map[string]string) ([]vc.Verif
 
 // incrementTimestamp increments the last_timestamp of the given service.
 func (s *sqlStore) incrementTimestamp(tx *gorm.DB, serviceID string) (*int, error) {
-	var service serviceRecord
-	// Lock (SELECT FOR UPDATE) discovery_service row to prevent concurrent updates to the same list, which could mess up the last Timestamp.
-	if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).
-		Where(serviceRecord{ID: serviceID}).
-		Find(&service).
-		Error; err != nil {
+	service, err := s.findAndLockService(tx, serviceID)
+	if err != nil {
 		return nil, err
 	}
 	service.ID = serviceID
@@ -252,19 +248,37 @@ func (s *sqlStore) incrementTimestamp(tx *gorm.DB, serviceID string) (*int, erro
 
 // setTimestamp sets the last_timestamp of the given service.
 func (s *sqlStore) setTimestamp(tx *gorm.DB, serviceID string, timestamp int) error {
-	var service serviceRecord
-	// Lock (SELECT FOR UPDATE) discovery_service row to prevent concurrent updates to the same list, which could mess up the last Timestamp.
-	if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).
-		Where(serviceRecord{ID: serviceID}).
-		Find(&service).
-		Error; err != nil {
+	service, err := s.findAndLockService(tx, serviceID)
+	if err != nil {
 		return err
 	}
 	service.ID = serviceID
 	service.LastLamportTimestamp = timestamp
 	return tx.Save(service).Error
 }
 
+// findAndLockService finds a service by ID and locks it, preventing concurrent updates to the same list.
+// This is required for atomically processing updates from the Discovery Server.
+func (s *sqlStore) findAndLockService(tx *gorm.DB, serviceID string) (serviceRecord, error) {
+	var service serviceRecord
+	// Lock (SELECT FOR UPDATE) discovery_service row to prevent concurrent updates to the same list, which could mess up the last Timestamp.
+	// Microsoft SQL server does not support the locking clause, so we have to use a raw query instead.
+	// See https://github.com/nuts-foundation/nuts-node/issues/3393
+	if tx.Dialector.Name() == "sqlserver" {
+		if err := tx.Raw("SELECT * FROM discovery_service WITH (UPDLOCK, ROWLOCK) WHERE id = ?", serviceID).Scan(&service).Error; err != nil {
+			return serviceRecord{}, err
+		}
+	} else {
+		if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).
+			Where(serviceRecord{ID: serviceID}).
+			Find(&service).
+			Error; err != nil {
+			return serviceRecord{}, err
+		}
+	}
+	return service, nil
+}
+
 // exists checks whether a presentation of the given subject is registered on a service.
 func (s *sqlStore) exists(serviceID string, credentialSubjectID string, presentationID string) (bool, error) {
 	var count int64

e2e-tests/oauth-flow/rfc021/do-test.sh

@@ -63,6 +63,18 @@ else
   exitWithDockerLogs 1
 fi
 
+# Register vendor B on Discovery Service
+echo "Registering vendor B on Discovery Service..."
+REQUEST="{\"registrationParameters\":{\"key\":\"value\"}}"
+RESPONSE=$(echo $REQUEST | curl -s -o /dev/null -w "%{http_code}" -X POST --data-binary @- http://localhost:28081/internal/discovery/v1/e2e-test/vendorB)
+if echo $RESPONSE == "200"; then
+  echo "Vendor B registered on Discovery Service"
+else
+  echo "FAILED: Could not register vendor B on Discovery Service" 1>&2
+  echo $RESPONSE
+  exitWithDockerLogs 1
+fi
+
 echo "---------------------------------------"
 echo "Perform OAuth 2.0 rfc021 flow..."
 echo "---------------------------------------"

e2e-tests/oauth-flow/rfc021/docker-compose.yml

@@ -13,6 +13,7 @@ services:
       # So we need to mount that file to the OS CA bundle location, otherwise did:web resolving will fail due to untrusted certs.
       - "../../tls-certs/truststore.pem:/etc/ssl/certs/Nuts_RootCA.pem:ro"
       - "./node-A/presentationexchangemapping.json:/opt/nuts/policies/presentationexchangemapping.json:ro"
+      - "./shared/discovery:/nuts/discovery:ro"
     healthcheck:
       interval: 1s # Make test run quicker by checking health status more often
   nodeA:
@@ -40,6 +41,7 @@ services:
       # did:web resolver uses the OS CA bundle, but e2e tests use a self-signed CA which can be found in truststore.pem
       # So we need to mount that file to the OS CA bundle location, otherwise did:web resolving will fail due to untrusted certs.
       - "../../tls-certs/truststore.pem:/etc/ssl/certs/Nuts_RootCA.pem:ro"
+      - "./shared/discovery:/nuts/discovery:ro"
     healthcheck:
       interval: 1s # Make test run quicker by checking health status more often
   nodeB:

e2e-tests/oauth-flow/rfc021/node-A/nuts.yaml

@@ -17,6 +17,11 @@ tls:
   truststorefile: /opt/nuts/truststore.pem
   certfile: /opt/nuts/certificate-and-key.pem
   certkeyfile: /opt/nuts/certificate-and-key.pem
+discovery:
+  definitions:
+    directory: /nuts/discovery
+  server:
+    ids: e2e-test
 vdr:
   didmethods:
     - web

e2e-tests/oauth-flow/rfc021/node-B/nuts.yaml

@@ -12,6 +12,9 @@ auth:
     - dummy
   irma:
     autoupdateschemas: false
+discovery:
+  definitions:
+    directory: /nuts/discovery
 tls:
   truststorefile: /opt/nuts/truststore.pem
   certfile: /opt/nuts/certificate-and-key.pem

e2e-tests/oauth-flow/rfc021/shared/discovery/definition.json

@@ -0,0 +1,49 @@
+{
+  "id": "e2e-test",
+  "endpoint": "http://nodeA-backend:8080/discovery/e2e-test",
+  "presentation_max_validity": 36000,
+  "presentation_definition": {
+    "id": "pd_eoverdracht_dev_care_organization",
+    "format": {
+      "ldp_vc": {
+        "proof_type": [
+          "JsonWebSignature2020"
+        ]
+      }
+    },
+    "input_descriptors": [
+      {
+        "id": "id_nuts_care_organization_cred",
+        "constraints": {
+          "fields": [
+            {
+              "path": [
+                "$.type"
+              ],
+              "filter": {
+                "type": "string",
+                "const": "NutsOrganizationCredential"
+              }
+            },
+            {
+              "path": [
+                "$.credentialSubject.organization.name"
+              ],
+              "filter": {
+                "type": "string"
+              }
+            },
+            {
+              "path": [
+                "$.credentialSubject.organization.city"
+              ],
+              "filter": {
+                "type": "string"
+              }
+            }
+          ]
+        }
+      }
+    ]
+  }
+}

vcr/revocation/statuslist2021_issuer.go

@@ -26,6 +26,7 @@ import (
 	"github.com/nuts-foundation/nuts-node/storage"
 	"github.com/nuts-foundation/nuts-node/vdr/resolver"
 	"net/url"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -165,9 +166,17 @@ func (cs *StatusList2021) Credential(ctx context.Context, issuerDID did.DID, pag
 	err = cs.db.Transaction(func(tx *gorm.DB) error {
 		// lock credentialRecord row for statusListCredentialURL since it will be updated.
 		// Revoke does the same to guarantee the DB always contains all revocations.
-		err = tx.Clauses(clause.Locking{Strength: clause.LockingStrengthUpdate}).
-			Find(new(credentialRecord), "subject_id = ?", statusListCredentialURL).
-			Error
+		// Microsoft SQL server does not support the locking clause, so we have to use a raw query instead.
+		// See https://github.com/nuts-foundation/nuts-node/issues/3393
+		if tx.Dialector.Name() == "sqlserver" {
+			err = tx.Raw("SELECT * FROM status_list_entry WITH (UPDLOCK, ROWLOCK) WHERE subject_id = ?", statusListCredentialURL).
+				Scan(new(credentialRecord)).
+				Error
+		} else {
+			err = tx.Clauses(clause.Locking{Strength: clause.LockingStrengthUpdate}).
+				Find(new(credentialRecord), "subject_id = ?", statusListCredentialURL).
+				Error
+		}
 		if err != nil {
 			return err
 		}
@@ -287,11 +296,32 @@ func (cs *StatusList2021) Entry(ctx context.Context, issuer did.DID, purpose Sta
 	credentialIssuer := new(credentialIssuerRecord)
 	for {
 		err := cs.db.Transaction(func(tx *gorm.DB) error {
-			// lock issuer's last page; iff it exists
-			err := tx.Clauses(clause.Locking{Strength: clause.LockingStrengthUpdate}).
-				Order("page").
-				Last(credentialIssuer, "issuer = ?", issuer.String()).
-				Error
+			// Find issuer's last page; if it exists. Lock all pages.
+			// Microsoft SQL server does not support the locking clause, so we have to use a raw query instead.
+			// See https://github.com/nuts-foundation/nuts-node/issues/3393
+			if tx.Dialector.Name() == "sqlserver" {
+				var pages []credentialIssuerRecord
+				if err = tx.Raw("SELECT * FROM status_list WITH (UPDLOCK, ROWLOCK) WHERE issuer = ?", issuer.String()).
+					Scan(&pages).
+					Error; err != nil {
+					return err
+				}
+				if len(pages) == 0 {
+					// mimic non-SQL Server behavior
+					err = gorm.ErrRecordNotFound
+				} else {
+					// get last page, order by page first
+					slices.SortFunc(pages, func(i, j credentialIssuerRecord) int {
+						return i.Page - j.Page
+					})
+					credentialIssuer = &pages[len(pages)-1]
+				}
+			} else {
+				err = tx.Clauses(clause.Locking{Strength: clause.LockingStrengthUpdate}).
+					Order("page").
+					Last(credentialIssuer, "issuer = ?", issuer.String()).
+					Error
+			}
 			if err != nil {
 				if !errors.Is(err, gorm.ErrRecordNotFound) {
 					return err