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

[Vincenzo Di Somma]

Improve error handling. Add explicit token validity check.

Improve error handling. Add explicit token validity check.

[Andrew W. Deane (andrew-w-deane)]

Instead of declaring jsonBuffer as an interface at 64+ and then casting to a map at 70+ could jsonBuffer just be declared as a map leaving json.Unmarshal to do the work?

At 75+ if USSO change there source and the json code field is no longer a string then we will panic. If 75+ was replaced with the code below we won't panic should the external source change.

return "", fmt.Errorf("%v",code)

Do you think it is worth changing the code to be on the safe side?

The above questions for 124+, 137+, and 142+ too.

[lp:~vds/usso/validity_check_and_better_auth_failure_handling]

41. By Vincenzo Di Somma on 2014-02-27

Avoiding useless interface/casting, using fmt to prevent failures due to possible changes in u1 sso.

[Vincenzo Di Somma (vds)]

> Instead of declaring jsonBuffer as an interface at 64+ and then casting to a
> map at 70+ could jsonBuffer just be declared as a map leaving json.Unmarshal
> to do the work?
>
> At 75+ if USSO change there source and the json code field is no longer a
> string then we will panic. If 75+ was replaced with the code below we won't
> panic should the external source change.
>
> return "", fmt.Errorf("%v",code)
>
> Do you think it is worth changing the code to be on the safe side?
>
> The above questions for 124+, 137+, and 142+ too.

All good suggestions, all fixed, thanks.

[lp:~vds/usso/validity_check_and_better_auth_failure_handling]

42. By Vincenzo Di Somma on 2014-02-27

Stop using errors module use fmt.Error instead.

43. By Vincenzo Di Somma on 2014-02-27

Stop using errors module use fmt.Error instead.

[Andrew W. Deane (andrew-w-deane)]

review: Approve

[Casey Marshall (cmars)]

review: Approve

=== modified file 'usso.go'
--- usso.go	2014-02-11 11:10:26 +0000
+++ usso.go	2014-02-27 17:23:23 +0000
@@ -1,12 +1,9 @@
 package usso
 
 import (
-	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"strings"
 )
@@ -56,7 +53,6 @@
 	}
 	jsonCredentials, err := json.Marshal(credentials)
 	if err != nil {
-		log.Printf("Error: %s\n", err)
 		return nil, err
 	}
 	response, err := http.Post(
@@ -67,20 +63,18 @@
 		return nil, err
 	}
 	if response.StatusCode == 404 {
-		return nil, errors.New("Wrong credentials.")
+		return nil, fmt.Errorf("Wrong credentials.")
 	}
 	if response.StatusCode != 200 && response.StatusCode != 201 {
 		return nil, fmt.Errorf("SSO Error: %s\n", response.Status)
 	}
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {
-		log.Println(err)
 		return nil, err
 	}
 	ssodata := SSOData{}
 	err = json.Unmarshal(body, &ssodata)
 	if err != nil {
-		log.Println(err)
 		return nil, err
 	}
 	ssodata.Realm = "API"
@@ -105,15 +99,28 @@
 	client := &http.Client{}
 	response, err := client.Do(request)
 	if err != nil {
-		fmt.Printf("Error: %s\n", err)
+		return "", err
 	}
+
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {
-		fmt.Println(err)
-	}
-	var b bytes.Buffer
-	b.Write(body)
-	return fmt.Sprint(b.String()), nil
+		return "", err
+	}
+	if response.StatusCode == 200 {
+		return string(body), nil
+	} else {
+		var jsonMap map[string]interface{}
+		err = json.Unmarshal(body, &jsonMap)
+		// In theory, this should never happen.
+		if err != nil {
+			return "", fmt.Errorf("NO_JSON_RESPONSE")
+		}
+		code, ok := jsonMap["code"]
+		if !ok {
+			return "", fmt.Errorf("NO_CODE")
+		}
+		return "", fmt.Errorf("%v", code)
+	}
 }
 
 // Given oauth credentials and a request, return it signed.
@@ -147,46 +154,35 @@
 	client := &http.Client{}
 	response, err := client.Do(request)
 	if err != nil {
-		fmt.Printf("Error: %s\n", err)
+		return "", err
 	}
 	body, err := ioutil.ReadAll(response.Body)
 	if err != nil {
-		fmt.Println(err)
-	}
-	var b bytes.Buffer
-	b.Write(body)
-	return fmt.Sprint(b.String()), nil
-}
-
-// Register the toke to the U1 File Sync Service.
-func (server UbuntuSSOServer) RegisterTokenToU1FileSync(ssodata *SSOData) (err error) {
-	rp := RequestParameters{
-		BaseURL:         server.tokenRegistrationUrl,
-		HTTPMethod:      "GET",
-		SignatureMethod: HMACSHA1{}}
-
-	request, err := http.NewRequest(rp.HTTPMethod, rp.BaseURL, nil)
-	if err != nil {
-		return err
-	}
-	ssodata.Realm = ""
-	err = SignRequest(ssodata, &rp, request)
-	if err != nil {
-		return err
-	}
-	client := &http.Client{}
-	response, err := client.Do(request)
-	if err != nil {
-		fmt.Printf("Error: %s\n", err)
-	}
-	if response.StatusCode != 200 {
-		body, err := ioutil.ReadAll(response.Body)
+		return "", err
+	}
+	if response.StatusCode == 200 {
+		return string(body), nil
+	} else {
+		var jsonMap map[string]interface{}
+		err = json.Unmarshal(body, &jsonMap)
+		// due to bug #1285176, it is possible to get non json code in the response.
 		if err != nil {
-			fmt.Println(err)
-		}
-		var b bytes.Buffer
-		b.Write(body)
-		errors.New(fmt.Sprint(b.String()))
-	}
-	return nil
+			return "", fmt.Errorf("INVALID_CREDENTIALS")
+		}
+		code, ok := jsonMap["code"]
+		if !ok {
+			return "", fmt.Errorf("NO_CODE")
+		}
+		return "", fmt.Errorf("%v", code)
+	}
+}
+
+// Verify the validity of the token, abusing the API to get the token details.
+func (server UbuntuSSOServer) IsTokenValid(ssodata *SSOData) (bool, error) {
+	details, err := server.GetTokenDetails(ssodata)
+	if details != "" && err == nil {
+		return true, nil
+	} else {
+		return false, err
+	}
 }
 
=== modified file 'usso_test.go'
--- usso_test.go	2014-01-22 16:58:23 +0000
+++ usso_test.go	2014-02-27 17:23:23 +0000
@@ -64,16 +64,6 @@
 		if r.URL.String() == "/api/v2/tokens/oauth/abcs" {
 			fmt.Fprint(w, tokenDetails)
 			return
-		}
-		if r.URL.String() == "/oauth/sso-finished-so-get-tokens/" {
-			fmt.Fprint(w, "ok")
-
-			concat := ""
-			for _, v := range r.Header["Authorization"] {
-				concat += v
-			}
-			requestContent = concat
-			return
 		} else {
 			http.Error(w, "404 page not found", http.StatusNotFound)
 			return
@@ -170,7 +160,7 @@
 	c.Assert(token_details, Equals, string(jsonTokenDetails))
 }
 
-func (suite *USSOTestSuite) TestRegisterToken(c *C) {
+func (suite *USSOTestSuite) TestTokenValidity(c *C) {
 	// Simulate a valid Ubuntu SSO Server response.
 	serverResponseData := map[string]string{
 		"date_updated": "2013-01-16 14:03:36",
@@ -184,16 +174,38 @@
 	if err != nil {
 		panic(err)
 	}
-	server := newTestServer(string(jsonServerResponseData), "{}", 200)
-	var testSSOServer = &UbuntuSSOServer{server.URL, server.URL + "/oauth/sso-finished-so-get-tokens/"}
+	tokenDetails := map[string]string{
+		"token_name":   tokenName,
+		"date_updated": "2014-01-22T13:35:49.867",
+		"token_key":    tokenKey,
+		"href":         "/api/v2/tokens/oauth/JckChNpbXxPRmPkElLglSnqnjsnGseWJmNqTJCWfUtNBSsGtoG",
+		"date_created": "2014-01-17T20:03:24.993",
+		"consumer_key": consumerKey,
+	}
+	jsonTokenDetails, err := json.Marshal(tokenDetails)
+	if err != nil {
+		panic(err)
+	}
+	server := newTestServer(string(jsonServerResponseData), string(jsonTokenDetails), 200)
+	var testSSOServer = &UbuntuSSOServer{server.URL, ""}
 	defer server.Close()
 	ssodata, err := testSSOServer.GetToken(email, password, tokenName)
-	err = testSSOServer.RegisterTokenToU1FileSync(ssodata)
+	// The returned information is correct.
+	token_details, err := testSSOServer.GetTokenDetails(ssodata)
 	c.Assert(err, IsNil)
-	c.Assert(true, Equals, strings.Contains(*server.requestContent, "oauth_consumer_key=\"rfyzhdQ\""))
-	c.Assert(true, Equals, strings.Contains(*server.requestContent, "oauth_token=\"abcs\""))
-	c.Assert(true, Equals, strings.Contains(*server.requestContent, "oauth_signature_method=\"HMAC-SHA1\""))
-	c.Assert(true, Equals, strings.Contains(*server.requestContent, "oauth_version=\"1.0\""))
-	c.Assert(true, Equals, strings.Contains(*server.requestContent, ""))
+	//The request that the fake Ubuntu SSO Server has the token details.
+	c.Assert(token_details, Equals, string(jsonTokenDetails))
+	validity, err := testSSOServer.IsTokenValid(ssodata)
+	c.Assert(validity, Equals, true)
+}
 
+// Check invalid token
+func (suite *USSOTestSuite) TestInvalidToken(c *C) {
+	server := newTestServer("{}", "{}", 200)
+	var testSSOServer = &UbuntuSSOServer{server.URL, ""}
+	defer server.Close()
+	ssodata := SSOData{"WRONG", "", "", "", "", ""}
+	validity, err := testSSOServer.IsTokenValid(&ssodata)
+	c.Assert(err, NotNil)
+	c.Assert(validity, Equals, false)
 }