Merge branch 'feature-rebac' into refactor-make-token

.github/CODEOWNERS

@@ -0,0 +1 @@
+*       @alesstimec @kian99 @ale8k @babakks @mina1460

.github/workflows/ci.yaml

@@ -44,7 +44,7 @@ jobs:
       - name: Start test environment
         run: docker compose up -d
       - name: Build and Test
-        run: go test -mod readonly ./... -timeout 30m
+        run: go test -mod readonly ./... -timeout 30m -cover
         env:
           JIMM_DSN: postgresql://jimm:jimm@localhost:5432/jimm
           JIMM_TEST_PGXDSN: postgresql://jimm:jimm@localhost:5432/jimm

Makefile

@@ -34,7 +34,7 @@ build/server: version/commit.txt version/version.txt
 	go build -tags version ./cmd/jimmsrv
 
 check: version/commit.txt version/version.txt
-	go test -p 1 -timeout 30m -tags version $(PROJECT)/...
+	go test -timeout 30m $(PROJECT)/... -cover
 
 install: version/commit.txt version/version.txt
 	go install -tags version $(INSTALL_FLAGS) -v $(PROJECT)/...

api/params/params.go

@@ -255,6 +255,10 @@ type ImportModelRequest struct {
 
 	// ModelTag is the tag of the model that is to be imported.
 	ModelTag string `json:"model-tag"`
+
+	// Owner specifies the new owner of the model after import.
+	// Can be empty to skip switching the owner.
+	Owner string `json:"owner"`
 }
 
 // Authorisation request parameters / responses:

cmd/jimmctl/cmd/importmodel.go

@@ -4,6 +4,7 @@ package cmd
 
 import (
 	"github.com/juju/cmd/v3"
+	"github.com/juju/gnuflag"
 	jujuapi "github.com/juju/juju/api"
 	jujucmd "github.com/juju/juju/cmd"
 	"github.com/juju/juju/cmd/modelcmd"
@@ -18,8 +19,16 @@ import (
 const importModelCommandDoc = `
 	import-model imports a model running on a controller to jimm.
 
+	When importing, it is necessary for JIMM to contain a set of cloud credentials
+	that represent a user's access to the incoming model's cloud. 
+
+	The --owner command is necessary when importing a model created by a 
+	local user and it will switch the model owner to the desired external user.
+	E.g. --owner my-user@external
+
 	Example:
 		jimmctl import-model <controller name> <model-uuid>
+		jimmctl import-model <controller name> <model-uuid> --owner <username>
 `
 
 // NewImportModelCommand returns a command to import a model.
@@ -50,6 +59,11 @@ func (c *importModelCommand) Info() *cmd.Info {
 	})
 }
 
+// SetFlags implements Command.SetFlags.
+func (c *importModelCommand) SetFlags(f *gnuflag.FlagSet) {
+	f.StringVar(&c.req.Owner, "owner", "", "switch the model owner to the desired user")
+}
+
 // Init implements the cmd.Command interface.
 func (c *importModelCommand) Init(args []string) error {
 	switch len(args) {

cmd/jimmctl/cmd/importmodel_test.go

@@ -50,6 +50,35 @@ func (s *importModelSuite) TestImportModelSuperuser(c *gc.C) {
 	model2.SetTag(names.NewModelTag(m.ModelUUID()))
 	err = s.JIMM.Database.GetModel(context.Background(), &model2)
 	c.Assert(err, gc.Equals, nil)
+	c.Check(model2.OwnerUsername, gc.Equals, "charlie@external")
+}
+
+func (s *importModelSuite) TestImportModelFromLocalUser(c *gc.C) {
+	s.AddController(c, "controller-1", s.APIInfo(c))
+	cct := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/charlie@external/cred")
+	s.UpdateCloudCredential(c, cct, jujuparams.CloudCredential{AuthType: "empty"})
+	// Add credentials for Alice on the test cloud, they are needed for the Alice user to become the new model owner
+	cctAlice := names.NewCloudCredentialTag(jimmtest.TestCloudName + "/alice@external/cred")
+	s.UpdateCloudCredential(c, cctAlice, jujuparams.CloudCredential{AuthType: "empty"})
+	mt := s.AddModel(c, names.NewUserTag("charlie@external"), "model-2", names.NewCloudTag(jimmtest.TestCloudName), jimmtest.TestCloudRegionName, cct)
+	var model dbmodel.Model
+	model.SetTag(mt)
+	err := s.JIMM.Database.GetModel(context.Background(), &model)
+	c.Assert(err, gc.Equals, nil)
+	err = s.JIMM.Database.DeleteModel(context.Background(), &model)
+	c.Assert(err, gc.Equals, nil)
+
+	// alice is superuser
+	bClient := s.userBakeryClient("alice")
+	_, err = cmdtesting.RunCommand(c, cmd.NewImportModelCommandForTesting(s.ClientStore(), bClient), "controller-1", mt.Id(), "--owner", "alice@external")
+	c.Assert(err, gc.IsNil)
+
+	var model2 dbmodel.Model
+	model2.SetTag(mt)
+	err = s.JIMM.Database.GetModel(context.Background(), &model2)
+	c.Assert(err, gc.Equals, nil)
+	c.Check(model2.CreatedAt.After(model.CreatedAt), gc.Equals, true)
+	c.Check(model2.OwnerUsername, gc.Equals, "alice@external")
 }
 
 func (s *importModelSuite) TestImportModelUnauthorized(c *gc.C) {

internal/dbmodel/model.go

@@ -113,6 +113,12 @@ func (m *Model) UserAccess(u *User) string {
 	return ""
 }
 
+// FromModelUpdate updates the model from the given ModelUpdate.
+func (m *Model) SwitchOwner(u *User) {
+	m.OwnerUsername = u.Username
+	m.Owner = *u
+}
+
 // FromJujuModelInfo converts jujuparams.ModelInfo into Model.
 func (m *Model) FromJujuModelInfo(info jujuparams.ModelInfo) error {
 	m.Name = info.Name

internal/jimm/controller.go

@@ -458,7 +458,7 @@ func (j *JIMM) GetUserControllerAccess(ctx context.Context, user *openfga.User,
 }
 
 // ImportModel imports model with the specified uuid from the controller.
-func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag) error {
+func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error {
 	const op = errors.Op("jimm.ImportModel")
 
 	isJIMMAdmin, err := openfga.IsAdministrator(ctx, user, j.ResourceTag())
@@ -490,7 +490,6 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa
 	if err != nil {
 		return errors.E(op, err)
 	}
-
 	model := dbmodel.Model{}
 	// fill in data from model info
 	err = model.FromJujuModelInfo(modelInfo)
@@ -500,42 +499,65 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa
 	model.ControllerID = controller.ID
 	model.Controller = controller
 
-	// fetch the model owner user
-	ownerTag, err := names.ParseUserTag(modelInfo.OwnerTag)
+	var ownerString string
+	if newOwner != "" {
+		// Switch the model to be owned by the specified user.
+		ownerString = names.UserTagKind + "-" + newOwner
+	} else {
+		// Use the model owner user
+		ownerString = modelInfo.OwnerTag
+	}
+	ownerTag, err := names.ParseUserTag(ownerString)
 	if err != nil {
 		return errors.E(op, err)
 	}
-	owner := dbmodel.User{}
-	owner.SetTag(ownerTag)
-	err = j.Database.GetUser(ctx, &owner)
+	if ownerTag.IsLocal() {
+		return errors.E(op, "cannot import model from local user, try --owner to switch the model owner")
+	}
+	ownerUser := dbmodel.User{}
+	ownerUser.SetTag(ownerTag)
+	err = j.Database.GetUser(ctx, &ownerUser)
 	if err != nil {
 		return errors.E(op, err)
 	}
-	model.OwnerUsername = owner.Username
-	model.Owner = owner
+	model.SwitchOwner(&ownerUser)
 
-	ownerUser := openfga.NewUser(&owner, j.OpenFGAClient)
-	if err := ownerUser.SetModelAccess(ctx, modelTag, ofganames.AdministratorRelation); err != nil {
+	// Note that only the new owner is given access. All previous users that had access according to Juju
+	// are discarded as access must now be governed by JIMM and OpenFGA.
+	model.Users = nil
+	ofgaUser := openfga.NewUser(&ownerUser, j.OpenFGAClient)
+	if err := ofgaUser.SetModelAccess(ctx, modelTag, ofganames.AdministratorRelation); err != nil {
 		zapctx.Error(
 			ctx,
 			"failed to set model admin",
-			zap.String("owner", owner.Username),
+			zap.String("owner", ownerUser.Username),
 			zap.String("model", modelTag.String()),
 			zap.Error(err),
 		)
 	}
 
+	// TODO(CSS-5458): Remove the below section on cloud credentials once we no longer persist the relation between
+	// cloud credentials and models
+
 	// fetch cloud credential used by the model
-	credentialTag, err := names.ParseCloudCredentialTag(modelInfo.CloudCredentialTag)
+	cloudTag, err := names.ParseCloudTag(modelInfo.CloudTag)
 	if err != nil {
-		return errors.E(op, err)
+		errors.E(op, err)
 	}
-	cloudCredential := dbmodel.CloudCredential{}
-	cloudCredential.SetTag(credentialTag)
-	err = j.Database.GetCloudCredential(ctx, &cloudCredential)
+	// Note that the model already has a cloud credential configured which it will use when deploying new
+	// applications. JIMM needs some cloud credential reference to be able to import the model so use any
+	// credential against the cloud the model is deployed against. Even using the correct cloud for the
+	// credential is not strictly necessary, but will help prevent the user thinking they can create new
+	// models on the incoming cloud.
+	allCredentials, err := j.Database.GetUserCloudCredentials(ctx, &ownerUser, cloudTag.Id())
 	if err != nil {
 		return errors.E(op, err)
 	}
+	if len(allCredentials) == 0 {
+		return errors.E(op, errors.CodeNotFound, fmt.Sprintf("Failed to find cloud credential for user %s on cloud %s", ownerUser.Username, cloudTag.Id()))
+	}
+	cloudCredential := allCredentials[0]
+
 	model.CloudCredentialID = cloudCredential.ID
 	model.CloudCredential = cloudCredential
 
@@ -545,6 +567,7 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa
 	}
 	err = j.Database.GetCloud(ctx, &cloud)
 	if err != nil {
+		zapctx.Error(ctx, "failed to get cloud", zap.String("cloud", cloud.Name))
 		return errors.E(op, err)
 	}
 
@@ -561,33 +584,6 @@ func (j *JIMM) ImportModel(ctx context.Context, user *openfga.User, controllerNa
 		return errors.E(op, "cloud region not found")
 	}
 
-	for i, userAccess := range model.Users {
-		u := userAccess.User
-		err = j.Database.GetUser(ctx, &u)
-		if err != nil {
-			return errors.E(op, err)
-		}
-		model.Users[i].Username = u.Username
-		model.Users[i].User = u
-
-		relation, err := ToModelRelation(userAccess.Access)
-		if err != nil {
-			return errors.E(op, err)
-		}
-
-		modelUser := openfga.NewUser(&u, j.OpenFGAClient)
-		if err := modelUser.SetModelAccess(ctx, modelTag, relation); err != nil {
-			zapctx.Error(
-				ctx,
-				"failed to set model access",
-				zap.String("user", u.Username),
-				zap.String("access", userAccess.Access),
-				zap.String("model", modelTag.String()),
-				zap.Error(err),
-			)
-		}
-	}
-
 	err = j.Database.AddModel(ctx, &model)
 	if err != nil {
 		if errors.ErrorCode(err) == errors.CodeAlreadyExists {