Fix did issuer encoding (#34)

* Fix did issuer encoding
Replaced the url.PathEncode with custom percent encoder

did_x509/did_x509.go

@@ -5,11 +5,13 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"github.com/nuts-foundation/go-did/did"
-	"github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert"
 	"net/url"
 	"regexp"
 	"strings"
+	"unicode"
+
+	"github.com/nuts-foundation/go-did/did"
+	"github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert"
 )
 
 type X509Did struct {
@@ -43,15 +45,37 @@ func CreateDid(signingCert, caCert *x509.Certificate, subjectAttributes []x509_c
 	policies := CreateOtherNamePolicies(otherNames)
 
 	subjectTypes, err := x509_cert.SelectSubjectTypes(signingCert, subjectAttributes...)
+	if err != nil {
+		return "", err
+	}
 
 	policies = append(policies, CreateSubjectPolicies(subjectTypes)...)
 
 	formattedDid, err := FormatDid(caCert, policies...)
 	return formattedDid, err
 }
+
+// PercentEncode encodes a string using percent encoding.
+// we can not use url.PathEscape because it does not escape : $ & + = : @ characters.
+// See https://github.com/golang/go/issues/27559#issuecomment-449652574
+func PercentEncode(input string) string {
+	var encoded strings.Builder
+	for _, r := range input {
+		if unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' || r == '_' || r == '.' {
+			encoded.WriteRune(r)
+		} else {
+			encoded.WriteString(fmt.Sprintf("%%%02X", r))
+		}
+	}
+	return encoded.String()
+}
+
 func ParseDid(didString string) (*X509Did, error) {
 	x509Did := X509Did{}
-	didObj := did.MustParseDID(didString)
+	didObj, err := did.ParseDID(didString)
+	if err != nil {
+		return nil, err
+	}
 	if didObj.Method != "x509" {
 		return nil, errors.New("invalid didString method")
 	}
@@ -96,7 +120,7 @@ func ParseDid(didString string) (*X509Did, error) {
 func CreateOtherNamePolicies(otherNames []*x509_cert.OtherNameValue) []string {
 	var policies []string
 	for _, otherName := range otherNames {
-		value := url.PathEscape(otherName.Value)
+		value := PercentEncode(otherName.Value)
 		fragments := []string{string(otherName.PolicyType), string(otherName.Type), value}
 		policy := strings.Join(fragments, ":")
 		policies = append(policies, policy)
@@ -107,7 +131,7 @@ func CreateOtherNamePolicies(otherNames []*x509_cert.OtherNameValue) []string {
 func CreateSubjectPolicies(subjectValues []*x509_cert.SubjectValue) []string {
 	var policies []string
 	for _, subjectValue := range subjectValues {
-		value := url.PathEscape(subjectValue.Value)
+		value := PercentEncode(subjectValue.Value)
 		fragments := []string{string(subjectValue.PolicyType), string(subjectValue.Type), value}
 		policy := strings.Join(fragments, ":")
 		policies = append(policies, policy)

did_x509/did_x509_test.go

@@ -3,15 +3,37 @@ package did_x509
 import (
 	"crypto/x509"
 	"encoding/base64"
-	"github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert"
-	"reflect"
 	"strings"
 	"testing"
+
+	"github.com/nuts-foundation/uzi-did-x509-issuer/x509_cert"
+	"github.com/stretchr/testify/assert"
 )
 
-// TestDefaultDidCreator_CreateDid tests the CreateDid function of DefaultDidProcessor by providing different certificate chains.
+func TestPercentEncode(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected string
+	}{
+		{"hello world", "hello%20world"},
+		{"[email protected]", "foo%40bar.com"},
+		{"100%", "100%25"},
+		{"a+b=c", "a%2Bb%3Dc"},
+		{"~!@#$%^&*()_+", "%7E%21%40%23%24%25%5E%26%2A%28%29_%2B"},
+		{"FauxCare & Co", "FauxCare%20%26%20Co"},
+	}
+
+	for _, test := range tests {
+		result := PercentEncode(test.input)
+		if result != test.expected {
+			t.Errorf("PercentEncode(%q) = %q; want %q", test.input, result, test.expected)
+		}
+	}
+}
+
+// TestCreateDid tests the CreateDid function of DefaultDidProcessor by providing different certificate chains.
 // It checks for correct DID generation and appropriate error messages.
-func TestDefaultDidCreator_CreateDidSingle(t *testing.T) {
+func TestCreateDidSingle(t *testing.T) {
 	type fields struct {
 	}
 	type args struct {
@@ -99,7 +121,7 @@ func TestDefaultDidCreator_CreateDidSingle(t *testing.T) {
 		})
 	}
 }
-func TestDefaultDidCreator_CreateDidDouble(t *testing.T) {
+func TestCreateDidDouble(t *testing.T) {
 	type fields struct {
 	}
 	type args struct {
@@ -183,9 +205,9 @@ func TestDefaultDidCreator_CreateDidDouble(t *testing.T) {
 	}
 }
 
-// TestDefaultDidCreator_ParseDid tests the ParseDid function of DefaultDidProcessor by providing different DID strings.
+// TestParseDid tests the ParseDid function of DefaultDidProcessor by providing different DID strings.
 // It checks for correct X509Did parsing and appropriate error messages.
-func TestDefaultDidCreator_ParseDid(t *testing.T) {
+func TestParseDid(t *testing.T) {
 	policies := []*x509_cert.GenericNameValue{
 		{
 			PolicyType: "san",
@@ -206,24 +228,30 @@ func TestDefaultDidCreator_ParseDid(t *testing.T) {
 		errMsg string
 	}{
 		{
-			name:   "Invalid DID method",
+			name:   "ok - happy path",
+			fields: fields{},
+			args:   args{didString: "did:x509:0:sha512:hash::san:otherName:A_BIG_STRING"},
+			want:   &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: policies},
+			errMsg: "",
+		},
+		{
+			name:   "nok - invalid DID method",
 			fields: fields{},
 			args:   args{didString: "did:abc:0:sha512:hash::san:otherName:A_BIG_STRING"},
 			want:   nil,
 			errMsg: "invalid didString method",
 		},
 		{
-			name:   "Invalid DID format",
+			name:   "nok - invalid DID format",
 			fields: fields{},
 			args:   args{didString: "did:x509:0:sha512::san:otherName:A_BIG_STRING"},
 			want:   nil,
 			errMsg: "invalid didString format, expected didString:x509:0:alg:hash::san:type:ura",
 		},
-		{
-			name:   "Happy path",
+		{name: "ok - correct unescaping",
 			fields: fields{},
-			args:   args{didString: "did:x509:0:sha512:hash::san:otherName:A_BIG_STRING"},
-			want:   &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: policies},
+			args:   args{didString: "did:x509:0:sha512:hash::san:otherName:hello%20world%20from%20FauxCare%20%26%20Co"},
+			want:   &X509Did{Version: "0", RootCertificateHashAlg: "sha512", RootCertificateHash: "hash", Policies: []*x509_cert.GenericNameValue{{PolicyType: "san", Type: "otherName", Value: "hello world from FauxCare & Co"}}},
 			errMsg: "",
 		},
 	}
@@ -232,20 +260,16 @@ func TestDefaultDidCreator_ParseDid(t *testing.T) {
 			got, err := ParseDid(tt.args.didString)
 			wantErr := tt.errMsg != ""
 			if (err != nil) != wantErr {
-				t.Errorf("DefaultDidProcessor.ParseDid() error = %v, expected error = %v", err, tt.errMsg)
+				t.Errorf("ParseDid() error = %v, expected error = %v", err, tt.errMsg)
 				return
 			} else if wantErr {
 				if err.Error() != tt.errMsg {
-					t.Errorf("DefaultDidProcessor.ParseDid() expected = \"%v\", got = \"%v\"", tt.errMsg, err.Error())
+					t.Errorf("ParseDid() expected = \"%v\", got = \"%v\"", tt.errMsg, err.Error())
 				}
 			}
 
-			if tt.want != nil && got != nil &&
-				(tt.want.Version != got.Version ||
-					tt.want.RootCertificateHashAlg != got.RootCertificateHashAlg ||
-					tt.want.RootCertificateHash != got.RootCertificateHash ||
-					!reflect.DeepEqual(tt.want.Policies, got.Policies)) {
-				t.Errorf("DefaultDidProcessor.ParseDid() = %v, want = %v", got, tt.want)
+			if tt.want != nil && got != nil {
+				assert.Equal(t, tt.want.Policies, got.Policies)
 			}
 		})
 	}

uzi_vc_issuer/ura_issuer.go

@@ -6,6 +6,7 @@ import (
 	"crypto/sha1"
 	"crypto/x509"
 	"encoding/base64"
+	"encoding/pem"
 	"errors"
 	"fmt"
 	"os"
@@ -392,8 +393,7 @@ func convertHeaders(headers map[string]interface{}) (jws.Headers, error) {
 	return hdr, nil
 }
 
-// uraCredential generates a VerifiableCredential for a given URA and UZI number, including the subject's DID.
-// It sets a 1-year expiration period from the current issuance date.
+// uraCredential builds a VerifiableCredential for a given URA and UZI number, including the subject's DID.
 func uraCredential(issuer string, expirationDate time.Time, otherNameValues []*x509_cert.OtherNameValue, subjectTypes []*x509_cert.SubjectValue, subjectDID subjectDID) (*vc.VerifiableCredential, error) {
 	iat := time.Now()
 	subject := map[string]interface{}{
@@ -407,8 +407,13 @@ func uraCredential(issuer string, expirationDate time.Time, otherNameValues []*x
 		subject[string(subjectType.Type)] = subjectType.Value
 	}
 
+	issuerDID, err := did.ParseDID(issuer)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse issuer DID '%s': %w", issuer, err)
+	}
+
 	id := did.DIDURL{
-		DID:      did.MustParseDID(issuer),
+		DID:      *issuerDID,
 		Fragment: uuid.NewString(),
 	}.URI()
 	return &vc.VerifiableCredential{

uzi_vc_issuer/ura_issuer_test.go

@@ -200,6 +200,18 @@ func TestIssue(t *testing.T) {
 
 		assert.Equal(t, validChain[0].NotAfter, *vc.ExpirationDate, "expiration date of VC must match signing certificate")
 	})
+
+	t.Run("ok - correct escaping of special characters", func(t *testing.T) {
+		validChain, err := NewValidCertificateChain("testdata/valid_chain.pem")
+		require.NoError(t, err, "failed to read chain")
+
+		validChain[0].Subject.Organization = []string{"FauxCare & Co"}
+
+		vc, err := Issue(validChain, validKey, "did:example:123", SubjectAttributes(x509_cert.SubjectTypeCountry, x509_cert.SubjectTypeOrganization))
+
+		assert.Equal(t, "did:x509:0:sha512:0OXDVLevEnf_sE-Ayopm0Yof_gmBwxwKZmzbDhKeAwj9vcsI_Q14TBArYsCftQTABLM-Vx9BB6zI05Me2aksaA::san:otherName:2.16.528.1.1007.99.2110-1-1111111-S-2222222-00.000-333333::subject:O:FauxCare%20%26%20Co", vc.Issuer.String())
+	})
+
 }
 
 func TestParsePemBytes(t *testing.T) {