Small refactors and improvement of comments

internal/sessiontest/handlers_test.go

@@ -154,7 +154,7 @@ func (th TestHandler) RequestPin(remainingAttempts int, callback irmaclient.PinH
 }
 func (th TestHandler) BindingRequired(bindingCode string) {
 	// Send binding code via channel to calling test. This is done such that
-	// calling tests can detect whether this handler wasn't called.
+	// calling tests can detect when this handler is skipped unexpectedly.
 	if th.bindingCodeChan != nil {
 		th.bindingCodeChan <- bindingCode
 		return

internal/sessiontest/main_test.go

@@ -244,9 +244,9 @@ func sessionHelperWithFrontendOptions(
 		frontendOptionsHandler(h)
 	}
 
-	bts, err := json.Marshal(qr)
+	qrjson, err := json.Marshal(qr)
 	require.NoError(t, err)
-	dismisser := client.NewSession(string(bts), h)
+	dismisser := client.NewSession(string(qrjson), h)
 
 	if bindingHandler != nil {
 		h.dismisser = &dismisser
@@ -262,7 +262,7 @@ func sessionHelper(t *testing.T, request irma.SessionRequest, sessiontype string
 	sessionHelperWithFrontendOptions(t, request, sessiontype, client, nil, nil)
 }
 
-func extractTransportFromDismisser(dismisser *irmaclient.SessionDismisser) *irma.HTTPTransport {
+func extractClientTransport(dismisser *irmaclient.SessionDismisser) *irma.HTTPTransport {
 	rct := reflect.ValueOf(dismisser).Elem().Elem().Elem().FieldByName("transport")
 	return reflect.NewAt(rct.Type(), unsafe.Pointer(rct.UnsafeAddr())).Elem().Interface().(*irma.HTTPTransport)
 }

internal/sessiontest/requestor_test.go

@@ -72,9 +72,9 @@ func requestorSessionHelper(t *testing.T, request irma.SessionRequest, client *i
 		h = &TestHandler{t, clientChan, client, nil, wait, "", nil, nil, nil}
 	}
 
-	bts, err := json.Marshal(qr)
+	j, err := json.Marshal(qr)
 	require.NoError(t, err)
-	dismisser := client.NewSession(string(bts), h)
+	dismisser := client.NewSession(string(j), h)
 
 	clientResult := <-clientChan
 	if opts&sessionOptionIgnoreError == 0 && clientResult != nil {
@@ -90,7 +90,7 @@ func requestorSessionHelper(t *testing.T, request irma.SessionRequest, client *i
 	require.Equal(t, backendToken, serverResult.Token)
 
 	if opts&sessionOptionRetryPost > 0 {
-		clientTransport := extractTransportFromDismisser(&dismisser)
+		clientTransport := extractClientTransport(&dismisser)
 		var result string
 		err := clientTransport.Post("proofs", &result, h.(*TestHandler).result)
 		require.NoError(t, err)

internal/sessiontest/session_test.go

@@ -131,7 +131,7 @@ func TestIssuanceBinding(t *testing.T) {
 			require.Equal(t, bindingCode, <-handler.bindingCodeChan)
 
 			// Check whether access to request endpoint is denied as long as binding is not finished
-			clientTransport := extractTransportFromDismisser(handler.dismisser)
+			clientTransport := extractClientTransport(handler.dismisser)
 			err := clientTransport.Get("request", struct{}{})
 			require.Error(t, err)
 			require.Equal(t, 403, err.(*irma.SessionError).RemoteStatus)

irma/cmd/session.go

@@ -113,9 +113,8 @@ func libraryRequest(
 		return nil, errors.WrapPrefix(err, "IRMA session failed", 0)
 	}
 
-	// Currently, the default session options are the same in all conditions,
-	// so do only fetch them when a change is requested
-	var sessionOptions *server.SessionOptions
+	// Enable binding if necessary
+	var sessionOptions *irma.SessionOptions
 	if binding {
 		optionsRequest := irma.NewOptionsRequest()
 		optionsRequest.BindingMethod = irma.BindingMethodPin
@@ -175,7 +174,7 @@ func serverRequest(
 
 	// Enable binding if necessary
 	var frontendTransport *irma.HTTPTransport
-	sessionOptions := &server.SessionOptions{}
+	sessionOptions := &irma.SessionOptions{}
 	if binding {
 		frontendTransport = irma.NewHTTPTransport(qr.URL, false)
 		frontendTransport.SetHeader(irma.AuthorizationHeader, string(frontendToken))
@@ -288,7 +287,7 @@ func postRequest(serverurl string, request irma.RequestorRequest, name, authmeth
 	return pkg.SessionPtr, pkg.FrontendToken, transport, err
 }
 
-func handleBinding(options *server.SessionOptions, statusChan chan server.Status, completeBinding func() error) (
+func handleBinding(options *irma.SessionOptions, statusChan chan server.Status, completeBinding func() error) (
 	server.Status, error) {
 	errorChan := make(chan error)
 	status := server.StatusInitialized
@@ -312,7 +311,7 @@ func handleBinding(options *server.SessionOptions, statusChan chan server.Status
 	}
 }
 
-func requestBindingPermission(options *server.SessionOptions, completeBinding func() error, errorChan chan error) {
+func requestBindingPermission(options *irma.SessionOptions, completeBinding func() error, errorChan chan error) {
 	if options.BindingMethod == irma.BindingMethodPin {
 		fmt.Println("\nBinding code:", options.BindingCode)
 		fmt.Println("Press Enter to confirm your device shows the same binding code; otherwise press Ctrl-C.")

irmaclient/session.go

@@ -234,7 +234,7 @@ func (client *Client) newQrSession(qr *irma.Qr, handler Handler) SessionDismisse
 	session.transport.SetHeader(irma.MinVersionHeader, min.String())
 	session.transport.SetHeader(irma.MaxVersionHeader, maxVersion.String())
 
-	// From protocol version 2.7 also a authorization header must be included.
+	// From protocol version 2.7 also an authorization header must be included.
 	clientAuth := ""
 	if maxVersion.Above(2, 6) {
 		clientAuth = common.NewSessionToken()
@@ -260,7 +260,7 @@ func (session *session) getSessionInfo() {
 
 	// Get the first IRMA protocol message and parse it
 	info := &irma.ClientRequest{
-		Request: session.request, // As request is an interface it need to be initialized with a specific instance.
+		Request: session.request, // As request is an interface, it need to be initialized with a specific instance.
 	}
 	// UnmarshalJSON of ClientRequest takes into account legacy protocols, so we do not have to check that here.
 	err := session.transport.Get("", info)

server/irmaserver/api.go

@@ -243,8 +243,8 @@ func (s *Server) CancelSession(backendToken irma.BackendToken) error {
 }
 
 // Requests a change of the session frontend options at the server.
-// Returns the updated options struct. Frontend options can only be
-// changed the irma client has not connected yet. Otherwise an error is returned.
+// Returns the updated session options struct. Frontend options can only be
+// changed when the client is not connected yet. Otherwise an error is returned.
 // Options that are not specified in the request, keep their old value.
 func SetFrontendOptions(backendToken irma.BackendToken, request *irma.OptionsRequest) (*irma.SessionOptions, error) {
 	return s.SetFrontendOptions(backendToken, request)

server/irmaserver/handle.go

@@ -84,7 +84,7 @@ func (session *session) handleGetInfo(min, max *irma.ProtocolVersion, clientAuth
 	}
 
 	if session.version.Below(2, 7) {
-		// These versions do not support binding, so the request is always returned immediately.
+		// These versions do not support the ClientRequest format, so send the SessionRequest.
 		request, rerr := session.getRequest()
 		return &request, rerr
 	}
@@ -295,7 +295,6 @@ func (s *Server) handleSessionGet(w http.ResponseWriter, r *http.Request) {
 	}
 	session := r.Context().Value("session").(*session)
 	clientAuth := irma.ClientAuthorization(r.Header.Get(irma.AuthorizationHeader))
-	// When session binding is supported by all clients, the legacy support can be removed
 	res, err := session.handleGetInfo(&min, &max, clientAuth)
 	server.WriteResponse(w, res, err)
 }

server/irmaserver/helpers.go

@@ -65,8 +65,8 @@ func (session *session) updateFrontendOptions(request *irma.OptionsRequest) (*ir
 	} else {
 		return nil, errors.New("Binding method unknown")
 	}
-	session.options.BindingMethod = request.BindingMethod
 
+	session.options.BindingMethod = request.BindingMethod
 	return &session.options, nil
 }
 
@@ -114,6 +114,7 @@ const retryTimeLimit = 10 * time.Second
 // checkCache returns a previously cached response, for replaying against multiple requests from
 // irmago's retryablehttp client, if:
 // - the same body was POSTed to the same endpoint as last time
+// - the body is not empty
 // - last time was not more than 10 seconds ago (retryablehttp client gives up before this)
 // - the session status is what it is expected to be when receiving the request for a second time.
 func (session *session) checkCache(endpoint string, message []byte) (int, []byte) {
@@ -514,7 +515,7 @@ func (s *Server) bindingMiddleware(next http.Handler) http.Handler {
 		}
 
 		// Endpoints behind the bindingMiddleware can only be accessed when the client is already connected
-		// and includes the right authorization header to prove we still talk to the same client as before.
+		// and the request includes the right authorization header to prove we still talk to the same client as before.
 		if session.status != server.StatusConnected {
 			server.WriteError(w, server.ErrorUnexpectedRequest, "Session not yet started or already finished")
 			return

server/wait_status.go

@@ -45,11 +45,12 @@ func subscribeSSE(transport *irma.HTTPTransport, statuschan chan Status, errorch
 	}()
 
 	err := sseclient.Notify(ctx, transport.Server+"statusevents", true, events)
-	if !cancelled {
-		close(events)
-		return err
+	// When sse was cancelled, an error is expected to be returned. The channels are already closed then.
+	if cancelled {
+		return nil
 	}
-	return nil
+	close(events)
+	return err
 }
 
 // poll recursively polls the session status until a final status is received.