diff --git a/app/controlplane/internal/server/grpc.go b/app/controlplane/internal/server/grpc.go index 4ee63ae5e..da7880fdd 100644 --- a/app/controlplane/internal/server/grpc.go +++ b/app/controlplane/internal/server/grpc.go @@ -206,9 +206,6 @@ func craftMiddleware(opts *Opts) []middleware.Middleware { selector.Server( usercontext.CheckUserHasAccess(opts.AuthConfig.AllowList, opts.UserUseCase), ).Match(allowListEnabled()).Build(), - selector.Server( - usercontext.CheckOrgRequirements(opts.CASBackendUseCase), - ).Match(requireFullyConfiguredOrgMatcher()).Build(), ).Match(requireCurrentUserMatcher()).Build(), ) @@ -259,15 +256,6 @@ func requireCurrentUserMatcher() selector.MatchFunc { } } -func requireFullyConfiguredOrgMatcher() selector.MatchFunc { - // We do not need to remove other endpoints since this matcher is called once the requireCurrentUserMatcher one has passed - const skipRegexp = "controlplane.v1.OCIRepositoryService/.*|controlplane.v1.ContextService/Current|/controlplane.v1.OrganizationService/.*|/controlplane.v1.AuthService/DeleteAccount|controlplane.v1.CASBackendService/.*|/controlplane.v1.UserService/.*|controlplane.v1.SigningService/.*" - return func(ctx context.Context, operation string) bool { - r := regexp.MustCompile(skipRegexp) - return !r.MatchString(operation) - } -} - func allowListEnabled() selector.MatchFunc { // the allow list should not affect the ability to know who you are and delete your account const skipRegexp = "controlplane.v1.ContextService/Current|/controlplane.v1.AuthService/DeleteAccount" diff --git a/app/controlplane/internal/server/grpc_test.go b/app/controlplane/internal/server/grpc_test.go index f6629e709..04008d43c 100644 --- a/app/controlplane/internal/server/grpc_test.go +++ b/app/controlplane/internal/server/grpc_test.go @@ -41,28 +41,6 @@ func TestRequireCurrentUserMatcher(t *testing.T) { } } -func TestRequireFullyConfiguredOrgMatcher(t *testing.T) { - testCases := []struct { - operation string - matches bool - }{ - {"/controlplane.v1.WorkflowService/List", true}, - {"/controlplane.v1.WorkflowRunService/List", true}, - {"/controlplane.v1.OCIRepositoryService/Save", false}, - {"/controlplane.v1.OrganizationService/ListMemberships", false}, - {"/controlplane.v1.OrganizationService/SetCurrent", false}, - {"/controlplane.v1.CASBackendService/List", false}, - {"/controlplane.v1.CASBackendService/Add", false}, - } - - matchFunc := requireFullyConfiguredOrgMatcher() - for _, op := range testCases { - if got, want := matchFunc(context.Background(), op.operation), op.matches; got != want { - assert.Equal(t, matchFunc(context.Background(), op.operation), op.matches) - } - } -} - func TestRequireRobotAccountMatcher(t *testing.T) { testCases := []struct { operation string diff --git a/app/controlplane/internal/usercontext/orgrequirements_middleware.go b/app/controlplane/internal/usercontext/orgrequirements_middleware.go deleted file mode 100644 index a82b69aad..000000000 --- a/app/controlplane/internal/usercontext/orgrequirements_middleware.go +++ /dev/null @@ -1,93 +0,0 @@ -// -// Copyright 2024 The Chainloop 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 usercontext - -import ( - "context" - "errors" - "fmt" - "time" - - v1 "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" - "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" - "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" - "github.com/go-kratos/kratos/v2/middleware" -) - -func CheckOrgRequirements(uc biz.CASBackendReader) middleware.Middleware { - return func(handler middleware.Handler) middleware.Handler { - return func(ctx context.Context, req interface{}) (interface{}, error) { - org := entities.CurrentOrg(ctx) - if org == nil { - // Make sure that this middleware is ran after WithCurrentUser - return nil, errors.New("organization not found") - } - - // 1 - Figure out main repository for this organization - repo, err := uc.FindDefaultBackend(ctx, org.ID) - if err != nil && !biz.IsNotFound(err) { - return nil, fmt.Errorf("checking for CAS backends in the org: %w", err) - } else if repo == nil { - return nil, v1.ErrorCasBackendErrorReasonRequired("your organization does not have a CAS Backend configured yet") - } - - // 2 - Perform a validation if needed - if shouldRevalidate(repo) { - repo, err = validateCASBackend(ctx, uc, repo) - if err != nil { - return nil, fmt.Errorf("validating CAS backend: %w", err) - } - } - - // 2 - compare the status - if repo.ValidationStatus != biz.CASBackendValidationOK { - return nil, v1.ErrorCasBackendErrorReasonInvalid("your CAS backend can't be reached") - } - - return handler(ctx, req) - } - } -} - -// validateRepoIfNeeded will re-run a validation and return the updated repository -func validateCASBackend(ctx context.Context, uc biz.CASBackendReader, repo *biz.CASBackend) (*biz.CASBackend, error) { - // re-run the validation - if err := uc.PerformValidation(ctx, repo.ID.String()); err != nil { - return nil, fmt.Errorf("performing validation: %w", err) - } - - // Reload repository to get the updated validation status - repo, err := uc.FindByIDInOrg(ctx, repo.OrganizationID.String(), repo.ID.String()) - if err != nil { - return nil, fmt.Errorf("reloading CAS backend: %w", err) - } - - return repo, nil -} - -const validationTimeOffset = 5 * time.Minute - -// Since this check happens synchronously on every request it has a big performance impact -// that's why we run it only in refresh windows -func shouldRevalidate(repo *biz.CASBackend) bool { - // If the validation is currently failed we want to make sure we re-validate - if repo.ValidationStatus == biz.CASBackendValidationFailed { - return true - } - - // if it has been more than validationTimeOffset since the last validation - return repo.ValidatedAt.Before(time.Now().Add(-validationTimeOffset)) -} diff --git a/app/controlplane/internal/usercontext/orgrequirements_middleware_test.go b/app/controlplane/internal/usercontext/orgrequirements_middleware_test.go deleted file mode 100644 index d4b530fe0..000000000 --- a/app/controlplane/internal/usercontext/orgrequirements_middleware_test.go +++ /dev/null @@ -1,102 +0,0 @@ -// -// Copyright 2023 The Chainloop 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 usercontext - -import ( - "context" - "errors" - "testing" - "time" - - "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" - "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/mocks" - "github.com/google/uuid" - "github.com/stretchr/testify/assert" -) - -func TestShouldRevalidate(t *testing.T) { - testCases := []struct { - name string - repoStatus biz.CASBackendValidationStatus - repoValidatedAt time.Time - expected bool - }{ - { - name: "should revalidate if status is not ok and new", - repoStatus: biz.CASBackendValidationFailed, - repoValidatedAt: time.Now(), - expected: true, - }, - { - name: "should revalidate if status is not ok and old", - repoStatus: biz.CASBackendValidationFailed, - repoValidatedAt: time.Now().Add(-2 * validationTimeOffset), - expected: true, - }, - { - name: "should revalidate if status is ok but validated at is too old", - repoStatus: biz.CASBackendValidationOK, - repoValidatedAt: time.Now().Add(-2 * validationTimeOffset), - expected: true, - }, - { - name: "should not revalidate if status is ok and validated at is recent", - repoStatus: biz.CASBackendValidationOK, - repoValidatedAt: time.Now().Add(-(validationTimeOffset - time.Second)), - expected: false, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - repo := &biz.CASBackend{ - ValidationStatus: tc.repoStatus, - ValidatedAt: toTimePtr(tc.repoValidatedAt), - } - - assert.Equal(t, tc.expected, shouldRevalidate(repo)) - }) - } -} - -func TestValidateCASBackend(t *testing.T) { - ctx := context.Background() - assert := assert.New(t) - repo := &biz.CASBackend{ID: uuid.New(), OrganizationID: uuid.New(), ValidatedAt: toTimePtr(time.Now())} - - t.Run("validation error", func(t *testing.T) { - useCase := mocks.NewCASBackendReader(t) - useCase.On("PerformValidation", ctx, repo.ID.String()).Return(errors.New("validation error")) - got, err := validateCASBackend(ctx, useCase, repo) - assert.Error(err) - assert.Nil(got) - }) - - t.Run("validation ok, returns updated repo", func(t *testing.T) { - useCase := mocks.NewCASBackendReader(t) - useCase.On("PerformValidation", ctx, repo.ID.String()).Return(nil) - - want := &biz.CASBackend{ID: repo.ID, ValidatedAt: toTimePtr(time.Now())} - useCase.On("FindByIDInOrg", ctx, repo.OrganizationID.String(), repo.ID.String()).Return(want, nil) - got, err := validateCASBackend(ctx, useCase, repo) - assert.NoError(err) - assert.Equal(want, got) - }) -} - -func toTimePtr(t time.Time) *time.Time { - return &t -}