diff --git a/internal/jimm/model.go b/internal/jimm/model.go index fd8d2c166..29055e29d 100644 --- a/internal/jimm/model.go +++ b/internal/jimm/model.go @@ -710,13 +710,7 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model return nil, errors.E(op, err) } - modelAccess, err := j.GetUserModelAccess(ctx, user, mt) - if err != nil { - return nil, errors.E(op, err) - } - if modelAccess == "" { - // If the user doesn't have any access on the model return an - // unauthorized error + if ok, err := user.IsModelReader(ctx, mt); !ok || err != nil { return nil, errors.E(op, errors.CodeUnauthorized, "unauthorized") } @@ -733,6 +727,19 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model return nil, errors.E(op, err) } + return j.mergeModelInfo(ctx, user, mi, m) +} + +// mergeModelInfo replaces fields on the juju model info object with +// information from JIMM where JIMM specific information should be used. +func (j *JIMM) mergeModelInfo(ctx context.Context, user *openfga.User, modelInfo *jujuparams.ModelInfo, jimmModel dbmodel.Model) (*jujuparams.ModelInfo, error) { + const op = errors.Op("jimm.mergeModelInfo") + + jimmSummary := jimmModel.ToJujuModelSummary() + modelInfo.CloudCredentialTag = jimmSummary.CloudCredentialTag + modelInfo.ControllerUUID = jimmSummary.ControllerUUID + modelInfo.OwnerTag = jimmSummary.OwnerTag + userAccess := make(map[string]string) for _, relation := range []openfga.Relation{ @@ -742,7 +749,7 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model ofganames.WriterRelation, ofganames.ReaderRelation, } { - usersWithSpecifiedRelation, err := openfga.ListUsersWithAccess(ctx, j.OpenFGAClient, mt, relation) + usersWithSpecifiedRelation, err := openfga.ListUsersWithAccess(ctx, j.OpenFGAClient, jimmModel.ResourceTag(), relation) if err != nil { return nil, errors.E(op, err) } @@ -756,6 +763,11 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model } } + modelAccess, err := j.GetUserModelAccess(ctx, user, jimmModel.ResourceTag()) + if err != nil { + return nil, errors.E(op, err) + } + users := make([]jujuparams.ModelUserInfo, 0, len(userAccess)) for username, access := range userAccess { // If the user does not contain an "@" sign (no domain), it means @@ -771,15 +783,15 @@ func (j *JIMM) ModelInfo(ctx context.Context, user *openfga.User, mt names.Model }) } } - mi.Users = users + modelInfo.Users = users if modelAccess != "admin" && modelAccess != "write" { // Users need "write" level access (or above) to see machine // information. - mi.Machines = nil + modelInfo.Machines = nil } - return mi, nil + return modelInfo, nil } // ModelStatus returns a jujuparams.ModelStatus for the given model. If diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go index f7e256803..c1d7e5973 100644 --- a/internal/jimm/model_test.go +++ b/internal/jimm/model_test.go @@ -1230,19 +1230,8 @@ const modelInfoTestEnvWithEveryoneAccess = modelInfoTestEnv + ` access: read ` -var modelInfoTests = []struct { - name string - env string - username string - uuid string - expectModelInfo *jujuparams.ModelInfo - expectError string -}{{ - name: "AdminUser", - env: modelInfoTestEnv, - username: "alice@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectModelInfo: &jujuparams.ModelInfo{ +func modelInfoTestExpectedModelInfo(canReadMachineInfo bool, limitedExpectedUsers []jujuparams.ModelUserInfo) *jujuparams.ModelInfo { + info := jujuparams.ModelInfo{ Name: "model-1", Type: "iaas", UUID: "00000002-0000-0000-0000-000000000001", @@ -1290,86 +1279,51 @@ var modelInfoTests = []struct { Level: "unsupported", }, AgentVersion: newVersion("1.2.3"), - }, -}, { - name: "WriteUser", - env: modelInfoTestEnv, - username: "bob@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectModelInfo: &jujuparams.ModelInfo{ - Name: "model-1", - Type: "iaas", - UUID: "00000002-0000-0000-0000-000000000001", - ControllerUUID: "00000001-0000-0000-0000-000000000001", - ProviderType: "test-provider", - DefaultSeries: "warty", - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-cloud-region", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC), - }, - Users: []jujuparams.ModelUserInfo{{ - UserName: "bob@canonical.com", - Access: "write", - }}, - Machines: []jujuparams.ModelMachineInfo{{ - Id: "0", - Hardware: jimmtest.ParseMachineHardware("arch=amd64 mem=8096 root-disk=10240 cores=1"), - InstanceId: "00000009-0000-0000-0000-0000000000000", - DisplayName: "Machine 0", - Status: "available", - Message: "OK!", - HasVote: true, - }, { - Id: "1", - Hardware: jimmtest.ParseMachineHardware("arch=amd64 mem=8096 root-disk=10240 cores=2"), - InstanceId: "00000009-0000-0000-0000-0000000000001", - DisplayName: "Machine 1", - Status: "available", - Message: "OK!", - HasVote: true, - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), - }, -}, { - name: "ReadUser", - env: modelInfoTestEnv, - username: "charlie@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectModelInfo: &jujuparams.ModelInfo{ - Name: "model-1", - Type: "iaas", - UUID: "00000002-0000-0000-0000-000000000001", - ControllerUUID: "00000001-0000-0000-0000-000000000001", - ProviderType: "test-provider", - DefaultSeries: "warty", - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-cloud-region", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC), - }, - Users: []jujuparams.ModelUserInfo{{ - UserName: "charlie@canonical.com", - Access: "read", - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), - }, + } + if !canReadMachineInfo { + info.Machines = nil + } + if limitedExpectedUsers != nil { + info.Users = limitedExpectedUsers + } + return &info +} + +var modelInfoTests = []struct { + name string + env string + username string + uuid string + originModelOwner string + expectModelInfo *jujuparams.ModelInfo + expectError string +}{{ + name: "AdminUser", + env: modelInfoTestEnv, + username: "alice@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + originModelOwner: names.NewUserTag("alice@canonical.com").String(), + expectModelInfo: modelInfoTestExpectedModelInfo(true, nil), +}, { + name: "WriteUser", + env: modelInfoTestEnv, + username: "bob@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + originModelOwner: names.NewUserTag("alice@canonical.com").String(), + expectModelInfo: modelInfoTestExpectedModelInfo(true, []jujuparams.ModelUserInfo{{ + UserName: "bob@canonical.com", + Access: "write", + }}), +}, { + name: "ReadUser", + env: modelInfoTestEnv, + username: "charlie@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + originModelOwner: names.NewUserTag("alice@canonical.com").String(), + expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{ + UserName: "charlie@canonical.com", + Access: "read", + }}), }, { name: "NoAccess", env: modelInfoTestEnv, @@ -1383,36 +1337,22 @@ var modelInfoTests = []struct { uuid: "00000002-0000-0000-0000-000000000002", expectError: "model not found", }, { - name: "Access through everyone user", - env: modelInfoTestEnvWithEveryoneAccess, - username: "diane@canonical.com", - uuid: "00000002-0000-0000-0000-000000000001", - expectModelInfo: &jujuparams.ModelInfo{ - Name: "model-1", - Type: "iaas", - UUID: "00000002-0000-0000-0000-000000000001", - ControllerUUID: "00000001-0000-0000-0000-000000000001", - ProviderType: "test-provider", - DefaultSeries: "warty", - CloudTag: names.NewCloudTag("test-cloud").String(), - CloudRegion: "test-cloud-region", - CloudCredentialTag: names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String(), - OwnerTag: names.NewUserTag("alice@canonical.com").String(), - Life: life.Value(state.Alive.String()), - Status: jujuparams.EntityStatus{ - Status: "available", - Info: "OK!", - Since: newDate(2020, 2, 20, 20, 2, 20, 0, time.UTC), - }, - Users: []jujuparams.ModelUserInfo{{ - UserName: "everyone@external", - Access: "read", - }}, - SLA: &jujuparams.ModelSLAInfo{ - Level: "unsupported", - }, - AgentVersion: newVersion("1.2.3"), - }, + name: "Access through everyone user", + env: modelInfoTestEnvWithEveryoneAccess, + username: "diane@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + originModelOwner: names.NewUserTag("alice@canonical.com").String(), + expectModelInfo: modelInfoTestExpectedModelInfo(false, []jujuparams.ModelUserInfo{{ + UserName: "everyone@external", + Access: "read", + }}), +}, { + name: "Owner field is replaced", + env: modelInfoTestEnv, + username: "alice@canonical.com", + uuid: "00000002-0000-0000-0000-000000000001", + originModelOwner: names.NewUserTag("bob").String(), + expectModelInfo: modelInfoTestExpectedModelInfo(true, nil), }, } @@ -1443,7 +1383,7 @@ func TestModelInfo(t *testing.T) { mi.CloudTag = names.NewCloudTag("test-cloud").String() mi.CloudRegion = "test-cloud-region" mi.CloudCredentialTag = names.NewCloudCredentialTag("test-cloud/alice@canonical.com/cred-1").String() - mi.OwnerTag = names.NewUserTag("alice@canonical.com").String() + mi.OwnerTag = test.originModelOwner mi.Life = life.Value(state.Alive.String()) mi.Status = jujuparams.EntityStatus{ Status: "available", diff --git a/internal/jimmtest/api.go b/internal/jimmtest/api.go index 20ba4a5cb..e68847c77 100644 --- a/internal/jimmtest/api.go +++ b/internal/jimmtest/api.go @@ -55,9 +55,12 @@ func (d *Dialer) Dial(_ context.Context, ctl *dbmodel.Controller, _ names.ModelT return nil, d.Err } atomic.AddInt64(&d.open, 1) - ctl.UUID = d.UUID if ctl.UUID == "" { - ctl.UUID = DefaultControllerUUID + if d.UUID == "" { + ctl.UUID = DefaultControllerUUID + } else { + ctl.UUID = d.UUID + } } ctl.AgentVersion = d.AgentVersion if ctl.AgentVersion == "" {