Merge lp:~vds/usso/support_sha1 into lp:usso
- support_sha1
- Merge into trunk
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 |
Related bugs: |
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
Description of the change
Although it's not nice yet, I'm proposing this branch to adds support to SHA1 oauth_signature
Jeroen T. Vermeulen (jtv) 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. :)
The notes are:
* I don't think signature() would produce the right output in the HMAC-SHA1 case. You use fmt.Sprint(
* The text composition in GetAuthorizatio
* 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 GetAuthorizatio
* 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...
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.
Jeroen T. Vermeulen (jtv) wrote : | # |
Where is that Google doc?
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 := newSingleServin
385 + server := newSingleServin
386 + string(
and
159 +func (suite *USSOTestSuite) TestNormalizeUR
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.
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 := newSingleServin
> string(
> 385 + server := newSingleServin
> 386 + string(
>
> and
>
> 159 +func (suite *USSOTestSuite) TestNormalizeUR
> 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.
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_
291 + `oauth_token="%s", `+
292 + `oauth_
293 + `oauth_
294 + `oauth_
295 + `oauth_nonce="%s", `+
296 + `oauth_
297 + url.QueryEscape
298 + url.QueryEscape
299 + oauth.Signature
300 + signature,
301 + url.QueryEscape
302 + url.QueryEscape
337 + base_string := fmt.Sprintf(
338 + oauth.HTTPMethod,
339 + url.QueryEscape
340 + url.QueryEscape
341 + url.QueryEscape
342 + url.QueryEscape
343 + url.QueryEscape
344 + url.QueryEscape
345 + url.QueryEscape
346 + url.QueryEscape
[3]
328 + switch oauth.Signature
329 + case "PLAINTEXT":
330 + return fmt.Sprintf(
331 + `%s%%26%s`,
332 + oauth.ConsumerS
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.
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_
> 291 + `oauth_token="%s", `+
> 292 + `oauth_
> 293 + `oauth_
> 294 + `oauth_
> 295 + `oauth_nonce="%s", `+
> 296 + `oauth_
> 297 + url.QueryEscape
> 298 + url.QueryEscape
> 299 + oauth.Signature
> 300 + signature,
> 301 + url.QueryEscape
> 302 + url.QueryEscape
>
>
> 337 + base_string := fmt.Sprintf(
> 338 + oauth.HTTPMethod,
> 339 + url.QueryEscape
> 340 + url.QueryEscape
> 341 +
> url.QueryEscape
> 342 + url.QueryEscape
> 343 +
> url.QueryEscape
> 344 +
> url.QueryEscape
> 345 +
> url.QueryEscape
> 346 + url.QueryEscape
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.Signature
> 329 + case "PLAINTEXT":
> 330 + return fmt.Sprintf(
> 331 + `%s%%26%s`,
> 332 + oauth.ConsumerS
> 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.
- 24. By Vincenzo Di Somma
-
Cleaning up.
- 25. By Vincenzo Di Somma
-
More cleaning up.
Graham Binns (gmb) wrote : | # |
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) DoSomethingToAS
Is maddeningly. However, breaking it up:
func (myStruct *MyStruct) DoSomethingToAS
someOtherPa
(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-
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 {
SomeFunctio
"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 {
SomeFunctio
"Here's a test of how to break up strings in Go. Note that we " +
"are using the concatenation operator '+' to combine the " +
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.
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.
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.
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
1 | === modified file 'example/usso_example.go' | |||
2 | --- example/usso_example.go 2013-01-21 16:57:31 +0000 | |||
3 | +++ example/usso_example.go 2013-01-25 11:39:20 +0000 | |||
4 | @@ -1,23 +1,27 @@ | |||
5 | 1 | package main | 1 | package main |
6 | 2 | 2 | ||
7 | 3 | import ( | 3 | import ( |
8 | 4 | "bytes" | ||
9 | 4 | "encoding/json" | 5 | "encoding/json" |
10 | 5 | "fmt" | 6 | "fmt" |
11 | 7 | "io/ioutil" | ||
12 | 6 | "launchpad.net/usso" | 8 | "launchpad.net/usso" |
13 | 9 | "net/http" | ||
14 | 7 | ) | 10 | ) |
15 | 8 | 11 | ||
19 | 9 | var email string | 12 | var email, password, tokenName, signature_method string |
17 | 10 | var password string | ||
18 | 11 | var tokenName string | ||
20 | 12 | 13 | ||
21 | 13 | func inputParams() { | 14 | func inputParams() { |
23 | 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" + |
24 | 16 | " to fetch authorisation tokens.") | ||
25 | 15 | fmt.Print("Enter email: ") | 17 | fmt.Print("Enter email: ") |
26 | 16 | fmt.Scanf("%s", &email) | 18 | fmt.Scanf("%s", &email) |
27 | 17 | fmt.Print("Enter password: ") | 19 | fmt.Print("Enter password: ") |
28 | 18 | fmt.Scanf("%s", &password) | 20 | fmt.Scanf("%s", &password) |
29 | 19 | fmt.Print("Enter token name: ") | 21 | fmt.Print("Enter token name: ") |
30 | 20 | fmt.Scanf("%s", &tokenName) | 22 | fmt.Scanf("%s", &tokenName) |
31 | 23 | fmt.Print("Enter signature method (PLAINTEXT or HMAC-SHA1): ") | ||
32 | 24 | fmt.Scanf("%s", &signature_method) | ||
33 | 21 | } | 25 | } |
34 | 22 | 26 | ||
35 | 23 | func main() { | 27 | func main() { |
36 | @@ -26,16 +30,41 @@ | |||
37 | 26 | // Fetch the tokens using usso.GetToken. | 30 | // Fetch the tokens using usso.GetToken. |
38 | 27 | fmt.Println("Fetching tokens from staging server...") | 31 | fmt.Println("Fetching tokens from staging server...") |
39 | 28 | server := usso.StagingUbuntuSSOServer | 32 | server := usso.StagingUbuntuSSOServer |
42 | 29 | // One would use server := usso.ProductionUbuntuSSOServer to use the production Ubuntu SSO Server. | 33 | // One would use server := usso.ProductionUbuntuSSOServer |
43 | 30 | token, err := server.GetToken(email, password, tokenName) | 34 | // to use the production Ubuntu SSO Server. |
44 | 35 | ssodata, err := server.GetToken(email, password, tokenName) | ||
45 | 31 | if err != nil { | 36 | if err != nil { |
46 | 32 | panic(err) | 37 | panic(err) |
47 | 33 | } | 38 | } |
48 | 34 | // Format the result as json for displaying it: | 39 | // Format the result as json for displaying it: |
50 | 35 | json_token, err := json.Marshal(token) | 40 | json_token, err := json.Marshal(ssodata) |
51 | 36 | if err != nil { | 41 | if err != nil { |
52 | 37 | panic(err) | 42 | panic(err) |
53 | 38 | } | 43 | } |
54 | 39 | fmt.Printf("Got tokens: %s\n", json_token) | 44 | fmt.Printf("Got tokens: %s\n", json_token) |
55 | 40 | 45 | ||
56 | 46 | ssodata.BaseURL = fmt.Sprintf( | ||
57 | 47 | "https://login.staging.ubuntu.com/api/v2/accounts/%s", | ||
58 | 48 | ssodata.ConsumerKey) | ||
59 | 49 | ssodata.HTTPMethod = "GET" | ||
60 | 50 | ssodata.SignatureMethod = signature_method | ||
61 | 51 | request, _ := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil) | ||
62 | 52 | ssodata.Sign(request) | ||
63 | 53 | |||
64 | 54 | if err != nil { | ||
65 | 55 | fmt.Printf("Error: %s\n", err) | ||
66 | 56 | } | ||
67 | 57 | // run the request | ||
68 | 58 | client := &http.Client{} | ||
69 | 59 | response, err := client.Do(request) | ||
70 | 60 | if err != nil { | ||
71 | 61 | fmt.Printf("Error: %s\n", err) | ||
72 | 62 | } | ||
73 | 63 | body, err := ioutil.ReadAll(response.Body) | ||
74 | 64 | if err != nil { | ||
75 | 65 | fmt.Println(err) | ||
76 | 66 | } | ||
77 | 67 | var b bytes.Buffer | ||
78 | 68 | b.Write(body) | ||
79 | 69 | fmt.Printf("response: %+v\n", b.String()) | ||
80 | 41 | } | 70 | } |
81 | 42 | 71 | ||
82 | === added file 'url.go' | |||
83 | --- url.go 1970-01-01 00:00:00 +0000 | |||
84 | +++ url.go 2013-01-25 11:39:20 +0000 | |||
85 | @@ -0,0 +1,41 @@ | |||
86 | 1 | package usso | ||
87 | 2 | |||
88 | 3 | import ( | ||
89 | 4 | "fmt" | ||
90 | 5 | "net/url" | ||
91 | 6 | "strings" | ||
92 | 7 | ) | ||
93 | 8 | |||
94 | 9 | func normalizeHost(scheme, host_spec string) string { | ||
95 | 10 | standard_ports := map[string]string{ | ||
96 | 11 | "http": "80", | ||
97 | 12 | "https": "443", | ||
98 | 13 | } | ||
99 | 14 | host_parts := strings.Split(host_spec, ":") | ||
100 | 15 | if len(host_parts) == 2 && host_parts[1] == standard_ports[scheme] { | ||
101 | 16 | // There's a port, but it's the default one. Leave it out. | ||
102 | 17 | return host_parts[0] | ||
103 | 18 | } | ||
104 | 19 | return host_spec | ||
105 | 20 | } | ||
106 | 21 | |||
107 | 22 | func NormalizeURL(input_url string) (string, error) { | ||
108 | 23 | parsed_url, err := url.Parse(input_url) | ||
109 | 24 | if err != nil { | ||
110 | 25 | return "", err | ||
111 | 26 | } | ||
112 | 27 | |||
113 | 28 | host := normalizeHost(parsed_url.Scheme, parsed_url.Host) | ||
114 | 29 | normalized_url := fmt.Sprintf("%v://%v%v", parsed_url.Scheme, host, parsed_url.Path) | ||
115 | 30 | return normalized_url, nil | ||
116 | 31 | } | ||
117 | 32 | |||
118 | 33 | func NormalizeParameters(parameters url.Values) (string, error) { | ||
119 | 34 | filtered_map := make(url.Values, len(parameters)) | ||
120 | 35 | for param, value := range parameters { | ||
121 | 36 | if param != "oauth_signature" { | ||
122 | 37 | filtered_map[param] = value | ||
123 | 38 | } | ||
124 | 39 | } | ||
125 | 40 | return filtered_map.Encode(), nil | ||
126 | 41 | } | ||
127 | 0 | 42 | ||
128 | === added file 'url_test.go' | |||
129 | --- url_test.go 1970-01-01 00:00:00 +0000 | |||
130 | +++ url_test.go 2013-01-25 11:39:20 +0000 | |||
131 | @@ -0,0 +1,84 @@ | |||
132 | 1 | package usso | ||
133 | 2 | |||
134 | 3 | import ( | ||
135 | 4 | "launchpad.net/gocheck" | ||
136 | 5 | "net/url" | ||
137 | 6 | ) | ||
138 | 7 | |||
139 | 8 | func (suite *USSOTestSuite) TestNormalizeURLReturnsBasicURL(c *gocheck.C) { | ||
140 | 9 | output, err := NormalizeURL("http://example.com/path") | ||
141 | 10 | c.Check(err, gocheck.Equals, nil) | ||
142 | 11 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
143 | 12 | } | ||
144 | 13 | |||
145 | 14 | func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPPort( | ||
146 | 15 | c *gocheck.C) { | ||
147 | 16 | output, err := NormalizeURL("http://example.com:80/path") | ||
148 | 17 | c.Check(err, gocheck.Equals, nil) | ||
149 | 18 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
150 | 19 | } | ||
151 | 20 | |||
152 | 21 | func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPSPort( | ||
153 | 22 | c *gocheck.C) { | ||
154 | 23 | output, err := NormalizeURL("https://example.com:443/path") | ||
155 | 24 | c.Check(err, gocheck.Equals, nil) | ||
156 | 25 | c.Check(output, gocheck.Equals, "https://example.com/path") | ||
157 | 26 | } | ||
158 | 27 | |||
159 | 28 | func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort( | ||
160 | 29 | c *gocheck.C) { | ||
161 | 30 | output, err := NormalizeURL("http://example.com:8080/") | ||
162 | 31 | c.Check(err, gocheck.Equals, nil) | ||
163 | 32 | c.Check(output, gocheck.Equals, "http://example.com:8080/") | ||
164 | 33 | } | ||
165 | 34 | |||
166 | 35 | func (suite *USSOTestSuite) TestNormalizeURLStripsParameters(c *gocheck.C) { | ||
167 | 36 | output, err := NormalizeURL("http://example.com/path?query=value¶m=arg") | ||
168 | 37 | c.Check(err, gocheck.Equals, nil) | ||
169 | 38 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
170 | 39 | } | ||
171 | 40 | |||
172 | 41 | func (suite *USSOTestSuite) TestNormalizeParametersReturnsParameters( | ||
173 | 42 | c *gocheck.C) { | ||
174 | 43 | output, err := NormalizeParameters(url.Values{"param": []string{"value"}}) | ||
175 | 44 | c.Check(err, gocheck.Equals, nil) | ||
176 | 45 | c.Check(output, gocheck.Equals, "param=value") | ||
177 | 46 | } | ||
178 | 47 | |||
179 | 48 | func (suite *USSOTestSuite) TestNormalizeParametersConcatenatesParameters( | ||
180 | 49 | c *gocheck.C) { | ||
181 | 50 | output, err := NormalizeParameters( | ||
182 | 51 | url.Values{"a": []string{"1"}, "b": []string{"2"}}) | ||
183 | 52 | c.Check(err, gocheck.Equals, nil) | ||
184 | 53 | c.Check(output, gocheck.Matches, "(a=1&b=2|b=2&a=1)") | ||
185 | 54 | } | ||
186 | 55 | |||
187 | 56 | func (suite *USSOTestSuite) TestNormalizeParametersSortsParameters( | ||
188 | 57 | c *gocheck.C) { | ||
189 | 58 | params := url.Values{ | ||
190 | 59 | "b": []string{"x"}, | ||
191 | 60 | "a": []string{"y"}, | ||
192 | 61 | } | ||
193 | 62 | output, err := NormalizeParameters(params) | ||
194 | 63 | c.Check(err, gocheck.Equals, nil) | ||
195 | 64 | c.Check(output, gocheck.Matches, "(a=y&b=x|b=x&a=y)") | ||
196 | 65 | } | ||
197 | 66 | |||
198 | 67 | func (suite *USSOTestSuite) TestNormalizeParametersEscapesParameters( | ||
199 | 68 | c *gocheck.C) { | ||
200 | 69 | output, err := NormalizeParameters(url.Values{"a&b": []string{"1"}}) | ||
201 | 70 | c.Check(err, gocheck.Equals, nil) | ||
202 | 71 | c.Check(output, gocheck.Equals, "a%26b=1") | ||
203 | 72 | } | ||
204 | 73 | |||
205 | 74 | func (suite *USSOTestSuite) TestNormalizeParametersOmitsOAuthSignature( | ||
206 | 75 | c *gocheck.C) { | ||
207 | 76 | params := url.Values{ | ||
208 | 77 | "a": []string{"1"}, | ||
209 | 78 | "oauth_signature": []string{"foobarsplatszot"}, | ||
210 | 79 | "z": []string{"26"}, | ||
211 | 80 | } | ||
212 | 81 | output, err := NormalizeParameters(params) | ||
213 | 82 | c.Check(err, gocheck.Equals, nil) | ||
214 | 83 | c.Check(output, gocheck.Matches, "(a=1&z=26|z=26&a=1)") | ||
215 | 84 | } | ||
216 | 0 | 85 | ||
217 | === modified file 'usso.go' | |||
218 | --- usso.go 2013-01-22 10:39:12 +0000 | |||
219 | +++ usso.go 2013-01-25 11:39:20 +0000 | |||
220 | @@ -1,10 +1,11 @@ | |||
221 | 1 | // Copyright 2013 Canonical Ltd. This software is licensed under the | ||
222 | 2 | // GNU Affero General Public License version 3 (see the file LICENSE). | ||
223 | 3 | |||
224 | 4 | package usso | 1 | package usso |
225 | 5 | 2 | ||
226 | 6 | import ( | 3 | import ( |
227 | 4 | "crypto/hmac" | ||
228 | 5 | "crypto/sha1" | ||
229 | 6 | "encoding/base64" | ||
230 | 7 | "encoding/json" | 7 | "encoding/json" |
231 | 8 | "fmt" | ||
232 | 8 | "io/ioutil" | 9 | "io/ioutil" |
233 | 9 | "log" | 10 | "log" |
234 | 10 | "math/rand" | 11 | "math/rand" |
235 | @@ -20,6 +21,16 @@ | |||
236 | 20 | rand.Seed(time.Now().UTC().UnixNano()) | 21 | rand.Seed(time.Now().UTC().UnixNano()) |
237 | 21 | } | 22 | } |
238 | 22 | 23 | ||
239 | 24 | func timestamp() string { | ||
240 | 25 | // Create a timestamp used in authorization header. | ||
241 | 26 | return strconv.Itoa(int(time.Now().Unix())) | ||
242 | 27 | } | ||
243 | 28 | |||
244 | 29 | func nonce() string { | ||
245 | 30 | // Create a nonce used in authorization header. | ||
246 | 31 | return strconv.Itoa(rand.Intn(100000000)) | ||
247 | 32 | } | ||
248 | 33 | |||
249 | 23 | type UbuntuSSOServer struct { | 34 | type UbuntuSSOServer struct { |
250 | 24 | baseUrl string | 35 | baseUrl string |
251 | 25 | } | 36 | } |
252 | @@ -38,12 +49,18 @@ | |||
253 | 38 | var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"} | 49 | var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"} |
254 | 39 | 50 | ||
255 | 40 | type SSOData struct { | 51 | type SSOData struct { |
262 | 41 | BaseURL string | 52 | // Contains the oauth data to perform a request. |
263 | 42 | ConsumerKey string `json:"consumer_key"` | 53 | HTTPMethod string `json:"-"` |
264 | 43 | ConsumerSecret string `json:"consumer_secret"` | 54 | BaseURL string `json:"-"` |
265 | 44 | TokenKey string `json:"token_key"` | 55 | Params url.Values `json:"-"` |
266 | 45 | TokenName string `json:"token_name"` | 56 | Nonce string `json:"-"` |
267 | 46 | TokenSecret string `json:"token_secret"` | 57 | Timestamp string `json:"-"` |
268 | 58 | SignatureMethod string `json:"-"` | ||
269 | 59 | ConsumerKey string `json:"consumer_key"` | ||
270 | 60 | ConsumerSecret string `json:"consumer_secret"` | ||
271 | 61 | TokenKey string `json:"token_key"` | ||
272 | 62 | TokenName string `json:"token_name"` | ||
273 | 63 | TokenSecret string `json:"token_secret"` | ||
274 | 47 | } | 64 | } |
275 | 48 | 65 | ||
276 | 49 | func (server UbuntuSSOServer) GetToken(email string, password string, tokenName string) (*SSOData, error) { | 66 | func (server UbuntuSSOServer) GetToken(email string, password string, tokenName string) (*SSOData, error) { |
277 | @@ -78,17 +95,74 @@ | |||
278 | 78 | return &ssodata, nil | 95 | return &ssodata, nil |
279 | 79 | } | 96 | } |
280 | 80 | 97 | ||
281 | 98 | func (oauth *SSOData) GetAuthorizationHeader() string { | ||
282 | 99 | // Sign the provided request. | ||
283 | 100 | if oauth.Nonce == "" { | ||
284 | 101 | oauth.Nonce = nonce() | ||
285 | 102 | } | ||
286 | 103 | if oauth.Timestamp == "" { | ||
287 | 104 | oauth.Timestamp = timestamp() | ||
288 | 105 | } | ||
289 | 106 | signature := oauth.signature() | ||
290 | 107 | |||
291 | 108 | auth := fmt.Sprintf( | ||
292 | 109 | `OAuth realm="API", `+ | ||
293 | 110 | `oauth_consumer_key="%s", `+ | ||
294 | 111 | `oauth_token="%s", `+ | ||
295 | 112 | `oauth_signature_method="%s", `+ | ||
296 | 113 | `oauth_signature="%s", `+ | ||
297 | 114 | `oauth_timestamp="%s", `+ | ||
298 | 115 | `oauth_nonce="%s", `+ | ||
299 | 116 | `oauth_version="1.0"`, | ||
300 | 117 | url.QueryEscape(oauth.ConsumerKey), | ||
301 | 118 | url.QueryEscape(oauth.TokenKey), | ||
302 | 119 | oauth.SignatureMethod, | ||
303 | 120 | signature, | ||
304 | 121 | url.QueryEscape(oauth.Timestamp), | ||
305 | 122 | url.QueryEscape(oauth.Nonce)) | ||
306 | 123 | |||
307 | 124 | return auth | ||
308 | 125 | } | ||
309 | 126 | |||
310 | 81 | func (oauth *SSOData) Sign(req *http.Request) error { | 127 | func (oauth *SSOData) Sign(req *http.Request) error { |
311 | 82 | // Sign the provided request. | 128 | // Sign the provided request. |
321 | 83 | auth := `OAuth realm="API", ` + | 129 | auth := oauth.GetAuthorizationHeader() |
313 | 84 | `oauth_consumer_key="` + url.QueryEscape(oauth.ConsumerKey) + `", ` + | ||
314 | 85 | `oauth_token="` + url.QueryEscape(oauth.TokenKey) + `", ` + | ||
315 | 86 | `oauth_signature_method="PLAINTEXT", ` + | ||
316 | 87 | `oauth_signature="` + url.QueryEscape( | ||
317 | 88 | oauth.ConsumerSecret+`&`+oauth.TokenSecret) + `", ` + | ||
318 | 89 | `oauth_timestamp="` + strconv.FormatInt(time.Now().Unix(), 10) + `", ` + | ||
319 | 90 | `oauth_nonce="` + strconv.Itoa(int(rand.Intn(99999999))) + `", ` + | ||
320 | 91 | `oauth_version="1.0"` | ||
322 | 92 | req.Header.Add("Authorization", auth) | 130 | req.Header.Add("Authorization", auth) |
323 | 93 | return nil | 131 | return nil |
324 | 94 | } | 132 | } |
325 | 133 | |||
326 | 134 | func (oauth *SSOData) signature() string { | ||
327 | 135 | // Depending on the signature method, create the signature from the | ||
328 | 136 | // consumer secret, the token secret and, if required, the URL. | ||
329 | 137 | // Supported signature methods are PLAINTEXT and HMAC-SHA1. | ||
330 | 138 | |||
331 | 139 | switch oauth.SignatureMethod { | ||
332 | 140 | case "PLAINTEXT": | ||
333 | 141 | return fmt.Sprintf( | ||
334 | 142 | `%s%%26%s`, | ||
335 | 143 | oauth.ConsumerSecret, | ||
336 | 144 | oauth.TokenSecret) | ||
337 | 145 | case "HMAC-SHA1": | ||
338 | 146 | base_url, _ := NormalizeURL(oauth.BaseURL) | ||
339 | 147 | params, _ := NormalizeParameters(oauth.Params) | ||
340 | 148 | base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`, | ||
341 | 149 | oauth.HTTPMethod, | ||
342 | 150 | url.QueryEscape(base_url), | ||
343 | 151 | url.QueryEscape(params), | ||
344 | 152 | url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey), | ||
345 | 153 | url.QueryEscape("&oauth_nonce="+oauth.Nonce), | ||
346 | 154 | url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod), | ||
347 | 155 | url.QueryEscape("&oauth_timestamp="+oauth.Timestamp), | ||
348 | 156 | url.QueryEscape("&oauth_token="+oauth.TokenKey), | ||
349 | 157 | url.QueryEscape("&oauth_version=1.0")) | ||
350 | 158 | hashfun := hmac.New(sha1.New, []byte( | ||
351 | 159 | oauth.ConsumerSecret+"&"+oauth.TokenSecret)) | ||
352 | 160 | hashfun.Write([]byte(base_string)) | ||
353 | 161 | rawsignature := hashfun.Sum(nil) | ||
354 | 162 | base64signature := make( | ||
355 | 163 | []byte, base64.StdEncoding.EncodedLen(len(rawsignature))) | ||
356 | 164 | base64.StdEncoding.Encode(base64signature, rawsignature) | ||
357 | 165 | return string(base64signature) | ||
358 | 166 | } | ||
359 | 167 | return "" | ||
360 | 168 | } | ||
361 | 95 | 169 | ||
362 | === modified file 'usso_test.go' | |||
363 | --- usso_test.go 2013-01-22 10:39:12 +0000 | |||
364 | +++ usso_test.go 2013-01-25 11:39:20 +0000 | |||
365 | @@ -1,6 +1,3 @@ | |||
366 | 1 | // Copyright 2013 Canonical Ltd. This software is licensed under the | ||
367 | 2 | // GNU Affero General Public License version 3 (see the file LICENSE). | ||
368 | 3 | |||
369 | 4 | package usso | 1 | package usso |
370 | 5 | 2 | ||
371 | 6 | import ( | 3 | import ( |
372 | @@ -49,7 +46,8 @@ | |||
373 | 49 | 46 | ||
374 | 50 | // newSingleServingServer create a single-serving test http server which will | 47 | // newSingleServingServer create a single-serving test http server which will |
375 | 51 | // return only one response as defined by the passed arguments. | 48 | // return only one response as defined by the passed arguments. |
377 | 52 | func newSingleServingServer(uri string, response string, code int) *SingleServingServer { | 49 | func newSingleServingServer( |
378 | 50 | uri string, response string, code int) *SingleServingServer { | ||
379 | 53 | var requestContent string | 51 | var requestContent string |
380 | 54 | var requested bool | 52 | var requested bool |
381 | 55 | handler := func(w http.ResponseWriter, r *http.Request) { | 53 | handler := func(w http.ResponseWriter, r *http.Request) { |
382 | @@ -89,16 +87,19 @@ | |||
383 | 89 | if err != nil { | 87 | if err != nil { |
384 | 90 | panic(err) | 88 | panic(err) |
385 | 91 | } | 89 | } |
387 | 92 | server := newSingleServingServer("/api/v2/tokens", string(jsonServerResponseData), 200) | 90 | server := newSingleServingServer("/api/v2/tokens", |
388 | 91 | string(jsonServerResponseData), 200) | ||
389 | 93 | var testSSOServer = &UbuntuSSOServer{server.URL} | 92 | var testSSOServer = &UbuntuSSOServer{server.URL} |
390 | 94 | defer server.Close() | 93 | defer server.Close() |
391 | 95 | 94 | ||
392 | 96 | // The returned information is correct. | 95 | // The returned information is correct. |
393 | 97 | ssodata, err := testSSOServer.GetToken(email, password, tokenName) | 96 | ssodata, err := testSSOServer.GetToken(email, password, tokenName) |
394 | 98 | c.Assert(err, IsNil) | 97 | c.Assert(err, IsNil) |
396 | 99 | expectedSSOData := &SSOData{ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenSecret: tokenSecret, TokenName: tokenName} | 98 | expectedSSOData := &SSOData{ConsumerKey: consumerKey, |
397 | 99 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, | ||
398 | 100 | TokenSecret: tokenSecret, TokenName: tokenName} | ||
399 | 100 | c.Assert(ssodata, DeepEquals, expectedSSOData) | 101 | c.Assert(ssodata, DeepEquals, expectedSSOData) |
401 | 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. |
402 | 102 | credentials := map[string]string{ | 103 | credentials := map[string]string{ |
403 | 103 | "email": email, | 104 | "email": email, |
404 | 104 | "password": password, | 105 | "password": password, |
405 | @@ -113,15 +114,45 @@ | |||
406 | 113 | 114 | ||
407 | 114 | func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) { | 115 | func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) { |
408 | 115 | baseUrl := "https://localhost" | 116 | baseUrl := "https://localhost" |
420 | 116 | ssoData := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenName: tokenName, TokenSecret: tokenSecret} | 117 | ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, |
421 | 117 | request, _ := http.NewRequest("GET", baseUrl, nil) | 118 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, |
422 | 118 | 119 | TokenName: tokenName, TokenSecret: tokenSecret} | |
423 | 119 | err := ssoData.Sign(request) | 120 | request, _ := http.NewRequest("GET", baseUrl, nil) |
424 | 120 | 121 | ssodata.HTTPMethod = "GET" | |
425 | 121 | c.Assert(err, IsNil) | 122 | ssodata.SignatureMethod = "PLAINTEXT" |
426 | 122 | authHeader := request.Header["Authorization"][0] | 123 | err := ssodata.Sign(request) |
427 | 123 | c.Assert(authHeader, Matches, `.*OAuth realm="API".*`) | 124 | c.Assert(err, IsNil) |
428 | 124 | c.Assert(authHeader, Matches, `.*oauth_consumer_key="`+url.QueryEscape(ssoData.ConsumerKey)+`".*`) | 125 | authHeader := request.Header["Authorization"][0] |
429 | 125 | c.Assert(authHeader, Matches, `.*oauth_token="`+url.QueryEscape(ssoData.TokenKey)+`".*`) | 126 | c.Assert(authHeader, Matches, `^OAuth.*`) |
430 | 126 | c.Assert(authHeader, Matches, `.*oauth_signature="`+url.QueryEscape(ssoData.ConsumerSecret+`&`+ssoData.TokenSecret)+`.*`) | 127 | c.Assert(authHeader, Matches, `.*realm="API".*`) |
431 | 128 | c.Assert(authHeader, Matches, | ||
432 | 129 | `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`) | ||
433 | 130 | c.Assert(authHeader, Matches, | ||
434 | 131 | `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`) | ||
435 | 132 | c.Assert(authHeader, Matches, | ||
436 | 133 | `.*oauth_signature="`+url.QueryEscape( | ||
437 | 134 | ssodata.ConsumerSecret+`&`+ssodata.TokenSecret)+`.*`) | ||
438 | 135 | } | ||
439 | 136 | |||
440 | 137 | func (suite *USSOTestSuite) TestSignRequestSHA1(c *C) { | ||
441 | 138 | // Test the request signing with oauth_signature_method = SHA1 | ||
442 | 139 | baseUrl := "https://localhost" | ||
443 | 140 | ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, | ||
444 | 141 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, | ||
445 | 142 | TokenName: tokenName, TokenSecret: tokenSecret, | ||
446 | 143 | Nonce: "10888885", Timestamp: "1358853126"} | ||
447 | 144 | request, _ := http.NewRequest("GET", baseUrl, nil) | ||
448 | 145 | ssodata.HTTPMethod = "GET" | ||
449 | 146 | ssodata.SignatureMethod = "HMAC-SHA1" | ||
450 | 147 | err := ssodata.Sign(request) | ||
451 | 148 | c.Assert(err, IsNil) | ||
452 | 149 | authHeader := request.Header["Authorization"][0] | ||
453 | 150 | c.Assert(authHeader, Matches, `^OAuth.*`) | ||
454 | 151 | c.Assert(authHeader, Matches, `.*realm="API".*`) | ||
455 | 152 | c.Assert(authHeader, Matches, | ||
456 | 153 | `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`) | ||
457 | 154 | c.Assert(authHeader, Matches, | ||
458 | 155 | `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`) | ||
459 | 156 | c.Assert(authHeader, Matches, | ||
460 | 157 | `.*oauth_signature="`+"amJnYeek4G9ObTgTiE2y6cwTyPg="+`.*`) | ||
461 | 127 | } | 158 | } |
Ok