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

Proposed by Vincenzo Di Somma
Status: Merged
Merge reported by: Vincenzo Di Somma
Merged at revision: not available
Proposed branch: lp:~vds/usso/support_sha1
Merge into: lp:usso
Diff against target: 461 lines (+302/-43)
5 files modified
example/usso_example.go (+36/-7)
url.go (+41/-0)
url_test.go (+84/-0)
usso.go (+92/-18)
usso_test.go (+49/-18)
To merge this branch: bzr merge lp:~vds/usso/support_sha1
Reviewer Review Type Date Requested Status
Matthew Williams (community) Approve
Review via email: mp+144350@code.launchpad.net

Commit message

Although it's not nice yet, I'm proposing this branch to adds support to SHA1 oauth_signature_method.

Description of the change

Although it's not nice yet, I'm proposing this branch to adds support to SHA1 oauth_signature_method.

To post a comment you must log in.
Revision history for this message
Matthew Williams (mattyw) wrote :

Ok

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :
Download full text (5.4 KiB)

Thanks for doing this! I have some notes. I don't know how urgent they are, since AIUI this code isn't functional yet, so perhaps it's best to land the code anyway, and then address any remaining points in a later branch. Maybe put TODO or XXX markers in the code? Just make sure not to forget. :)

The notes are:

 * I don't think signature() would produce the right output in the HMAC-SHA1 case. You use fmt.Sprint("%s&%s%s", a, b, c, d, e, f, g, h, i, j) but Sprint() doesn't format. So it'll just put that format string at the front as-is, with all the other values appended to it: "%s&%s%sAaaBbbCccDdd" etc. You probably want fmt.Sprintf(). But then you'll also need to provide a format string that consumes all the values you pass! The current code has a format string for 3 parameters, followed by about 10 parameters. This kind of thing is annoyingly hard to test for, because you have a large amount of text and you need to fish out of it the things that you expect to see. Things that you _don't_ expect to see are hard to detect. Normally an end-to-end test would reveal those, but not when the real OAuth server isn't part of the test setup. There are ways to deal with this problem, and I'll try to make them gradually clearer over the rest of this review.

 * The text composition in GetAuthorizationHeader() and signature() looks very repetitive, both within each function and between the two. Also the repeated code leaves out the query-escaping here and there just because it can. Is there no easy way to regularize that into a standard-library call or loop, so that it's easier to avoid little one-off mistakes? I know it would be very easy in Python, although it does look to be a bit harder in Go. Especially if you want to pass a map and get consistently-sorted output.

 * Speaking of which: does signature() need the parameters to be written in a consistent order? The python SSO code sorted them, but I'm not sure net/url's Values.Encode() in Go promises any kind of consistency. I didn't get around to resolving this in my code. :-(

 * Is there any particular reason why SSOData.Timestamp and SSOData.Nonce are initialized inside GetAuthorizationHeader? It seems to me that should be SSOData's responsibility.

 * You introduce some line-breaking. Do we have a standard for doing that in Go? The Go way of dealing with long lines, apparently, is to let them run for as long as they want to. If you do want to break them up, the Cloud Engineering convention in Python at least is to put the first line break *right after* the opening parenthesis or brace that contains the thing you want to break up. *Except* in function definitions, where you continue the parameter list until the line gets too long and then break the line. You do it that way in some places, but not consistently. My personal instinct is to go with the Go way, and if the line gets too long, take that as a signal that something has become too complicated and needs refactoring.

 * signature() returns the empty string in the case of an unknown signature method. Is that meant to indicate an error to the caller, or is it meant as a sensible fallback? If it's an error condi...

Read more...

Revision history for this message
Vincenzo Di Somma (vds) wrote :

> Thanks for doing this! I have some notes. I don't know how urgent they are,
> since AIUI this code isn't functional yet, so perhaps it's best to land the
> code anyway, and then address any remaining points in a later branch. Maybe
> put TODO or XXX markers in the code? Just make sure not to forget. :)

Thanks for this notes, but, if we don't want to forget all this obeservation the right place were to put them is the google doc, once the branch is landed, no one will come back here to read the comments.
I agree with the need for better tests and error handling which has been ignored for a while, both are already listed in the google doc as next work item.

I'll fix the bugs you spotted and then I guess we can land this branch and keep improving the library in the following branches.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Where is that Google doc?

Revision history for this message
Raphaël Badin (rvb) wrote :

I'll try to review that in details later today but just a quick notes:

[0]

+ fmt.Print("Enter signature method (PLAINTEXT or HMAC-SHA1): ")
+ fmt.Scanf("%s", &signature_method)

How about we ditch that and perform the call for the two available signatures? The goal of this example is to run a test (that covers as much code as possible) against a real server in the quickest way possible.

[1]

22 - fmt.Println("This application will query the staging Ubuntu SSO Server to fetch authorisation tokens.")
23 + fmt.Println("This application will query the staging Ubuntu SSO Server" +
24 + " to fetch authorisation tokens.")

and

384 - server := newSingleServingServer("/api/v2/tokens", string(jsonServerResponseData), 200)
385 + server := newSingleServingServer("/api/v2/tokens",
386 + string(jsonServerResponseData), 200)

and

159 +func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort(
160 + c *gocheck.C) {

etc.

I think Jeroen's remark about the formatting is right. It's best if we try to use the same style as the other Go projects (lp:juju-core for instance) and use long lines instead of wrapping things like that.

Revision history for this message
Vincenzo Di Somma (vds) wrote :

> I'll try to review that in details later today but just a quick notes:
>
> [0]
>
> + fmt.Print("Enter signature method (PLAINTEXT or HMAC-SHA1): ")
> + fmt.Scanf("%s", &signature_method)
>
> How about we ditch that and perform the call for the two available signatures?
> The goal of this example is to run a test (that covers as much code as
> possible) against a real server in the quickest way possible.

It's just an example but I don't really see a reason for two http request.

> [1]
>
> 22 - fmt.Println("This application will query the staging Ubuntu
> SSO Server to fetch authorisation tokens.")
> 23 + fmt.Println("This application will query the staging Ubuntu
> SSO Server" +
> 24 + " to fetch authorisation tokens.")
>
> and
>
> 384 - server := newSingleServingServer("/api/v2/tokens",
> string(jsonServerResponseData), 200)
> 385 + server := newSingleServingServer("/api/v2/tokens",
> 386 + string(jsonServerResponseData), 200)
>
> and
>
> 159 +func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort(
> 160 + c *gocheck.C) {
>
> etc.
>
> I think Jeroen's remark about the formatting is right. It's best if we try to
> use the same style as the other Go projects (lp:juju-core for instance) and
> use long lines instead of wrapping things like that.

I understand juju-core uses long lines, but omnibus, for example, doesn't. I only see cons in long lines and no pros.

Revision history for this message
Raphaël Badin (rvb) wrote :

[2]

These two blocks seem very redundant to me, isn't it possible refactor that a bit to unify/simplify it a bit?

289 + `OAuth realm="API", `+
290 + `oauth_consumer_key="%s", `+
291 + `oauth_token="%s", `+
292 + `oauth_signature_method="%s", `+
293 + `oauth_signature="%s", `+
294 + `oauth_timestamp="%s", `+
295 + `oauth_nonce="%s", `+
296 + `oauth_version="1.0"`,
297 + url.QueryEscape(oauth.ConsumerKey),
298 + url.QueryEscape(oauth.TokenKey),
299 + oauth.SignatureMethod,
300 + signature,
301 + url.QueryEscape(oauth.Timestamp),
302 + url.QueryEscape(oauth.Nonce))

337 + base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`,
338 + oauth.HTTPMethod,
339 + url.QueryEscape(base_url),
340 + url.QueryEscape(params),
341 + url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey),
342 + url.QueryEscape("&oauth_nonce="+oauth.Nonce),
343 + url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod),
344 + url.QueryEscape("&oauth_timestamp="+oauth.Timestamp),
345 + url.QueryEscape("&oauth_token="+oauth.TokenKey),
346 + url.QueryEscape("&oauth_version=1.0"))

[3]

328 + switch oauth.SignatureMethod {
329 + case "PLAINTEXT":
330 + return fmt.Sprintf(
331 + `%s%%26%s`,
332 + oauth.ConsumerSecret,
333 + oauth.TokenSecret)
334 + case "HMAC-SHA1":

This is probably something that we can fix later but implementing the signature like this looks like a design flaw to me. I think we should be using the strategy pattern here: define a "signing" interface and have two different types implement it (one with a plain text signature, the other with a SHA1 signature).
This would provide a much better encapsulation of the signing code and would make it much more easy to add new signature types if we ever need to.

Revision history for this message
Vincenzo Di Somma (vds) wrote :

> [2]
>
> These two blocks seem very redundant to me, isn't it possible refactor that a
> bit to unify/simplify it a bit?
>
> 289 + `OAuth realm="API", `+
> 290 + `oauth_consumer_key="%s", `+
> 291 + `oauth_token="%s", `+
> 292 + `oauth_signature_method="%s", `+
> 293 + `oauth_signature="%s", `+
> 294 + `oauth_timestamp="%s", `+
> 295 + `oauth_nonce="%s", `+
> 296 + `oauth_version="1.0"`,
> 297 + url.QueryEscape(oauth.ConsumerKey),
> 298 + url.QueryEscape(oauth.TokenKey),
> 299 + oauth.SignatureMethod,
> 300 + signature,
> 301 + url.QueryEscape(oauth.Timestamp),
> 302 + url.QueryEscape(oauth.Nonce))
>
>
> 337 + base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`,
> 338 + oauth.HTTPMethod,
> 339 + url.QueryEscape(base_url),
> 340 + url.QueryEscape(params),
> 341 +
> url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey),
> 342 + url.QueryEscape("&oauth_nonce="+oauth.Nonce),
> 343 +
> url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod),
> 344 +
> url.QueryEscape("&oauth_timestamp="+oauth.Timestamp),
> 345 +
> url.QueryEscape("&oauth_token="+oauth.TokenKey),
> 346 + url.QueryEscape("&oauth_version=1.0"))

Nope, they look the same but they produce two different string, unifing them will not make them any simpler to read, quite the opposite.

> [3]
>
> 328 + switch oauth.SignatureMethod {
> 329 + case "PLAINTEXT":
> 330 + return fmt.Sprintf(
> 331 + `%s%%26%s`,
> 332 + oauth.ConsumerSecret,
> 333 + oauth.TokenSecret)
> 334 + case "HMAC-SHA1":
>
> This is probably something that we can fix later but implementing the
> signature like this looks like a design flaw to me. I think we should be
> using the strategy pattern here: define a "signing" interface and have two
> different types implement it (one with a plain text signature, the other with
> a SHA1 signature).
> This would provide a much better encapsulation of the signing code and would
> make it much more easy to add new signature types if we ever need to.

Apart from the error handling, that will be added in a following branch, I think that a strategy pattern would be over engineering or just YAGNI.

lp:~vds/usso/support_sha1 updated
24. By Vincenzo Di Somma

Cleaning up.

25. By Vincenzo Di Somma

More cleaning up.

Revision history for this message
Graham Binns (gmb) wrote :
Download full text (3.6 KiB)

Just to add my two penn'orth here...

> * You introduce some line-breaking. Do we have a standard for doing
> that in Go? The Go way of dealing with long lines, apparently, is to
> let them run for as long as they want to. If you do want to break
> them up, the Cloud Engineering convention in Python at least is to
> put the first line break *right after* the opening parenthesis or
> brace that contains the thing you want to break up. *Except* in
> function definitions, where you continue the parameter list until the
> line gets too long and then break the line. You do it that way in
> some places, but not consistently. My personal instinct is to go with
> the Go way, and if the line gets too long, take that as a signal that
> something has become too complicated and needs refactoring.

We don't have a standard for this (we need to have some Golang coding
standards agreed across at least the CE team, preferably across the
company).

Now, to get my personal bias out of the way first: I hate long lines. I
like my 78-character limits, thank-you very much.

That said, Go is an interesting problem, and as you say the convention
(by sheer weight of peer pressure, if nothing else) seems to be "let the
line run on and on and on for as long as it needs."

For instance, long function definitions are a pain in the arse to read:

func (myStruct *MyStruct) DoSomethingToAStruct (param1, param2, param3, someOtherParameterWithAStupidlyLongName string) (string, string, string, error) {

Is maddeningly. However, breaking it up:

func (myStruct *MyStruct) DoSomethingToAStruct (param1, param2, param3,
    someOtherParameterWithAStupidlyLongName string)
    (string, string, string, error) {

Is also a bit maddening (particularly that brace. Don't get me started).

Now, I don't know what The Right Thing is here - I'm going to write an email to the -tech list about general Go coding standards, and that's probably the right place for a discussion - in our project we've broken lines to make them fit; I'm happy to continue that semi-psychosis-inducing convention for now, mostly because the other way is also semi-psychosis-inducing.

Finally, a note on multiple lines of parameters and the formatting thereof.

> If you do want to break them up, the Cloud Engineering convention in Python
> at least is to put the first line break *right after* the opening parenthesis
> or brace that contains the thing you want to break up.

You can't do that in Go, at least with strings. For example, if you do this:

func Bleurgh() error {
    SomeFunctionCall(
        "Here's a test of how to break up strings in Go. Note that we " +
        "are using the concatenation operator '+' to combine the " +
        "strings. Isn't that great? Looks just like JavaScript. " +
        "Kill me now.")
}

and then run gofmt on it (which we do habitually; if gofmt needs to change things, your code ain't getting into trunk), you get this piece of joy:

func Bleurgh() error {
    SomeFunctionCall(
        "Here's a test of how to break up strings in Go. Note that we " +
            "are using the concatenation operator '+' to combine the " +
            "strings. Isn't that great? Lo...

Read more...

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Gentlemen, in my humble opinion this branch is spending too much time out of the codebase. I'd love to help implement all these little fixes, and am currently available to do so, but that doesn't work very well with an ongoing merge proposal.

So does anyone mind if we list our grievances for later and get this branch landed in its non-functioning state? I'll start a to-do list in the Google document so that we don't forget to fix the things that were bothering us. Should also help us prioritize them.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Ahem. I said “non-functioning” but actually it looks like Vincenzo fixed the Sprint()/Sprintf() bug! Sorry about that. All the more reason to get this in soon.

Revision history for this message
Vincenzo Di Somma (vds) wrote :

Most of the bugs have been addressed in following branches, now it's fully functional and tested. I agree with you, once it lands we can go trough the code again and fix what's left.

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

> Most of the bugs have been addressed in following branches, now it's fully
> functional and tested. I agree with you, once it lands we can go trough the
> code again and fix what's left.

Yes, let's get this landed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'example/usso_example.go'
--- example/usso_example.go 2013-01-21 16:57:31 +0000
+++ example/usso_example.go 2013-01-25 11:39:20 +0000
@@ -1,23 +1,27 @@
1package main1package main
22
3import (3import (
4 "bytes"
4 "encoding/json"5 "encoding/json"
5 "fmt"6 "fmt"
7 "io/ioutil"
6 "launchpad.net/usso"8 "launchpad.net/usso"
9 "net/http"
7)10)
811
9var email string12var email, password, tokenName, signature_method string
10var password string
11var tokenName string
1213
13func inputParams() {14func inputParams() {
14 fmt.Println("This application will query the staging Ubuntu SSO Server to fetch authorisation tokens.")15 fmt.Println("This application will query the staging Ubuntu SSO Server" +
16 " to fetch authorisation tokens.")
15 fmt.Print("Enter email: ")17 fmt.Print("Enter email: ")
16 fmt.Scanf("%s", &email)18 fmt.Scanf("%s", &email)
17 fmt.Print("Enter password: ")19 fmt.Print("Enter password: ")
18 fmt.Scanf("%s", &password)20 fmt.Scanf("%s", &password)
19 fmt.Print("Enter token name: ")21 fmt.Print("Enter token name: ")
20 fmt.Scanf("%s", &tokenName)22 fmt.Scanf("%s", &tokenName)
23 fmt.Print("Enter signature method (PLAINTEXT or HMAC-SHA1): ")
24 fmt.Scanf("%s", &signature_method)
21}25}
2226
23func main() {27func main() {
@@ -26,16 +30,41 @@
26 // Fetch the tokens using usso.GetToken.30 // Fetch the tokens using usso.GetToken.
27 fmt.Println("Fetching tokens from staging server...")31 fmt.Println("Fetching tokens from staging server...")
28 server := usso.StagingUbuntuSSOServer32 server := usso.StagingUbuntuSSOServer
29 // One would use server := usso.ProductionUbuntuSSOServer to use the production Ubuntu SSO Server.33 // One would use server := usso.ProductionUbuntuSSOServer
30 token, err := server.GetToken(email, password, tokenName)34 // to use the production Ubuntu SSO Server.
35 ssodata, err := server.GetToken(email, password, tokenName)
31 if err != nil {36 if err != nil {
32 panic(err)37 panic(err)
33 }38 }
34 // Format the result as json for displaying it:39 // Format the result as json for displaying it:
35 json_token, err := json.Marshal(token)40 json_token, err := json.Marshal(ssodata)
36 if err != nil {41 if err != nil {
37 panic(err)42 panic(err)
38 }43 }
39 fmt.Printf("Got tokens: %s\n", json_token)44 fmt.Printf("Got tokens: %s\n", json_token)
4045
46 ssodata.BaseURL = fmt.Sprintf(
47 "https://login.staging.ubuntu.com/api/v2/accounts/%s",
48 ssodata.ConsumerKey)
49 ssodata.HTTPMethod = "GET"
50 ssodata.SignatureMethod = signature_method
51 request, _ := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil)
52 ssodata.Sign(request)
53
54 if err != nil {
55 fmt.Printf("Error: %s\n", err)
56 }
57 // run the request
58 client := &http.Client{}
59 response, err := client.Do(request)
60 if err != nil {
61 fmt.Printf("Error: %s\n", err)
62 }
63 body, err := ioutil.ReadAll(response.Body)
64 if err != nil {
65 fmt.Println(err)
66 }
67 var b bytes.Buffer
68 b.Write(body)
69 fmt.Printf("response: %+v\n", b.String())
41}70}
4271
=== added file 'url.go'
--- url.go 1970-01-01 00:00:00 +0000
+++ url.go 2013-01-25 11:39:20 +0000
@@ -0,0 +1,41 @@
1package usso
2
3import (
4 "fmt"
5 "net/url"
6 "strings"
7)
8
9func normalizeHost(scheme, host_spec string) string {
10 standard_ports := map[string]string{
11 "http": "80",
12 "https": "443",
13 }
14 host_parts := strings.Split(host_spec, ":")
15 if len(host_parts) == 2 && host_parts[1] == standard_ports[scheme] {
16 // There's a port, but it's the default one. Leave it out.
17 return host_parts[0]
18 }
19 return host_spec
20}
21
22func NormalizeURL(input_url string) (string, error) {
23 parsed_url, err := url.Parse(input_url)
24 if err != nil {
25 return "", err
26 }
27
28 host := normalizeHost(parsed_url.Scheme, parsed_url.Host)
29 normalized_url := fmt.Sprintf("%v://%v%v", parsed_url.Scheme, host, parsed_url.Path)
30 return normalized_url, nil
31}
32
33func NormalizeParameters(parameters url.Values) (string, error) {
34 filtered_map := make(url.Values, len(parameters))
35 for param, value := range parameters {
36 if param != "oauth_signature" {
37 filtered_map[param] = value
38 }
39 }
40 return filtered_map.Encode(), nil
41}
042
=== added file 'url_test.go'
--- url_test.go 1970-01-01 00:00:00 +0000
+++ url_test.go 2013-01-25 11:39:20 +0000
@@ -0,0 +1,84 @@
1package usso
2
3import (
4 "launchpad.net/gocheck"
5 "net/url"
6)
7
8func (suite *USSOTestSuite) TestNormalizeURLReturnsBasicURL(c *gocheck.C) {
9 output, err := NormalizeURL("http://example.com/path")
10 c.Check(err, gocheck.Equals, nil)
11 c.Check(output, gocheck.Equals, "http://example.com/path")
12}
13
14func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPPort(
15 c *gocheck.C) {
16 output, err := NormalizeURL("http://example.com:80/path")
17 c.Check(err, gocheck.Equals, nil)
18 c.Check(output, gocheck.Equals, "http://example.com/path")
19}
20
21func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPSPort(
22 c *gocheck.C) {
23 output, err := NormalizeURL("https://example.com:443/path")
24 c.Check(err, gocheck.Equals, nil)
25 c.Check(output, gocheck.Equals, "https://example.com/path")
26}
27
28func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort(
29 c *gocheck.C) {
30 output, err := NormalizeURL("http://example.com:8080/")
31 c.Check(err, gocheck.Equals, nil)
32 c.Check(output, gocheck.Equals, "http://example.com:8080/")
33}
34
35func (suite *USSOTestSuite) TestNormalizeURLStripsParameters(c *gocheck.C) {
36 output, err := NormalizeURL("http://example.com/path?query=value&param=arg")
37 c.Check(err, gocheck.Equals, nil)
38 c.Check(output, gocheck.Equals, "http://example.com/path")
39}
40
41func (suite *USSOTestSuite) TestNormalizeParametersReturnsParameters(
42 c *gocheck.C) {
43 output, err := NormalizeParameters(url.Values{"param": []string{"value"}})
44 c.Check(err, gocheck.Equals, nil)
45 c.Check(output, gocheck.Equals, "param=value")
46}
47
48func (suite *USSOTestSuite) TestNormalizeParametersConcatenatesParameters(
49 c *gocheck.C) {
50 output, err := NormalizeParameters(
51 url.Values{"a": []string{"1"}, "b": []string{"2"}})
52 c.Check(err, gocheck.Equals, nil)
53 c.Check(output, gocheck.Matches, "(a=1&b=2|b=2&a=1)")
54}
55
56func (suite *USSOTestSuite) TestNormalizeParametersSortsParameters(
57 c *gocheck.C) {
58 params := url.Values{
59 "b": []string{"x"},
60 "a": []string{"y"},
61 }
62 output, err := NormalizeParameters(params)
63 c.Check(err, gocheck.Equals, nil)
64 c.Check(output, gocheck.Matches, "(a=y&b=x|b=x&a=y)")
65}
66
67func (suite *USSOTestSuite) TestNormalizeParametersEscapesParameters(
68 c *gocheck.C) {
69 output, err := NormalizeParameters(url.Values{"a&b": []string{"1"}})
70 c.Check(err, gocheck.Equals, nil)
71 c.Check(output, gocheck.Equals, "a%26b=1")
72}
73
74func (suite *USSOTestSuite) TestNormalizeParametersOmitsOAuthSignature(
75 c *gocheck.C) {
76 params := url.Values{
77 "a": []string{"1"},
78 "oauth_signature": []string{"foobarsplatszot"},
79 "z": []string{"26"},
80 }
81 output, err := NormalizeParameters(params)
82 c.Check(err, gocheck.Equals, nil)
83 c.Check(output, gocheck.Matches, "(a=1&z=26|z=26&a=1)")
84}
085
=== modified file 'usso.go'
--- usso.go 2013-01-22 10:39:12 +0000
+++ usso.go 2013-01-25 11:39:20 +0000
@@ -1,10 +1,11 @@
1// Copyright 2013 Canonical Ltd. This software is licensed under the
2// GNU Affero General Public License version 3 (see the file LICENSE).
3
4package usso1package usso
52
6import (3import (
4 "crypto/hmac"
5 "crypto/sha1"
6 "encoding/base64"
7 "encoding/json"7 "encoding/json"
8 "fmt"
8 "io/ioutil"9 "io/ioutil"
9 "log"10 "log"
10 "math/rand"11 "math/rand"
@@ -20,6 +21,16 @@
20 rand.Seed(time.Now().UTC().UnixNano())21 rand.Seed(time.Now().UTC().UnixNano())
21}22}
2223
24func timestamp() string {
25 // Create a timestamp used in authorization header.
26 return strconv.Itoa(int(time.Now().Unix()))
27}
28
29func nonce() string {
30 // Create a nonce used in authorization header.
31 return strconv.Itoa(rand.Intn(100000000))
32}
33
23type UbuntuSSOServer struct {34type UbuntuSSOServer struct {
24 baseUrl string35 baseUrl string
25}36}
@@ -38,12 +49,18 @@
38var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"}49var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"}
3950
40type SSOData struct {51type SSOData struct {
41 BaseURL string52 // Contains the oauth data to perform a request.
42 ConsumerKey string `json:"consumer_key"`53 HTTPMethod string `json:"-"`
43 ConsumerSecret string `json:"consumer_secret"`54 BaseURL string `json:"-"`
44 TokenKey string `json:"token_key"`55 Params url.Values `json:"-"`
45 TokenName string `json:"token_name"`56 Nonce string `json:"-"`
46 TokenSecret string `json:"token_secret"`57 Timestamp string `json:"-"`
58 SignatureMethod string `json:"-"`
59 ConsumerKey string `json:"consumer_key"`
60 ConsumerSecret string `json:"consumer_secret"`
61 TokenKey string `json:"token_key"`
62 TokenName string `json:"token_name"`
63 TokenSecret string `json:"token_secret"`
47}64}
4865
49func (server UbuntuSSOServer) GetToken(email string, password string, tokenName string) (*SSOData, error) {66func (server UbuntuSSOServer) GetToken(email string, password string, tokenName string) (*SSOData, error) {
@@ -78,17 +95,74 @@
78 return &ssodata, nil95 return &ssodata, nil
79}96}
8097
98func (oauth *SSOData) GetAuthorizationHeader() string {
99 // Sign the provided request.
100 if oauth.Nonce == "" {
101 oauth.Nonce = nonce()
102 }
103 if oauth.Timestamp == "" {
104 oauth.Timestamp = timestamp()
105 }
106 signature := oauth.signature()
107
108 auth := fmt.Sprintf(
109 `OAuth realm="API", `+
110 `oauth_consumer_key="%s", `+
111 `oauth_token="%s", `+
112 `oauth_signature_method="%s", `+
113 `oauth_signature="%s", `+
114 `oauth_timestamp="%s", `+
115 `oauth_nonce="%s", `+
116 `oauth_version="1.0"`,
117 url.QueryEscape(oauth.ConsumerKey),
118 url.QueryEscape(oauth.TokenKey),
119 oauth.SignatureMethod,
120 signature,
121 url.QueryEscape(oauth.Timestamp),
122 url.QueryEscape(oauth.Nonce))
123
124 return auth
125}
126
81func (oauth *SSOData) Sign(req *http.Request) error {127func (oauth *SSOData) Sign(req *http.Request) error {
82 // Sign the provided request.128 // Sign the provided request.
83 auth := `OAuth realm="API", ` +129 auth := oauth.GetAuthorizationHeader()
84 `oauth_consumer_key="` + url.QueryEscape(oauth.ConsumerKey) + `", ` +
85 `oauth_token="` + url.QueryEscape(oauth.TokenKey) + `", ` +
86 `oauth_signature_method="PLAINTEXT", ` +
87 `oauth_signature="` + url.QueryEscape(
88 oauth.ConsumerSecret+`&`+oauth.TokenSecret) + `", ` +
89 `oauth_timestamp="` + strconv.FormatInt(time.Now().Unix(), 10) + `", ` +
90 `oauth_nonce="` + strconv.Itoa(int(rand.Intn(99999999))) + `", ` +
91 `oauth_version="1.0"`
92 req.Header.Add("Authorization", auth)130 req.Header.Add("Authorization", auth)
93 return nil131 return nil
94}132}
133
134func (oauth *SSOData) signature() string {
135 // Depending on the signature method, create the signature from the
136 // consumer secret, the token secret and, if required, the URL.
137 // Supported signature methods are PLAINTEXT and HMAC-SHA1.
138
139 switch oauth.SignatureMethod {
140 case "PLAINTEXT":
141 return fmt.Sprintf(
142 `%s%%26%s`,
143 oauth.ConsumerSecret,
144 oauth.TokenSecret)
145 case "HMAC-SHA1":
146 base_url, _ := NormalizeURL(oauth.BaseURL)
147 params, _ := NormalizeParameters(oauth.Params)
148 base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`,
149 oauth.HTTPMethod,
150 url.QueryEscape(base_url),
151 url.QueryEscape(params),
152 url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey),
153 url.QueryEscape("&oauth_nonce="+oauth.Nonce),
154 url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod),
155 url.QueryEscape("&oauth_timestamp="+oauth.Timestamp),
156 url.QueryEscape("&oauth_token="+oauth.TokenKey),
157 url.QueryEscape("&oauth_version=1.0"))
158 hashfun := hmac.New(sha1.New, []byte(
159 oauth.ConsumerSecret+"&"+oauth.TokenSecret))
160 hashfun.Write([]byte(base_string))
161 rawsignature := hashfun.Sum(nil)
162 base64signature := make(
163 []byte, base64.StdEncoding.EncodedLen(len(rawsignature)))
164 base64.StdEncoding.Encode(base64signature, rawsignature)
165 return string(base64signature)
166 }
167 return ""
168}
95169
=== modified file 'usso_test.go'
--- usso_test.go 2013-01-22 10:39:12 +0000
+++ usso_test.go 2013-01-25 11:39:20 +0000
@@ -1,6 +1,3 @@
1// Copyright 2013 Canonical Ltd. This software is licensed under the
2// GNU Affero General Public License version 3 (see the file LICENSE).
3
4package usso1package usso
52
6import (3import (
@@ -49,7 +46,8 @@
4946
50// newSingleServingServer create a single-serving test http server which will47// newSingleServingServer create a single-serving test http server which will
51// return only one response as defined by the passed arguments.48// return only one response as defined by the passed arguments.
52func newSingleServingServer(uri string, response string, code int) *SingleServingServer {49func newSingleServingServer(
50 uri string, response string, code int) *SingleServingServer {
53 var requestContent string51 var requestContent string
54 var requested bool52 var requested bool
55 handler := func(w http.ResponseWriter, r *http.Request) {53 handler := func(w http.ResponseWriter, r *http.Request) {
@@ -89,16 +87,19 @@
89 if err != nil {87 if err != nil {
90 panic(err)88 panic(err)
91 }89 }
92 server := newSingleServingServer("/api/v2/tokens", string(jsonServerResponseData), 200)90 server := newSingleServingServer("/api/v2/tokens",
91 string(jsonServerResponseData), 200)
93 var testSSOServer = &UbuntuSSOServer{server.URL}92 var testSSOServer = &UbuntuSSOServer{server.URL}
94 defer server.Close()93 defer server.Close()
9594
96 // The returned information is correct.95 // The returned information is correct.
97 ssodata, err := testSSOServer.GetToken(email, password, tokenName)96 ssodata, err := testSSOServer.GetToken(email, password, tokenName)
98 c.Assert(err, IsNil)97 c.Assert(err, IsNil)
99 expectedSSOData := &SSOData{ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenSecret: tokenSecret, TokenName: tokenName}98 expectedSSOData := &SSOData{ConsumerKey: consumerKey,
99 ConsumerSecret: consumerSecret, TokenKey: tokenKey,
100 TokenSecret: tokenSecret, TokenName: tokenName}
100 c.Assert(ssodata, DeepEquals, expectedSSOData)101 c.Assert(ssodata, DeepEquals, expectedSSOData)
101 // The request that the fake Ubuntu SSO Server got contained the credentials.102 //The request that the fake Ubuntu SSO Server got contained the credentials.
102 credentials := map[string]string{103 credentials := map[string]string{
103 "email": email,104 "email": email,
104 "password": password,105 "password": password,
@@ -113,15 +114,45 @@
113114
114func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) {115func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) {
115 baseUrl := "https://localhost"116 baseUrl := "https://localhost"
116 ssoData := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenName: tokenName, TokenSecret: tokenSecret}117 ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey,
117 request, _ := http.NewRequest("GET", baseUrl, nil)118 ConsumerSecret: consumerSecret, TokenKey: tokenKey,
118119 TokenName: tokenName, TokenSecret: tokenSecret}
119 err := ssoData.Sign(request)120 request, _ := http.NewRequest("GET", baseUrl, nil)
120121 ssodata.HTTPMethod = "GET"
121 c.Assert(err, IsNil)122 ssodata.SignatureMethod = "PLAINTEXT"
122 authHeader := request.Header["Authorization"][0]123 err := ssodata.Sign(request)
123 c.Assert(authHeader, Matches, `.*OAuth realm="API".*`)124 c.Assert(err, IsNil)
124 c.Assert(authHeader, Matches, `.*oauth_consumer_key="`+url.QueryEscape(ssoData.ConsumerKey)+`".*`)125 authHeader := request.Header["Authorization"][0]
125 c.Assert(authHeader, Matches, `.*oauth_token="`+url.QueryEscape(ssoData.TokenKey)+`".*`)126 c.Assert(authHeader, Matches, `^OAuth.*`)
126 c.Assert(authHeader, Matches, `.*oauth_signature="`+url.QueryEscape(ssoData.ConsumerSecret+`&`+ssoData.TokenSecret)+`.*`)127 c.Assert(authHeader, Matches, `.*realm="API".*`)
128 c.Assert(authHeader, Matches,
129 `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`)
130 c.Assert(authHeader, Matches,
131 `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`)
132 c.Assert(authHeader, Matches,
133 `.*oauth_signature="`+url.QueryEscape(
134 ssodata.ConsumerSecret+`&`+ssodata.TokenSecret)+`.*`)
135}
136
137func (suite *USSOTestSuite) TestSignRequestSHA1(c *C) {
138 // Test the request signing with oauth_signature_method = SHA1
139 baseUrl := "https://localhost"
140 ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey,
141 ConsumerSecret: consumerSecret, TokenKey: tokenKey,
142 TokenName: tokenName, TokenSecret: tokenSecret,
143 Nonce: "10888885", Timestamp: "1358853126"}
144 request, _ := http.NewRequest("GET", baseUrl, nil)
145 ssodata.HTTPMethod = "GET"
146 ssodata.SignatureMethod = "HMAC-SHA1"
147 err := ssodata.Sign(request)
148 c.Assert(err, IsNil)
149 authHeader := request.Header["Authorization"][0]
150 c.Assert(authHeader, Matches, `^OAuth.*`)
151 c.Assert(authHeader, Matches, `.*realm="API".*`)
152 c.Assert(authHeader, Matches,
153 `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`)
154 c.Assert(authHeader, Matches,
155 `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`)
156 c.Assert(authHeader, Matches,
157 `.*oauth_signature="`+"amJnYeek4G9ObTgTiE2y6cwTyPg="+`.*`)
127}158}

Subscribers

People subscribed via source and target branches

to all changes: