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

Proposed by John Lenton
Status: Merged
Approved by: John Lenton
Approved revision: 181
Merged at revision: 180
Proposed branch: lp:~chipaca/ubuntu-push/shuffle-signing-bits
Merge into: lp:ubuntu-push/automatic
Diff against target: 200 lines (+69/-40)
5 files modified
client/client.go (+21/-1)
client/client_test.go (+36/-1)
client/session/session.go (+5/-17)
client/session/session_test.go (+6/-21)
debian/changelog (+1/-0)
To merge this branch: bzr merge lp:~chipaca/ubuntu-push/shuffle-signing-bits
Reviewer Review Type Date Requested Status
Roberto Alsina (community) Approve
Review via email: mp+222783@code.launchpad.net

This proposal supersedes a proposal from 2014-06-11.

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Roberto Alsina (ralsina) wrote :

Other than the text conflict it looks good.

review: Approve
181. By John Lenton

merged trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'client/client.go'
2--- client/client.go 2014-05-21 10:03:18 +0000
3+++ client/client.go 2014-06-11 13:44:07 +0000
4@@ -28,6 +28,7 @@
5 "fmt"
6 "io/ioutil"
7 "os"
8+ "os/exec"
9 "strings"
10
11 "launchpad.net/go-dbus/v1"
12@@ -160,7 +161,26 @@
13 ExpectAllRepairedTime: client.config.ExpectAllRepairedTime.TimeDuration(),
14 PEM: client.pem,
15 Info: info,
16- AuthHelper: client.config.AuthHelper,
17+ AuthGetter: client.getAuthorization,
18+ }
19+}
20+
21+// getAuthorization gets the authorization blob to send to the server
22+func (client *PushClient) getAuthorization() string {
23+ client.log.Debugf("getting authorization")
24+ // using a helper, for now at least
25+ if len(client.config.AuthHelper) == 0 {
26+ // do nothing if helper is unset or empty
27+ return ""
28+ }
29+
30+ auth, err := exec.Command(client.config.AuthHelper[0], client.config.AuthHelper[1:]...).Output()
31+ if err != nil {
32+ // For now we just log the error, as we don't want to block unauthorized users
33+ client.log.Errorf("unable to get the authorization token from the account: %v", err)
34+ return ""
35+ } else {
36+ return strings.TrimSpace(string(auth))
37 }
38 }
39
40
41=== modified file 'client/client_test.go'
42--- client/client_test.go 2014-05-20 13:56:37 +0000
43+++ client/client_test.go 2014-06-11 13:44:07 +0000
44@@ -272,7 +272,7 @@
45 ExpectAllRepairedTime: 30 * time.Minute,
46 PEM: cli.pem,
47 Info: info,
48- AuthHelper: []string{"auth", "helper"},
49+ AuthGetter: func() string { return "" },
50 }
51 // sanity check that we are looking at all fields
52 vExpected := reflect.ValueOf(expected)
53@@ -284,6 +284,11 @@
54 }
55 // finally compare
56 conf := cli.deriveSessionConfig(info)
57+ // compare authGetter by string
58+ c.Check(fmt.Sprintf("%v", conf.AuthGetter), Equals, fmt.Sprintf("%v", cli.getAuthorization))
59+ // and set it to nil
60+ conf.AuthGetter = nil
61+ expected.AuthGetter = nil
62 c.Check(conf, DeepEquals, expected)
63 }
64
65@@ -951,3 +956,33 @@
66 c.Assert(err, NotNil)
67 c.Check(cs.log.Captured(), Matches, "(?msi).*showing notification: no way$")
68 }
69+
70+/*****************************************************************
71+ getAuthorization() tests
72+******************************************************************/
73+
74+func (cs *clientSuite) TestGetAuthorizationIgnoresErrors(c *C) {
75+ cli := NewPushClient(cs.configPath, cs.leveldbPath)
76+ cli.configure()
77+ cli.config.AuthHelper = []string{"sh", "-c", "echo hello; false"}
78+
79+ c.Check(cli.getAuthorization(), Equals, "")
80+}
81+
82+func (cs *clientSuite) TestGetAuthorizationGetsIt(c *C) {
83+ cli := NewPushClient(cs.configPath, cs.leveldbPath)
84+ cli.configure()
85+ cli.config.AuthHelper = []string{"echo", "hello"}
86+
87+ c.Check(cli.getAuthorization(), Equals, "hello")
88+}
89+
90+func (cs *clientSuite) TestGetAuthorizationWorksIfUnsetOrNil(c *C) {
91+ cli := NewPushClient(cs.configPath, cs.leveldbPath)
92+ cli.log = cs.log
93+
94+ c.Assert(cli.config, NotNil)
95+ c.Check(cli.getAuthorization(), Equals, "")
96+ cli.configure()
97+ c.Check(cli.getAuthorization(), Equals, "")
98+}
99
100=== modified file 'client/session/session.go'
101--- client/session/session.go 2014-05-23 05:59:38 +0000
102+++ client/session/session.go 2014-06-11 13:44:07 +0000
103@@ -26,7 +26,6 @@
104 "fmt"
105 "math/rand"
106 "net"
107- "os/exec"
108 "strings"
109 "sync"
110 "sync/atomic"
111@@ -87,7 +86,7 @@
112 ExpectAllRepairedTime time.Duration
113 PEM []byte
114 Info map[string]interface{}
115- AuthHelper []string
116+ AuthGetter func() string
117 }
118
119 // ClientSession holds a client<->server session and its configuration.
120@@ -244,21 +243,10 @@
121 // addAuthorization gets the authorization blob to send to the server
122 // and adds it to the session.
123 func (sess *ClientSession) addAuthorization() error {
124- sess.Log.Debugf("adding authorization")
125- // using a helper, for now at least
126- if len(sess.AuthHelper) == 0 {
127- // do nothing if helper is unset or empty
128- return nil
129- }
130-
131- auth, err := exec.Command(sess.AuthHelper[0], sess.AuthHelper[1:]...).Output()
132- if err != nil {
133- // For now we just log the error, as we don't want to block unauthorized users
134- sess.Log.Errorf("unable to get the authorization token from the account: %v", err)
135- } else {
136- sess.auth = strings.TrimSpace(string(auth))
137- }
138-
139+ if sess.AuthGetter != nil {
140+ sess.Log.Debugf("adding authorization")
141+ sess.auth = sess.AuthGetter()
142+ }
143 return nil
144 }
145
146
147=== modified file 'client/session/session_test.go'
148--- client/session/session_test.go 2014-05-23 05:59:38 +0000
149+++ client/session/session_test.go 2014-06-11 13:44:07 +0000
150@@ -354,33 +354,18 @@
151
152 func (cs *clientSessionSuite) TestAddAuthorizationAddsAuthorization(c *C) {
153 sess := &ClientSession{Log: cs.log}
154- sess.AuthHelper = []string{"echo", "some auth"}
155+ sess.AuthGetter = func() string { return "some auth" }
156 c.Assert(sess.auth, Equals, "")
157 err := sess.addAuthorization()
158 c.Assert(err, IsNil)
159 c.Check(sess.auth, Equals, "some auth")
160 }
161
162-func (cs *clientSessionSuite) TestAddAuthorizationIgnoresErrors(c *C) {
163- sess := &ClientSession{Log: cs.log}
164- sess.AuthHelper = []string{"sh", "-c", "echo hello; false"}
165-
166- c.Assert(sess.auth, Equals, "")
167- err := sess.addAuthorization()
168- c.Assert(err, IsNil)
169- c.Check(sess.auth, Equals, "")
170-}
171-
172-func (cs *clientSessionSuite) TestAddAuthorizationSkipsIfUnsetOrNil(c *C) {
173- sess := &ClientSession{Log: cs.log}
174- sess.AuthHelper = nil
175- c.Assert(sess.auth, Equals, "")
176- err := sess.addAuthorization()
177- c.Assert(err, IsNil)
178- c.Check(sess.auth, Equals, "")
179-
180- sess.AuthHelper = []string{}
181- err = sess.addAuthorization()
182+func (cs *clientSessionSuite) TestAddAuthorizationSkipsIfUnset(c *C) {
183+ sess := &ClientSession{Log: cs.log}
184+ sess.AuthGetter = nil
185+ c.Assert(sess.auth, Equals, "")
186+ err := sess.addAuthorization()
187 c.Assert(err, IsNil)
188 c.Check(sess.auth, Equals, "")
189 }
190
191=== modified file 'debian/changelog'
192--- debian/changelog 2014-06-06 12:13:37 +0000
193+++ debian/changelog 2014-06-11 13:44:07 +0000
194@@ -9,6 +9,7 @@
195
196 [ John R. Lenton ]
197 * Switch dbus api to retrieve app name from dbus path.
198+ * Move signing bits up from session to client, for reuse by service.
199
200 -- John R. Lenton <john.lenton@canonical.com> Fri, 06 Jun 2014 12:02:56 +0100
201

Subscribers

People subscribed via source and target branches