Merge pull request #504 from privacybydesign/fix-credential-removal-logs

Fix credential removal logs

Author: w-ensink

CHANGELOG.md

@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+## [0.19.1] - 2025-10-13
+### Fix
+- Bug in `irmaclient` that caused attributes to be stored in the wrong order in credential removal logs
+
 ## [0.19.0] - 2025-09-30
 ### Changed
 - Remove legacy storage from irmaclient
@@ -580,6 +584,7 @@ This release contains several large new features. In particular, the shoulder su
 - Combined issuance-disclosure requests with two schemes one of which has a keyshare server now work as expected
 - Various other bugfixes
 
+[0.19.1]: https://github.com/privacybydesign/irmago/compare/v0.19.0...v0.19.1
 [0.19.0]: https://github.com/privacybydesign/irmago/compare/v0.18.1...v0.19.0
 [0.18.1]: https://github.com/privacybydesign/irmago/compare/v0.18.0...v0.18.1
 [0.18.0]: https://github.com/privacybydesign/irmago/compare/v0.17.1...v0.18.0

internal/sessiontest/client_integration_test.go

@@ -60,7 +60,7 @@ func testDoubleSdJwtIssuanceReplacesInstances(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	info := client.CredentialInfoList()
 	require.Len(t, info, 3)
@@ -72,7 +72,7 @@ func testDoubleSdJwtIssuanceReplacesInstances(t *testing.T) {
 
 	require.Equal(t, 10, int(*cred.InstanceCount))
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	info = client.CredentialInfoList()
 	require.Len(t, info, 3)
@@ -95,7 +95,7 @@ func testCredentialInstanceCount(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	info := client.CredentialInfoList()
 	require.Len(t, info, 3)
@@ -153,6 +153,20 @@ func testLogsForCombinedIssuanceAndDisclosure(t *testing.T) {
 	require.Len(t, latestLog.IssuanceLog.Credentials, 1)
 }
 
+func createMijnOverheidIssuanceRequest() *irma.IssuanceRequest {
+	return irma.NewIssuanceRequest([]*irma.CredentialRequest{
+		{
+			CredentialTypeID: irma.NewCredentialTypeIdentifier("irma-demo.MijnOverheid.fullName"),
+			Attributes: map[string]string{
+				"firstnames": "Barry",
+				"firstname":  "",
+				"familyname": "Batsbak",
+				"prefix":     "Sir",
+			},
+		},
+	})
+}
+
 func performCombinedIssuanceAndDisclosureSession(t *testing.T, client *irmaclient.Client, irmaServer *IrmaServer) {
 	regularIssuanceRequest := irma.NewIssuanceRequest([]*irma.CredentialRequest{
 		{
@@ -268,7 +282,7 @@ func testRemoveStorageWithOnlyIdemixCredentials(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueIdemixOnlyToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIdemixOnlyIssuanceRequest())
 
 	require.NoError(t, client.RemoveStorage())
 }
@@ -287,32 +301,55 @@ func testRemoveStorageEmptyClient(t *testing.T) {
 	require.NoError(t, client.RemoveStorage())
 }
 
-func testIdemixAndSdJwtCombinedRemovalLog(t *testing.T) {
-	conf := irmaServerConfWithSdJwtEnabled(t)
-	irmaServer := StartIrmaServer(t, conf)
-	defer irmaServer.Stop()
+func testIdemixOnlyCredentialRemovalLog(t *testing.T) {
+	// This test makes sure the attributes in the log are not in incorrect order.
+	// It solves a bug we had where the list of attributes had an incorrect order due to a (unordered) map being converted to a list.
+	tester := func(t *testing.T) {
+		conf := IrmaServerConfigurationWithTempStorage(t)
+		irmaServer := StartIrmaServer(t, conf)
+		defer irmaServer.Stop()
 
-	keyshareServer := testkeyshare.StartKeyshareServer(t, logger, irma.NewSchemeManagerIdentifier("test"), 0)
-	defer keyshareServer.Stop()
+		keyshareServer := testkeyshare.StartKeyshareServer(t, logger, irma.NewSchemeManagerIdentifier("test"), 0)
+		defer keyshareServer.Stop()
 
-	client := createClient(t)
-	defer client.Close()
+		client := createClient(t)
+		defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+		performIrmaIssuanceSession(t, client, irmaServer, createMijnOverheidIssuanceRequest())
 
-	credentials := client.CredentialInfoList()
-	emailCreds := collectCredentialsWithId(credentials, "test.test.email")
+		credentials := client.CredentialInfoList()
+		emailCreds := collectCredentialsWithId(credentials, "irma-demo.MijnOverheid.fullName")
 
-	require.NoError(t, client.RemoveCredentialsByHash(hashByFormat(emailCreds)))
+		require.NoError(t, client.RemoveCredentialsByHash(hashByFormat(emailCreds)))
 
-	logs, err := client.LoadNewestLogs(100)
-	require.NoError(t, err)
+		logs, err := client.LoadNewestLogs(100)
+		require.NoError(t, err)
+
+		require.Equal(t, logs[0].Type, irmaclient.LogType_CredentialRemoval)
+		removalLog := logs[0].RemovalLog
+
+		// one credential was removed
+		require.Len(t, removalLog.Credentials, 1)
+
+		credential := removalLog.Credentials[0]
+
+		require.Contains(t, credential.Formats, irmaclient.Format_Idemix)
+
+		attrs := credential.Attributes
 
-	requireIdemixAndSdJwtCredentialRemovalLog(t, logs[0])
+		require.Equal(t, attrs["firstnames"], "Barry")
+		require.Equal(t, attrs["firstname"], "")
+		require.Equal(t, attrs["familyname"], "Batsbak")
+		require.Equal(t, attrs["prefix"], "Sir")
+	}
+
+	for range 10 {
+		tester(t)
+	}
 }
 
-func testIdemixOnlyCredentialRemovalLog(t *testing.T) {
-	conf := IrmaServerConfigurationWithTempStorage(t)
+func testIdemixAndSdJwtCombinedRemovalLog(t *testing.T) {
+	conf := irmaServerConfWithSdJwtEnabled(t)
 	irmaServer := StartIrmaServer(t, conf)
 	defer irmaServer.Stop()
 
@@ -322,7 +359,7 @@ func testIdemixOnlyCredentialRemovalLog(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueIdemixOnlyToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIdemixOnlyIssuanceRequest())
 
 	credentials := client.CredentialInfoList()
 	emailCreds := collectCredentialsWithId(credentials, "test.test.email")
@@ -405,7 +442,7 @@ func testDoubleSdJwtIssuanceFailsAfterRevocationListUpdate(t *testing.T) {
 	time.Sleep(4 * time.Second)
 
 	// Execute first issuance
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	info := client.CredentialInfoList()
 	require.Len(t, info, 3)
@@ -450,19 +487,6 @@ func testDoubleSdJwtIssuanceFailsAfterRevocationListUpdate(t *testing.T) {
 	require.Equal(t, 10, int(*cred.InstanceCount))
 }
 
-func requireIdemixAndSdJwtCredentialRemovalLog(t *testing.T, log irmaclient.LogInfo) {
-	require.Equal(t, log.Type, irmaclient.LogType_CredentialRemoval)
-
-	credLog := log.RemovalLog.Credentials[0]
-
-	require.Equal(t, credLog.CredentialType, "test.test.email")
-	require.Contains(t, credLog.Formats, irmaclient.Format_Idemix)
-	require.Contains(t, credLog.Formats, irmaclient.Format_SdJwtVc)
-	require.Equal(t, credLog.Attributes, map[string]string{
-		"email": "test@gmail.com",
-	})
-}
-
 func requireIdemixOnlyCredentialRemovalLog(t *testing.T, log irmaclient.LogInfo) {
 	require.Equal(t, log.Type, irmaclient.LogType_CredentialRemoval)
 	require.Equal(t, log.RemovalLog.Credentials, []irmaclient.CredentialLog{
@@ -487,7 +511,7 @@ func testIrmaDisclosureSessionLogs(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueIdemixOnlyToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIdemixOnlyIssuanceRequest())
 	performIrmaDisclosureSession(t, client, irmaServer)
 
 	logs, err := client.LoadNewestLogs(100)
@@ -508,7 +532,7 @@ func testIrmaSignatureSessionLogs(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueIdemixOnlyToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIdemixOnlyIssuanceRequest())
 	performIrmaSignatureSession(t, client, irmaServer)
 
 	logs, err := client.LoadNewestLogs(100)
@@ -565,7 +589,7 @@ func testEudiSessionLogs(t *testing.T) {
 	// only keyshare enrollment log should be there
 	require.Len(t, logs, 1)
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	logs, err = client.LoadNewestLogs(100)
 	require.NoError(t, err)
@@ -636,7 +660,7 @@ func testDeletingCombinedCredentialDeletesBothFormats(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	credentialInfoList := client.CredentialInfoList()
 	require.Len(t, credentialInfoList, 3)
@@ -661,7 +685,7 @@ func testIdemixAndSdJwtShowUpAsSeparateCredentialInfos(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	credentialInfoList := client.CredentialInfoList()
 	require.Len(t, credentialInfoList, 3)
@@ -681,7 +705,7 @@ func testIdemixAndSdJwtCombinedIssuance(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 }
 
 func testDiscloseOverOpenID4VP(t *testing.T) {
@@ -695,7 +719,7 @@ func testDiscloseOverOpenID4VP(t *testing.T) {
 	client := createClient(t)
 	defer client.Close()
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 	discloseOverOpenID4VP(t, client, testdata.OpenID4VP_DirectPost_Host)
 	discloseOverOpenID4VP(t, client, testdata.OpenID4VP_DirectPostJwt_Host)
 }
@@ -730,35 +754,15 @@ func discloseOverOpenID4VP(t *testing.T, client *irmaclient.Client, openid4vpHos
 	require.True(t, sessionHandler.AwaitSessionEnd())
 }
 
-func issueIdemixOnlyToClient(t *testing.T, client *irmaclient.Client, irmaServer *IrmaServer) {
-	req := irma.NewIssuanceRequest([]*irma.CredentialRequest{
+func createIdemixOnlyIssuanceRequest() *irma.IssuanceRequest {
+	return irma.NewIssuanceRequest([]*irma.CredentialRequest{
 		{
 			CredentialTypeID: irma.NewCredentialTypeIdentifier("test.test.email"),
 			Attributes: map[string]string{
 				"email": "test@gmail.com",
 			},
 		},
 	})
-	session := startIrmaSessionAtServer(t, irmaServer, req)
-	sessionHandler := irmaclient.NewMockSessionHandler(t)
-
-	client.NewSession(session, sessionHandler)
-	permissionReq := sessionHandler.AwaitPermissionRequest()
-
-	permissionReq.PermissionHandler(true, nil)
-	sessionHandler.AwaitSessionEnd()
-}
-
-func issueSdJwtAndIdemixToClient(t *testing.T, client *irmaclient.Client, irmaServer *IrmaServer) {
-	sessionReq := createIrmaIssuanceRequestWithSdJwts("test.test.email", "email")
-	sessionRequestJson := startIrmaSessionAtServer(t, irmaServer, sessionReq)
-
-	sessionHandler := irmaclient.NewMockSessionHandler(t)
-	client.NewSession(sessionRequestJson, sessionHandler)
-	details := sessionHandler.AwaitPermissionRequest()
-	details.PermissionHandler(true, nil)
-
-	require.True(t, sessionHandler.AwaitSessionEnd())
 }
 
 func failIssueSdJwtAndIdemixToClient(t *testing.T, client *irmaclient.Client, irmaServer *IrmaServer) {

internal/sessiontest/ios_logo_path_bug_test.go

@@ -33,7 +33,7 @@ func test_iOSLogoPathBugEudiLogs(t *testing.T) {
 	client, handler := createClientWithStorageAndSigner(t, storagePath, irmaConfigurationPath, signer)
 	keyshareEnrollClient(t, client, handler)
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 	discloseOverOpenID4VP(t, client, testdata.OpenID4VP_DirectPost_Host)
 
 	logs, err := client.LoadNewestLogs(1)
@@ -89,7 +89,7 @@ func test_iOSLogoPathBug(t *testing.T) {
 	client, handler := createClientWithStorageAndSigner(t, storagePath, irmaConfigurationPath, signer)
 	keyshareEnrollClient(t, client, handler)
 
-	issueSdJwtAndIdemixToClient(t, client, irmaServer)
+	performIrmaIssuanceSession(t, client, irmaServer, createIrmaIssuanceRequestWithSdJwts("test.test.email", "email"))
 
 	logs, err := client.LoadNewestLogs(1)
 	require.NoError(t, err)

irmaclient/client.go

@@ -292,7 +292,7 @@ func (client *Client) RemoveCredentialsByHash(hashByFormat map[CredentialFormat]
 	}
 
 	info := relevantCreds[0]
-	logEntry, err := createRemovalLog(info.Identifier(), info.Attributes, formats)
+	logEntry, err := createRemovalLog(client.GetIrmaConfiguration(), info.Identifier(), info.Attributes, formats)
 	if err != nil {
 		return fmt.Errorf("failed to create delete log: %v", err)
 	}
@@ -301,13 +301,17 @@ func (client *Client) RemoveCredentialsByHash(hashByFormat map[CredentialFormat]
 }
 
 func createRemovalLog(
+	irmaConfiguration *irma.Configuration,
 	credentialType irma.CredentialTypeIdentifier,
 	attributes map[irma.AttributeTypeIdentifier]irma.TranslatedString,
 	formats []CredentialFormat,
 ) (*LogEntry, error) {
 	attrs := []irma.TranslatedString{}
-	for _, attr := range attributes {
-		attrs = append(attrs, attr)
+
+	// loop over it in the order as defined in the scheme
+	for _, t := range irmaConfiguration.CredentialTypes[credentialType].AttributeTypes {
+		id := t.GetAttributeTypeIdentifier()
+		attrs = append(attrs, attributes[id])
 	}
 
 	return &LogEntry{

version.go

@@ -5,4 +5,4 @@
 package irma
 
 // Version of the IRMA command line and libraries
-const Version = "0.19.0"
+const Version = "0.19.1"