Merge lp:~vds/usso/better_interface into lp:usso
- better_interface
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 20 |
Proposed branch: | lp:~vds/usso/better_interface |
Merge into: | lp:usso |
Prerequisite: | lp:~vds/usso/change_license |
Diff against target: |
588 lines (+408/-52) 7 files modified
example/usso_example.go (+41/-7) oauth.go (+126/-0) oauth_test.go (+52/-0) url.go (+45/-0) url_test.go (+88/-0) usso.go (+48/-25) usso_test.go (+8/-20) |
To merge this branch: | bzr merge lp:~vds/usso/better_interface |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code | Approve | |
Review via email: mp+145174@code.launchpad.net |
This proposal supersedes a proposal from 2013-01-25.
Commit message
Description of the change
Refactoring the interface and handling errors a little better. There are no unit tests for the main interface, but the example shows that they work as expected. Tests will be added in a following branch, if we decide to spend more time on this project.
Vincenzo Di Somma (vds) wrote : | # |
> [1]
Right, I'll fix that.
> [2]
> [3]
> [4]
All this comments are about example/
> [5]
> [6]
Will do.
Graham Binns (gmb) wrote : | # |
> > [1]
>
> Right, I'll fix that.
>
> > [2]
> > [3]
> > [4]
>
> All this comments are about example/
> how to use the library, that's why there's some commented code and the error
> checking is not very accurate, I can still fix them if you like.
Ah, I see. Sorry :). That's fine then :).
- 18. By Vincenzo Di Somma
-
Fixing comments and little code style.
- 19. By Vincenzo Di Somma
-
Commented tests.
Graham Binns (gmb) wrote : | # |
Hi Vincenzo,
Thanks for addressing my concerns. I still think the documentation on the tests needs tweaking. The documentation you've added does describe the expected behaviour for which we're testing, but it does it in a rather opaque way. I've pastebinned a diff of how I think the documentation should read: https:/
Generally, comments on tests should be written as something like:
// Foo() returns an instance of Bar when its returnBar parameter is set to true.
func (suite *FooTestSuite) TestFooReturnsB
...
}
Vincenzo Di Somma (vds) wrote : | # |
Ok, you are right, I'll fix it.
- 20. By Vincenzo Di Somma
-
Commented tests in a more reasonable way thanks to gmb.
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-29 14:29:23 +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,46 @@ | |||
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 | // This would be the easiest way to get the account data. | ||
57 | 47 | //accounts, _ := server.GetAccounts(ssodata) | ||
58 | 48 | //fmt.Printf("Got accounts info: %s\n", accounts) | ||
59 | 49 | |||
60 | 50 | // But this shows how to sign a generic request. | ||
61 | 51 | ssodata.BaseURL = fmt.Sprintf( | ||
62 | 52 | "https://login.staging.ubuntu.com/api/v2/accounts/%s", | ||
63 | 53 | ssodata.ConsumerKey) | ||
64 | 54 | ssodata.HTTPMethod = "GET" | ||
65 | 55 | ssodata.SignatureMethod = signature_method | ||
66 | 56 | request, _ := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil) | ||
67 | 57 | usso.SignRequest(ssodata, request) | ||
68 | 58 | |||
69 | 59 | if err != nil { | ||
70 | 60 | fmt.Printf("Error: %s\n", err) | ||
71 | 61 | } | ||
72 | 62 | // run the request | ||
73 | 63 | client := &http.Client{} | ||
74 | 64 | response, err := client.Do(request) | ||
75 | 65 | if err != nil { | ||
76 | 66 | fmt.Printf("Error: %s\n", err) | ||
77 | 67 | } | ||
78 | 68 | body, err := ioutil.ReadAll(response.Body) | ||
79 | 69 | if err != nil { | ||
80 | 70 | fmt.Println(err) | ||
81 | 71 | } | ||
82 | 72 | var b bytes.Buffer | ||
83 | 73 | b.Write(body) | ||
84 | 74 | fmt.Printf("response: %+v\n", b.String()) | ||
85 | 41 | } | 75 | } |
86 | 42 | 76 | ||
87 | === added file 'oauth.go' | |||
88 | --- oauth.go 1970-01-01 00:00:00 +0000 | |||
89 | +++ oauth.go 2013-01-29 14:29:23 +0000 | |||
90 | @@ -0,0 +1,126 @@ | |||
91 | 1 | package usso | ||
92 | 2 | |||
93 | 3 | import ( | ||
94 | 4 | "crypto/hmac" | ||
95 | 5 | "crypto/sha1" | ||
96 | 6 | "encoding/base64" | ||
97 | 7 | "errors" | ||
98 | 8 | "fmt" | ||
99 | 9 | "math/rand" | ||
100 | 10 | "net/http" | ||
101 | 11 | "net/url" | ||
102 | 12 | "strconv" | ||
103 | 13 | "time" | ||
104 | 14 | ) | ||
105 | 15 | |||
106 | 16 | // Initialize the random generator. | ||
107 | 17 | func init() { | ||
108 | 18 | rand.Seed(time.Now().UTC().UnixNano()) | ||
109 | 19 | } | ||
110 | 20 | |||
111 | 21 | // Create a timestamp used in authorization header. | ||
112 | 22 | func timestamp() string { | ||
113 | 23 | return strconv.Itoa(int(time.Now().Unix())) | ||
114 | 24 | } | ||
115 | 25 | |||
116 | 26 | // Create a nonce used in authorization header. | ||
117 | 27 | func nonce() string { | ||
118 | 28 | return strconv.Itoa(rand.Intn(100000000)) | ||
119 | 29 | } | ||
120 | 30 | |||
121 | 31 | // Contains the oauth data to perform a request. | ||
122 | 32 | type SSOData struct { | ||
123 | 33 | HTTPMethod string `json:"-"` | ||
124 | 34 | BaseURL string `json:"-"` | ||
125 | 35 | Params url.Values `json:"-"` | ||
126 | 36 | Nonce string `json:"-"` | ||
127 | 37 | Timestamp string `json:"-"` | ||
128 | 38 | SignatureMethod string `json:"-"` | ||
129 | 39 | ConsumerKey string `json:"consumer_key"` | ||
130 | 40 | ConsumerSecret string `json:"consumer_secret"` | ||
131 | 41 | TokenKey string `json:"token_key"` | ||
132 | 42 | TokenName string `json:"token_name"` | ||
133 | 43 | TokenSecret string `json:"token_secret"` | ||
134 | 44 | } | ||
135 | 45 | |||
136 | 46 | // Depending on the signature method, create the signature from the | ||
137 | 47 | // consumer secret, the token secret and, if required, the URL. | ||
138 | 48 | // Supported signature methods are PLAINTEXT and HMAC-SHA1. | ||
139 | 49 | func (oauth *SSOData) signature() (string, error) { | ||
140 | 50 | switch oauth.SignatureMethod { | ||
141 | 51 | case "PLAINTEXT": | ||
142 | 52 | return fmt.Sprintf( | ||
143 | 53 | `%s%%26%s`, | ||
144 | 54 | oauth.ConsumerSecret, | ||
145 | 55 | oauth.TokenSecret), nil | ||
146 | 56 | case "HMAC-SHA1": | ||
147 | 57 | base_url, err := NormalizeURL(oauth.BaseURL) | ||
148 | 58 | if err != nil { | ||
149 | 59 | return "", err | ||
150 | 60 | } | ||
151 | 61 | params, err := NormalizeParameters(oauth.Params) | ||
152 | 62 | if err != nil { | ||
153 | 63 | return "", err | ||
154 | 64 | } | ||
155 | 65 | base_string := fmt.Sprintf(`%s&%s&%s%s%s%s%s%s%s`, | ||
156 | 66 | oauth.HTTPMethod, | ||
157 | 67 | url.QueryEscape(base_url), | ||
158 | 68 | url.QueryEscape(params), | ||
159 | 69 | url.QueryEscape("oauth_consumer_key="+oauth.ConsumerKey), | ||
160 | 70 | url.QueryEscape("&oauth_nonce="+oauth.Nonce), | ||
161 | 71 | url.QueryEscape("&oauth_signature_method="+oauth.SignatureMethod), | ||
162 | 72 | url.QueryEscape("&oauth_timestamp="+oauth.Timestamp), | ||
163 | 73 | url.QueryEscape("&oauth_token="+oauth.TokenKey), | ||
164 | 74 | url.QueryEscape("&oauth_version=1.0")) | ||
165 | 75 | hashfun := hmac.New(sha1.New, []byte( | ||
166 | 76 | oauth.ConsumerSecret+"&"+oauth.TokenSecret)) | ||
167 | 77 | hashfun.Write([]byte(base_string)) | ||
168 | 78 | rawsignature := hashfun.Sum(nil) | ||
169 | 79 | base64signature := make( | ||
170 | 80 | []byte, base64.StdEncoding.EncodedLen(len(rawsignature))) | ||
171 | 81 | base64.StdEncoding.Encode(base64signature, rawsignature) | ||
172 | 82 | return string(base64signature), nil | ||
173 | 83 | default: | ||
174 | 84 | return "", errors.New( | ||
175 | 85 | "usso/oauth: Oauth Signature Method not supported.") | ||
176 | 86 | } | ||
177 | 87 | return "", nil | ||
178 | 88 | } | ||
179 | 89 | |||
180 | 90 | // Sign the provided request. | ||
181 | 91 | func (oauth *SSOData) GetAuthorizationHeader() (string, error) { | ||
182 | 92 | if oauth.Nonce == "" { | ||
183 | 93 | oauth.Nonce = nonce() | ||
184 | 94 | } | ||
185 | 95 | if oauth.Timestamp == "" { | ||
186 | 96 | oauth.Timestamp = timestamp() | ||
187 | 97 | } | ||
188 | 98 | signature, err := oauth.signature() | ||
189 | 99 | if err != nil { | ||
190 | 100 | return "", err | ||
191 | 101 | } | ||
192 | 102 | auth := fmt.Sprintf( | ||
193 | 103 | `OAuth realm="API", `+ | ||
194 | 104 | `oauth_consumer_key="%s", `+ | ||
195 | 105 | `oauth_token="%s", `+ | ||
196 | 106 | `oauth_signature_method="%s", `+ | ||
197 | 107 | `oauth_signature="%s", `+ | ||
198 | 108 | `oauth_timestamp="%s", `+ | ||
199 | 109 | `oauth_nonce="%s", `+ | ||
200 | 110 | `oauth_version="1.0"`, | ||
201 | 111 | url.QueryEscape(oauth.ConsumerKey), | ||
202 | 112 | url.QueryEscape(oauth.TokenKey), | ||
203 | 113 | oauth.SignatureMethod, | ||
204 | 114 | signature, | ||
205 | 115 | url.QueryEscape(oauth.Timestamp), | ||
206 | 116 | url.QueryEscape(oauth.Nonce)) | ||
207 | 117 | |||
208 | 118 | return auth, nil | ||
209 | 119 | } | ||
210 | 120 | |||
211 | 121 | // Sign the provided request. | ||
212 | 122 | func (oauth *SSOData) SignRequest(req *http.Request) error { | ||
213 | 123 | auth, error := oauth.GetAuthorizationHeader() | ||
214 | 124 | req.Header.Add("Authorization", auth) | ||
215 | 125 | return error | ||
216 | 126 | } | ||
217 | 0 | 127 | ||
218 | === added file 'oauth_test.go' | |||
219 | --- oauth_test.go 1970-01-01 00:00:00 +0000 | |||
220 | +++ oauth_test.go 2013-01-29 14:29:23 +0000 | |||
221 | @@ -0,0 +1,52 @@ | |||
222 | 1 | package usso | ||
223 | 2 | |||
224 | 3 | import ( | ||
225 | 4 | . "launchpad.net/gocheck" | ||
226 | 5 | "net/http" | ||
227 | 6 | "net/url" | ||
228 | 7 | ) | ||
229 | 8 | |||
230 | 9 | func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) { | ||
231 | 10 | baseUrl := "https://localhost" | ||
232 | 11 | ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, | ||
233 | 12 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, | ||
234 | 13 | TokenName: tokenName, TokenSecret: tokenSecret} | ||
235 | 14 | request, _ := http.NewRequest("GET", baseUrl, nil) | ||
236 | 15 | ssodata.HTTPMethod = "GET" | ||
237 | 16 | ssodata.SignatureMethod = "PLAINTEXT" | ||
238 | 17 | err := ssodata.SignRequest(request) | ||
239 | 18 | c.Assert(err, IsNil) | ||
240 | 19 | authHeader := request.Header["Authorization"][0] | ||
241 | 20 | c.Assert(authHeader, Matches, `^OAuth.*`) | ||
242 | 21 | c.Assert(authHeader, Matches, `.*realm="API".*`) | ||
243 | 22 | c.Assert(authHeader, Matches, | ||
244 | 23 | `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`) | ||
245 | 24 | c.Assert(authHeader, Matches, | ||
246 | 25 | `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`) | ||
247 | 26 | c.Assert(authHeader, Matches, | ||
248 | 27 | `.*oauth_signature="`+url.QueryEscape( | ||
249 | 28 | ssodata.ConsumerSecret+`&`+ssodata.TokenSecret)+`.*`) | ||
250 | 29 | } | ||
251 | 30 | |||
252 | 31 | // Test the request signing with oauth_signature_method = SHA1 | ||
253 | 32 | func (suite *USSOTestSuite) TestSignRequestSHA1(c *C) { | ||
254 | 33 | baseUrl := "https://localhost" | ||
255 | 34 | ssodata := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, | ||
256 | 35 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, | ||
257 | 36 | TokenName: tokenName, TokenSecret: tokenSecret, | ||
258 | 37 | Nonce: "10888885", Timestamp: "1358853126"} | ||
259 | 38 | request, _ := http.NewRequest("GET", baseUrl, nil) | ||
260 | 39 | ssodata.HTTPMethod = "GET" | ||
261 | 40 | ssodata.SignatureMethod = "HMAC-SHA1" | ||
262 | 41 | err := ssodata.SignRequest(request) | ||
263 | 42 | c.Assert(err, IsNil) | ||
264 | 43 | authHeader := request.Header["Authorization"][0] | ||
265 | 44 | c.Assert(authHeader, Matches, `^OAuth.*`) | ||
266 | 45 | c.Assert(authHeader, Matches, `.*realm="API".*`) | ||
267 | 46 | c.Assert(authHeader, Matches, | ||
268 | 47 | `.*oauth_consumer_key="`+url.QueryEscape(ssodata.ConsumerKey)+`".*`) | ||
269 | 48 | c.Assert(authHeader, Matches, | ||
270 | 49 | `.*oauth_token="`+url.QueryEscape(ssodata.TokenKey)+`".*`) | ||
271 | 50 | c.Assert(authHeader, Matches, | ||
272 | 51 | `.*oauth_signature="`+"amJnYeek4G9ObTgTiE2y6cwTyPg="+`.*`) | ||
273 | 52 | } | ||
274 | 0 | 53 | ||
275 | === added file 'url.go' | |||
276 | --- url.go 1970-01-01 00:00:00 +0000 | |||
277 | +++ url.go 2013-01-29 14:29:23 +0000 | |||
278 | @@ -0,0 +1,45 @@ | |||
279 | 1 | package usso | ||
280 | 2 | |||
281 | 3 | import ( | ||
282 | 4 | "fmt" | ||
283 | 5 | "net/url" | ||
284 | 6 | "strings" | ||
285 | 7 | ) | ||
286 | 8 | |||
287 | 9 | // Remove the standard ports from the URL. | ||
288 | 10 | func normalizeHost(scheme, host_spec string) string { | ||
289 | 11 | standard_ports := map[string]string{ | ||
290 | 12 | "http": "80", | ||
291 | 13 | "https": "443", | ||
292 | 14 | } | ||
293 | 15 | host_parts := strings.Split(host_spec, ":") | ||
294 | 16 | if len(host_parts) == 2 && host_parts[1] == standard_ports[scheme] { | ||
295 | 17 | // There's a port, but it's the default one. Leave it out. | ||
296 | 18 | return host_parts[0] | ||
297 | 19 | } | ||
298 | 20 | return host_spec | ||
299 | 21 | } | ||
300 | 22 | |||
301 | 23 | // Normalize the URL according to OAuth specs. | ||
302 | 24 | func NormalizeURL(input_url string) (string, error) { | ||
303 | 25 | parsed_url, err := url.Parse(input_url) | ||
304 | 26 | if err != nil { | ||
305 | 27 | return "", err | ||
306 | 28 | } | ||
307 | 29 | |||
308 | 30 | host := normalizeHost(parsed_url.Scheme, parsed_url.Host) | ||
309 | 31 | normalized_url := fmt.Sprintf( | ||
310 | 32 | "%v://%v%v", parsed_url.Scheme, host, parsed_url.Path) | ||
311 | 33 | return normalized_url, nil | ||
312 | 34 | } | ||
313 | 35 | |||
314 | 36 | // Normalize the parameters in the query string according to OAuth specs. | ||
315 | 37 | func NormalizeParameters(parameters url.Values) (string, error) { | ||
316 | 38 | filtered_map := make(url.Values, len(parameters)) | ||
317 | 39 | for param, value := range parameters { | ||
318 | 40 | if param != "oauth_signature" { | ||
319 | 41 | filtered_map[param] = value | ||
320 | 42 | } | ||
321 | 43 | } | ||
322 | 44 | return filtered_map.Encode(), nil | ||
323 | 45 | } | ||
324 | 0 | 46 | ||
325 | === added file 'url_test.go' | |||
326 | --- url_test.go 1970-01-01 00:00:00 +0000 | |||
327 | +++ url_test.go 2013-01-29 14:29:23 +0000 | |||
328 | @@ -0,0 +1,88 @@ | |||
329 | 1 | package usso | ||
330 | 2 | |||
331 | 3 | import ( | ||
332 | 4 | "launchpad.net/gocheck" | ||
333 | 5 | "net/url" | ||
334 | 6 | ) | ||
335 | 7 | |||
336 | 8 | // When NormalizeURL() is passed a simple URL, it will make no changes | ||
337 | 9 | // to it. | ||
338 | 10 | func (suite *USSOTestSuite) TestNormalizeURLReturnsBasicURL(c *gocheck.C) { | ||
339 | 11 | output, err := NormalizeURL("http://example.com/path") | ||
340 | 12 | c.Check(err, gocheck.Equals, nil) | ||
341 | 13 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
342 | 14 | } | ||
343 | 15 | |||
344 | 16 | // NormalizeURL() strips the ":80" from http:// URLs that contain it. | ||
345 | 17 | func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPPort( | ||
346 | 18 | c *gocheck.C) { | ||
347 | 19 | output, err := NormalizeURL("http://example.com:80/path") | ||
348 | 20 | c.Check(err, gocheck.Equals, nil) | ||
349 | 21 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
350 | 22 | } | ||
351 | 23 | |||
352 | 24 | // NormalizeURL() strips the ":443" from https:// URLs that contain it. | ||
353 | 25 | func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPSPort( | ||
354 | 26 | c *gocheck.C) { | ||
355 | 27 | output, err := NormalizeURL("https://example.com:443/path") | ||
356 | 28 | c.Check(err, gocheck.Equals, nil) | ||
357 | 29 | c.Check(output, gocheck.Equals, "https://example.com/path") | ||
358 | 30 | } | ||
359 | 31 | |||
360 | 32 | // NormalizeURL() does not remove non-standard ports from the URL. | ||
361 | 33 | func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort( | ||
362 | 34 | c *gocheck.C) { | ||
363 | 35 | output, err := NormalizeURL("http://example.com:8080/") | ||
364 | 36 | c.Check(err, gocheck.Equals, nil) | ||
365 | 37 | c.Check(output, gocheck.Equals, "http://example.com:8080/") | ||
366 | 38 | } | ||
367 | 39 | |||
368 | 40 | // NormalizeURL() strips the query string from URLs. | ||
369 | 41 | func (suite *USSOTestSuite) TestNormalizeURLStripsParameters(c *gocheck.C) { | ||
370 | 42 | output, err := NormalizeURL("http://example.com/path?query=value¶m=arg") | ||
371 | 43 | c.Check(err, gocheck.Equals, nil) | ||
372 | 44 | c.Check(output, gocheck.Equals, "http://example.com/path") | ||
373 | 45 | } | ||
374 | 46 | |||
375 | 47 | // NormalizeParameters() takes a url.Values instance and returns an | ||
376 | 48 | // encoded key=value string containing the parameters in that instance. | ||
377 | 49 | func (suite *USSOTestSuite) TestNormalizeParametersReturnsParameters( | ||
378 | 50 | c *gocheck.C) { | ||
379 | 51 | output, err := NormalizeParameters(url.Values{"param": []string{"value"}}) | ||
380 | 52 | c.Check(err, gocheck.Equals, nil) | ||
381 | 53 | c.Check(output, gocheck.Equals, "param=value") | ||
382 | 54 | } | ||
383 | 55 | |||
384 | 56 | // NormalizeParameters() encodes multiple key/value parameters as a | ||
385 | 57 | // query string. | ||
386 | 58 | func (suite *USSOTestSuite) TestNormalizeParametersConcatenatesParameters( | ||
387 | 59 | c *gocheck.C) { | ||
388 | 60 | output, err := NormalizeParameters( | ||
389 | 61 | url.Values{"a": []string{"1"}, "b": []string{"2"}}) | ||
390 | 62 | c.Check(err, gocheck.Equals, nil) | ||
391 | 63 | c.Check(output, gocheck.Matches, "(a=1&b=2|b=2&a=1)") | ||
392 | 64 | } | ||
393 | 65 | |||
394 | 66 | // NormalizeParameters() escapes the parameters correctly when encoding | ||
395 | 67 | // them as a query string. | ||
396 | 68 | func (suite *USSOTestSuite) TestNormalizeParametersEscapesParameters( | ||
397 | 69 | c *gocheck.C) { | ||
398 | 70 | output, err := NormalizeParameters(url.Values{"a&b": []string{"1"}}) | ||
399 | 71 | c.Check(err, gocheck.Equals, nil) | ||
400 | 72 | c.Check(output, gocheck.Equals, "a%26b=1") | ||
401 | 73 | } | ||
402 | 74 | |||
403 | 75 | // If oauth_signature appears in the parameters passed to | ||
404 | 76 | // NormalizeParameters(), it is omitted in the returned string as it does not | ||
405 | 77 | // have to be included in the computation of the new oauth_signature. | ||
406 | 78 | func (suite *USSOTestSuite) TestNormalizeParametersOmitsOAuthSignature( | ||
407 | 79 | c *gocheck.C) { | ||
408 | 80 | params := url.Values{ | ||
409 | 81 | "a": []string{"1"}, | ||
410 | 82 | "oauth_signature": []string{"foobarsplatszot"}, | ||
411 | 83 | "z": []string{"26"}, | ||
412 | 84 | } | ||
413 | 85 | output, err := NormalizeParameters(params) | ||
414 | 86 | c.Check(err, gocheck.Equals, nil) | ||
415 | 87 | c.Check(output, gocheck.Matches, "(a=1&z=26|z=26&a=1)") | ||
416 | 88 | } | ||
417 | 0 | 89 | ||
418 | === modified file 'usso.go' | |||
419 | --- usso.go 2013-01-25 11:38:14 +0000 | |||
420 | +++ usso.go 2013-01-29 14:29:23 +0000 | |||
421 | @@ -1,13 +1,13 @@ | |||
422 | 1 | package usso | 1 | package usso |
423 | 2 | 2 | ||
424 | 3 | import ( | 3 | import ( |
425 | 4 | "bytes" | ||
426 | 4 | "encoding/json" | 5 | "encoding/json" |
427 | 6 | "fmt" | ||
428 | 5 | "io/ioutil" | 7 | "io/ioutil" |
429 | 6 | "log" | 8 | "log" |
430 | 7 | "math/rand" | 9 | "math/rand" |
431 | 8 | "net/http" | 10 | "net/http" |
432 | 9 | "net/url" | ||
433 | 10 | "strconv" | ||
434 | 11 | "strings" | 11 | "strings" |
435 | 12 | "time" | 12 | "time" |
436 | 13 | ) | 13 | ) |
437 | @@ -26,6 +26,12 @@ | |||
438 | 26 | return server.baseUrl + "/api/v2/tokens" | 26 | return server.baseUrl + "/api/v2/tokens" |
439 | 27 | } | 27 | } |
440 | 28 | 28 | ||
441 | 29 | // AccountURL returns the URL where the Ubuntu SSO account information can be | ||
442 | 30 | // requested. | ||
443 | 31 | func (server UbuntuSSOServer) AccountsURL() string { | ||
444 | 32 | return server.baseUrl + "/api/v2/accounts/" | ||
445 | 33 | } | ||
446 | 34 | |||
447 | 29 | // ProductionUbuntuSSOServer represents the production Ubuntu SSO server | 35 | // ProductionUbuntuSSOServer represents the production Ubuntu SSO server |
448 | 30 | // located at https://login.ubuntu.com. | 36 | // located at https://login.ubuntu.com. |
449 | 31 | var ProductionUbuntuSSOServer = UbuntuSSOServer{"https://login.ubuntu.com"} | 37 | var ProductionUbuntuSSOServer = UbuntuSSOServer{"https://login.ubuntu.com"} |
450 | @@ -34,16 +40,10 @@ | |||
451 | 34 | // at https://login.staging.ubuntu.com. Use it for testing. | 40 | // at https://login.staging.ubuntu.com. Use it for testing. |
452 | 35 | var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"} | 41 | var StagingUbuntuSSOServer = UbuntuSSOServer{"https://login.staging.ubuntu.com"} |
453 | 36 | 42 | ||
464 | 37 | type SSOData struct { | 43 | // Giving user credentials and token name, retrieves oauth credentials |
465 | 38 | BaseURL string | 44 | // for the users, the oauth credentials can be used later to sign requests. |
466 | 39 | ConsumerKey string `json:"consumer_key"` | 45 | func (server UbuntuSSOServer) GetToken( |
467 | 40 | ConsumerSecret string `json:"consumer_secret"` | 46 | email string, password string, tokenName string) (*SSOData, error) { |
458 | 41 | TokenKey string `json:"token_key"` | ||
459 | 42 | TokenName string `json:"token_name"` | ||
460 | 43 | TokenSecret string `json:"token_secret"` | ||
461 | 44 | } | ||
462 | 45 | |||
463 | 46 | func (server UbuntuSSOServer) GetToken(email string, password string, tokenName string) (*SSOData, error) { | ||
468 | 47 | credentials := map[string]string{ | 47 | credentials := map[string]string{ |
469 | 48 | "email": email, | 48 | "email": email, |
470 | 49 | "password": password, | 49 | "password": password, |
471 | @@ -75,17 +75,40 @@ | |||
472 | 75 | return &ssodata, nil | 75 | return &ssodata, nil |
473 | 76 | } | 76 | } |
474 | 77 | 77 | ||
488 | 78 | func (oauth *SSOData) Sign(req *http.Request) error { | 78 | // Returns all the Ubuntu SSO information related to this account. |
489 | 79 | // Sign the provided request. | 79 | func (server UbuntuSSOServer) GetAccounts(ssodata *SSOData) (string, error) { |
490 | 80 | auth := `OAuth realm="API", ` + | 80 | ssodata.BaseURL = server.AccountsURL() + ssodata.ConsumerKey |
491 | 81 | `oauth_consumer_key="` + url.QueryEscape(oauth.ConsumerKey) + `", ` + | 81 | ssodata.HTTPMethod = "GET" |
492 | 82 | `oauth_token="` + url.QueryEscape(oauth.TokenKey) + `", ` + | 82 | ssodata.SignatureMethod = "HMAC-SHA1" |
493 | 83 | `oauth_signature_method="PLAINTEXT", ` + | 83 | request, err := http.NewRequest(ssodata.HTTPMethod, ssodata.BaseURL, nil) |
494 | 84 | `oauth_signature="` + url.QueryEscape( | 84 | if err != nil { |
495 | 85 | oauth.ConsumerSecret+`&`+oauth.TokenSecret) + `", ` + | 85 | return "", err |
496 | 86 | `oauth_timestamp="` + strconv.FormatInt(time.Now().Unix(), 10) + `", ` + | 86 | } |
497 | 87 | `oauth_nonce="` + strconv.Itoa(int(rand.Intn(99999999))) + `", ` + | 87 | err = SignRequest(ssodata, request) |
498 | 88 | `oauth_version="1.0"` | 88 | if err != nil { |
499 | 89 | req.Header.Add("Authorization", auth) | 89 | return "", err |
500 | 90 | return nil | 90 | } |
501 | 91 | client := &http.Client{} | ||
502 | 92 | response, err := client.Do(request) | ||
503 | 93 | if err != nil { | ||
504 | 94 | fmt.Printf("Error: %s\n", err) | ||
505 | 95 | } | ||
506 | 96 | body, err := ioutil.ReadAll(response.Body) | ||
507 | 97 | if err != nil { | ||
508 | 98 | fmt.Println(err) | ||
509 | 99 | } | ||
510 | 100 | var b bytes.Buffer | ||
511 | 101 | b.Write(body) | ||
512 | 102 | return fmt.Sprint(b.String()), nil | ||
513 | 103 | } | ||
514 | 104 | |||
515 | 105 | // Given oauth credentials and a request, return it signed. | ||
516 | 106 | func SignRequest(ssodata *SSOData, request *http.Request) error { | ||
517 | 107 | return ssodata.SignRequest(request) | ||
518 | 108 | } | ||
519 | 109 | |||
520 | 110 | // Given oauth credentials return a valid http authorization header. | ||
521 | 111 | func GetAuthorizationHeader(ssodata *SSOData) (string, error) { | ||
522 | 112 | header, err := ssodata.GetAuthorizationHeader() | ||
523 | 113 | return header, err | ||
524 | 91 | } | 114 | } |
525 | 92 | 115 | ||
526 | === modified file 'usso_test.go' | |||
527 | --- usso_test.go 2013-01-25 11:38:14 +0000 | |||
528 | +++ usso_test.go 2013-01-29 14:29:23 +0000 | |||
529 | @@ -7,7 +7,6 @@ | |||
530 | 7 | . "launchpad.net/gocheck" | 7 | . "launchpad.net/gocheck" |
531 | 8 | "net/http" | 8 | "net/http" |
532 | 9 | "net/http/httptest" | 9 | "net/http/httptest" |
533 | 10 | "net/url" | ||
534 | 11 | "testing" | 10 | "testing" |
535 | 12 | ) | 11 | ) |
536 | 13 | 12 | ||
537 | @@ -46,7 +45,8 @@ | |||
538 | 46 | 45 | ||
539 | 47 | // newSingleServingServer create a single-serving test http server which will | 46 | // newSingleServingServer create a single-serving test http server which will |
540 | 48 | // return only one response as defined by the passed arguments. | 47 | // return only one response as defined by the passed arguments. |
542 | 49 | func newSingleServingServer(uri string, response string, code int) *SingleServingServer { | 48 | func newSingleServingServer( |
543 | 49 | uri string, response string, code int) *SingleServingServer { | ||
544 | 50 | var requestContent string | 50 | var requestContent string |
545 | 51 | var requested bool | 51 | var requested bool |
546 | 52 | handler := func(w http.ResponseWriter, r *http.Request) { | 52 | handler := func(w http.ResponseWriter, r *http.Request) { |
547 | @@ -86,16 +86,19 @@ | |||
548 | 86 | if err != nil { | 86 | if err != nil { |
549 | 87 | panic(err) | 87 | panic(err) |
550 | 88 | } | 88 | } |
552 | 89 | server := newSingleServingServer("/api/v2/tokens", string(jsonServerResponseData), 200) | 89 | server := newSingleServingServer("/api/v2/tokens", |
553 | 90 | string(jsonServerResponseData), 200) | ||
554 | 90 | var testSSOServer = &UbuntuSSOServer{server.URL} | 91 | var testSSOServer = &UbuntuSSOServer{server.URL} |
555 | 91 | defer server.Close() | 92 | defer server.Close() |
556 | 92 | 93 | ||
557 | 93 | // The returned information is correct. | 94 | // The returned information is correct. |
558 | 94 | ssodata, err := testSSOServer.GetToken(email, password, tokenName) | 95 | ssodata, err := testSSOServer.GetToken(email, password, tokenName) |
559 | 95 | c.Assert(err, IsNil) | 96 | c.Assert(err, IsNil) |
561 | 96 | expectedSSOData := &SSOData{ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenSecret: tokenSecret, TokenName: tokenName} | 97 | expectedSSOData := &SSOData{ConsumerKey: consumerKey, |
562 | 98 | ConsumerSecret: consumerSecret, TokenKey: tokenKey, | ||
563 | 99 | TokenSecret: tokenSecret, TokenName: tokenName} | ||
564 | 97 | c.Assert(ssodata, DeepEquals, expectedSSOData) | 100 | c.Assert(ssodata, DeepEquals, expectedSSOData) |
566 | 98 | // The request that the fake Ubuntu SSO Server got contained the credentials. | 101 | //The request that the fake Ubuntu SSO Server got contained the credentials. |
567 | 99 | credentials := map[string]string{ | 102 | credentials := map[string]string{ |
568 | 100 | "email": email, | 103 | "email": email, |
569 | 101 | "password": password, | 104 | "password": password, |
570 | @@ -107,18 +110,3 @@ | |||
571 | 107 | } | 110 | } |
572 | 108 | c.Assert(*server.requestContent, Equals, string(expectedRequestContent)) | 111 | c.Assert(*server.requestContent, Equals, string(expectedRequestContent)) |
573 | 109 | } | 112 | } |
574 | 110 | |||
575 | 111 | func (suite *USSOTestSuite) TestSignRequestPlainText(c *C) { | ||
576 | 112 | baseUrl := "https://localhost" | ||
577 | 113 | ssoData := SSOData{BaseURL: baseUrl, ConsumerKey: consumerKey, ConsumerSecret: consumerSecret, TokenKey: tokenKey, TokenName: tokenName, TokenSecret: tokenSecret} | ||
578 | 114 | request, _ := http.NewRequest("GET", baseUrl, nil) | ||
579 | 115 | |||
580 | 116 | err := ssoData.Sign(request) | ||
581 | 117 | |||
582 | 118 | c.Assert(err, IsNil) | ||
583 | 119 | authHeader := request.Header["Authorization"][0] | ||
584 | 120 | c.Assert(authHeader, Matches, `.*OAuth realm="API".*`) | ||
585 | 121 | c.Assert(authHeader, Matches, `.*oauth_consumer_key="`+url.QueryEscape(ssoData.ConsumerKey)+`".*`) | ||
586 | 122 | c.Assert(authHeader, Matches, `.*oauth_token="`+url.QueryEscape(ssoData.TokenKey)+`".*`) | ||
587 | 123 | c.Assert(authHeader, Matches, `.*oauth_signature="`+url.QueryEscape(ssoData.ConsumerSecret+`&`+ssoData.TokenSecret)+`.*`) | ||
588 | 124 | } |
Hi Vincenzo,
Thanks for this branch. There are quite a few changes that need to be
made, but they're mostly cosmetic - don't be alarmed at the size of the
review :)
I've grouped things together under common headers (so [6] refers to all
the functions without documentation, for example). That makes it a bit
non-linear, but I think it's clearer that way.
I'm marking this needs-fixing because the review is so large - just to
make it disappear from the review queue, really.
[1]
19 +var email, password, tokenName, signature_method string
We should either be using camelCase or underscore_ separated names, not
both, I think. It's up to you which.
[2]
56 + // This would be the easiest way to get the account data. GetAccounts( ssodata)
57 + //accounts, _ := server.
58 + //fmt.Printf("Got accounts info: %s\n", accounts)
I'm not overly fond of leaving commented-out lines in code just for
documentation purposes. We should add this to a README rather than
leaving it here.
[3]
74 + response, err := client.Do(request) ReadAll( response. Body)
75 + if err != nil {
76 + fmt.Printf("Error: %s\n", err)
77 + }
78 + body, err := ioutil.
79 + if err != nil {
80 + fmt.Println(err)
81 + }
I may be reading this wrong (not being a proper Gopher yet and all) but
I _think_ that if client.Do(request) returns an error we should probably
exit early rather than try to do something with the response, right?
Unless the response body has something useful in it, which we can't
guarantee.
[4]
82 + var b bytes.Buffer "response: %+v\n", b.String())
83 + b.Write(body)
84 + fmt.Printf(
Is there any reason to print out the body here, other than to see what's
happened?
[5]
106 +func init() { time.Now( ).UTC() .UnixNano( ))
107 + // Initialize the random generator.
108 + rand.Seed(
109 +}
111 +func timestamp() string { Itoa(int( time.Now( ).Unix( )))
112 + // Create a timestamp used in authorization header.
113 + return strconv.
114 +}
116 +func nonce() string { Itoa(rand. Intn(100000000) )
117 + // Create a nonce used in authorization header.
118 + return strconv.
119 +}
136 +func (oauth *SSOData) signature() (string, error) {
137 + // Depending on the signature method, create the signature from the
138 + // consumer secret, the token secret and, if required, the URL.
139 + // Supported signature methods are PLAINTEXT and HMAC-SHA1.
457 +func (server UbuntuSSOServer) GetToken(
458 + // Giving user credentials and token name, retrieves oauth credentials
459 + // for the users, the oauth credentials can be used later to sign requests.
460 + email string, password string, tokenName string) (*SSOData, error) {
481 +func (server UbuntuSSOServer) GetAccounts(ssodata *SSOData) (string, error) {
508 +func SignRequest(ssodata *SSOData, request *http.Request) error {
509 + // Given oauth credentials and a request, return it signed.
513 +func GetAuthorizatio nHeader( ssodata *SSOData) (string, error) {
514 + // Given oauth credentials return a valid http authorization header.
These comments should go before the function declaration (I don't know
if godoc does The Right Thing with comments after the declaration, but
most other places do it before.)
[6]
181 +func (oauth *SSOData) GetAut...