Avoid duplicate everyone check (#1327)

* avoid duplicate everyone checks * make method not exported
canonical · Aug 28, 2024 · 6c1d24d · 6c1d24d
1 parent 98c95bf
commit 6c1d24d
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 75 deletions.
diff --git a/internal/jimm/applicationoffer.go b/internal/jimm/applicationoffer.go
@@ -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",

diff --git a/internal/jimm/cloud.go b/internal/jimm/cloud.go
@@ -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
 }
 
@@ -84,7 +72,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 == "" {
@@ -95,30 +82,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

diff --git a/internal/jimm/cloud_test.go b/internal/jimm/cloud_test.go
@@ -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",
 	}
@@ -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"))
@@ -204,13 +197,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",
 	}
@@ -230,7 +216,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{
@@ -239,7 +225,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

diff --git a/internal/jimm/controller.go b/internal/jimm/controller.go
@@ -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))
 			}
 		}

diff --git a/internal/jimm/export_test.go b/internal/jimm/export_test.go
@@ -56,3 +56,7 @@ func (j *JIMM) GetUser(ctx context.Context, identifier string) (*openfga.User, e
 func (j *JIMM) UpdateUserLastLogin(ctx context.Context, identifier string) error {
 	return j.updateUserLastLogin(ctx, identifier)
 }
+
+func (j *JIMM) EveryoneUser() *openfga.User {
+	return j.everyoneUser()
+}
diff --git a/internal/jimm/utils.go b/internal/jimm/utils.go
@@ -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 {