Skip to content

Commit

Permalink
avoid duplicate everyone checks
Browse files Browse the repository at this point in the history
  • Loading branch information
kian99 committed Aug 26, 2024
1 parent aa71655 commit 2bfa86e
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 75 deletions.
11 changes: 1 addition & 10 deletions internal/jimm/applicationoffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,7 @@ func (j *JIMM) Offer(ctx context.Context, user *openfga.User, offer AddApplicati
zap.String("application-offer", doc.UUID))
}

everyoneIdentity, err := dbmodel.NewIdentity(ofganames.EveryoneUser)
if err != nil {
return errors.E(op, err)
}

everyone := openfga.NewUser(
everyoneIdentity,
j.OpenFGAClient,
)
if err := everyone.SetApplicationOfferAccess(ctx, doc.ResourceTag(), ofganames.ReaderRelation); err != nil {
if err := j.EveryoneUser().SetApplicationOfferAccess(ctx, doc.ResourceTag(), ofganames.ReaderRelation); err != nil {
zapctx.Error(
ctx,
"failed relation between user and application offer",
Expand Down
37 changes: 0 additions & 37 deletions internal/jimm/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,6 @@ import (
// GetUserCloudAccess returns users access level for the specified cloud.
func (j *JIMM) GetUserCloudAccess(ctx context.Context, user *openfga.User, cloud names.CloudTag) (string, error) {
accessLevel := user.GetCloudAccess(ctx, cloud)
if accessLevel == ofganames.NoRelation {
everyoneTag := names.NewUserTag(ofganames.EveryoneUser)
everyoneIdentity, err := dbmodel.NewIdentity(everyoneTag.Id())
if err != nil {
return "", err
}
everyone := openfga.NewUser(
everyoneIdentity,
j.OpenFGAClient,
)
accessLevel = everyone.GetCloudAccess(ctx, cloud)
}
return ToCloudAccessString(accessLevel), nil
}

Expand Down Expand Up @@ -106,7 +94,6 @@ func (j *JIMM) ForEachUserCloud(ctx context.Context, user *openfga.User, f func(
if err != nil {
return errors.E(op, err, "cannot load clouds")
}
seen := make(map[string]bool, len(clouds))
for _, cloud := range clouds {
userAccess := ToCloudAccessString(user.GetCloudAccess(ctx, cloud.ResourceTag()))
if userAccess == "" {
Expand All @@ -117,30 +104,6 @@ func (j *JIMM) ForEachUserCloud(ctx context.Context, user *openfga.User, f func(
if err := f(&cloud); err != nil {
return err
}
seen[cloud.Name] = true
}

// Also include "public" clouds
everyoneDB, err := dbmodel.NewIdentity(ofganames.EveryoneUser)
if err != nil {
return errors.E(op, err)
}

everyone := openfga.NewUser(everyoneDB, j.OpenFGAClient)

for _, cloud := range clouds {
if seen[cloud.Name] {
continue
}
userAccess := ToCloudAccessString(everyone.GetCloudAccess(ctx, cloud.ResourceTag()))
if userAccess == "" {
// if user does not have access to the cloud,
// we skip this cloud
continue
}
if err := f(&cloud); err != nil {
return err
}
}

return nil
Expand Down
35 changes: 18 additions & 17 deletions internal/jimm/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ func TestGetCloud(t *testing.T) {
)
c.Assert(err, qt.IsNil)

everyoneIdentity, err := dbmodel.NewIdentity(ofganames.EveryoneUser)
c.Assert(err, qt.IsNil)
everyone := openfga.NewUser(
everyoneIdentity,
client,
)

cloud := &dbmodel.Cloud{
Name: "test-cloud-1",
}
Expand All @@ -106,7 +99,7 @@ func TestGetCloud(t *testing.T) {
err = client.AddCloudController(context.Background(), cloud2.ResourceTag(), j.ResourceTag())
c.Assert(err, qt.IsNil)

err = everyone.SetCloudAccess(context.Background(), cloud2.ResourceTag(), ofganames.CanAddModelRelation)
err = j.EveryoneUser().SetCloudAccess(context.Background(), cloud2.ResourceTag(), ofganames.CanAddModelRelation)
c.Assert(err, qt.IsNil)

_, err = j.GetCloud(ctx, alice, names.NewCloudTag("test-cloud-0"))
Expand Down Expand Up @@ -212,6 +205,21 @@ func TestDefaultCloud(t *testing.T) {

_, err = j.DefaultCloud(ctx, alice)
c.Assert(err, qt.ErrorMatches, "multiple clouds available; please specify one")

// Verify adding everyone access also works
// Remove old access first.
err = alice.UnsetCloudAccess(context.Background(), cloud.ResourceTag(), ofganames.AdministratorRelation)
c.Assert(err, qt.IsNil)
err = alice.UnsetCloudAccess(context.Background(), cloud2.ResourceTag(), ofganames.AdministratorRelation)
c.Assert(err, qt.IsNil)

err = j.EveryoneUser().SetCloudAccess(context.Background(), cloud.ResourceTag(), ofganames.AdministratorRelation)
c.Assert(err, qt.IsNil)

defaultCloud, err = j.DefaultCloud(ctx, alice)
c.Assert(err, qt.IsNil)
c.Assert(defaultCloud.String(), qt.Equals, "cloud-test-cloud-1")

}

func TestForEachCloud(t *testing.T) {
Expand Down Expand Up @@ -262,13 +270,6 @@ func TestForEachCloud(t *testing.T) {
)
daphne.JimmAdmin = true

everyoneIdentity, err := dbmodel.NewIdentity(ofganames.EveryoneUser)
c.Assert(err, qt.IsNil)
everyone := openfga.NewUser(
everyoneIdentity,
client,
)

cloud := &dbmodel.Cloud{
Name: "test-cloud-1",
}
Expand All @@ -288,7 +289,7 @@ func TestForEachCloud(t *testing.T) {

err = bob.SetCloudAccess(ctx, cloud2.ResourceTag(), ofganames.CanAddModelRelation)
c.Assert(err, qt.IsNil)
err = everyone.SetCloudAccess(ctx, cloud2.ResourceTag(), ofganames.CanAddModelRelation)
err = j.EveryoneUser().SetCloudAccess(ctx, cloud2.ResourceTag(), ofganames.CanAddModelRelation)
c.Assert(err, qt.IsNil)

cloud3 := &dbmodel.Cloud{
Expand All @@ -297,7 +298,7 @@ func TestForEachCloud(t *testing.T) {
err = j.Database.AddCloud(ctx, cloud3)
c.Assert(err, qt.IsNil)

err = everyone.SetCloudAccess(ctx, cloud3.ResourceTag(), ofganames.CanAddModelRelation)
err = j.EveryoneUser().SetCloudAccess(ctx, cloud3.ResourceTag(), ofganames.CanAddModelRelation)
c.Assert(err, qt.IsNil)

var clds []dbmodel.Cloud
Expand Down
12 changes: 1 addition & 11 deletions internal/jimm/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,17 +278,7 @@ func (j *JIMM) AddController(ctx context.Context, user *openfga.User, ctl *dbmod
// If this cloud is the one used by the controller model then
// it is available to all users. Other clouds require `juju grant-cloud` to add permissions.
if cloud.ResourceTag().String() == modelSummary.CloudTag {
everyoneTag := names.NewUserTag(ofganames.EveryoneUser)
everyoneIdentity, err := dbmodel.NewIdentity(everyoneTag.Id())
if err != nil {
zapctx.Error(ctx, "failed to create identity model", zap.Error(err))
return errors.E(op, err)
}
everyone := openfga.NewUser(
everyoneIdentity,
j.OpenFGAClient,
)
if err := everyone.SetCloudAccess(ctx, cloud.ResourceTag(), ofganames.CanAddModelRelation); err != nil {
if err := j.EveryoneUser().SetCloudAccess(ctx, cloud.ResourceTag(), ofganames.CanAddModelRelation); err != nil {
zapctx.Error(ctx, "failed to grant everyone add-model access", zap.Error(err))
}
}
Expand Down
8 changes: 8 additions & 0 deletions internal/jimm/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,20 @@ import (
"github.com/canonical/jimm/v3/internal/dbmodel"
"github.com/canonical/jimm/v3/internal/errors"
"github.com/canonical/jimm/v3/internal/openfga"
ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
)

/**
* Authorisation utilities
**/

// EveryoneUser is a convenience method to retrieve the "everyone" user
// whose permissions will translate into granting all users with access.
func (j *JIMM) EveryoneUser() *openfga.User {
everyoneIdentity := &dbmodel.Identity{Name: ofganames.EveryoneUser}
return openfga.NewUser(everyoneIdentity, j.OpenFGAClient)
}

// checkJimmAdmin checks if the user is a JIMM admin.
func (j *JIMM) checkJimmAdmin(user *openfga.User) error {
if !user.JimmAdmin {
Expand Down

0 comments on commit 2bfa86e

Please sign in to comment.