From 25e748331fbfe02d87f0bad75b8f8f91defaab8b Mon Sep 17 00:00:00 2001 From: alesstimec Date: Fri, 8 Dec 2023 12:46:39 +0100 Subject: [PATCH] Add-model check when creating a model When creating a model we check that the owner has an add-model permission on the cloud. --- internal/jimm/cloudcredential_test.go | 4 ++++ internal/jimm/model.go | 21 +++++++++++++------ internal/jimm/model_test.go | 29 +++++++++++++++++++++++++++ internal/jujuclient/storage_test.go | 6 +++--- 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/internal/jimm/cloudcredential_test.go b/internal/jimm/cloudcredential_test.go index 37fd9c997..9e7209105 100644 --- a/internal/jimm/cloudcredential_test.go +++ b/internal/jimm/cloudcredential_test.go @@ -361,6 +361,10 @@ func TestUpdateCloudCredential(t *testing.T) { err = alice.SetCloudAccess(context.Background(), cloud.ResourceTag(), ofganames.AdministratorRelation) c.Assert(err, qt.IsNil) + e := openfga.NewUser(&eve, client) + err = e.SetCloudAccess(context.Background(), cloud.ResourceTag(), ofganames.CanAddModelRelation) + c.Assert(err, qt.IsNil) + controller1 := dbmodel.Controller{ Name: "test-controller-1", UUID: "00000000-0000-0000-0000-0000-0000000000001", diff --git a/internal/jimm/model.go b/internal/jimm/model.go index ca0225ca0..b35d0dbbf 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -255,6 +255,7 @@ func (b *modelBuilder) WithCloudCredential(credentialTag names.CloudCredentialTa b.err = errors.E(err, fmt.Sprintf("failed to fetch cloud credentials %s", credential.Path())) } b.credential = &credential + return b } @@ -509,10 +510,10 @@ func (b *modelBuilder) JujuModelInfo() *jujuparams.ModelInfo { func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCreateArgs) (_ *jujuparams.ModelInfo, err error) { const op = errors.Op("jimm.AddModel") - owner := dbmodel.User{ + owner := &dbmodel.User{ Username: args.Owner.Id(), } - err = j.Database.GetUser(ctx, &owner) + err = j.Database.GetUser(ctx, owner) if err != nil { return nil, errors.E(op, err) } @@ -522,10 +523,8 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") } - // TODO(Kian) CSS-6081 Missing check for add-model access on the desired cloud. - builder := newModelBuilder(ctx, j) - builder = builder.WithOwner(&owner) + builder = builder.WithOwner(owner) builder = builder.WithName(args.Name) if err := builder.Error(); err != nil { return nil, errors.E(op, err) @@ -564,6 +563,16 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea return nil, errors.E(op, err) } + // at this point we know which cloud will host the model and + // we must check the user has add-model permission on the cloud + canAddModel, err := openfga.NewUser(owner, j.OpenFGAClient).IsAllowedAddModel(ctx, builder.cloud.ResourceTag()) + if err != nil { + return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") + } + if !canAddModel { + return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") + } + // fetch cloud region defaults if args.Cloud != (names.CloudTag{}) && builder.cloudRegion != "" { cloudRegionDefaults := dbmodel.CloudDefaults{ @@ -620,7 +629,7 @@ func (j *JIMM) AddModel(ctx context.Context, user *openfga.User, args *ModelCrea zap.String("model", builder.model.UUID.String), ) } - err = openfga.NewUser(&owner, j.OpenFGAClient).SetModelAccess(ctx, names.NewModelTag(mi.UUID), ofganames.AdministratorRelation) + err = openfga.NewUser(owner, j.OpenFGAClient).SetModelAccess(ctx, names.NewModelTag(mi.UUID), ofganames.AdministratorRelation) if err != nil { zapctx.Error( ctx, diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index 44a244f84..67c24c45b 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -158,6 +158,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model user-defaults: - user: alice@external defaults: @@ -268,6 +271,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model user-defaults: - user: alice@external defaults: @@ -379,6 +385,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model cloud-credentials: - name: test-credential-1 owner: alice@external @@ -469,6 +478,11 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: admin + - user: bob@external + access: add-model cloud-credentials: - name: test-credential-1 owner: alice@external @@ -564,6 +578,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model cloud-credentials: - name: test-credential-1 owner: alice@external @@ -627,6 +644,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model cloud-credentials: - name: test-credential-1 owner: alice@external @@ -677,6 +697,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model - name: test-cloud-2 type: test-provider regions: @@ -752,6 +775,9 @@ clouds: type: test-provider regions: - name: test-region-1 + users: + - user: alice@external + access: add-model cloud-credentials: - name: test-credential-1 owner: alice@external @@ -3482,6 +3508,9 @@ clouds: regions: - name: test-region-1 - name: test-region-2 + users: + - user: alice@external + access: add-model user-defaults: - user: alice@external controller-access: superuser diff --git a/internal/jujuclient/storage_test.go b/internal/jujuclient/storage_test.go index 271d62375..20ec0a983 100644 --- a/internal/jujuclient/storage_test.go +++ b/internal/jujuclient/storage_test.go @@ -21,9 +21,9 @@ var _ = gc.Suite(&storageSuite{}) func (s *storageSuite) TestListFilesystems(c *gc.C) { ctx := context.Background() - cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/bob@external/pw1").String() + cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/bob@external/pw1") cred := jujuparams.TaggedCredential{ - Tag: cct, + Tag: cct.String(), Credential: jujuparams.CloudCredential{ AuthType: "userpass", Attributes: map[string]string{ @@ -51,7 +51,7 @@ func (s *storageSuite) TestListFilesystems(c *gc.C) { err = s.API.CreateModel(ctx, &jujuparams.ModelCreateArgs{ Name: "model-1", OwnerTag: names.NewUserTag("bob@external").String(), - CloudCredentialTag: cct, + CloudCredentialTag: cct.String(), }, &modelInfo) c.Assert(err, gc.Equals, nil) uuid := modelInfo.UUID