feat: remove usages of model.Users

The model user collection is being removed in the move to 4.0. Usages of
it are being replaced with GetModelUsers on the model service and the
collection in mongo will soon be deleted.

As part of this, the model migration user doc is also removed from
state. This is used when checking if a user trying to access a migrated
model gets told it has been migrated or not. If the user had no
permissions they were not told. This check can no longer be performed.
TODOs have been put in the code and a card created on Jira to address
this.

apiserver/admin.go

@@ -350,7 +350,7 @@ func (a *admin) authenticate(ctx context.Context, req params.LoginRequest) (*aut
 }
 
 func (a *admin) maybeEmitRedirectError(modelUUID model.UUID, authTag names.Tag) error {
-	userTag, ok := authTag.(names.UserTag)
+	_, ok := authTag.(names.UserTag)
 	if !ok {
 		return nil
 	}
@@ -377,7 +377,8 @@ func (a *admin) maybeEmitRedirectError(modelUUID model.UUID, authTag names.Tag)
 	// granted access, do not return a redirect error.
 	// We need to return redirects if possible for anonymous logins in order
 	// to ensure post-migration operation of CMRs.
-	if mig == nil || (userTag.Id() != api.AnonymousUsername && mig.ModelUserAccess(userTag) == permission.NoAccess) {
+	// TODO(aflynn): reinstate check for unauthorised user (JUJU-6669).
+	if mig == nil {
 		return nil
 	}
 

apiserver/admin_test.go

@@ -616,9 +616,10 @@ func (s *loginSuite) TestMigratedModelLogin(c *gc.C) {
 	// Attempt to open an API connection to the migrated model as a user
 	// that had NO access to the model before it got migrated. The server
 	// should return a not-authorized error when attempting to log in.
-	info.Tag = names.NewUserTag("some-other-user")
-	_, err = api.Open(context.Background(), info, fastDialOpts)
-	c.Assert(params.ErrCode(errors.Cause(err)), gc.Equals, params.CodeUnauthorized)
+	// TODO(aflynn): reinstate check for unauthorised user (JUJU-6669).
+	//info.Tag = names.NewUserTag("some-other-user")
+	//_, err = api.Open(context.Background(), info, fastDialOpts)
+	//c.Assert(params.ErrCode(errors.Cause(err)), gc.Equals, params.CodeUnauthorized)
 
 	// Attempt to open an API connection to the migrated model as the
 	// anonymous user; this should also be allowed on account of CMRs.

apiserver/common/modelmanagerinterface.go

@@ -199,7 +199,9 @@ func (st modelManagerStateShim) GetBackend(modelUUID string) (ModelManagerBacken
 			return nil, nil, errors.Trace(mErr)
 		}
 
-		if mig == nil || mig.ModelUserAccess(st.user) == permission.NoAccess {
+		// TODO(aflynn): Also return this error if, in the migration info, the
+		// user had access to the migrated model (JUJU-6669).
+		if mig == nil {
 			return nil, nil, errors.Trace(err) // return original NotFound error
 		}
 

apiserver/facades/client/client/backend.go

@@ -65,7 +65,6 @@ type Model interface {
 	Config() (*config.Config, error)
 	Owner() names.UserTag
 	AddUser(state.UserAccessSpec) (permission.UserAccess, error)
-	Users() ([]permission.UserAccess, error)
 	StatusHistory(status.StatusHistoryFilter) ([]status.StatusInfo, error)
 	LatestToolsVersion() version.Number
 	Status() (status.StatusInfo, error)

apiserver/facades/client/controller/controller.go

@@ -82,6 +82,9 @@ type ModelService interface {
 	Model(ctx context.Context, uuid coremodel.UUID) (coremodel.Model, error)
 	// ControllerModel returns the model used for housing the Juju controller.
 	ControllerModel(ctx context.Context) (coremodel.Model, error)
+	// GetModelUsers will retrieve basic information about users with permissions on
+	// the given model UUID.
+	GetModelUsers(ctx context.Context, modelUUID coremodel.UUID) ([]coremodel.ModelUserInfo, error)
 }
 
 // ModelConfigService provides access to the model configuration.
@@ -1038,17 +1041,18 @@ func makeModelInfo(ctx context.Context, st *state.State,
 		return empty, ul, errors.Trace(err)
 	}
 
-	users, err := model.Users()
+	modelID := coremodel.UUID(model.UUID())
+
+	users, err := modelService.GetModelUsers(ctx, modelID)
 	if err != nil {
 		return empty, ul, errors.Trace(err)
 	}
 	ul.users = set.NewStrings()
 	for _, u := range users {
-		ul.users.Add(u.UserName.Name())
+		ul.users.Add(u.Name.Name())
 	}
 
 	// Retrieve agent version for the model.
-	modelID := coremodel.UUID(model.UUID())
 	modelInfo, err := modelService.Model(ctx, modelID)
 	if err != nil {
 		return empty, userList{}, fmt.Errorf("getting model %q: %w", modelID, err)

state/model.go

@@ -19,7 +19,6 @@ import (
 
 	"github.com/juju/juju/controller"
 	"github.com/juju/juju/core/constraints"
-	"github.com/juju/juju/core/permission"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/environs/config"
 	internalpassword "github.com/juju/juju/internal/password"
@@ -370,11 +369,6 @@ func (ctlr *Controller) NewModel(configSchemaGetter config.ConfigSchemaSourceGet
 		_, _ = probablyUpdateStatusHistory(newSt.db(), newModel.Kind(), modelGlobalKey, modelGlobalKey, modelStatusDoc)
 	}
 
-	_, err = newSt.SetUserAccess(newModel.Owner(), newModel.ModelTag(), permission.AdminAccess)
-	if err != nil {
-		return nil, nil, errors.Annotate(err, "granting admin permission to the owner")
-	}
-
 	// TODO(storage) - we need to add the default storage pools using the new dqlite model service
 	//storageService, registry, err := newSt.storageServices()
 	//if err != nil {
@@ -820,43 +814,6 @@ func (m *Model) AllEndpointBindings() (map[string]*Bindings, error) {
 	return appEndpointBindings, nil
 }
 
-// Users returns a slice of all users for this model.
-func (m *Model) Users() ([]permission.UserAccess, error) {
-	coll, closer := m.st.db().GetCollection(modelUsersC)
-	defer closer()
-
-	var userDocs []userAccessDoc
-	err := coll.Find(nil).All(&userDocs)
-	if err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	var modelUsers []permission.UserAccess
-	for _, doc := range userDocs {
-		// check if the User belonging to this model user has
-		// been deleted, in this case we should not return it.
-		userTag := names.NewUserTag(doc.UserName)
-		if userTag.IsLocal() {
-			_, err := m.st.User(userTag)
-			if err != nil {
-				if !IsDeletedUserError(err) {
-					// We ignore deleted users for now. So if it is not a
-					// DeletedUserError we return the error.
-					return nil, errors.Trace(err)
-				}
-				continue
-			}
-		}
-		mu, err := NewModelUserAccess(m.st, doc)
-		if err != nil {
-			return nil, errors.Trace(err)
-		}
-		modelUsers = append(modelUsers, mu)
-	}
-
-	return modelUsers, nil
-}
-
 // IsControllerModel returns a boolean indicating whether
 // this model is responsible for running a controller.
 func (m *Model) IsControllerModel() bool {

state/modelmigration.go

@@ -17,7 +17,6 @@ import (
 	"gopkg.in/macaroon.v2"
 
 	"github.com/juju/juju/core/migration"
-	"github.com/juju/juju/core/permission"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/internal/mongo"
 )
@@ -94,10 +93,6 @@ type ModelMigration interface {
 	// Refresh updates the contents of the ModelMigration from the
 	// underlying state.
 	Refresh() error
-
-	// ModelUserAccess returns the type of access that the given tag had to
-	// the model prior to it being migrated.
-	ModelUserAccess(names.Tag) permission.Access
 }
 
 // MinionReports indicates the sets of agents whose migration minion
@@ -159,14 +154,6 @@ type modelMigDoc struct {
 	// TargetMacaroons holds the macaroons to use with TargetAuthTag
 	// when authenticating.
 	TargetMacaroons string `bson:"target-macaroons,omitempty"`
-
-	// The list of users and their access-level to the model being migrated.
-	ModelUsers []modelMigUserDoc `bson:"model-users,omitempty"`
-}
-
-type modelMigUserDoc struct {
-	UserID string            `bson:"user_id"`
-	Access permission.Access `bson:"access"`
 }
 
 // modelMigStatusDoc tracks the progress of a migration attempt for a
@@ -644,18 +631,6 @@ func (mig *modelMigration) Refresh() error {
 	return nil
 }
 
-// ModelUserAccess implements ModelMigration.
-func (mig *modelMigration) ModelUserAccess(tag names.Tag) permission.Access {
-	id := tag.Id()
-	for _, user := range mig.doc.ModelUsers {
-		if user.UserID == id {
-			return user.Access
-		}
-	}
-
-	return permission.NoAccess
-}
-
 // MigrationSpec holds the information required to create a
 // ModelMigration instance.
 type MigrationSpec struct {
@@ -722,11 +697,6 @@ func (st *State) CreateMigration(spec MigrationSpec) (ModelMigration, error) {
 			return nil, errors.Trace(err)
 		}
 
-		userDocs, err := modelUserDocs(model)
-		if err != nil {
-			return nil, errors.Trace(err)
-		}
-
 		id := fmt.Sprintf("%s:%d", modelUUID, attempt)
 		doc = modelMigDoc{
 			Id:                    id,
@@ -740,7 +710,6 @@ func (st *State) CreateMigration(spec MigrationSpec) (ModelMigration, error) {
 			TargetAuthTag:         spec.TargetInfo.AuthTag.String(),
 			TargetPassword:        spec.TargetInfo.Password,
 			TargetMacaroons:       macsJSON,
-			ModelUsers:            userDocs,
 		}
 
 		statusDoc = modelMigStatusDoc{
@@ -788,23 +757,6 @@ func (st *State) CreateMigration(spec MigrationSpec) (ModelMigration, error) {
 	}, nil
 }
 
-func modelUserDocs(m *Model) ([]modelMigUserDoc, error) {
-	users, err := m.Users()
-	if err != nil {
-		return nil, err
-	}
-
-	var docs []modelMigUserDoc
-	for _, user := range users {
-		docs = append(docs, modelMigUserDoc{
-			UserID: user.UserTag.Id(),
-			Access: user.Access,
-		})
-	}
-
-	return docs, nil
-}
-
 func macaroonsToJSON(m []macaroon.Slice) (string, error) {
 	if len(m) == 0 {
 		return "", nil

state/modelmigration_test.go

@@ -15,7 +15,6 @@ import (
 	"gopkg.in/macaroon.v2"
 
 	"github.com/juju/juju/core/migration"
-	"github.com/juju/juju/core/permission"
 	"github.com/juju/juju/internal/testing"
 	"github.com/juju/juju/internal/testing/factory"
 	"github.com/juju/juju/internal/uuid"
@@ -850,30 +849,6 @@ func (s *MigrationSuite) TestWatchMinionReportsMultiModel(c *gc.C) {
 	wc3.AssertOneChange()
 }
 
-func (s *MigrationSuite) TestModelUserAccess(c *gc.C) {
-	model, err := s.State2.Model()
-	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(model.MigrationMode(), gc.Equals, state.MigrationModeNone)
-
-	// Get users that had access to the model before the migration
-	modelUsers, err := model.Users()
-	c.Assert(err, jc.ErrorIsNil)
-	c.Assert(len(modelUsers), gc.Not(gc.Equals), 0)
-
-	mig, err := s.State2.CreateMigration(s.stdSpec)
-	c.Assert(err, jc.ErrorIsNil)
-
-	for _, modelUser := range modelUsers {
-		c.Logf("check that migration doc lists user %q having permission %q", modelUser.UserTag, modelUser.Access)
-		perm := mig.ModelUserAccess(modelUser.UserTag)
-		c.Assert(perm, gc.Equals, modelUser.Access)
-	}
-
-	// Querying for any other user should yield permission.NoAccess
-	perm := mig.ModelUserAccess(names.NewUserTag("bogus"))
-	c.Assert(perm, gc.Equals, permission.NoAccess)
-}
-
 func (s *MigrationSuite) createStatusWatcher(c *gc.C, st *state.State) (
 	state.NotifyWatcher, statetesting.NotifyWatcherC,
 ) {