Merge pull request #4 from ory-am/coverage

tests: increased coverage

account/handler/handler.go

@@ -2,7 +2,7 @@ package handler
 
 import (
 	"encoding/json"
-	valid "github.com/asaskevich/govalidator"
+	"github.com/asaskevich/govalidator"
 	"github.com/gorilla/mux"
 	. "github.com/ory-am/hydra/account"
 	hydcon "github.com/ory-am/hydra/context"
@@ -30,24 +30,24 @@ func (h *Handler) SetRoutes(r *mux.Router, extractor func(h hydcon.ContextHandle
 func (h *Handler) Create(extractor func(h hydcon.ContextHandler) hydcon.ContextHandler) hydcon.ContextHandler {
 	return extractor(h.m.IsAuthenticated(h.m.IsAuthorized(hydcon.ContextHandlerFunc(
 		func(ctx context.Context, rw http.ResponseWriter, req *http.Request) {
-			type payload struct {
-				Email    string `json:"email",valid:"email,required"`
-				Password string `json:"password",valid:"required"`
-				Data     string `json:"data",valid:"json"`
+			type Payload struct {
+				Email    string `valid:"email,required" json:"email" `
+				Password string `valid:"length(6|254),required" json:"password"`
+				Data     string `valid:"optional,json", json:"data"`
 			}
 
-			var p payload
+			var p Payload
 			decoder := json.NewDecoder(req.Body)
 			if err := decoder.Decode(&p); err != nil {
 				http.Error(rw, err.Error(), http.StatusBadRequest)
 				return
 			}
 
-			result, err := valid.ValidateStruct(p)
-			if err != nil {
-				http.Error(rw, err.Error(), http.StatusBadRequest)
-				return
-			} else if !result {
+			if v, err := govalidator.ValidateStruct(p); !v {
+				if err != nil {
+					http.Error(rw, err.Error(), http.StatusBadRequest)
+					return
+				}
 				http.Error(rw, "Payload did not validate.", http.StatusBadRequest)
 				return
 			}

account/handler/handler_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/ory-am/hydra/handler/middleware"
 	"github.com/ory-am/hydra/hash"
 	"github.com/ory-am/ladon/policy"
+	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
@@ -38,7 +39,12 @@ func TestMain(m *testing.M) {
 	if err != nil {
 		log.Fatalf("Could not connect to database: %s", err)
 	}
-	defer c.KillRemove()
+	defer func() {
+		err := c.KillRemove()
+		if err != nil {
+			panic(err.Error())
+		}
+	}()
 
 	s = hydra.New(&hash.BCrypt{10}, db)
 	if err := s.CreateSchemas(); err != nil {
@@ -51,10 +57,10 @@ func TestMain(m *testing.M) {
 }
 
 type payload struct {
-	ID       string `json:"id"`
-	Email    string `json:"email"`
-	Password string `json:"password"`
-	Data     string `json:"data"`
+	ID       string `json:"id,omitempty"`
+	Email    string `json:"email,omitempty"`
+	Password string `json:"password,omitempty"`
+	Data     string `json:"data,omitempty"`
 }
 
 type test struct {
@@ -111,7 +117,52 @@ var cases = []*test{
 		[]policy.Policy{
 			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
 		},
-		&payload{Email: "[email protected]", Password: "secret", Data: "{}"},
+		&payload{Email: uuid.New() + "@foobar.com", Data: "{}"},
+		http.StatusBadRequest, 0, 0,
+	},
+	&test{
+		&account.DefaultAccount{ID: "peter"},
+		&jwt.Token{Valid: true},
+		[]policy.Policy{
+			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
+		},
+		&payload{Email: uuid.New() + "@foobar.com", Password: "123", Data: "{}"},
+		http.StatusBadRequest, 0, 0,
+	},
+	&test{
+		&account.DefaultAccount{ID: "peter"},
+		&jwt.Token{Valid: true},
+		[]policy.Policy{
+			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
+		},
+		&payload{Email: "notemail", Password: "secret", Data: "{}"},
+		http.StatusBadRequest, 0, 0,
+	},
+	&test{
+		&account.DefaultAccount{ID: "peter"},
+		&jwt.Token{Valid: true},
+		[]policy.Policy{
+			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
+		},
+		&payload{Email: uuid.New() + "@bar.com", Password: "", Data: "{}"},
+		http.StatusBadRequest, 0, 0,
+	},
+	&test{
+		&account.DefaultAccount{ID: "peter"},
+		&jwt.Token{Valid: true},
+		[]policy.Policy{
+			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
+		},
+		&payload{Email: uuid.New() + "@bar.com", Password: "secret", Data: "not json"},
+		http.StatusBadRequest, 0, 0,
+	},
+	&test{
+		&account.DefaultAccount{ID: "peter"},
+		&jwt.Token{Valid: true},
+		[]policy.Policy{
+			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
+		},
+		&payload{Email: uuid.New() + "@bar.com", Password: "secret", Data: "{}"},
 		http.StatusOK, http.StatusForbidden, http.StatusForbidden,
 	},
 	&test{
@@ -121,7 +172,7 @@ var cases = []*test{
 			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users"}, []string{"create"}},
 			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{".*"}, []string{"get"}},
 		},
-		&payload{Email: "2@bar.com", Password: "secret", Data: "{}"},
+		&payload{Email: uuid.New() + "@bar.com", Password: "secret", Data: "{}"},
 		http.StatusOK, http.StatusOK, http.StatusForbidden,
 	},
 	&test{
@@ -132,7 +183,7 @@ var cases = []*test{
 			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{".*"}, []string{"get"}},
 			&policy.DefaultPolicy{"", "", []string{"peter"}, policy.AllowAccess, []string{"/users/.*"}, []string{"delete"}},
 		},
-		&payload{Email: "3@bar.com", Password: "secret", Data: "{}"},
+		&payload{Email: uuid.New() + "@bar.com", Password: "secret", Data: "{}"},
 		http.StatusOK, http.StatusOK, http.StatusAccepted,
 	},
 }
@@ -153,9 +204,11 @@ func TestCreateGetDelete(t *testing.T) {
 		req := request(testCase)
 		res := httptest.NewRecorder()
 		router.ServeHTTP(res, req)
-		assert.Equal(t, code, res.Code, "Case %d, %s", k, name)
+		assert.Equal(t, code, res.Code, `Case %d, %s: %s`, k, name, res.Body.Bytes())
 		if http.StatusOK == res.Code || http.StatusAccepted == res.Code {
 			finish(testCase, res)
+		} else if res.Code == http.StatusNotFound {
+			log.Printf("404 case %d: %s", k, testCase.createData.ID)
 		}
 	}
 
@@ -170,7 +223,7 @@ func TestCreateGetDelete(t *testing.T) {
 		}, func(c *test, res *httptest.ResponseRecorder) {
 			code = res.Code
 			result := res.Body.Bytes()
-			log.Printf("POST ok /users: %s", result)
+			log.Printf("POST case %d /users: %s", k, result)
 			require.Nil(t, json.Unmarshal(result, &p))
 			assert.Equal(t, c.createData.Email, p.Email)
 			assert.Equal(t, c.createData.Data, p.Data)
@@ -187,7 +240,7 @@ func TestCreateGetDelete(t *testing.T) {
 		}, func(c *test, res *httptest.ResponseRecorder) {
 			code = res.Code
 			result := res.Body.Bytes()
-			log.Printf("GET ok /users/%s: %s", p.ID, result)
+			log.Printf("GET case %d /users/%s: %s", k, p.ID, result)
 			require.Nil(t, json.Unmarshal(result, &p))
 			assert.Equal(t, c.createData.Email, p.Email)
 			assert.Equal(t, c.createData.Data, p.Data)
@@ -203,7 +256,7 @@ func TestCreateGetDelete(t *testing.T) {
 			return req
 		}, func(c *test, res *httptest.ResponseRecorder) {
 			code = res.Code
-			log.Printf("DELETE ok /users/%s", p.ID)
+			log.Printf("DELETE case %d /users/%s", k, p.ID)
 		})
 
 		if code != http.StatusAccepted {

account/postgres/store.go

@@ -2,6 +2,7 @@ package postgres
 
 import (
 	"database/sql"
+	"errors"
 	"github.com/ory-am/hydra/account"
 	"github.com/ory-am/hydra/hash"
 	"log"
@@ -14,6 +15,8 @@ const accountSchema = `CREATE TABLE account (
 	data		 json
 )`
 
+var ErrNotFound = errors.New("Not found")
+
 type Store struct {
 	hasher hash.Hasher
 	db     *sql.DB
@@ -103,7 +106,7 @@ func (s *Store) Delete(id string) (err error) {
 func (s *Store) Authenticate(email, password string) (account.Account, error) {
 	var a account.DefaultAccount
 	// Query account
-	row := s.db.QueryRow("SELECT id, email, password, data FROM account WHERE email=$1 LIMIT 1", email)
+	row := s.db.QueryRow("SELECT id, email, password, data FROM account WHERE email=$1", email)
 
 	// Hydrate struct with data
 	if err := row.Scan(&a.ID, &a.Email, &a.Password, &a.Data); err != nil {

account/postgres/store_test.go

@@ -136,6 +136,8 @@ func TestAuthenticate(t *testing.T) {
 	assert.NotNil(t, err)
 	_, err = store.Authenticate("doesnotexist@foo", "secret")
 	assert.NotNil(t, err)
+	_, err = store.Authenticate("", "")
+	assert.NotNil(t, err)
 
 	result, err := store.Authenticate("5@bar", "secret")
 	assert.Nil(t, err)

context/auth.go

@@ -101,20 +101,10 @@ func PoliciesFromContext(ctx context.Context) ([]policy.Policy, error) {
 
 func IsAuthenticatedFromContext(ctx context.Context) bool {
 	a, b := ctx.Value(authKey).(*authorization)
-	return b && a.token.Valid
+	return (b && a.token != nil && a.token.Valid)
 }
 
 func NewContextFromAuthValues(ctx context.Context, subject account.Account, token *jwt.Token, policies []policy.Policy) context.Context {
-	if subject == nil {
-		subject = &account.DefaultAccount{}
-	}
-	if token == nil {
-		token = &jwt.Token{}
-	}
-	if policies == nil {
-		policies = []policy.Policy{}
-	}
-
 	return context.WithValue(ctx, authKey, &authorization{subject, token, policies})
 }
 

handler/middleware/middleware_test.go

@@ -81,7 +81,7 @@ var cases = []test{
 		&jwt.Token{Valid: true},
 		[]policy.Policy{},
 		"", "",
-		true, false,
+		false, false,
 	},
 	test{
 		&account.DefaultAccount{ID: "max"},

hash/bcrypt_test.go

@@ -1,7 +1,7 @@
 package hash
 
 import (
-	"code.google.com/p/go-uuid/uuid"
+	"github.com/pborman/uuid"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )

jwt/jwt.go

@@ -3,6 +3,7 @@ package jwt
 import (
 	"errors"
 	"fmt"
+	"github.com/RangelReale/osin"
 	"github.com/dgrijalva/jwt-go"
 	"io"
 	"io/ioutil"
@@ -115,22 +116,49 @@ func (j *JWT) SignToken(claims map[string]interface{}, header map[string]interfa
 		return "", errors.New("You may not override the typ header key.")
 	}
 
-	c := func(a, b map[string]interface{}) map[string]interface{} {
-		for k, w := range b {
-			if _, ok := a[k]; !ok {
-				continue
-			}
-			a[k] = w
-		}
-		return a
-	}
-
 	token := jwt.New(jwt.SigningMethodRS256)
 	token.Claims = claims
-	token.Header = c(token.Header, header)
+	token.Header = merge(token.Header, header)
 	ecdsaKey, err := jwt.ParseRSAPrivateKeyFromPEM(j.privateKey)
 	if err != nil {
 		return "", err
 	}
 	return token.SignedString(ecdsaKey)
 }
+
+func merge(a, b map[string]interface{}) map[string]interface{} {
+	for k, w := range b {
+		if _, ok := a[k]; !ok {
+			continue
+		}
+		a[k] = w
+	}
+	return a
+}
+
+type Map struct {
+	Data map[string]interface{}
+}
+
+func (j *JWT) GenerateAccessToken(data *osin.AccessData, generateRefresh bool) (accessToken string, refreshToken string, err error) {
+	claims := map[string]interface{}{"cid": data.Client.GetId(), "exp": data.ExpireAt().Unix()}
+	extra, ok := data.UserData.(*Map)
+	if !ok {
+		extra = &Map{Data: map[string]interface{}{}}
+	}
+
+	claims = merge(claims, extra.Data)
+	accessToken, err = j.SignToken(claims, map[string]interface{}{})
+	if err != nil {
+		return "", "", err
+	} else if !generateRefresh {
+		return
+	}
+
+	claims = merge(map[string]interface{}{"at": accessToken}, claims)
+	refreshToken, err = j.SignToken(claims, map[string]interface{}{})
+	if err != nil {
+		return "", "", err
+	}
+	return
+}