Merge lp:~vds/usso/handle_order_of_parameters into lp:usso

Proposed by Vincenzo Di Somma
Status: Merged
Approved by: Vincenzo Di Somma
Approved revision: 25
Merged at revision: 25
Proposed branch: lp:~vds/usso/handle_order_of_parameters
Merge into: lp:usso
Diff against target: 506 lines (+191/-141)
6 files modified
example/usso_example.go (+7/-12)
oauth.go (+90/-71)
oauth_test.go (+41/-26)
url.go (+36/-19)
url_test.go (+2/-2)
usso.go (+15/-11)
To merge this branch: bzr merge lp:~vds/usso/handle_order_of_parameters
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Domas Monkus Approve
Review via email: mp+145936@code.launchpad.net

Commit message

"Got rid of useless error, using a struct to handle the oauth_signature_method, separated ssodata from temporary request parameters, fixed tests and example accordingly.

Description of the change

"Got rid of useless error, using a struct to handle the oauth_signature_method, separated ssodata from temporary request parameters, fixed tests and example accordingly.

To post a comment you must log in.
Revision history for this message
Domas Monkus (tasdomas) wrote :

Looks ok.

review: Approve
Revision history for this message
Graham Binns (gmb) wrote :

Hi Vincenzo,

Thank you for this branch! There are a couple of things you need to add before you land this, but there's nothing that needs me to re-review it after you've made the changes, so feel free to make them and then mark the MP as approved.

[1]

149 +func (PLAINTEXT) Name() string { return "PLAINTEXT" }
150 +func (PLAINTEXT) Signature(

160 +func (HMACSHA1) Name() string { return "HMAC-SHA1" }
161 +func (HMACSHA1) Signature(

These all need some documentation comments.

[2]

275 +// Test the request signing with oauth_signature_method = PLAINTEXT
301 // Test the request signing with oauth_signature_method = SHA1

We should avoid the "Test the..." style when writing documentation for tests. Instead, we should write things like:

  // It is possible to sign a request with a PLAINTEXT oauth_signature_method.
  // It is possible to sign a request with a SHA1 oauth_signature_method.

Put another way, the documentation on a test should be something that can be declared true if the test passes, false otherwise.

review: Approve (code)
25. By Vincenzo Di Somma

Fixing/adding comments and cleaning up.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'example/usso_example.go'
2--- example/usso_example.go 2013-01-25 16:39:11 +0000
3+++ example/usso_example.go 2013-02-05 12:38:20 +0000
4@@ -9,7 +9,7 @@
5 "net/http"
6 )
7
8-var email, password, tokenName, signature_method string
9+var email, password string
10
11 func inputParams() {
12 fmt.Println("This application will query the staging Ubuntu SSO Server" +
13@@ -19,9 +19,6 @@
14 fmt.Print("Enter password: ")
15 fmt.Scanf("%s", &password)
16 fmt.Print("Enter token name: ")
17- fmt.Scanf("%s", &tokenName)
18- fmt.Print("Enter signature method (PLAINTEXT or HMAC-SHA1): ")
19- fmt.Scanf("%s", &signature_method)
20 }
21
22 func main() {
23@@ -32,7 +29,7 @@
24 server := usso.StagingUbuntuSSOServer
25 // One would use server := usso.ProductionUbuntuSSOServer
26 // to use the production Ubuntu SSO Server.
27- ssodata, err := server.GetToken(email, password, tokenName)
28+ ssodata, err := server.GetToken(email, password, "usso")
29 if err != nil {
30 panic(err)
31 }
32@@ -48,14 +45,12 @@
33 //fmt.Printf("Got accounts info: %s\n", accounts)
34
35 // But this shows how to sign a generic request.
36- ssodata.BaseURL = fmt.Sprintf(
37+ rp := usso.RequestParameters{BaseURL: fmt.Sprintf(
38 "https://login.staging.ubuntu.com/api/v2/accounts/%s",
39- ssodata.ConsumerKey)
40- ssodata.HTTPMethod = "GET"
41- ssodata.SignatureMethod = signature_method
42- request, _ := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil)
43- usso.SignRequest(ssodata, request)
44-
45+ ssodata.ConsumerKey), HTTPMethod: "GET",
46+ SignatureMethod: usso.HMACSHA1{}}
47+ request, _ := http.NewRequest(rp.HTTPMethod, rp.BaseURL, nil)
48+ usso.SignRequest(ssodata, &rp, request)
49 if err != nil {
50 fmt.Printf("Error: %s\n", err)
51 }
52
53=== modified file 'oauth.go'
54--- oauth.go 2013-01-29 11:23:37 +0000
55+++ oauth.go 2013-02-05 12:38:20 +0000
56@@ -4,7 +4,6 @@
57 "crypto/hmac"
58 "crypto/sha1"
59 "encoding/base64"
60- "errors"
61 "fmt"
62 "math/rand"
63 "net/http"
64@@ -30,72 +29,91 @@
65
66 // Contains the oauth data to perform a request.
67 type SSOData struct {
68- HTTPMethod string `json:"-"`
69- BaseURL string `json:"-"`
70- Params url.Values `json:"-"`
71- Nonce string `json:"-"`
72- Timestamp string `json:"-"`
73- SignatureMethod string `json:"-"`
74- ConsumerKey string `json:"consumer_key"`
75- ConsumerSecret string `json:"consumer_secret"`
76- TokenKey string `json:"token_key"`
77- TokenName string `json:"token_name"`
78- TokenSecret string `json:"token_secret"`
79-}
80-
81-// Depending on the signature method, create the signature from the
82-// consumer secret, the token secret and, if required, the URL.
83-// Supported signature methods are PLAINTEXT and HMAC-SHA1.
84-func (oauth *SSOData) signature() (string, error) {
85- switch oauth.SignatureMethod {
86- case "PLAINTEXT":
87- return fmt.Sprintf(
88- `%s%%26%s`,
89- oauth.ConsumerSecret,
90- oauth.TokenSecret), nil
91- case "HMAC-SHA1":
92- base_url, err := NormalizeURL(oauth.BaseURL)
93- if err != nil {
94- return "", err
95- }
96- params, err := NormalizeParameters(oauth.Params)
97- if err != nil {
98- return "", err
99- }
100- base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`,
101- oauth.HTTPMethod,
102- url.QueryEscape(base_url),
103- url.QueryEscape(params),
104- url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey),
105- url.QueryEscape("&oauth_nonce="+oauth.Nonce),
106- url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod),
107- url.QueryEscape("&oauth_timestamp="+oauth.Timestamp),
108- url.QueryEscape("&oauth_token="+oauth.TokenKey),
109- url.QueryEscape("&oauth_version=1.0"))
110- hashfun := hmac.New(sha1.New, []byte(
111- oauth.ConsumerSecret+"&"+oauth.TokenSecret))
112- hashfun.Write([]byte(base_string))
113- rawsignature := hashfun.Sum(nil)
114- base64signature := make(
115- []byte, base64.StdEncoding.EncodedLen(len(rawsignature)))
116- base64.StdEncoding.Encode(base64signature, rawsignature)
117- return string(base64signature), nil
118- default:
119- return "", errors.New(
120- "usso/oauth: Oauth Signature Method not supported.")
121- }
122- return "", nil
123+ ConsumerKey string `json:"consumer_key"`
124+ ConsumerSecret string `json:"consumer_secret"`
125+ TokenKey string `json:"token_key"`
126+ TokenName string `json:"token_name"`
127+ TokenSecret string `json:"token_secret"`
128+}
129+
130+type RequestParameters struct {
131+ HTTPMethod string
132+ BaseURL string
133+ Params url.Values
134+ Nonce string
135+ Timestamp string
136+ SignatureMethod SignatureMethod
137+}
138+
139+type SignatureMethod interface {
140+ Name() string
141+ Signature(
142+ ssodata *SSOData, rp *RequestParameters) (string, error)
143+}
144+
145+type PLAINTEXT struct{}
146+
147+// Return the name of the signature method, used to compose the
148+// Authentication Header.
149+func (PLAINTEXT) Name() string { return "PLAINTEXT" }
150+
151+// Calculate the oaut_signature part of the Authentication Header.
152+func (PLAINTEXT) Signature(
153+ ssodata *SSOData, rp *RequestParameters) (string, error) {
154+ return fmt.Sprintf(
155+ `%s&%s`,
156+ ssodata.ConsumerSecret,
157+ ssodata.TokenSecret), nil
158+}
159+
160+type HMACSHA1 struct{}
161+
162+// Return the name of the signature method, used to compose the
163+// Authentication Header.
164+func (HMACSHA1) Name() string { return "HMAC-SHA1" }
165+
166+// Calculate the oaut_signature part of the Authentication Header.
167+func (HMACSHA1) Signature(
168+ ssodata *SSOData, rp *RequestParameters) (string, error) {
169+ baseUrl, err := NormalizeURL(rp.BaseURL)
170+ if err != nil {
171+ return "", err
172+ }
173+ params, err := NormalizeParameters(rp.Params)
174+ if err != nil {
175+ return "", err
176+ }
177+ baseString := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`,
178+ rp.HTTPMethod,
179+ url.QueryEscape(baseUrl),
180+ url.QueryEscape(params),
181+ url.QueryEscape("oauth_consumer_key="+ssodata.ConsumerKey),
182+ url.QueryEscape("&oauth_nonce="+rp.Nonce),
183+ url.QueryEscape(
184+ "&oauth_signature_method="+string(rp.SignatureMethod.Name())),
185+ url.QueryEscape("&oauth_timestamp="+rp.Timestamp),
186+ url.QueryEscape("&oauth_token="+ssodata.TokenKey),
187+ url.QueryEscape("&oauth_version=1.0"))
188+ hashfun := hmac.New(sha1.New, []byte(
189+ ssodata.ConsumerSecret+"&"+ssodata.TokenSecret))
190+ hashfun.Write([]byte(baseString))
191+ rawsignature := hashfun.Sum(nil)
192+ base64signature := make(
193+ []byte, base64.StdEncoding.EncodedLen(len(rawsignature)))
194+ base64.StdEncoding.Encode(base64signature, rawsignature)
195+ return string(base64signature), nil
196 }
197
198 // Sign the provided request.
199-func (oauth *SSOData) GetAuthorizationHeader() (string, error) {
200- if oauth.Nonce == "" {
201- oauth.Nonce = nonce()
202- }
203- if oauth.Timestamp == "" {
204- oauth.Timestamp = timestamp()
205- }
206- signature, err := oauth.signature()
207+func (ssodata *SSOData) GetAuthorizationHeader(
208+ rp *RequestParameters) (string, error) {
209+ if rp.Nonce == "" {
210+ rp.Nonce = nonce()
211+ }
212+ if rp.Timestamp == "" {
213+ rp.Timestamp = timestamp()
214+ }
215+ signature, err := rp.SignatureMethod.Signature(ssodata, rp)
216 if err != nil {
217 return "", err
218 }
219@@ -108,19 +126,20 @@
220 `oauth_timestamp="%s", `+
221 `oauth_nonce="%s", `+
222 `oauth_version="1.0"`,
223- url.QueryEscape(oauth.ConsumerKey),
224- url.QueryEscape(oauth.TokenKey),
225- oauth.SignatureMethod,
226+ url.QueryEscape(ssodata.ConsumerKey),
227+ url.QueryEscape(ssodata.TokenKey),
228+ rp.SignatureMethod.Name(),
229 signature,
230- url.QueryEscape(oauth.Timestamp),
231- url.QueryEscape(oauth.Nonce))
232+ url.QueryEscape(rp.Timestamp),
233+ url.QueryEscape(rp.Nonce))
234
235 return auth, nil
236 }
237
238 // Sign the provided request.
239-func (oauth *SSOData) SignRequest(req *http.Request) error {
240- auth, error := oauth.GetAuthorizationHeader()
241+func (ssodata *SSOData) SignRequest(
242+ rp *RequestParameters, req *http.Request) error {
243+ auth, error := ssodata.GetAuthorizationHeader(rp)
244 req.Header.Add("Authorization", auth)
245 return error
246 }
247
248=== modified file 'oauth_test.go'
249--- oauth_test.go 2013-01-29 11:23:37 +0000
250+++ oauth_test.go 2013-02-05 12:38:20 +0000
251@@ -6,47 +6,62 @@
252 "net/url"
253 )
254
255-func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) {
256+type OAuthTestSuite struct {
257+ ssodata SSOData
258+ rp RequestParameters
259+ request *http.Request
260+}
261+
262+var _ = Suite(&OAuthTestSuite{})
263+
264+func (suite *OAuthTestSuite) SetUpTest(c *C) {
265 baseUrl := "https://localhost"
266- ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey,
267+ suite.ssodata = SSOData{ConsumerKey: consumerKey,
268 ConsumerSecret: consumerSecret, TokenKey: tokenKey,
269 TokenName: tokenName, TokenSecret: tokenSecret}
270- request, _ := http.NewRequest("GET", baseUrl, nil)
271- ssodata.HTTPMethod = "GET"
272- ssodata.SignatureMethod = "PLAINTEXT"
273- err := ssodata.SignRequest(request)
274- c.Assert(err, IsNil)
275- authHeader := request.Header["Authorization"][0]
276+ suite.rp = RequestParameters{BaseURL: baseUrl, HTTPMethod: "GET",
277+ Nonce: "10888885", Timestamp: "1358853126"}
278+ suite.request, _ = http.NewRequest("GET", baseUrl, nil)
279+}
280+
281+// It is possible to sign a request with oauth_signature_method = PLAINTEXT
282+func (suite *OAuthTestSuite) TestSignRequestPlainText(c *C) {
283+ suite.rp.SignatureMethod = PLAINTEXT{}
284+ err := suite.ssodata.SignRequest(&suite.rp, suite.request)
285+ if err != nil {
286+ c.Log(err)
287+ c.FailNow()
288+ }
289+ authHeader := suite.request.Header["Authorization"][0]
290 c.Assert(authHeader, Matches, `^OAuth.*`)
291 c.Assert(authHeader, Matches, `.*realm="API".*`)
292 c.Assert(authHeader, Matches,
293- `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`)
294+ `.*oauth_consumer_key="`+url.QueryEscape(
295+ suite.ssodata.ConsumerKey)+`".*`)
296 c.Assert(authHeader, Matches,
297- `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`)
298+ `.*oauth_token="`+url.QueryEscape(suite.ssodata.TokenKey)+`".*`)
299 c.Assert(authHeader, Matches,
300 `.*oauth_signature="`+url.QueryEscape(
301- ssodata.ConsumerSecret+`&`+ssodata.TokenSecret)+`.*`)
302+ suite.ssodata.ConsumerSecret)+`&`+url.QueryEscape(
303+ suite.ssodata.TokenSecret)+`.*`)
304 }
305
306-// Test the request signing with oauth_signature_method = SHA1
307-func (suite *USSOTestSuite) TestSignRequestSHA1(c *C) {
308- baseUrl := "https://localhost"
309- ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey,
310- ConsumerSecret: consumerSecret, TokenKey: tokenKey,
311- TokenName: tokenName, TokenSecret: tokenSecret,
312- Nonce: "10888885", Timestamp: "1358853126"}
313- request, _ := http.NewRequest("GET", baseUrl, nil)
314- ssodata.HTTPMethod = "GET"
315- ssodata.SignatureMethod = "HMAC-SHA1"
316- err := ssodata.SignRequest(request)
317- c.Assert(err, IsNil)
318- authHeader := request.Header["Authorization"][0]
319+// It is possible to sign a request with oauth_signature_method = SHA1
320+func (suite *OAuthTestSuite) TestSignRequestSHA1(c *C) {
321+ suite.rp.SignatureMethod = HMACSHA1{}
322+ err := suite.ssodata.SignRequest(&suite.rp, suite.request)
323+ if err != nil {
324+ c.Log(err)
325+ c.FailNow()
326+ }
327+ authHeader := suite.request.Header["Authorization"][0]
328 c.Assert(authHeader, Matches, `^OAuth.*`)
329 c.Assert(authHeader, Matches, `.*realm="API".*`)
330 c.Assert(authHeader, Matches,
331- `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`)
332+ `.*oauth_consumer_key="`+url.QueryEscape(
333+ suite.ssodata.ConsumerKey)+`".*`)
334 c.Assert(authHeader, Matches,
335- `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`)
336+ `.*oauth_token="`+url.QueryEscape(suite.ssodata.TokenKey)+`".*`)
337 c.Assert(authHeader, Matches,
338 `.*oauth_signature="`+"amJnYeek4G9ObTgTiE2y6cwTyPg="+`.*`)
339 }
340
341=== modified file 'url.go'
342--- url.go 2013-01-29 11:23:37 +0000
343+++ url.go 2013-02-05 12:38:20 +0000
344@@ -3,43 +3,60 @@
345 import (
346 "fmt"
347 "net/url"
348+ "sort"
349 "strings"
350 )
351
352 // Remove the standard ports from the URL.
353-func normalizeHost(scheme, host_spec string) string {
354- standard_ports := map[string]string{
355+func normalizeHost(scheme, hostSpec string) string {
356+ standardPorts := map[string]string{
357 "http": "80",
358 "https": "443",
359 }
360- host_parts := strings.Split(host_spec, ":")
361- if len(host_parts) == 2 && host_parts[1] == standard_ports[scheme] {
362+ hostParts := strings.Split(hostSpec, ":")
363+ if len(hostParts) == 2 && hostParts[1] == standardPorts[scheme] {
364 // There's a port, but it's the default one. Leave it out.
365- return host_parts[0]
366+ return hostParts[0]
367 }
368- return host_spec
369+ return hostSpec
370 }
371
372 // Normalize the URL according to OAuth specs.
373-func NormalizeURL(input_url string) (string, error) {
374- parsed_url, err := url.Parse(input_url)
375+func NormalizeURL(inputUrl string) (string, error) {
376+ parsedUrl, err := url.Parse(inputUrl)
377 if err != nil {
378 return "", err
379 }
380
381- host := normalizeHost(parsed_url.Scheme, parsed_url.Host)
382- normalized_url := fmt.Sprintf(
383- "%v://%v%v", parsed_url.Scheme, host, parsed_url.Path)
384- return normalized_url, nil
385+ host := normalizeHost(parsedUrl.Scheme, parsedUrl.Host)
386+ normalizedUrl := fmt.Sprintf(
387+ "%v://%v%v", parsedUrl.Scheme, host, parsedUrl.Path)
388+ return normalizedUrl, nil
389 }
390
391 // Normalize the parameters in the query string according to OAuth specs.
392+// url.Values.Encode encoded the GET parameters in a consistent order
393+// we do the encoding ourselves.
394 func NormalizeParameters(parameters url.Values) (string, error) {
395- filtered_map := make(url.Values, len(parameters))
396- for param, value := range parameters {
397- if param != "oauth_signature" {
398- filtered_map[param] = value
399- }
400- }
401- return filtered_map.Encode(), nil
402+ filteredMap := make(url.Values, len(parameters))
403+ keys := make([]string, len(parameters))
404+ i := 0
405+ for key, _ := range parameters {
406+ keys[i] = key
407+ i++
408+ }
409+ sort.Strings(keys)
410+ for _, key := range keys {
411+ if key != "oauth_signature" {
412+ filteredMap[key] = parameters[key]
413+ }
414+ }
415+ parts := make([]string, 0, len(filteredMap))
416+ for _, key := range keys {
417+ prefix := url.QueryEscape(key) + "="
418+ for _, v := range filteredMap[key] {
419+ parts = append(parts, prefix+url.QueryEscape(v))
420+ }
421+ }
422+ return strings.Join(parts, "&"), nil
423 }
424
425=== modified file 'url_test.go'
426--- url_test.go 2013-01-29 14:27:25 +0000
427+++ url_test.go 2013-02-05 12:38:20 +0000
428@@ -60,7 +60,7 @@
429 output, err := NormalizeParameters(
430 url.Values{"a": []string{"1"}, "b": []string{"2"}})
431 c.Check(err, gocheck.Equals, nil)
432- c.Check(output, gocheck.Matches, "(a=1&b=2|b=2&a=1)")
433+ c.Check(output, gocheck.Matches, "(a=1&b=2)")
434 }
435
436 // NormalizeParameters() escapes the parameters correctly when encoding
437@@ -84,5 +84,5 @@
438 }
439 output, err := NormalizeParameters(params)
440 c.Check(err, gocheck.Equals, nil)
441- c.Check(output, gocheck.Matches, "(a=1&z=26|z=26&a=1)")
442+ c.Check(output, gocheck.Matches, "(a=1&z=26)")
443 }
444
445=== modified file 'usso.go'
446--- usso.go 2013-01-29 11:23:37 +0000
447+++ usso.go 2013-02-05 12:38:20 +0000
448@@ -49,7 +49,7 @@
449 "password": password,
450 "token_name": tokenName,
451 }
452- json_credentials, err := json.Marshal(credentials)
453+ jsonCredentials, err := json.Marshal(credentials)
454 if err != nil {
455 log.Printf("Error: %s\n", err)
456 return nil, err
457@@ -57,7 +57,7 @@
458 response, err := http.Post(
459 server.tokenURL(),
460 "application/json",
461- strings.NewReader(string(json_credentials)))
462+ strings.NewReader(string(jsonCredentials)))
463 if err != nil {
464 return nil, err
465 }
466@@ -77,14 +77,16 @@
467
468 // Returns all the Ubuntu SSO information related to this account.
469 func (server UbuntuSSOServer) GetAccounts(ssodata *SSOData) (string, error) {
470- ssodata.BaseURL = server.AccountsURL() + ssodata.ConsumerKey
471- ssodata.HTTPMethod = "GET"
472- ssodata.SignatureMethod = "HMAC-SHA1"
473- request, err := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil)
474+ rp := RequestParameters{
475+ BaseURL: server.AccountsURL() + ssodata.ConsumerKey,
476+ HTTPMethod: "GET",
477+ SignatureMethod: HMACSHA1{}}
478+
479+ request, err := http.NewRequest(rp.HTTPMethod, rp.BaseURL, nil)
480 if err != nil {
481 return "", err
482 }
483- err = SignRequest(ssodata, request)
484+ err = SignRequest(ssodata, &rp, request)
485 if err != nil {
486 return "", err
487 }
488@@ -103,12 +105,14 @@
489 }
490
491 // Given oauth credentials and a request, return it signed.
492-func SignRequest(ssodata *SSOData, request *http.Request) error {
493- return ssodata.SignRequest(request)
494+func SignRequest(
495+ ssodata *SSOData, rp *RequestParameters, request *http.Request) error {
496+ return ssodata.SignRequest(rp, request)
497 }
498
499 // Given oauth credentials return a valid http authorization header.
500-func GetAuthorizationHeader(ssodata *SSOData) (string, error) {
501- header, err := ssodata.GetAuthorizationHeader()
502+func GetAuthorizationHeader(
503+ ssodata *SSOData, rp *RequestParameters) (string, error) {
504+ header, err := ssodata.GetAuthorizationHeader(rp)
505 return header, err
506 }

Subscribers

People subscribed via source and target branches

to all changes: