diff --git a/pkg/app/api/grpcapi/BUILD.bazel b/pkg/app/api/grpcapi/BUILD.bazel index 4682943851..662b0fae53 100644 --- a/pkg/app/api/grpcapi/BUILD.bazel +++ b/pkg/app/api/grpcapi/BUILD.bazel @@ -6,6 +6,7 @@ go_library( "api.go", "deployment_config_templates.go", "piped_api.go", + "utils.go", "web_api.go", ":deployment_config_templates.embed", #keep ], @@ -50,8 +51,10 @@ go_test( "//pkg/datastore:go_default_library", "//pkg/datastore/datastoretest:go_default_library", "//pkg/model:go_default_library", + "//pkg/rpc/rpcauth:go_default_library", "@com_github_golang_mock//gomock:go_default_library", "@com_github_stretchr_testify//assert:go_default_library", + "@org_uber_go_zap//:go_default_library", ], ) diff --git a/pkg/app/api/grpcapi/api.go b/pkg/app/api/grpcapi/api.go index ecd7bbb0eb..c28caf3d95 100644 --- a/pkg/app/api/grpcapi/api.go +++ b/pkg/app/api/grpcapi/api.go @@ -16,7 +16,9 @@ package grpcapi import ( "context" + "errors" + "github.com/google/uuid" "go.uber.org/zap" "google.golang.org/grpc" "google.golang.org/grpc/codes" @@ -25,6 +27,8 @@ import ( "github.com/pipe-cd/pipe/pkg/app/api/commandstore" "github.com/pipe-cd/pipe/pkg/app/api/service/apiservice" "github.com/pipe-cd/pipe/pkg/datastore" + "github.com/pipe-cd/pipe/pkg/model" + "github.com/pipe-cd/pipe/pkg/rpc/rpcauth" ) // API implements the behaviors for the gRPC definitions of API. @@ -58,10 +62,86 @@ func (a *API) Register(server *grpc.Server) { apiservice.RegisterAPIServiceServer(server, a) } -func (a *API) AddApplication(_ context.Context, _ *apiservice.AddApplicationRequest) (*apiservice.AddApplicationResponse, error) { - return nil, status.Error(codes.Unimplemented, "") +func (a *API) AddApplication(ctx context.Context, req *apiservice.AddApplicationRequest) (*apiservice.AddApplicationResponse, error) { + key, err := requireAPIKey(ctx, model.APIKey_READ_WRITE, a.logger) + if err != nil { + return nil, err + } + + piped, err := getPiped(ctx, a.pipedStore, req.PipedId, a.logger) + if err != nil { + return nil, err + } + + if key.ProjectId != piped.ProjectId { + return nil, status.Error(codes.InvalidArgument, "Requested piped does not belong to your project") + } + + gitpath, err := makeGitPath( + req.GitPath.Repo.Id, + req.GitPath.Path, + req.GitPath.ConfigFilename, + piped, + a.logger, + ) + if err != nil { + return nil, err + } + + app := model.Application{ + Id: uuid.New().String(), + Name: req.Name, + EnvId: req.EnvId, + PipedId: req.PipedId, + ProjectId: key.ProjectId, + GitPath: gitpath, + Kind: req.Kind, + CloudProvider: req.CloudProvider, + } + err = a.applicationStore.AddApplication(ctx, &app) + if errors.Is(err, datastore.ErrAlreadyExists) { + return nil, status.Error(codes.AlreadyExists, "The application already exists") + } + if err != nil { + a.logger.Error("failed to create application", zap.Error(err)) + return nil, status.Error(codes.Internal, "Failed to create application") + } + + return &apiservice.AddApplicationResponse{ + ApplicationId: app.Id, + }, nil } -func (a *API) SyncApplication(_ context.Context, _ *apiservice.SyncApplicationRequest) (*apiservice.SyncApplicationResponse, error) { +func (a *API) SyncApplication(ctx context.Context, _ *apiservice.SyncApplicationRequest) (*apiservice.SyncApplicationResponse, error) { + _, err := requireAPIKey(ctx, model.APIKey_READ_WRITE, a.logger) + if err != nil { + return nil, err + } + return nil, status.Error(codes.Unimplemented, "") } + +// requireAPIKey checks the existence of an API key inside the given context +// and ensures that it has enough permissions for the give role. +func requireAPIKey(ctx context.Context, role model.APIKey_Role, logger *zap.Logger) (*model.APIKey, error) { + key, err := rpcauth.ExtractAPIKey(ctx) + if err != nil { + return nil, err + } + + switch key.Role { + case model.APIKey_READ_WRITE: + return key, nil + + case model.APIKey_READ_ONLY: + if role == model.APIKey_READ_ONLY { + return key, nil + } + logger.Warn("detected an API key that has insufficient permissions", zap.String("key", key.Id)) + return nil, status.Error(codes.PermissionDenied, "Permission denied") + + default: + logger.Warn("detected an API key that has an invalid role", zap.String("key", key.Id)) + return nil, status.Error(codes.PermissionDenied, "Invalid role") + } +} diff --git a/pkg/app/api/grpcapi/api_test.go b/pkg/app/api/grpcapi/api_test.go index 99a44504f1..d9d0c8f29d 100644 --- a/pkg/app/api/grpcapi/api_test.go +++ b/pkg/app/api/grpcapi/api_test.go @@ -13,3 +13,92 @@ // limitations under the License. package grpcapi + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + + "github.com/pipe-cd/pipe/pkg/model" + "github.com/pipe-cd/pipe/pkg/rpc/rpcauth" +) + +func TestRequireAPIKey(t *testing.T) { + testcases := []struct { + name string + key *model.APIKey + requireRole model.APIKey_Role + expectedKey *model.APIKey + expectedErr string + }{ + { + name: "ok: using READ_ONLY to read", + key: &model.APIKey{ + Role: model.APIKey_READ_ONLY, + }, + requireRole: model.APIKey_READ_ONLY, + expectedKey: &model.APIKey{ + Role: model.APIKey_READ_ONLY, + }, + }, + { + name: "ok: using READ_WRITE to read", + key: &model.APIKey{ + Role: model.APIKey_READ_WRITE, + }, + requireRole: model.APIKey_READ_ONLY, + expectedKey: &model.APIKey{ + Role: model.APIKey_READ_WRITE, + }, + }, + { + name: "ok: using READ_WRITE to write", + key: &model.APIKey{ + Role: model.APIKey_READ_WRITE, + }, + requireRole: model.APIKey_READ_WRITE, + expectedKey: &model.APIKey{ + Role: model.APIKey_READ_WRITE, + }, + }, + { + name: "invalid: using READ_ONLY to write", + key: &model.APIKey{ + Role: model.APIKey_READ_ONLY, + }, + requireRole: model.APIKey_READ_WRITE, + expectedErr: "rpc error: code = PermissionDenied desc = Permission denied", + }, + { + name: "invalid: invalid role", + key: &model.APIKey{ + Role: -1, + }, + requireRole: model.APIKey_READ_ONLY, + expectedErr: "rpc error: code = PermissionDenied desc = Invalid role", + }, + { + name: "invalid: api key was not included", + requireRole: model.APIKey_READ_ONLY, + expectedErr: "rpc error: code = Unauthenticated desc = Unauthenticated", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + if tc.key != nil { + ctx = rpcauth.ContextWithAPIKey(ctx, tc.key) + } + key, err := requireAPIKey(ctx, tc.requireRole, zap.NewNop()) + assert.Equal(t, tc.expectedKey, key) + if err != nil { + assert.Equal(t, tc.expectedErr, err.Error()) + } else { + assert.Equal(t, tc.expectedErr, "") + } + }) + } +} diff --git a/pkg/app/api/grpcapi/utils.go b/pkg/app/api/grpcapi/utils.go new file mode 100644 index 0000000000..ea2d8511c1 --- /dev/null +++ b/pkg/app/api/grpcapi/utils.go @@ -0,0 +1,68 @@ +// Copyright 2020 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package grpcapi + +import ( + "context" + "errors" + + "go.uber.org/zap" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + "github.com/pipe-cd/pipe/pkg/datastore" + "github.com/pipe-cd/pipe/pkg/git" + "github.com/pipe-cd/pipe/pkg/model" +) + +func getPiped(ctx context.Context, store datastore.PipedStore, id string, logger *zap.Logger) (*model.Piped, error) { + piped, err := store.GetPiped(ctx, id) + if errors.Is(err, datastore.ErrNotFound) { + return nil, status.Error(codes.NotFound, "Piped is not found") + } + if err != nil { + logger.Error("failed to get piped", zap.Error(err)) + return nil, status.Error(codes.Internal, "Failed to get piped") + } + + return piped, nil +} + +// makeGitPath returns an ApplicationGitPath by adding Repository info and GitPath URL to given args. +func makeGitPath(repoID, path, cfgFilename string, piped *model.Piped, logger *zap.Logger) (*model.ApplicationGitPath, error) { + var repo *model.ApplicationGitRepository + for _, r := range piped.Repositories { + if r.Id == repoID { + repo = r + break + } + } + if repo == nil { + return nil, status.Error(codes.Internal, "The requested repository is not found") + } + + u, err := git.MakeDirURL(repo.Remote, path, repo.Branch) + if err != nil { + logger.Error("failed to make GitPath URL", zap.Error(err)) + return nil, status.Error(codes.Internal, "Failed to make GitPath URL") + } + + return &model.ApplicationGitPath{ + Repo: repo, + Path: path, + ConfigFilename: cfgFilename, + Url: u, + }, nil +} diff --git a/pkg/app/api/grpcapi/web_api.go b/pkg/app/api/grpcapi/web_api.go index 0c92711ebf..33e296cccf 100644 --- a/pkg/app/api/grpcapi/web_api.go +++ b/pkg/app/api/grpcapi/web_api.go @@ -18,7 +18,6 @@ import ( "context" "errors" "fmt" - "strings" "time" "github.com/google/uuid" @@ -313,7 +312,7 @@ func (a *WebAPI) GetPiped(ctx context.Context, req *webservice.GetPipedRequest) return nil, err } - piped, err := a.getPiped(ctx, req.PipedId) + piped, err := getPiped(ctx, a.pipedStore, req.PipedId, a.logger) if err != nil { return nil, err } @@ -329,18 +328,6 @@ func (a *WebAPI) GetPiped(ctx context.Context, req *webservice.GetPipedRequest) }, nil } -func (a *WebAPI) getPiped(ctx context.Context, pipedID string) (*model.Piped, error) { - piped, err := a.pipedStore.GetPiped(ctx, pipedID) - if errors.Is(err, datastore.ErrNotFound) { - return nil, status.Error(codes.NotFound, "Piped is not found") - } - if err != nil { - a.logger.Error("failed to get piped", zap.Error(err)) - return nil, status.Error(codes.Internal, "Failed to get piped") - } - return piped, nil -} - // validatePipedBelongsToProject checks if the given piped belongs to the given project. // It gives back error unless the piped belongs to the project. func (a *WebAPI) validatePipedBelongsToProject(ctx context.Context, pipedID, projectID string) error { @@ -352,7 +339,7 @@ func (a *WebAPI) validatePipedBelongsToProject(ctx context.Context, pipedID, pro return nil } - piped, err := a.getPiped(ctx, pipedID) + piped, err := getPiped(ctx, a.pipedStore, pipedID, a.logger) if err != nil { return err } @@ -372,15 +359,26 @@ func (a *WebAPI) AddApplication(ctx context.Context, req *webservice.AddApplicat return nil, err } - // The path to the application directory must be relative. - if strings.HasPrefix(req.GitPath.Path, "/") { - return nil, status.Error(codes.InvalidArgument, "The path must be a relative path") + piped, err := getPiped(ctx, a.pipedStore, req.PipedId, a.logger) + if err != nil { + return nil, err + } + + if piped.ProjectId != claims.Role.ProjectId { + return nil, status.Error(codes.InvalidArgument, "Requested piped does not belong to your project") } - gitpath, err := a.makeGitPath(ctx, req.GitPath.Repo.Id, req.GitPath.Path, req.GitPath.ConfigFilename, req.PipedId, claims.Role.ProjectId) + gitpath, err := makeGitPath( + req.GitPath.Repo.Id, + req.GitPath.Path, + req.GitPath.ConfigFilename, + piped, + a.logger, + ) if err != nil { return nil, err } + app := model.Application{ Id: uuid.New().String(), Name: req.Name, @@ -405,45 +403,6 @@ func (a *WebAPI) AddApplication(ctx context.Context, req *webservice.AddApplicat }, nil } -// makeGitPath returns an ApplicationGitPath by adding Repository info and GitPath URL to given args. -func (a *WebAPI) makeGitPath(ctx context.Context, repoID, path, cfgFilename, pipedID, projectID string) (*model.ApplicationGitPath, error) { - piped, err := a.getPiped(ctx, pipedID) - if err != nil { - return nil, err - } - if err := a.validatePipedBelongsToProject(ctx, pipedID, projectID); err != nil { - return nil, err - } - - var repo *model.ApplicationGitRepository - for _, r := range piped.Repositories { - if r.Id == repoID { - repo = r - break - } - } - if repo == nil { - a.logger.Error("repository not found", - zap.String("repo-id", repoID), - zap.String("piped-id", pipedID), - zap.Error(err), - ) - return nil, status.Error(codes.Internal, "The repository is not found") - } - - u, err := git.MakeDirURL(repo.Remote, path, repo.Branch) - if err != nil { - a.logger.Error("failed to make GitPath URL", zap.Error(err)) - return nil, status.Error(codes.Internal, "Failed to make GitPath URL") - } - return &model.ApplicationGitPath{ - Repo: repo, - Path: path, - ConfigFilename: cfgFilename, - Url: u, - }, nil -} - func (a *WebAPI) EnableApplication(ctx context.Context, req *webservice.EnableApplicationRequest) (*webservice.EnableApplicationResponse, error) { if err := a.updateApplicationEnable(ctx, req.ApplicationId, true); err != nil { return nil, err @@ -628,7 +587,7 @@ func (a *WebAPI) GenerateApplicationSealedSecret(ctx context.Context, req *webse return nil, err } - piped, err := a.getPiped(ctx, req.PipedId) + piped, err := getPiped(ctx, a.pipedStore, req.PipedId, a.logger) if err != nil { return nil, err } diff --git a/pkg/model/BUILD.bazel b/pkg/model/BUILD.bazel index 18a7cb4019..daa2d1c43c 100644 --- a/pkg/model/BUILD.bazel +++ b/pkg/model/BUILD.bazel @@ -76,6 +76,7 @@ go_test( size = "small", srcs = [ "apikey_test.go", + "common_test.go", "image_name_test.go", "model_test.go", "piped_test.go", diff --git a/pkg/model/common.proto b/pkg/model/common.proto index 018fbfafe9..e1050fa294 100644 --- a/pkg/model/common.proto +++ b/pkg/model/common.proto @@ -30,7 +30,7 @@ enum ApplicationKind { message ApplicationGitPath { // The repository that was configured at piped. ApplicationGitRepository repo = 1 [(validate.rules).message.required = true]; - string path = 2 [(validate.rules).string.min_len = 1]; + string path = 2 [(validate.rules).string.pattern = "^[^/].+$"]; string config_path = 3 [deprecated=true]; string config_filename = 4; string url = 5; diff --git a/pkg/model/common_test.go b/pkg/model/common_test.go new file mode 100644 index 0000000000..66c9f73449 --- /dev/null +++ b/pkg/model/common_test.go @@ -0,0 +1,74 @@ +// Copyright 2020 The PipeCD Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package model + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestApplicationGitPathValidate(t *testing.T) { + testcases := []struct { + name string + gitPath *ApplicationGitPath + expectedErr string + }{ + { + name: "invalid: missing repo", + gitPath: &ApplicationGitPath{}, + expectedErr: "invalid ApplicationGitPath.Repo: value is required", + }, + { + name: "invalid: missing path", + gitPath: &ApplicationGitPath{ + Repo: &ApplicationGitRepository{ + Id: "id", + }, + }, + expectedErr: `invalid ApplicationGitPath.Path: value does not match regex pattern "^[^/].+$"`, + }, + { + name: "invalid: path must be relative", + gitPath: &ApplicationGitPath{ + Repo: &ApplicationGitRepository{ + Id: "id", + }, + Path: "/kubernetes/simple", + }, + expectedErr: `invalid ApplicationGitPath.Path: value does not match regex pattern "^[^/].+$"`, + }, + { + name: "ok", + gitPath: &ApplicationGitPath{ + Repo: &ApplicationGitRepository{ + Id: "id", + }, + Path: "kuberntes/simple", + }, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + err := tc.gitPath.Validate() + var errMsg string + if err != nil { + errMsg = err.Error() + } + assert.Equal(t, tc.expectedErr, errMsg) + }) + } +} diff --git a/pkg/rpc/rpcauth/interceptor.go b/pkg/rpc/rpcauth/interceptor.go index 84732a7bb0..dc334d3b3e 100644 --- a/pkg/rpc/rpcauth/interceptor.go +++ b/pkg/rpc/rpcauth/interceptor.go @@ -163,11 +163,16 @@ func APIKeyUnaryServerInterceptor(verifier APIKeyVerifier, logger *zap.Logger) g logger.Warn("unable to verify api key", zap.Error(err)) return nil, errUnauthenticated } - ctx = context.WithValue(ctx, apiKeyKey, apiKey) + ctx = ContextWithAPIKey(ctx, apiKey) return handler(ctx, req) } } +// ContextWithAPIKey returns a new context in which the given API key was attached. +func ContextWithAPIKey(ctx context.Context, k *model.APIKey) context.Context { + return context.WithValue(ctx, apiKeyKey, k) +} + // ExtractAPIKey returns the verified API key inside the given context. func ExtractAPIKey(ctx context.Context) (*model.APIKey, error) { k, ok := ctx.Value(apiKeyKey).(*model.APIKey)