From 16a895e1416541c7b4ff78aad25e296b33159a76 Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Fri, 2 Mar 2018 12:25:05 +0100 Subject: [PATCH] Detect if the user to init on Tenant already has a project on OSO (OSIO#1446) Include option in User service to support custom HTTP transport Rename vars in controller.Tenant to avoid collision with packages (`user` and `cluster`) Add test on the controller.TenantController.Setup() method using go-vcr Fixes openshiftio/openshift.io#1446 Signed-off-by: Xavier Coulon --- auth/auth_client.go | 3 +- cluster/resolve.go | 3 + controller/tenant.go | 75 ++++++++-------- controller/tenant_test.go | 119 +++++++++++++++++++++++++ controller/tenants_test.go | 10 +-- test/data/controller/setup_tenant.yaml | 84 +++++++++++++++++ test/recorder/recorder.go | 24 +++-- user/user_service.go | 16 ++-- 8 files changed, 276 insertions(+), 58 deletions(-) create mode 100644 controller/tenant_test.go create mode 100644 test/data/controller/setup_tenant.yaml diff --git a/auth/auth_client.go b/auth/auth_client.go index 38e9d9f..b03ae94 100644 --- a/auth/auth_client.go +++ b/auth/auth_client.go @@ -8,7 +8,6 @@ import ( authclient "github.com/fabric8-services/fabric8-tenant/auth/client" "github.com/fabric8-services/fabric8-tenant/configuration" - "github.com/fabric8-services/fabric8-wit/log" goaclient "github.com/goadesign/goa/client" ) @@ -33,7 +32,7 @@ func NewClient(authURL, token string, options ...configuration.HTTPClientOption) }) client.Host = u.Host client.Scheme = u.Scheme - log.Debug(nil, map[string]interface{}{"host": client.Host, "scheme": client.Scheme}, "initializing auth client") + // log.Debug(nil, map[string]interface{}{"host": client.Host, "scheme": client.Scheme}, "initializing auth client") return client, nil } diff --git a/cluster/resolve.go b/cluster/resolve.go index 2058d12..3572bcb 100644 --- a/cluster/resolve.go +++ b/cluster/resolve.go @@ -3,6 +3,8 @@ package cluster import ( "context" "fmt" + + "github.com/fabric8-services/fabric8-wit/log" ) // Resolve a func to resolve a cluster @@ -12,6 +14,7 @@ type Resolve func(ctx context.Context, target string) (*Cluster, error) func NewResolve(clusters []*Cluster) Resolve { return func(ctx context.Context, target string) (*Cluster, error) { for _, cluster := range clusters { + log.Debug(nil, map[string]interface{}{"target_url": cleanURL(target), "cluster_url": cleanURL(cluster.APIURL)}, "comparing URLs...") if cleanURL(target) == cleanURL(cluster.APIURL) { return cluster, nil } diff --git a/controller/tenant.go b/controller/tenant.go index 32c8849..be8871c 100644 --- a/controller/tenant.go +++ b/controller/tenant.go @@ -55,50 +55,49 @@ func NewTenantController( // Setup runs the setup action. func (c *TenantController) Setup(ctx *app.SetupTenantContext) error { - userToken := goajwt.ContextJWT(ctx) - if userToken == nil { + usrToken := goajwt.ContextJWT(ctx) + if usrToken == nil { return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Missing JWT token")) } - ttoken := &TenantToken{token: userToken} - exists := c.tenantService.Exists(ttoken.Subject()) - if exists { + tenantToken := &TenantToken{token: usrToken} + if c.tenantService.Exists(tenantToken.Subject()) { return ctx.Conflict() } - // fetch the cluster the user belongs to - user, err := c.userService.GetUser(ctx, ttoken.Subject()) + usr, err := c.userService.GetUser(ctx, tenantToken.Subject()) if err != nil { return jsonapi.JSONErrorResponse(ctx, err) } - if user.Cluster == nil { + if usr.Cluster == nil { log.Error(ctx, nil, "no cluster defined for tenant") return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, fmt.Errorf("unable to provision to undefined cluster"))) } // fetch the users cluster token - openshiftUsername, openshiftUserToken, err := c.resolveTenant(ctx, *user.Cluster, userToken.Raw) + openshiftUsername, openshiftUserToken, err := c.resolveTenant(ctx, *usr.Cluster, usrToken.Raw) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch tenant token from auth") return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Could not resolve user token")) } // fetch the cluster info - cluster, err := c.resolveCluster(ctx, *user.Cluster) + clustr, err := c.resolveCluster(ctx, *usr.Cluster) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch cluster") return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err)) } + log.Info(ctx, map[string]interface{}{"cluster_api_url": clustr.APIURL, "user_id": tenantToken.Subject().String()}, "resolved cluster for user") // create openshift config - openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, user, cluster.User, cluster.Token, cluster.APIURL) - tenant := &tenant.Tenant{ID: ttoken.Subject(), Email: ttoken.Email()} + openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, usr, clustr.User, clustr.Token, clustr.APIURL) + tenant := &tenant.Tenant{ID: tenantToken.Subject(), Email: tenantToken.Email()} err = c.tenantService.SaveTenant(tenant) if err != nil { log.Error(ctx, map[string]interface{}{ @@ -132,48 +131,48 @@ func (c *TenantController) Setup(ctx *app.SetupTenantContext) error { // Update runs the setup action. func (c *TenantController) Update(ctx *app.UpdateTenantContext) error { - userToken := goajwt.ContextJWT(ctx) - if userToken == nil { + usrToken := goajwt.ContextJWT(ctx) + if usrToken == nil { return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Missing JWT token")) } - ttoken := &TenantToken{token: userToken} - tenant, err := c.tenantService.GetTenant(ttoken.Subject()) + tenantToken := &TenantToken{token: usrToken} + tenant, err := c.tenantService.GetTenant(tenantToken.Subject()) if err != nil { - return jsonapi.JSONErrorResponse(ctx, errors.NewNotFoundError("tenants", ttoken.Subject().String())) + return jsonapi.JSONErrorResponse(ctx, errors.NewNotFoundError("tenants", tenantToken.Subject().String())) } // fetch the cluster the user belongs to - user, err := c.userService.GetUser(ctx, ttoken.Subject()) + usr, err := c.userService.GetUser(ctx, tenantToken.Subject()) if err != nil { return jsonapi.JSONErrorResponse(ctx, err) } - if user.Cluster == nil { + if usr.Cluster == nil { log.Error(ctx, nil, "no cluster defined for tenant") return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, fmt.Errorf("unable to provision to undefined cluster"))) } // fetch the users cluster token - openshiftUsername, _, err := c.resolveTenant(ctx, *user.Cluster, userToken.Raw) + openshiftUsername, _, err := c.resolveTenant(ctx, *usr.Cluster, usrToken.Raw) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch tenant token from auth") return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Could not resolve user token")) } - cluster, err := c.resolveCluster(ctx, *user.Cluster) + clustr, err := c.resolveCluster(ctx, *usr.Cluster) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch cluster") return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err)) } - + log.Info(ctx, map[string]interface{}{"cluster_api_url": clustr.APIURL, "user_id": tenantToken.Subject().String()}, "resolved cluster for user") // create openshift config - openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, user, cluster.User, cluster.Token, cluster.APIURL) + openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, usr, clustr.User, clustr.Token, clustr.APIURL) go func() { ctx := ctx @@ -199,39 +198,39 @@ func (c *TenantController) Update(ctx *app.UpdateTenantContext) error { // Clean runs the setup action for the tenant namespaces. func (c *TenantController) Clean(ctx *app.CleanTenantContext) error { - userToken := goajwt.ContextJWT(ctx) - if userToken == nil { + usrToken := goajwt.ContextJWT(ctx) + if usrToken == nil { return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Missing JWT token")) } - ttoken := &TenantToken{token: userToken} + tenantToken := &TenantToken{token: usrToken} // fetch the cluster the user belongs to - user, err := c.userService.GetUser(ctx, ttoken.Subject()) + usr, err := c.userService.GetUser(ctx, tenantToken.Subject()) if err != nil { return jsonapi.JSONErrorResponse(ctx, err) } // fetch the users cluster token - openshiftUsername, _, err := c.resolveTenant(ctx, *user.Cluster, userToken.Raw) + openshiftUsername, _, err := c.resolveTenant(ctx, *usr.Cluster, usrToken.Raw) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch tenant token from auth") return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Could not resolve user token")) } - cluster, err := c.resolveCluster(ctx, *user.Cluster) + clustr, err := c.resolveCluster(ctx, *usr.Cluster) if err != nil { log.Error(ctx, map[string]interface{}{ "err": err, - "cluster_url": *user.Cluster, + "cluster_url": *usr.Cluster, }, "unable to fetch cluster") return jsonapi.JSONErrorResponse(ctx, errors.NewInternalError(ctx, err)) } // create openshift config - openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, user, cluster.User, cluster.Token, cluster.APIURL) + openshiftConfig := openshift.NewConfig(c.defaultOpenshiftConfig, usr, clustr.User, clustr.Token, clustr.APIURL) err = openshift.CleanTenant(ctx, openshiftConfig, openshiftUsername, c.templateVars) if err != nil { @@ -248,8 +247,8 @@ func (c *TenantController) Show(ctx *app.ShowTenantContext) error { return jsonapi.JSONErrorResponse(ctx, errors.NewUnauthorizedError("Missing JWT token")) } - ttoken := &TenantToken{token: token} - tenantID := ttoken.Subject() + tenantToken := &TenantToken{token: token} + tenantID := tenantToken.Subject() tenant, err := c.tenantService.GetTenant(tenantID) if err != nil { return jsonapi.JSONErrorResponse(ctx, errors.NewNotFoundError("tenants", tenantID.String())) diff --git a/controller/tenant_test.go b/controller/tenant_test.go new file mode 100644 index 0000000..522d29a --- /dev/null +++ b/controller/tenant_test.go @@ -0,0 +1,119 @@ +package controller_test + +import ( + "context" + "testing" + + jwt "github.com/dgrijalva/jwt-go" + "github.com/fabric8-services/fabric8-tenant/app/test" + "github.com/fabric8-services/fabric8-tenant/cluster" + "github.com/fabric8-services/fabric8-tenant/configuration" + "github.com/fabric8-services/fabric8-tenant/controller" + "github.com/fabric8-services/fabric8-tenant/openshift" + "github.com/fabric8-services/fabric8-tenant/tenant" + testsupport "github.com/fabric8-services/fabric8-tenant/test" + "github.com/fabric8-services/fabric8-tenant/test/gormsupport" + "github.com/fabric8-services/fabric8-tenant/test/recorder" + "github.com/fabric8-services/fabric8-tenant/token" + "github.com/fabric8-services/fabric8-tenant/user" + "github.com/fabric8-services/fabric8-wit/resource" + "github.com/goadesign/goa" + goajwt "github.com/goadesign/goa/middleware/security/jwt" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type TenantControllerTestSuite struct { + gormsupport.DBTestSuite +} + +func TestTenantController(t *testing.T) { + resource.Require(t, resource.Database) + suite.Run(t, &TenantControllerTestSuite{DBTestSuite: gormsupport.NewDBTestSuite("../config.yaml")}) +} + +func (s *TenantControllerTestSuite) TestInitTenant() { + // given + r, err := recorder.New("../test/data/controller/setup_tenant", recorder.WithJWTMatcher()) + require.NoError(s.T(), err) + defer r.Stop() + authURL := "http://authservice" + resolveToken := token.NewResolve(authURL, configuration.WithRoundTripper(r.Transport)) + // tok, err := testsupport.NewToken("tenant_service", "../test/private_key.pem") + // require.NoError(s.T(), err) + svc := goa.New("Tenants-service") + saToken, err := testsupport.NewToken("tenant_service", "../test/private_key.pem") + require.NoError(s.T(), err) + clusterService := cluster.NewService( + authURL, + saToken.Raw, + resolveToken, + token.NewGPGDecypter("foo"), + configuration.WithRoundTripper(r.Transport), + ) + clusters, err := clusterService.GetClusters(context.Background()) + require.NoError(s.T(), err) + resolveCluster := cluster.NewResolve(clusters) + resolveTenant := func(ctx context.Context, target, userToken string) (user, accessToken string, err error) { + // log.Debug(ctx, map[string]interface{}{"user_token": userToken}, "attempting to resolve tenant for user...") + return resolveToken(ctx, target, userToken, false, token.PlainText) // no need to use "forcePull=true" to validate the user's token on the target. + } + tenantService := tenant.NewDBService(s.DB) + userService := user.NewService( + authURL, + saToken.Raw, + configuration.WithRoundTripper(r.Transport), + ) + defaultOpenshiftConfig := openshift.Config{} + templateVars := make(map[string]string) + ctrl := controller.NewTenantController(svc, tenantService, userService, resolveTenant, resolveCluster, defaultOpenshiftConfig, templateVars) + require.NotNil(s.T(), ctrl) + + s.T().Run("accepted", func(t *testing.T) { + // given + tenantID := "83fdcae2-634f-4a52-958a-f723cb621700" + ctx, err := createValidUserContext(tenantID, "user_foo@bar.com") + require.NoError(t, err) + // when/then + test.SetupTenantAccepted(t, ctx, svc, ctrl) + }) + + s.T().Run("fail", func(t *testing.T) { + + t.Run("user already exists", func(t *testing.T) { + // given + tenantID := "83fdcae2-634f-4a52-958a-f723cb621700" + ctx, err := createValidUserContext(tenantID, "user_foo@bar.com") + require.NoError(t, err) + // when/then + test.SetupTenantAccepted(t, ctx, svc, ctrl) + }) + + t.Run("missing token", func(t *testing.T) { + // given + test.SetupTenantUnauthorized(t, context.Background(), svc, ctrl) + }) + }) + +} + +func createValidUserContext(userID, email string) (context.Context, error) { + claims := jwt.MapClaims{} + var token *jwt.Token = nil + if userID != "" { + claims["sub"] = userID + claims["email"] = email + token = jwt.NewWithClaims(jwt.SigningMethodRS512, claims) + // use the test private key to sign the token + key, err := testsupport.PrivateKey("../test/private_key.pem") + if err != nil { + return nil, err + } + signed, err := token.SignedString(key) + if err != nil { + return nil, err + } + token.Raw = signed + } + return goajwt.WithJWT(context.Background(), token), nil +} diff --git a/controller/tenants_test.go b/controller/tenants_test.go index 02e22df..f2de91d 100644 --- a/controller/tenants_test.go +++ b/controller/tenants_test.go @@ -23,13 +23,13 @@ import ( "github.com/stretchr/testify/suite" ) -type TenantControllerTestSuite struct { +type TenantsControllerTestSuite struct { gormsupport.DBTestSuite } -func TestTenantController(t *testing.T) { +func TestTenantsController(t *testing.T) { resource.Require(t, resource.Database) - suite.Run(t, &TenantControllerTestSuite{DBTestSuite: gormsupport.NewDBTestSuite("../config.yaml")}) + suite.Run(t, &TenantsControllerTestSuite{DBTestSuite: gormsupport.NewDBTestSuite("../config.yaml")}) } var resolveCluster = func(ctx context.Context, target string) (*cluster.Cluster, error) { @@ -44,7 +44,7 @@ var resolveCluster = func(ctx context.Context, target string) (*cluster.Cluster, }, nil } -func (s *TenantControllerTestSuite) TestShowTenants() { +func (s *TenantsControllerTestSuite) TestShowTenants() { s.T().Run("OK", func(t *testing.T) { // given @@ -82,7 +82,7 @@ func (s *TenantControllerTestSuite) TestShowTenants() { }) } -func (s *TenantControllerTestSuite) TestSearchTenants() { +func (s *TenantsControllerTestSuite) TestSearchTenants() { // given svc := goa.New("Tenants-service") diff --git a/test/data/controller/setup_tenant.yaml b/test/data/controller/setup_tenant.yaml new file mode 100644 index 0000000..fd5302b --- /dev/null +++ b/test/data/controller/setup_tenant.yaml @@ -0,0 +1,84 @@ +--- +version: 1 +interactions: +- request: + method: GET + url: http://authservice/api/clusters/ + headers: + sub: ["tenant_service"] # will be compared against the `sub` claim in the incoming request's token + response: + status: 200 OK + code: 200 + body: '{ + "data":[ + { + "name": "cluster_name", + "api-url": "http://api.cluster1/", + "console-url": "http://console.cluster1/", + "metrics-url": "http://metrics.cluster1/", + "logging-url": "http://logs.cluster1/", + "app-dns": "foo" + } + ] + }' +- request: + method: GET + url: http://api.cluster1/apis/user.openshift.io/v1/users/~ + headers: + sub: ["tenant_service"] # will be compared against the `sub` claim in the incoming request's token + response: + status: 200 OK + code: 200 + body: '{ + "kind":"User", + "apiVersion":"user.openshift.io/v1", + "metadata":{ + "name":"tenant_service", + }, + "identities":[], + "groups":[] + }' +- request: + method: GET + url: http://authservice/api/token?for=http%3A%2F%2Fapi.cluster1%2F&force_pull=false + headers: + sub: ["tenant_service"] # will be compared against the `sub` claim in the incoming request's token + response: + status: 200 OK + code: 200 + body: '{ + "access_token": "jA0ECQMCbp6BH9LCNqNg0sDNAcVRutDsLnJvXYKwHnaQ+7K5kJEw4HOIFRLp77nfh/3CSnNBgY2gZc8d/o0gYww01C4LpUdvyPB/m/Mohp1/yQ2ErIsJDSiYWBgVA/sUPhZf3vY/JYDfg4XOMVA28C7L3ritvlzHT2IwQkZw65NPnJRNvaFA+5fSNqR5XUIKZ84kWotIpyvTwzS/4yAjVAlnu6W/hU6SRDfABPH3Tw/QJPigoRpwyLzN7hVprfXW7Kq54Qx/tmYmceeals0DYfuThKDfx7r2LHW0Yq6A5yzzkKueUaEm4sqJhGkdwf1LsOa+F+/T5MMT7DCiaX88FOkVuQ6cgmCGWbVz5x4fo9fEIy0f56WQAOZwR8clHXS6/pSuj4R5axeX67e3ijAZokiTi3eGCvVmgo2Zb/I49oXCreZQ6kUDQUvu/lwcBDxnrZeOcpftb0OiGNWvCdjwinxgRf0n3FgK+ZYcTxpdYewgOKTauCDhC64rf6hYeV72r6/416zRiaQrvatfJEXLC7mtw1qmALP64WBJOxqpkw==", + "token_type": "bearer", + "username": "tenant_service" + }' +- request: + method: GET + url: http://authservice/api/users/83fdcae2-634f-4a52-958a-f723cb621700 + headers: + sub: ["tenant_service"] # will be compared against the `sub` claim in the incoming request's token + response: + status: 200 OK + code: 200 + body: '{ + "data": { + "attributes": { + "username": "user_foo", + "email": "user_foo@bar.com", + "cluster": "http://api.cluster1/" + } + } + }' +- request: + method: GET + url: http://authservice/api/token?for=http%3A%2F%2Fapi.cluster1%2F&force_pull=false + headers: + sub: ["83fdcae2-634f-4a52-958a-f723cb621700"] # will be compared against the `sub` claim in the incoming request's token + response: + status: 200 OK + code: 200 + body: '{ + "access_token": "jA0ECQMCtCG1bfGEQbxg0kABEQ6nh/A4tMGGkHMHJtLDtFLyXh28IuLvoyGjsZtWPV0LHwN+EEsTtu90BQGbWFdBv+2Wiedk9eE3h08lwA8m", + "token_type": "bearer", + "username": "tenant_service" + }' + diff --git a/test/recorder/recorder.go b/test/recorder/recorder.go index 3e1fe27..093b3a0 100644 --- a/test/recorder/recorder.go +++ b/test/recorder/recorder.go @@ -54,12 +54,12 @@ func JWTMatcher() cassette.Matcher { // check the request URI and method if httpRequest.Method != cassetteRequest.Method || (httpRequest.URL != nil && httpRequest.URL.String() != cassetteRequest.URL) { - log.Debug(nil, map[string]interface{}{ - "httpRequest_method": httpRequest.Method, - "cassetteRequest_method": cassetteRequest.Method, - "httpRequest_url": httpRequest.URL, - "cassetteRequest_url": cassetteRequest.URL, - }, "Cassette method/url doesn't match with the current request") + // log.Debug(nil, map[string]interface{}{ + // "httpRequest_method": httpRequest.Method, + // "cassetteRequest_method": cassetteRequest.Method, + // "httpRequest_url": httpRequest.URL, + // "cassetteRequest_url": cassetteRequest.URL, + // }, "Cassette method/url doesn't match with the current request") return false } @@ -72,9 +72,17 @@ func JWTMatcher() cassette.Matcher { return false } claims := token.Claims.(jwt.MapClaims) - if sub, found := cassetteRequest.Headers["sub"]; found { - return sub[0] == claims["sub"] + sub, found := cassetteRequest.Headers["sub"] + if found && sub[0] == claims["sub"] { + return true } + log.Debug(nil, map[string]interface{}{ + "cassetteRequest_method": cassetteRequest.Method, + "cassetteRequest_url": cassetteRequest.URL, + "cassetteRequest_sub": sub, + "claim_sub": claims["sub"], + }, "Authorization header's 'sub' claim doesn't match with the current request") + return false } } diff --git a/user/user_service.go b/user/user_service.go index 20a1676..0df8043 100644 --- a/user/user_service.go +++ b/user/user_service.go @@ -5,6 +5,7 @@ import ( "github.com/fabric8-services/fabric8-tenant/auth" authclient "github.com/fabric8-services/fabric8-tenant/auth/client" + "github.com/fabric8-services/fabric8-tenant/configuration" goaclient "github.com/goadesign/goa/client" "github.com/pkg/errors" uuid "github.com/satori/go.uuid" @@ -16,17 +17,22 @@ type Service interface { } // NewService creates a new User service -func NewService(authURL string, serviceToken string) Service { - return &userService{authURL: authURL, serviceToken: serviceToken} +func NewService(authURL string, serviceToken string, options ...configuration.HTTPClientOption) Service { + return &userService{ + authURL: authURL, + serviceToken: serviceToken, + clientOptions: options, + } } type userService struct { - authURL string - serviceToken string + authURL string + serviceToken string + clientOptions []configuration.HTTPClientOption } func (s *userService) GetUser(ctx context.Context, id uuid.UUID) (*authclient.UserDataAttributes, error) { - c, err := auth.NewClient(s.authURL, s.serviceToken) + c, err := auth.NewClient(s.authURL, s.serviceToken, s.clientOptions...) if err != nil { return nil, err }