Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix duplicate discovery results #3515

Merged
merged 5 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 53 additions & 12 deletions discovery/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/nuts-foundation/go-did/did"
"github.com/nuts-foundation/nuts-node/vcr/credential/store"
"slices"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -267,25 +268,19 @@ func (s *sqlStore) get(serviceID string, startAfter int) (map[string]vc.Verifiab
func (s *sqlStore) search(serviceID string, query map[string]string, allowUnvalidated bool) ([]vc.VerifiablePresentation, error) {
// first only select columns also used in group by clause
// if the query is empty, there's no need to do a join
stmt := s.db.Model(&presentationRecord{}).
stmt := s.db.Model(&presentationRecord{}).Select("discovery_presentation.id").
Where("service_id = ?", serviceID)
if !allowUnvalidated {
stmt = stmt.Where("validated != 0")
}
// remove wildcards to prevent unneeded join on credential
filteredQuery := make(map[string]string)
for k, v := range query {
if strings.TrimSpace(v) != "*" {
filteredQuery[k] = v
}
}
if len(filteredQuery) > 0 {
stmt = stmt.Joins("inner join discovery_credential ON discovery_credential.presentation_id = discovery_presentation.id")
stmt = store.CredentialStore{}.BuildSearchStatement(stmt, "discovery_credential.credential_id", filteredQuery)
if len(query) > 0 {
stmt = applyQuery(stmt, query)
}
stmt = stmt.Group("discovery_presentation.id")

var matches []presentationRecord
if err := stmt.Preload("Credentials").Preload("Credentials.Credential").Find(&matches).Error; err != nil {
main := s.db.Preload("Credentials").Preload("Credentials.Credential").Model(&presentationRecord{}).Where("id in (?)", stmt)
if err := main.Find(&matches).Error; err != nil {
return nil, err
}
var results []vc.VerifiablePresentation
Expand All @@ -302,6 +297,52 @@ func (s *sqlStore) search(serviceID string, query map[string]string, allowUnvali
return results, nil
}

// applyQuery is like vcr/credential/store/sql.go#BuildSearchStatement but for searching VPs a group by is needed which also requires a sub query
// at that point a generic search statement is not maintainable
func applyQuery(stmt *gorm.DB, query map[string]string) *gorm.DB {
propertyColumns := map[string]string{
"id": "credential.id",
"issuer": "credential.issuer",
"type": "credential.type",
"credentialSubject.id": "credential.subject_id",
}

stmt = stmt.Joins("inner join discovery_credential ON discovery_credential.presentation_id = discovery_presentation.id")
stmt = stmt.Joins("inner join credential ON credential.id = discovery_credential.credential_id")
numProps := 0
for jsonPath, value := range query {
// sort out wildcard mode: prefix and postfix asterisks (*) are replaced with %, which then is used in a LIKE query.
// an asterisk is translated to IS NOT NULL
// Otherwise, exact match (=) is used.
var op = "= ?"
if strings.TrimSpace(value) == "*" {
op = "is not null"
value = ""
} else {
if strings.HasPrefix(value, "*") {
value = "%" + value[1:]
op = "LIKE ?"
}
// and or
if strings.HasSuffix(value, "*") {
value = value[:len(value)-1] + "%"
op = "LIKE ?"
}
}
if column := propertyColumns[jsonPath]; column != "" {
stmt = stmt.Where(column+" "+op, value)
} else {
// This property is not present as column, but indexed as key-value property.
// Multiple (inner) joins to filter on a dynamic number of properties to filter on is not pretty, but it works
alias := "p" + strconv.Itoa(numProps)
numProps++
// for an IS NOT NULL query, the value is ignored
stmt = stmt.Joins("inner join credential_prop "+alias+" ON "+alias+".credential_id = credential.id AND "+alias+".path = ? AND "+alias+".value "+op, jsonPath, value)
}
}
return stmt
}

// incrementTimestamp increments the last_timestamp of the given service. USed by server.
func (s *sqlStore) incrementTimestamp(tx *gorm.DB, serviceID string, seed string) (*int, error) {
service, err := s.findAndLockService(tx, serviceID)
Expand Down
7 changes: 7 additions & 0 deletions discovery/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ func Test_sqlStore_search(t *testing.T) {
}, true)
require.NoError(t, err)
require.Len(t, actualVPs, 0)

t.Run("wildcard", func(t *testing.T) {
actualVPs, err = c.search(testServiceID, map[string]string{"credentialSubject.person.noName": "*"}, true)
require.NoError(t, err)
require.Len(t, actualVPs, 0)
})

})
}

Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/discovery/run-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ sleep 2
echo "---------------------------------------"
echo "Searching for care organization registration on Discovery Server..."
echo "---------------------------------------"
RESPONSE=$(curl -s --insecure "http://localhost:18081/internal/discovery/v1/dev:eOverdracht2023?credentialSubject.organization.name=Care*")
RESPONSE=$(curl -s --insecure "http://localhost:18081/internal/discovery/v1/dev:eOverdracht2023?credentialSubject.organization.name=Care*&credentialSubject.organization.city=*")
NUM_ITEMS=$(echo $RESPONSE | jq length)
if [ $NUM_ITEMS -eq 1 ]; then
echo "Registration found"
Expand Down
10 changes: 10 additions & 0 deletions e2e-tests/oauth-flow/rfc021/do-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ else
exitWithDockerLogs 1
fi

# Search for registration using wildcards, results in complicated DB query
RESPONSE=$(curl -s --insecure http://localhost:28081/internal/discovery/v1/e2e-test?credentialSubject.organization.name=*)
NUM_ITEMS=$(echo $RESPONSE | jq length)
if [ $NUM_ITEMS -eq 1 ]; then
echo "Registration found"
else
echo "FAILED: Could not find registration" 1>&2
exitWithDockerLogs 1
fi

echo "---------------------------------------"
echo "Perform OAuth 2.0 rfc021 flow..."
echo "---------------------------------------"
Expand Down
Loading