Merge pull request #1484 from alesstimec/simplify-model-access-checks

refactor[internal/openfga] simplifies some model access checks

internal/jimm/model.go

@@ -1081,10 +1081,10 @@ func (j *JIMM) RevokeModelAccess(ctx context.Context, user *openfga.User, mt nam
 		return errors.E(op, errors.CodeBadRequest, fmt.Sprintf("failed to recognize given access: %q", access), err)
 	}
 
-	requiredAccess := "admin"
+	requiredAccess := ofganames.AdministratorRelation
 	if user.Tag() == ut {
 		// If the user is attempting to revoke their own access.
-		requiredAccess = "read"
+		requiredAccess = ofganames.ReaderRelation
 	}
 
 	err = j.doModel(ctx, user, mt, requiredAccess, func(_ *dbmodel.Model, _ API) error {
@@ -1260,7 +1260,7 @@ func (j *JIMM) ValidateModelUpgrade(ctx context.Context, user *openfga.User, mt
 // returned from the dial operation. If the given function returns an error
 // that error will be returned with the code unmasked.
 func (j *JIMM) doModelAdmin(ctx context.Context, user *openfga.User, mt names.ModelTag, f func(*dbmodel.Model, API) error) error {
-	return j.doModel(ctx, user, mt, "admin", f)
+	return j.doModel(ctx, user, mt, ofganames.AdministratorRelation, f)
 }
 
 // GetUserModelAccess returns the access level a user has against a specific model.
@@ -1269,7 +1269,7 @@ func (j *JIMM) GetUserModelAccess(ctx context.Context, user *openfga.User, model
 	return ToModelAccessString(accessLevel), nil
 }
 
-func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTag, access string, f func(*dbmodel.Model, API) error) error {
+func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTag, requireRelation openfga.Relation, f func(*dbmodel.Model, API) error) error {
 	const op = errors.Op("jimm.doModel")
 	zapctx.Info(ctx, string(op))
 
@@ -1280,11 +1280,12 @@ func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTa
 		return errors.E(op, err)
 	}
 
-	accessLevel, err := j.GetUserModelAccess(ctx, user, mt)
+	hasAccess, err := user.HasModelRelation(ctx, mt, requireRelation)
 	if err != nil {
 		return errors.E(op, err)
 	}
-	if !allowedModelAccess[access][accessLevel] {
+
+	if !hasAccess {
 		// If the user doesn't have correct access on the model return
 		// an unauthorized error.
 		return errors.E(op, errors.CodeUnauthorized, "unauthorized")
@@ -1301,21 +1302,6 @@ func (j *JIMM) doModel(ctx context.Context, user *openfga.User, mt names.ModelTa
 	return nil
 }
 
-var allowedModelAccess = map[string]map[string]bool{
-	"admin": {
-		"admin": true,
-	},
-	"write": {
-		"admin": true,
-		"write": true,
-	},
-	"read": {
-		"admin": true,
-		"write": true,
-		"read":  true,
-	},
-}
-
 // ChangeModelCredential changes the credential used with a model on both
 // the controller and the local database.
 func (j *JIMM) ChangeModelCredential(ctx context.Context, user *openfga.User, modelTag names.ModelTag, cloudCredentialTag names.CloudCredentialTag) error {

internal/jujuapi/cloud.go

@@ -17,6 +17,7 @@ import (
 	"github.com/canonical/jimm/v3/internal/jimm"
 	"github.com/canonical/jimm/v3/internal/jujuapi/rpc"
 	"github.com/canonical/jimm/v3/internal/openfga"
+	ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
 )
 
 func init() {
@@ -276,30 +277,18 @@ func collapseUpdateCredentialResults(args jujuparams.TaggedCredentials, updateRe
 	return results
 }
 
-func userModelAccess(ctx context.Context, user *openfga.User, model names.ModelTag) (string, error) {
-	isModelAdmin, err := openfga.IsAdministrator(ctx, user, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if isModelAdmin {
-		return "admin", nil
-	}
-	hasWriteAccess, err := user.IsModelWriter(ctx, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if hasWriteAccess {
-		return "write", nil
-	}
-	hasReadAccess, err := user.IsModelReader(ctx, model)
-	if err != nil {
-		return "", errors.E(err)
-	}
-	if hasReadAccess {
-		return "read", nil
+func userModelAccess(ctx context.Context, user *openfga.User, model names.ModelTag) string {
+	userRelation := user.GetModelAccess(ctx, model)
+	switch userRelation {
+	case ofganames.AdministratorRelation:
+		return "admin"
+	case ofganames.WriterRelation:
+		return "write"
+	case ofganames.ReaderRelation:
+		return "read"
+	default:
+		return ""
 	}
-
-	return "", nil
 }
 
 // CredentialContents implements the CredentialContents method of the Cloud (v5) facade.
@@ -326,13 +315,9 @@ func getIdentityCredentials(ctx context.Context, user *openfga.User, j JIMM, arg
 		}
 		mas := make([]jujuparams.ModelAccess, len(c.Models))
 		for i, m := range c.Models {
-			userModelAccess, err := userModelAccess(ctx, user, m.ResourceTag())
-			if err != nil {
-				return nil, errors.E(err)
-			}
 			mas[i] = jujuparams.ModelAccess{
 				Model:  m.Name,
-				Access: userModelAccess,
+				Access: userModelAccess(ctx, user, m.ResourceTag()),
 			}
 		}
 		return &jujuparams.ControllerCredentialInfo{

internal/openfga/user.go

@@ -79,6 +79,15 @@ func (u *User) IsModelWriter(ctx context.Context, resource names.ModelTag) (bool
 	return isWriter, nil
 }
 
+// HasModelRelation returns true if user has the specified relation to the model.
+func (u *User) HasModelRelation(ctx context.Context, resource names.ModelTag, relation Relation) (bool, error) {
+	hasRelation, err := checkRelation(ctx, u, resource, relation)
+	if err != nil {
+		return false, errors.E(err)
+	}
+	return hasRelation, nil
+}
+
 // IsServiceAccountAdmin returns true if the user has administrator relation to the service account.
 func (u *User) IsServiceAccountAdmin(ctx context.Context, clientID jimmnames.ServiceAccountTag) (bool, error) {
 	isAdmin, err := checkRelation(ctx, u, clientID, ofganames.AdministratorRelation)

internal/openfga/user_test.go

@@ -133,6 +133,18 @@ func (s *userTestSuite) TestModelAccess(c *gc.C) {
 	allowed, err = adamUser.IsModelWriter(ctx, model)
 	c.Assert(err, gc.IsNil)
 	c.Assert(allowed, gc.Equals, false)
+
+	allowed, err = eveUser.HasModelRelation(ctx, model, ofganames.ReaderRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, true)
+
+	allowed, err = eveUser.HasModelRelation(ctx, model, ofganames.AdministratorRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, true)
+
+	allowed, err = adamUser.HasModelRelation(ctx, model, ofganames.ReaderRelation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(allowed, gc.Equals, false)
 }
 
 func (s *userTestSuite) TestSetModelAccess(c *gc.C) {