Ubuntu Push Notifications

Merge lp:~diogobaeder/ubuntu-push/auth into lp:ubuntu-push/automatic

  1. auth
  2. Merge into automatic
Proposed by Diogo Baeder
Status: Merged
Approved by: Diogo Baeder
Approved revision: 131
Merged at revision: 131
Proposed branch: lp:~diogobaeder/ubuntu-push/auth
Merge into: lp:ubuntu-push/automatic
Diff against target: 332 lines (+82/-51)
4 files modified
client/client.go (+2/-19)
client/client_test.go (+2/-17)
client/session/session.go (+29/-10)
client/session/session_test.go (+49/-5)
To merge this branch: bzr merge lp:~diogobaeder/ubuntu-push/auth
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+216179@code.launchpad.net

Commit message

Moving the authorization retrieval to the client session start, so that it can be retried at a later time

Description of the change

Moving the authorization retrieval to the client session start, so that it can be retried at a later time

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) :
review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (14.4 KiB)

The attempt to merge lp:~diogobaeder/ubuntu-push/auth into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 gopkg.in/qml.v0 gopkg.in/niemeyer/uoneauth.v1
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/launchpad.net/gocheck" now at <email address hidden>
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/gopkg.in/qml.v0" now at master
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/gopkg.in/niemeyer/uoneauth.v1" now at v1
go install launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 gopkg.in/qml.v0 gopkg.in/niemeyer/uoneauth.v1
go test launchpad.net/ubuntu-push launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/urldispatcher launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/levelmap launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/whoopsie launchpad.net/ubuntu-push/whoopsie/identifier launchpad.net/ubuntu-push/whoopsie/identifier/testing
? launchpad.net/ubuntu-push [no test files]
ok launchpad.net/ubuntu-push/bus 0.008s
ok launchpad.net/ubuntu-push/bus/connectivity 1.192s
ok launchpad.net/ubuntu-push/bus/networkmanager 0.090s
ok launchpad.net/ubuntu-push/bus/notifications 0.028s
ok launchpad.net/ubuntu-push/bus/systemimage 0.010s
ok launchpad.net/ubuntu-push/bus/testing 0.018s
ok launchpad.net/ubuntu-push/bus/urldispatcher 0.006s
ok launchpad.net/ubuntu-push/client 0.083s
ok launchpad.net/ubuntu-push/client/gethosts 0.712s
ok launchpad.net/ubuntu-push/client/session 0.193s
ok launchpad.net/ubuntu-push/client/session/levelmap 0.059s
ok launchpad.net/ubuntu-push/config 0.011s
ok launchpad.net/ubuntu-push/external/murmur3 0.004s
ok launchpad.net/ubuntu-push/logger 0.008s
ok launchpad.net/ubuntu-push/protocol 0.011s
ok launchpad.net/ubuntu-push/...

Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (14.4 KiB)

The attempt to merge lp:~diogobaeder/ubuntu-push/auth into lp:ubuntu-push/automatic failed. Below is the output from the failed tests.

rm -f -r /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin
mkdir -p /mnt/tarmac/cache/ubuntu-push-automatic/go-ws/pkg
go get -u launchpad.net/godeps
go get -d -u launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 gopkg.in/qml.v0 gopkg.in/niemeyer/uoneauth.v1
/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/bin/godeps -u dependencies.tsv
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/launchpad.net/gocheck" now at <email address hidden>
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/gopkg.in/qml.v0" now at master
"/mnt/tarmac/cache/ubuntu-push-automatic/go-ws/src/gopkg.in/niemeyer/uoneauth.v1" now at v1
go install launchpad.net/gocheck launchpad.net/go-dbus/v1 launchpad.net/go-xdg/v0 code.google.com/p/gosqlite/sqlite3 gopkg.in/qml.v0 gopkg.in/niemeyer/uoneauth.v1
go test launchpad.net/ubuntu-push launchpad.net/ubuntu-push/bus launchpad.net/ubuntu-push/bus/connectivity launchpad.net/ubuntu-push/bus/networkmanager launchpad.net/ubuntu-push/bus/notifications launchpad.net/ubuntu-push/bus/systemimage launchpad.net/ubuntu-push/bus/testing launchpad.net/ubuntu-push/bus/urldispatcher launchpad.net/ubuntu-push/client launchpad.net/ubuntu-push/client/gethosts launchpad.net/ubuntu-push/client/session launchpad.net/ubuntu-push/client/session/levelmap launchpad.net/ubuntu-push/config launchpad.net/ubuntu-push/external/murmur3 launchpad.net/ubuntu-push/logger launchpad.net/ubuntu-push/protocol launchpad.net/ubuntu-push/server launchpad.net/ubuntu-push/server/api launchpad.net/ubuntu-push/server/broker launchpad.net/ubuntu-push/server/broker/simple launchpad.net/ubuntu-push/server/broker/testing launchpad.net/ubuntu-push/server/broker/testsuite launchpad.net/ubuntu-push/server/dev launchpad.net/ubuntu-push/server/listener launchpad.net/ubuntu-push/server/session launchpad.net/ubuntu-push/server/store launchpad.net/ubuntu-push/testing launchpad.net/ubuntu-push/testing/condition launchpad.net/ubuntu-push/util launchpad.net/ubuntu-push/whoopsie launchpad.net/ubuntu-push/whoopsie/identifier launchpad.net/ubuntu-push/whoopsie/identifier/testing
? launchpad.net/ubuntu-push [no test files]
ok launchpad.net/ubuntu-push/bus 0.007s
ok launchpad.net/ubuntu-push/bus/connectivity 1.194s
ok launchpad.net/ubuntu-push/bus/networkmanager 0.093s
ok launchpad.net/ubuntu-push/bus/notifications 0.027s
ok launchpad.net/ubuntu-push/bus/systemimage 0.006s
ok launchpad.net/ubuntu-push/bus/testing 0.017s
ok launchpad.net/ubuntu-push/bus/urldispatcher 0.017s
ok launchpad.net/ubuntu-push/client 0.158s
ok launchpad.net/ubuntu-push/client/gethosts 0.712s
ok launchpad.net/ubuntu-push/client/session 0.190s
ok launchpad.net/ubuntu-push/client/session/levelmap 0.030s
ok launchpad.net/ubuntu-push/config 0.010s
ok launchpad.net/ubuntu-push/external/murmur3 0.004s
ok launchpad.net/ubuntu-push/logger 0.016s
ok launchpad.net/ubuntu-push/protocol 0.010s
ok launchpad.net/ubuntu-push/...

lp:~diogobaeder/ubuntu-push/auth updated
131. By Diogo Baeder

Putting function replacement into SetUpSuite to avoid race condition

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-04-16 09:40:59 +0000
3+++ client/client.go 2014-04-17 15:43:47 +0000
4@@ -43,11 +43,6 @@
5 "launchpad.net/ubuntu-push/whoopsie/identifier"
6 )
7
8-var (
9- getAuthorization = util.GetAuthorization
10- shouldGetAuth = false
11-)
12-
13 // ClientConfig holds the client configuration
14 type ClientConfig struct {
15 connectivity.ConnectivityConfig // q.v.
16@@ -85,7 +80,6 @@
17 actionsCh <-chan notifications.RawActionReply
18 session *session.ClientSession
19 sessionConnectedCh chan uint32
20- auth string
21 }
22
23 var ACTION_ID_SNOWFLAKE = "::ubuntu-push-client::"
24@@ -120,16 +114,6 @@
25 client.log = logger.NewSimpleLogger(os.Stderr, client.config.LogLevel.Level())
26 qml.SetLogger(client.log)
27
28- // grab the authorization string from the accounts
29- // TODO: remove this condition when we have a way to deal with failing authorizations
30- if shouldGetAuth {
31- auth, err := getAuthorization()
32- if err != nil {
33- return fmt.Errorf("unable to get the authorization token from the account: %v", err)
34- }
35- client.auth = auth
36- }
37-
38 // overridden for testing
39 client.idder = identifier.New()
40 client.notificationsEndp = bus.SessionBus.Endpoint(notifications.BusAddress, client.log)
41@@ -162,9 +146,8 @@
42 ExchangeTimeout: client.config.ExchangeTimeout.TimeDuration(),
43 HostsCachingExpiryTime: client.config.HostsCachingExpiryTime.TimeDuration(),
44 ExpectAllRepairedTime: client.config.ExpectAllRepairedTime.TimeDuration(),
45- PEM: client.pem,
46- Info: info,
47- Authorization: client.auth,
48+ PEM: client.pem,
49+ Info: info,
50 }
51 }
52
53
54=== modified file 'client/client_test.go'
55--- client/client_test.go 2014-04-15 13:09:40 +0000
56+++ client/client_test.go 2014-04-17 15:43:47 +0000
57@@ -85,17 +85,11 @@
58 config.IgnoreParsedFlags = true // because configure() uses <flags>
59 cs.timeouts = util.SwapTimeouts([]time.Duration{0})
60 cs.leveldbPath = ""
61- getAuthorization = func() (string, error) {
62- return "some auth", nil
63- }
64- shouldGetAuth = true
65 }
66
67 func (cs *clientSuite) TearDownSuite(c *C) {
68 util.SwapTimeouts(cs.timeouts)
69 cs.timeouts = nil
70- getAuthorization = util.GetAuthorization
71- shouldGetAuth = false
72 }
73
74 func (cs *clientSuite) writeTestConfig(overrides map[string]interface{}) {
75@@ -206,14 +200,6 @@
76 c.Assert(cli.connCh, NotNil)
77 }
78
79-func (cs *clientSuite) TestConfigureSetsUpAuth(c *C) {
80- cli := NewPushClient(cs.configPath, cs.leveldbPath)
81- c.Check(cli.auth, Equals, "")
82- err := cli.configure()
83- c.Assert(err, IsNil)
84- c.Assert(cli.auth, Equals, "some auth")
85-}
86-
87 func (cs *clientSuite) TestConfigureBailsOnBadFilename(c *C) {
88 cli := NewPushClient("/does/not/exist", cs.leveldbPath)
89 err := cli.configure()
90@@ -279,9 +265,8 @@
91 ExchangeTimeout: 10 * time.Millisecond,
92 HostsCachingExpiryTime: 1 * time.Hour,
93 ExpectAllRepairedTime: 30 * time.Minute,
94- PEM: cli.pem,
95- Info: info,
96- Authorization: "some auth",
97+ PEM: cli.pem,
98+ Info: info,
99 }
100 // sanity check that we are looking at all fields
101 vExpected := reflect.ValueOf(expected)
102
103=== modified file 'client/session/session.go'
104--- client/session/session.go 2014-04-16 12:26:24 +0000
105+++ client/session/session.go 2014-04-17 15:43:47 +0000
106@@ -38,7 +38,11 @@
107 "launchpad.net/ubuntu-push/util"
108 )
109
110-var wireVersionBytes = []byte{protocol.ProtocolWireVersion}
111+var (
112+ wireVersionBytes = []byte{protocol.ProtocolWireVersion}
113+ getAuthorization = util.GetAuthorization
114+ shouldGetAuth = false
115+)
116
117 type Notification struct {
118 TopLevel int64
119@@ -84,7 +88,6 @@
120 ExpectAllRepairedTime time.Duration
121 PEM []byte
122 Info map[string]interface{}
123- Authorization string
124 }
125
126 // ClientSession holds a client<->server session and its configuration.
127@@ -145,7 +148,6 @@
128 TLS: &tls.Config{},
129 stateP: &state,
130 timeSince: time.Since,
131- auth: conf.Authorization,
132 }
133 if sess.PEM != nil {
134 cp := x509.NewCertPool()
135@@ -201,6 +203,21 @@
136 return nil
137 }
138
139+// checkAuthorization checks the authorization within the phone
140+func (sess *ClientSession) checkAuthorization() error {
141+ // grab the authorization string from the accounts
142+ // TODO: remove this condition when we have a way to deal with failing authorizations
143+ if shouldGetAuth {
144+ auth, err := getAuthorization()
145+ if err != nil {
146+ // For now we just log the error, as we don't want to block unauthorized users
147+ sess.Log.Errorf("unable to get the authorization token from the account: %v", err)
148+ }
149+ sess.auth = auth
150+ }
151+ return nil
152+}
153+
154 func (sess *ClientSession) resetHosts() {
155 sess.deliveryHosts = nil
156 }
157@@ -453,13 +470,15 @@
158
159 // run calls connect, and if it works it calls start, and if it works
160 // it runs loop in a goroutine, and ships its return value over ErrCh.
161-func (sess *ClientSession) run(closer func(), hostGetter, connecter, starter, looper func() error) error {
162+func (sess *ClientSession) run(closer func(), authChecker, hostGetter, connecter, starter, looper func() error) error {
163 closer()
164- err := hostGetter()
165- if err != nil {
166- return err
167- }
168- err = connecter()
169+ if err := authChecker(); err != nil {
170+ return err
171+ }
172+ if err := hostGetter(); err != nil {
173+ return err
174+ }
175+ err := connecter()
176 if err == nil {
177 err = starter()
178 if err == nil {
179@@ -489,7 +508,7 @@
180 // keep on trying.
181 panic("can't Dial() without a protocol constructor.")
182 }
183- return sess.run(sess.doClose, sess.getHosts, sess.connect, sess.start, sess.loop)
184+ return sess.run(sess.doClose, sess.checkAuthorization, sess.getHosts, sess.connect, sess.start, sess.loop)
185 }
186
187 func init() {
188
189=== modified file 'client/session/session_test.go'
190--- client/session/session_test.go 2014-04-16 13:28:33 +0000
191+++ client/session/session_test.go 2014-04-17 15:43:47 +0000
192@@ -34,10 +34,10 @@
193
194 "launchpad.net/ubuntu-push/client/gethosts"
195 "launchpad.net/ubuntu-push/client/session/levelmap"
196- "launchpad.net/ubuntu-push/logger"
197 "launchpad.net/ubuntu-push/protocol"
198 helpers "launchpad.net/ubuntu-push/testing"
199 "launchpad.net/ubuntu-push/testing/condition"
200+ "launchpad.net/ubuntu-push/util"
201 )
202
203 func TestSession(t *testing.T) { TestingT(t) }
204@@ -165,14 +165,26 @@
205 /////
206
207 type clientSessionSuite struct {
208- log logger.Logger
209+ log *helpers.TestLogger
210 lvls func() (levelmap.LevelMap, error)
211 }
212
213+func (cs *clientSessionSuite) SetUpSuite(c *C) {
214+ getAuthorization = func() (string, error) {
215+ return "some auth", nil
216+ }
217+ shouldGetAuth = true
218+}
219+
220 func (cs *clientSessionSuite) SetUpTest(c *C) {
221 cs.log = helpers.NewTestLogger(c, "debug")
222 }
223
224+func (cs *clientSessionSuite) TearDownSuite(c *C) {
225+ getAuthorization = util.GetAuthorization
226+ shouldGetAuth = false
227+}
228+
229 // in-memory level map testing
230 var _ = Suite(&clientSessionSuite{lvls: levelmap.NewLevelMap})
231
232@@ -182,6 +194,7 @@
233 var _ = Suite(&clientSqlevelsSessionSuite{})
234
235 func (cs *clientSqlevelsSessionSuite) SetUpSuite(c *C) {
236+ cs.clientSessionSuite.SetUpSuite(c)
237 cs.lvls = func() (levelmap.LevelMap, error) { return levelmap.NewSqliteLevelMap(":memory:") }
238 }
239
240@@ -342,6 +355,18 @@
241 }
242
243 /****************************************************************
244+ checkAuthorization() tests
245+****************************************************************/
246+
247+func (cs *clientSessionSuite) TestChecksAuthorizationFromServer(c *C) {
248+ sess := &ClientSession{}
249+ c.Assert(sess.auth, Equals, "")
250+ err := sess.checkAuthorization()
251+ c.Assert(err, IsNil)
252+ c.Check(sess.auth, Equals, "some auth")
253+}
254+
255+/****************************************************************
256 startConnectionAttempt()/nextHostToTry()/started tests
257 ****************************************************************/
258
259@@ -942,8 +967,7 @@
260 "bar": "baz",
261 }
262 conf := ClientSessionConfig{
263- Info: info,
264- Authorization: "some auth",
265+ Info: info,
266 }
267 sess, err := NewSession("", conf, "wah", cs.lvls, cs.log)
268 c.Assert(err, IsNil)
269@@ -962,7 +986,7 @@
270 msg, ok := takeNext(downCh).(protocol.ConnectMsg)
271 c.Check(ok, Equals, true)
272 c.Check(msg.DeviceId, Equals, "wah")
273- c.Check(msg.Authorization, Equals, "some auth")
274+ c.Check(msg.Authorization, Equals, "")
275 c.Check(msg.Info, DeepEquals, info)
276 upCh <- nil // no error
277 upCh <- protocol.ConnAckMsg{
278@@ -979,6 +1003,22 @@
279 run() tests
280 ****************************************************************/
281
282+func (cs *clientSessionSuite) TestRunBailsIfAuthCheckFails(c *C) {
283+ sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
284+ c.Assert(err, IsNil)
285+ failure := errors.New("TestRunBailsIfAuthCheckFails")
286+ has_closed := false
287+ err = sess.run(
288+ func() { has_closed = true },
289+ func() error { return failure },
290+ nil,
291+ nil,
292+ nil,
293+ nil)
294+ c.Check(err, Equals, failure)
295+ c.Check(has_closed, Equals, true)
296+}
297+
298 func (cs *clientSessionSuite) TestRunBailsIfHostGetterFails(c *C) {
299 sess, err := NewSession("", dummyConf, "wah", cs.lvls, cs.log)
300 c.Assert(err, IsNil)
301@@ -986,6 +1026,7 @@
302 has_closed := false
303 err = sess.run(
304 func() { has_closed = true },
305+ func() error { return nil },
306 func() error { return failure },
307 nil,
308 nil,
309@@ -1001,6 +1042,7 @@
310 err = sess.run(
311 func() {},
312 func() error { return nil },
313+ func() error { return nil },
314 func() error { return failure },
315 nil,
316 nil)
317@@ -1015,6 +1057,7 @@
318 func() {},
319 func() error { return nil },
320 func() error { return nil },
321+ func() error { return nil },
322 func() error { return failure },
323 nil)
324 c.Check(err, Equals, failure)
325@@ -1034,6 +1077,7 @@
326 func() error { return nil },
327 func() error { return nil },
328 func() error { return nil },
329+ func() error { return nil },
330 func() error { sess.MsgCh <- notf; return <-failureCh })
331 c.Check(err, Equals, nil)
332 // if run doesn't error it sets up the channels

Subscribers

People subscribed via source and target branches