Refactor registration flow

configstore.go

@@ -356,20 +356,20 @@ func (store *ConfigurationStore) Download(set *IrmaIdentifierSet) (*IrmaIdentifi
 			}
 			downloaded.Issuers[issid] = struct{}{}
 		}
-		for issid, list := range set.PublicKeys {
-			for _, count := range list {
-				pk, err := store.PublicKey(issid, count)
-				if err != nil {
+	}
+	for issid, list := range set.PublicKeys {
+		for _, count := range list {
+			pk, err := store.PublicKey(issid, count)
+			if err != nil {
+				return nil, err
+			}
+			if pk == nil {
+				manager := issid.SchemeManagerIdentifier()
+				suffix := fmt.Sprintf("/%s/PublicKeys/%d.xml", issid.Name(), count)
+				path := fmt.Sprintf("%s/%s/%s", store.path, manager.String(), suffix)
+				if transport.GetFile(store.SchemeManagers[manager].URL+suffix, path); err != nil {
 					return nil, err
 				}
-				if pk == nil {
-					manager := issid.SchemeManagerIdentifier()
-					suffix := fmt.Sprintf("/%s/PublicKeys/%d.xml", issid.Name(), count)
-					path := fmt.Sprintf("%s/%s/%s", store.path, manager.String(), suffix)
-					if transport.GetFile(store.SchemeManagers[manager].URL+suffix, path); err != nil {
-						return nil, err
-					}
-				}
 			}
 		}
 	}

irmago_test.go

@@ -27,14 +27,12 @@ func TestMain(m *testing.M) {
 	os.Exit(retCode)
 }
 
-type IgnoringKeyshareHandler struct{}
+type IgnoringClientHandler struct{}
 
-func (i *IgnoringKeyshareHandler) UpdateConfigurationStore(new *IrmaIdentifierSet) {}
-func (i *IgnoringKeyshareHandler) UpdateAttributes()                               {}
-func (i *IgnoringKeyshareHandler) RegistrationError(err error)                     {}
-func (i *IgnoringKeyshareHandler) RegistrationSuccess()                            {}
-func (i *IgnoringKeyshareHandler) StartRegistration(m *SchemeManager, callback func(e, p string)) {
-}
+func (i *IgnoringClientHandler) UpdateConfigurationStore(new *IrmaIdentifierSet)              {}
+func (i *IgnoringClientHandler) UpdateAttributes()                                            {}
+func (i *IgnoringClientHandler) RegistrationError(manager SchemeManagerIdentifier, err error) {}
+func (i *IgnoringClientHandler) RegistrationSuccess(manager SchemeManagerIdentifier)          {}
 
 func parseStorage(t *testing.T) *CredentialManager {
 	exists, err := PathExists("testdata/storage/test")
@@ -46,7 +44,7 @@ func parseStorage(t *testing.T) *CredentialManager {
 		"testdata/storage/test",
 		"testdata/irma_configuration",
 		"testdata/oldstorage",
-		&IgnoringKeyshareHandler{},
+		&IgnoringClientHandler{},
 	)
 	require.NoError(t, err)
 	return manager

manager.go

@@ -46,18 +46,18 @@ type CredentialManager struct {
 	storage storage
 
 	// Other state
-	ConfigurationStore    *ConfigurationStore
-	irmaConfigurationPath string
-	androidStoragePath    string
-	handler               ClientHandler
+	ConfigurationStore        *ConfigurationStore
+	UnenrolledKeyshareServers []SchemeManagerIdentifier
+	irmaConfigurationPath     string
+	androidStoragePath        string
+	handler                   ClientHandler
 }
 
 // KeyshareHandler is used for asking the user for his email address and PIN,
 // for registering at a keyshare server.
 type KeyshareHandler interface {
-	StartRegistration(manager *SchemeManager, registrationCallback func(email, pin string))
-	RegistrationError(err error)
-	RegistrationSuccess()
+	RegistrationError(manager SchemeManagerIdentifier, err error)
+	RegistrationSuccess(manager SchemeManagerIdentifier)
 }
 
 type ClientHandler interface {
@@ -142,12 +142,8 @@ func NewCredentialManager(
 		cm.paillierKey(false)
 	}
 
-	unenrolled := cm.unenrolledKeyshareServers()
-	switch len(unenrolled) {
-	case 0: // nop
-	case 1:
-		cm.KeyshareEnroll(unenrolled[0], cm.handler)
-	default:
+	cm.UnenrolledKeyshareServers = cm.unenrolledKeyshareServers()
+	if len(cm.UnenrolledKeyshareServers) > 1 {
 		return nil, errors.New("Too many keyshare servers")
 	}
 
@@ -596,28 +592,27 @@ func (cm *CredentialManager) paillierKeyWorker(wait bool, ch chan bool) {
 	}
 }
 
-func (cm *CredentialManager) unenrolledKeyshareServers() []*SchemeManager {
-	list := []*SchemeManager{}
+func (cm *CredentialManager) unenrolledKeyshareServers() []SchemeManagerIdentifier {
+	list := []SchemeManagerIdentifier{}
 	for name, manager := range cm.ConfigurationStore.SchemeManagers {
-		if _, contains := cm.keyshareServers[name]; len(manager.KeyshareServer) > 0 && !contains {
-			list = append(list, manager)
+		if _, contains := cm.keyshareServers[name]; manager.Distributed() && !contains {
+			list = append(list, manager.Identifier())
 		}
 	}
 	return list
 }
 
 // KeyshareEnroll attempts to register at the keyshare server of the specified scheme manager.
-func (cm *CredentialManager) KeyshareEnroll(manager *SchemeManager, handler KeyshareHandler) {
-	handler.StartRegistration(manager, func(email, pin string) {
-		go func() {
-			err := cm.keyshareEnrollWorker(manager.Identifier(), email, pin)
-			if err != nil {
-				handler.RegistrationError(err)
-			} else {
-				handler.RegistrationSuccess()
-			}
-		}()
-	})
+func (cm *CredentialManager) KeyshareEnroll(manager SchemeManagerIdentifier, email, pin string) {
+	go func() {
+		err := cm.keyshareEnrollWorker(manager, email, pin)
+		cm.UnenrolledKeyshareServers = cm.unenrolledKeyshareServers()
+		if err != nil {
+			cm.handler.RegistrationError(manager, err)
+		} else {
+			cm.handler.RegistrationSuccess(manager)
+		}
+	}()
 }
 
 func (cm *CredentialManager) keyshareEnrollWorker(managerID SchemeManagerIdentifier, email, pin string) error {

requests.go

@@ -181,27 +181,27 @@ func newIssuanceState() (*issuanceState, error) {
 	}, nil
 }
 
-func (dr *IssuanceRequest) Identifiers() *IrmaIdentifierSet {
-	if dr.identifiers == nil {
-		dr.identifiers = &IrmaIdentifierSet{
+func (ir *IssuanceRequest) Identifiers() *IrmaIdentifierSet {
+	if ir.identifiers == nil {
+		ir.identifiers = &IrmaIdentifierSet{
 			SchemeManagers:  map[SchemeManagerIdentifier]struct{}{},
 			Issuers:         map[IssuerIdentifier]struct{}{},
 			CredentialTypes: map[CredentialTypeIdentifier]struct{}{},
 			PublicKeys:      map[IssuerIdentifier][]int{},
 		}
 
-		for _, credreq := range dr.Credentials {
+		for _, credreq := range ir.Credentials {
 			issuer := credreq.CredentialTypeID.IssuerIdentifier()
-			dr.identifiers.SchemeManagers[issuer.SchemeManagerIdentifier()] = struct{}{}
-			dr.identifiers.Issuers[issuer] = struct{}{}
-			dr.identifiers.CredentialTypes[*credreq.CredentialTypeID] = struct{}{}
-			if dr.identifiers.PublicKeys[issuer] == nil {
-				dr.identifiers.PublicKeys[issuer] = []int{}
+			ir.identifiers.SchemeManagers[issuer.SchemeManagerIdentifier()] = struct{}{}
+			ir.identifiers.Issuers[issuer] = struct{}{}
+			ir.identifiers.CredentialTypes[*credreq.CredentialTypeID] = struct{}{}
+			if ir.identifiers.PublicKeys[issuer] == nil {
+				ir.identifiers.PublicKeys[issuer] = []int{}
 			}
-			dr.identifiers.PublicKeys[issuer] = append(dr.identifiers.PublicKeys[issuer], credreq.KeyCounter)
+			ir.identifiers.PublicKeys[issuer] = append(ir.identifiers.PublicKeys[issuer], credreq.KeyCounter)
 		}
 
-		for _, disjunction := range dr.Disclose {
+		for _, disjunction := range ir.Disclose {
 			for _, attr := range disjunction.Attributes {
 				var cti CredentialTypeIdentifier
 				if !attr.IsCredential() {
@@ -209,13 +209,13 @@ func (dr *IssuanceRequest) Identifiers() *IrmaIdentifierSet {
 				} else {
 					cti = NewCredentialTypeIdentifier(attr.String())
 				}
-				dr.identifiers.SchemeManagers[cti.IssuerIdentifier().SchemeManagerIdentifier()] = struct{}{}
-				dr.identifiers.Issuers[cti.IssuerIdentifier()] = struct{}{}
-				dr.identifiers.CredentialTypes[cti] = struct{}{}
+				ir.identifiers.SchemeManagers[cti.IssuerIdentifier().SchemeManagerIdentifier()] = struct{}{}
+				ir.identifiers.Issuers[cti.IssuerIdentifier()] = struct{}{}
+				ir.identifiers.CredentialTypes[cti] = struct{}{}
 			}
 		}
 	}
-	return dr.identifiers
+	return ir.identifiers
 }
 
 // ToDisclose returns the attributes that must be disclosed in this issuance session.

session.go

@@ -24,6 +24,7 @@ type Handler interface {
 	Cancelled(action Action)
 	Failure(action Action, err *SessionError)
 	UnsatisfiableRequest(action Action, missing AttributeDisjunctionList)
+	MissingKeyshareServer(manager SchemeManagerIdentifier)
 
 	RequestIssuancePermission(request IssuanceRequest, ServerName string, callback PermissionHandler)
 	RequestVerificationPermission(request DisclosureRequest, ServerName string, callback PermissionHandler)
@@ -46,7 +47,6 @@ type session struct {
 	irmaSession IrmaSession
 	transport   *HTTPTransport
 	choice      *DisclosureChoice
-	newmanager  *SchemeManager
 	downloaded  *IrmaIdentifierSet
 }
 
@@ -184,6 +184,17 @@ func (session *session) start() {
 		}
 	}
 
+	// Check if we are registered to all involved keyshare servers
+	for id := range session.irmaSession.Identifiers().SchemeManagers {
+		distributed := session.credManager.ConfigurationStore.SchemeManagers[id].Distributed()
+		_, registered := session.credManager.keyshareServers[id]
+		if distributed && !registered {
+			session.transport.Delete()
+			session.Handler.MissingKeyshareServer(id)
+			return
+		}
+	}
+
 	// Download missing credential types/issuers/public keys from the scheme manager
 	if session.downloaded, err = session.credManager.ConfigurationStore.Download(session.irmaSession.Identifiers()); err != nil {
 		session.Handler.Failure(
@@ -351,39 +362,21 @@ func (session *session) managerSession() {
 			session.Handler.Cancelled(session.Action) // No need to DELETE session here
 			return
 		}
-		session.newmanager = manager
+		if err := session.credManager.ConfigurationStore.AddSchemeManager(manager); err != nil {
+			session.Handler.Failure(session.Action, &SessionError{})
+			return
+		}
 		if manager.Distributed() {
-			session.credManager.KeyshareEnroll(manager, KeyshareHandler(session))
-		} else {
-			session.RegistrationSuccess()
+			session.credManager.UnenrolledKeyshareServers = session.credManager.unenrolledKeyshareServers()
 		}
+		session.credManager.handler.UpdateConfigurationStore(
+			&IrmaIdentifierSet{
+				SchemeManagers:  map[SchemeManagerIdentifier]struct{}{manager.Identifier(): {}},
+				Issuers:         map[IssuerIdentifier]struct{}{},
+				CredentialTypes: map[CredentialTypeIdentifier]struct{}{},
+			},
+		)
+		session.Handler.Success(session.Action)
 	})
 	return
 }
-
-func (session *session) StartRegistration(manager *SchemeManager, callback func(email, pin string)) {
-	session.credManager.handler.StartRegistration(manager, callback)
-}
-
-func (session *session) RegistrationError(err error) {
-	session.Handler.Failure(session.Action, &SessionError{Err: err}) // TODO
-	session.credManager.handler.RegistrationError(err)
-}
-
-func (session *session) RegistrationSuccess() {
-	if err := session.credManager.ConfigurationStore.AddSchemeManager(session.newmanager); err != nil {
-		session.Handler.Failure(session.Action, &SessionError{})
-		return
-	}
-	session.credManager.handler.UpdateConfigurationStore(
-		&IrmaIdentifierSet{
-			SchemeManagers:  map[SchemeManagerIdentifier]struct{}{session.newmanager.Identifier(): {}},
-			Issuers:         map[IssuerIdentifier]struct{}{},
-			CredentialTypes: map[CredentialTypeIdentifier]struct{}{},
-		},
-	)
-	session.Handler.Success(session.Action)
-	if session.newmanager.Distributed() {
-		session.credManager.handler.RegistrationSuccess()
-	}
-}

session_test.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/go-errors/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -19,6 +20,10 @@ type TestHandler struct {
 	manager *CredentialManager
 }
 
+func (th TestHandler) MissingKeyshareServer(manager SchemeManagerIdentifier) {
+	th.Failure(ActionUnknown, &SessionError{Err: errors.Errorf("Missing keyshare server %s", manager.String())})
+}
+
 func (th TestHandler) StatusUpdate(action Action, status Status) {}
 func (th TestHandler) Success(action Action) {
 	th.c <- nil

updates.go

@@ -156,6 +156,7 @@ func (cm *CredentialManager) ParseAndroidStorage() (present bool, err error) {
 			return
 		}
 	}
+	cm.UnenrolledKeyshareServers = cm.unenrolledKeyshareServers()
 
 	if err = cm.storage.StorePaillierKeys(cm.paillierKeyCache); err != nil {
 		return