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. 57 + //accounts, _ := server.GetAccounts(ssodata) 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) 75 + if err != nil { 76 + fmt.Printf("Error: %s\n", err) 77 + } 78 + body, err := ioutil.ReadAll(response.Body) 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 83 + b.Write(body) 84 + fmt.Printf("response: %+v\n", b.String()) Is there any reason to print out the body here, other than to see what's happened? [5] 106 +func init() { 107 + // Initialize the random generator. 108 + rand.Seed(time.Now().UTC().UnixNano()) 109 +} 111 +func timestamp() string { 112 + // Create a timestamp used in authorization header. 113 + return strconv.Itoa(int(time.Now().Unix())) 114 +} 116 +func nonce() string { 117 + // Create a nonce used in authorization header. 118 + return strconv.Itoa(rand.Intn(100000000)) 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 GetAuthorizationHeader(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) GetAuthorizationHeader() (string, error) { 288 +func normalizeHost(scheme, host_spec string) string { 301 +func NormalizeURL(input_url string) (string, error) { 312 +func NormalizeParameters(parameters url.Values) (string, error) { 333 +func (suite *USSOTestSuite) TestNormalizeURLReturnsBasicURL(c *gocheck.C) { 339 +func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPPort( 346 +func (suite *USSOTestSuite) TestNormalizeURLStripsStandardHTTPSPort( 353 +func (suite *USSOTestSuite) TestNormalizeURLLeavesNonstandardPort( 360 +func (suite *USSOTestSuite) TestNormalizeURLStripsParameters(c *gocheck.C) {j 366 +func (suite *USSOTestSuite) TestNormalizeParametersReturnsParameters( 373 +func (suite *USSOTestSuite) TestNormalizeParametersConcatenatesParameters( 381 +func (suite *USSOTestSuite) TestNormalizeParametersSortsParameters( 392 +func (suite *USSOTestSuite) TestNormalizeParametersEscapesParameters( 399 +func (suite *USSOTestSuite) TestNormalizeParametersOmitsOAuthSignature( 535 +func newSingleServingServer( 536 + uri string, response string, code int) *SingleServingServer { These functions need documentation (and the Test* functions need to follow the test documentation guidelines - see [7]). [7] 253 +func (suite *USSOTestSuite) TestSignRequestSHA1(c *C) { 254 + // Test the request signing with oauth_signature_method = SHA1 The documentation on a test should be a statement of the expected behaviour being tested. e.g.: // It is possible to sign a request using SHA1 by setting // SSOdata's SignatureMethod to "HMAC-SHA1" before calling // SSOData.SignRequest() The comment should also come _before_ the function declaration.