Skip to content

Commit

Permalink
Add-model check when creating a model
Browse files Browse the repository at this point in the history
When creating a model we check that the owner has an add-model permission
on the cloud.
  • Loading branch information
alesstimec committed Dec 11, 2023
1 parent 8ad7773 commit 25e7483
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 9 deletions.
4 changes: 4 additions & 0 deletions internal/jimm/cloudcredential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
21 changes: 15 additions & 6 deletions internal/jimm/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down
29 changes: 29 additions & 0 deletions internal/jimm/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions internal/jujuclient/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 25e7483

Please sign in to comment.