Merge pull request #356 from privacybydesign/fix-verify-pin-callback

Fix: IRMA session gets stuck in communicating status when user is requested to confirm PIN

CHANGELOG.md

@@ -4,7 +4,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-## Unreleased
+## [0.14.2] - 2023-10-25
+### Fixed
+- IRMA session gets stuck in communicating status when user is requested to confirm PIN in `irmaclient`
 
 ## [0.14.1] - 2023-10-18
 ### Fixed
@@ -468,6 +470,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.14.2]: https://github.com/privacybydesign/irmago/compare/v0.14.1...v0.14.2
 [0.14.1]: https://github.com/privacybydesign/irmago/compare/v0.14.0...v0.14.1
 [0.14.0]: https://github.com/privacybydesign/irmago/compare/v0.13.3...v0.14.0
 [0.13.3]: https://github.com/privacybydesign/irmago/compare/v0.13.2...v0.13.3

internal/sessiontest/helper_handlers_test.go

@@ -139,7 +139,10 @@ func (th TestHandler) RequestVerificationPermission(request *irma.DisclosureRequ
 	if th.wait != 0 {
 		time.Sleep(th.wait)
 	}
-	callback(true, &choice)
+	// Do callback asynchronously to simulate user giving permission.
+	time.AfterFunc(100*time.Millisecond, func() {
+		callback(true, &choice)
+	})
 }
 func (th TestHandler) RequestIssuancePermission(request *irma.IssuanceRequest, satisfiable bool, candidates [][]irmaclient.DisclosureCandidates, ServerName *irma.RequestorInfo, callback irmaclient.PermissionHandler) {
 	th.RequestVerificationPermission(&request.DisclosureRequest, satisfiable, candidates, ServerName, callback)
@@ -148,10 +151,16 @@ func (th TestHandler) RequestSignaturePermission(request *irma.SignatureRequest,
 	th.RequestVerificationPermission(&request.DisclosureRequest, satisfiable, candidates, ServerName, callback)
 }
 func (th TestHandler) RequestSchemeManagerPermission(manager *irma.SchemeManager, callback func(proceed bool)) {
-	callback(true)
+	// Do callback asynchronously to simulate user giving permission.
+	time.AfterFunc(100*time.Millisecond, func() {
+		callback(true)
+	})
 }
 func (th TestHandler) RequestPin(remainingAttempts int, callback irmaclient.PinHandler) {
-	callback(true, "12345")
+	// Do callback asynchronously to simulate user entering pin.
+	time.AfterFunc(100*time.Millisecond, func() {
+		callback(true, "12345")
+	})
 }
 func (th TestHandler) PairingRequired(pairingCode string) {
 	// Send pairing code via channel to calling test. This is done such that

internal/sessiontest/keyshare_test.go

@@ -80,7 +80,9 @@ func TestKeyshareAttributeRenewal(t *testing.T) {
 
 	// Validate that keyshare attribute is invalid.
 	disclosureRequest := getDisclosureRequest(irma.NewAttributeTypeIdentifier("test.test.mijnirma.email"))
-	doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil, optionUnsatisfiableRequest)
+	result := doSession(t, disclosureRequest, client, irmaServer, nil, nil, nil, optionUnsatisfiableRequest)
+	// Session remains active when being unsatisfiable, so we have to close it manually.
+	result.Dismisser.Dismiss()
 
 	// Do a PIN verification. This should detect the invalid keyshare attribute and renew it.
 	valid, _, _, err := client.KeyshareVerifyPin("12345", irma.NewSchemeManagerIdentifier("test"))

irmaclient/keyshare.go

@@ -165,7 +165,9 @@ func newKeyshareSession(
 	}
 
 	ks.sessionHandler.KeysharePin()
-	return ks, ks.VerifyPin(-1)
+	authenticated := make(chan bool, 1)
+	ks.VerifyPin(-1, authenticated)
+	return ks, <-authenticated
 }
 
 func (kss *keyshareServer) tokenValid(conf *irma.Configuration) bool {
@@ -191,29 +193,37 @@ func (kss *keyshareServer) tokenValid(conf *irma.Configuration) bool {
 
 // VerifyPin asks for a pin, repeatedly if necessary, informing the handler of success or failure.
 // It returns whether the authentication was successful or not.
-func (ks *keyshareSession) VerifyPin(attempts int) bool {
+func (ks *keyshareSession) VerifyPin(attempts int, authenticated chan bool) {
 	ks.pinRequestor.RequestPin(attempts, PinHandler(func(proceed bool, pin string) {
 		if !proceed {
 			ks.sessionHandler.KeyshareCancelled()
+			authenticated <- false
 			return
 		}
 		success, attemptsRemaining, blocked, manager, err := ks.verifyPinAttempt(pin)
 		if err != nil {
 			ks.sessionHandler.KeyshareError(&manager, err)
+			authenticated <- false
 			return
 		}
 		if blocked != 0 {
 			ks.sessionHandler.KeyshareBlocked(manager, blocked)
+			authenticated <- false
 			return
 		}
 		if success {
-			ks.sessionHandler.KeysharePinOK()
+			if ok := ks.keyshareServer.tokenValid(ks.client.Configuration); ok {
+				ks.sessionHandler.KeysharePinOK()
+				authenticated <- true
+			} else {
+				ks.sessionHandler.KeyshareError(&manager, errors.New("keyshare token invalid after successful authentication"))
+				authenticated <- false
+			}
 			return
 		}
 		// Not successful but no error and not yet blocked: try again
-		ks.VerifyPin(attemptsRemaining)
+		ks.VerifyPin(attemptsRemaining, authenticated)
 	}))
-	return ks.keyshareServer.tokenValid(ks.client.Configuration)
 }
 
 // challengeRequestJWTExpiry is the expiry of the JWT sent to the keyshareserver at
@@ -370,8 +380,9 @@ func (ks *keyshareSession) GetCommitments() {
 				// (but only if we did not ask for a PIN earlier)
 				ks.pinCheck = false
 				ks.sessionHandler.KeysharePin()
-				authenticated := ks.VerifyPin(-1)
-				if authenticated {
+				authenticated := make(chan bool, 1)
+				ks.VerifyPin(-1, authenticated)
+				if <-authenticated {
 					ks.GetCommitments()
 				}
 				return

version.go

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