Merge lp:~chipaca/ubuntu-push/shuffle-signing-bits into lp:ubuntu-push/automatic

[John Lenton]

Move the signing bits about a bit (up from session to client, for reuse from service).

Move the signing bits about a bit (up from session to client, for reuse from service).

[Roberto Alsina (ralsina)]

Other than the text conflict it looks good.

review: Approve

[lp:~chipaca/ubuntu-push/shuffle-signing-bits]

181. By John Lenton on 2014-06-11

merged trunk

=== modified file 'client/client.go'
--- client/client.go	2014-05-21 10:03:18 +0000
+++ client/client.go	2014-06-11 13:44:07 +0000
@@ -28,6 +28,7 @@
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"strings"
 
 	"launchpad.net/go-dbus/v1"
@@ -160,7 +161,26 @@
 		ExpectAllRepairedTime:  client.config.ExpectAllRepairedTime.TimeDuration(),
 		PEM:        client.pem,
 		Info:       info,
-		AuthHelper: client.config.AuthHelper,
+		AuthGetter: client.getAuthorization,
+	}
+}
+
+// getAuthorization gets the authorization blob to send to the server
+func (client *PushClient) getAuthorization() string {
+	client.log.Debugf("getting authorization")
+	// using a helper, for now at least
+	if len(client.config.AuthHelper) == 0 {
+		// do nothing if helper is unset or empty
+		return ""
+	}
+
+	auth, err := exec.Command(client.config.AuthHelper[0], client.config.AuthHelper[1:]...).Output()
+	if err != nil {
+		// For now we just log the error, as we don't want to block unauthorized users
+		client.log.Errorf("unable to get the authorization token from the account: %v", err)
+		return ""
+	} else {
+		return strings.TrimSpace(string(auth))
 	}
 }
 
 
=== modified file 'client/client_test.go'
--- client/client_test.go	2014-05-20 13:56:37 +0000
+++ client/client_test.go	2014-06-11 13:44:07 +0000
@@ -272,7 +272,7 @@
 		ExpectAllRepairedTime:  30 * time.Minute,
 		PEM:        cli.pem,
 		Info:       info,
-		AuthHelper: []string{"auth", "helper"},
+		AuthGetter: func() string { return "" },
 	}
 	// sanity check that we are looking at all fields
 	vExpected := reflect.ValueOf(expected)
@@ -284,6 +284,11 @@
 	}
 	// finally compare
 	conf := cli.deriveSessionConfig(info)
+	// compare authGetter by string
+	c.Check(fmt.Sprintf("%v", conf.AuthGetter), Equals, fmt.Sprintf("%v", cli.getAuthorization))
+	// and set it to nil
+	conf.AuthGetter = nil
+	expected.AuthGetter = nil
 	c.Check(conf, DeepEquals, expected)
 }
 
@@ -951,3 +956,33 @@
 	c.Assert(err, NotNil)
 	c.Check(cs.log.Captured(), Matches, "(?msi).*showing notification: no way$")
 }
+
+/*****************************************************************
+    getAuthorization() tests
+******************************************************************/
+
+func (cs *clientSuite) TestGetAuthorizationIgnoresErrors(c *C) {
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	cli.configure()
+	cli.config.AuthHelper = []string{"sh", "-c", "echo hello; false"}
+
+	c.Check(cli.getAuthorization(), Equals, "")
+}
+
+func (cs *clientSuite) TestGetAuthorizationGetsIt(c *C) {
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	cli.configure()
+	cli.config.AuthHelper = []string{"echo", "hello"}
+
+	c.Check(cli.getAuthorization(), Equals, "hello")
+}
+
+func (cs *clientSuite) TestGetAuthorizationWorksIfUnsetOrNil(c *C) {
+	cli := NewPushClient(cs.configPath, cs.leveldbPath)
+	cli.log = cs.log
+
+	c.Assert(cli.config, NotNil)
+	c.Check(cli.getAuthorization(), Equals, "")
+	cli.configure()
+	c.Check(cli.getAuthorization(), Equals, "")
+}
 
=== modified file 'client/session/session.go'
--- client/session/session.go	2014-05-23 05:59:38 +0000
+++ client/session/session.go	2014-06-11 13:44:07 +0000
@@ -26,7 +26,6 @@
 	"fmt"
 	"math/rand"
 	"net"
-	"os/exec"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -87,7 +86,7 @@
 	ExpectAllRepairedTime  time.Duration
 	PEM                    []byte
 	Info                   map[string]interface{}
-	AuthHelper             []string
+	AuthGetter             func() string
 }
 
 // ClientSession holds a client<->server session and its configuration.
@@ -244,21 +243,10 @@
 // addAuthorization gets the authorization blob to send to the server
 // and adds it to the session.
 func (sess *ClientSession) addAuthorization() error {
-	sess.Log.Debugf("adding authorization")
-	// using a helper, for now at least
-	if len(sess.AuthHelper) == 0 {
-		// do nothing if helper is unset or empty
-		return nil
-	}
-
-	auth, err := exec.Command(sess.AuthHelper[0], sess.AuthHelper[1:]...).Output()
-	if err != nil {
-		// For now we just log the error, as we don't want to block unauthorized users
-		sess.Log.Errorf("unable to get the authorization token from the account: %v", err)
-	} else {
-		sess.auth = strings.TrimSpace(string(auth))
-	}
-
+	if sess.AuthGetter != nil {
+		sess.Log.Debugf("adding authorization")
+		sess.auth = sess.AuthGetter()
+	}
 	return nil
 }
 
 
=== modified file 'client/session/session_test.go'
--- client/session/session_test.go	2014-05-23 05:59:38 +0000
+++ client/session/session_test.go	2014-06-11 13:44:07 +0000
@@ -354,33 +354,18 @@
 
 func (cs *clientSessionSuite) TestAddAuthorizationAddsAuthorization(c *C) {
 	sess := &ClientSession{Log: cs.log}
-	sess.AuthHelper = []string{"echo", "some auth"}
+	sess.AuthGetter = func() string { return "some auth" }
 	c.Assert(sess.auth, Equals, "")
 	err := sess.addAuthorization()
 	c.Assert(err, IsNil)
 	c.Check(sess.auth, Equals, "some auth")
 }
 
-func (cs *clientSessionSuite) TestAddAuthorizationIgnoresErrors(c *C) {
-	sess := &ClientSession{Log: cs.log}
-	sess.AuthHelper = []string{"sh", "-c", "echo hello; false"}
-
-	c.Assert(sess.auth, Equals, "")
-	err := sess.addAuthorization()
-	c.Assert(err, IsNil)
-	c.Check(sess.auth, Equals, "")
-}
-
-func (cs *clientSessionSuite) TestAddAuthorizationSkipsIfUnsetOrNil(c *C) {
-	sess := &ClientSession{Log: cs.log}
-	sess.AuthHelper = nil
-	c.Assert(sess.auth, Equals, "")
-	err := sess.addAuthorization()
-	c.Assert(err, IsNil)
-	c.Check(sess.auth, Equals, "")
-
-	sess.AuthHelper = []string{}
-	err = sess.addAuthorization()
+func (cs *clientSessionSuite) TestAddAuthorizationSkipsIfUnset(c *C) {
+	sess := &ClientSession{Log: cs.log}
+	sess.AuthGetter = nil
+	c.Assert(sess.auth, Equals, "")
+	err := sess.addAuthorization()
 	c.Assert(err, IsNil)
 	c.Check(sess.auth, Equals, "")
 }
 
=== modified file 'debian/changelog'
--- debian/changelog	2014-06-06 12:13:37 +0000
+++ debian/changelog	2014-06-11 13:44:07 +0000
@@ -9,6 +9,7 @@
 
   [ John R. Lenton ]
   * Switch dbus api to retrieve app name from dbus path.
+  * Move signing bits up from session to client, for reuse by service.
 
  -- John R. Lenton <[e-mail]>  Fri, 06 Jun 2014 12:02:56 +0100