Merge pull request #5526 from perrito666/acls_add_write

WIP: Acls add write



(Review request: http://reviews.vapour.ws/r/4973/)

api/modelmanager/modelmanager.go

@@ -120,6 +120,8 @@ func ParseModelAccess(access string) (params.ModelAccessPermission, error) {
 		accessPermission = params.ModelReadAccess
 	case permission.ModelWriteAccess:
 		accessPermission = params.ModelWriteAccess
+	case permission.ModelAdminAccess:
+		accessPermission = params.ModelAdminAccess
 	default:
 		return fail, errors.Errorf("unsupported model access permission %v", modelAccess)
 	}

api/usermanager/client_test.go

@@ -60,9 +60,9 @@ func (s *usermanagerSuite) TestAddUserWithModelAccess(c *gc.C) {
 	for i, u := range users {
 		modelUserTags[i] = u.UserTag()
 		if u.UserTag().Name() == "foobar" {
-			c.Assert(u.ReadOnly(), jc.IsTrue)
+			c.Assert(u.IsReadOnly(), jc.IsTrue)
 		} else {
-			c.Assert(u.ReadOnly(), jc.IsFalse)
+			c.Assert(u.IsReadOnly(), jc.IsFalse)
 		}
 	}
 	c.Assert(modelUserTags, jc.SameContents, []names.UserTag{

apiserver/admin.go

@@ -146,18 +146,18 @@ func (a *admin) doLogin(req params.LoginRequest, loginVersion int) (params.Login
 	}
 
 	var maybeUserInfo *params.AuthUserInfo
-	var envUser *state.ModelUser
+	var modelUser *state.ModelUser
 	// Send back user info if user
 	if isUser && !serverOnlyLogin {
 		maybeUserInfo = &params.AuthUserInfo{
 			Identity:       entity.Tag().String(),
 			LastConnection: lastConnection,
 		}
-		envUser, err = a.root.state.ModelUser(entity.Tag().(names.UserTag))
+		modelUser, err = a.root.state.ModelUser(entity.Tag().(names.UserTag))
 		if err != nil {
 			return fail, errors.Annotatef(err, "missing ModelUser for logged in user %s", entity.Tag())
 		}
-		maybeUserInfo.ReadOnly = envUser.ReadOnly()
+		maybeUserInfo.ReadOnly = modelUser.IsReadOnly()
 		if maybeUserInfo.ReadOnly {
 			logger.Debugf("model user %s is READ ONLY", entity.Tag())
 		}
@@ -201,8 +201,8 @@ func (a *admin) doLogin(req params.LoginRequest, loginVersion int) (params.Login
 		loginResult.Facades = facades
 	}
 
-	if envUser != nil {
-		authedApi = newClientAuthRoot(authedApi, envUser)
+	if modelUser != nil {
+		authedApi = newClientAuthRoot(authedApi, modelUser)
 	}
 
 	a.root.rpcConn.ServeFinder(authedApi, serverError)

apiserver/client/client_test.go

@@ -83,8 +83,8 @@ func (s *serverSuite) TestModelUsersInfo(c *gc.C) {
 
 	localUser1 := s.makeLocalModelUser(c, "ralphdoe", "Ralph Doe")
 	localUser2 := s.makeLocalModelUser(c, "samsmith", "Sam Smith")
-	remoteUser1 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "bobjohns@ubuntuone", DisplayName: "Bob Johns"})
-	remoteUser2 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "nicshaw@idprovider", DisplayName: "Nic Shaw"})
+	remoteUser1 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "bobjohns@ubuntuone", DisplayName: "Bob Johns", Access: state.WriteAccess})
+	remoteUser2 := s.Factory.MakeModelUser(c, &factory.ModelUserParams{User: "nicshaw@idprovider", DisplayName: "Nic Shaw", Access: state.WriteAccess})
 
 	results, err := s.client.ModelUserInfo()
 	c.Assert(err, jc.ErrorIsNil)
@@ -98,21 +98,21 @@ func (s *serverSuite) TestModelUsersInfo(c *gc.C) {
 			&params.ModelUserInfo{
 				UserName:    owner.UserName(),
 				DisplayName: owner.DisplayName(),
-				Access:      "write",
+				Access:      "admin",
 			},
 		}, {
 			localUser1,
 			&params.ModelUserInfo{
 				UserName:    "ralphdoe@local",
 				DisplayName: "Ralph Doe",
-				Access:      "write",
+				Access:      "admin",
 			},
 		}, {
 			localUser2,
 			&params.ModelUserInfo{
 				UserName:    "samsmith@local",
 				DisplayName: "Sam Smith",
-				Access:      "write",
+				Access:      "admin",
 			},
 		}, {
 			remoteUser1,

apiserver/client_auth_root.go

@@ -34,14 +34,21 @@ func (r *clientAuthRoot) FindMethod(rootName string, version int, methodName str
 	if err != nil {
 		return nil, err
 	}
-	if r.user.ReadOnly() {
+
+	// ReadOnly User
+	if r.user.IsReadOnly() {
 		canCall := isCallAllowableByReadOnlyUser(rootName, methodName) ||
 			isCallReadOnly(rootName, methodName)
 		if !canCall {
 			return nil, errors.Trace(common.ErrPerm)
 		}
 	}
 
+	// Check if our call requires higher access than the user has.
+	if doesCallRequireAdmin(rootName, methodName) && !r.user.IsAdmin() {
+		return nil, errors.Trace(common.ErrPerm)
+	}
+
 	return caller, nil
 }
 
@@ -54,3 +61,12 @@ func isCallAllowableByReadOnlyUser(facade, _ /*method*/ string) bool {
 	// endpoint rather than a model oriented one.
 	return restrictedRootNames.Contains(facade)
 }
+
+func doesCallRequireAdmin(facade, method string) bool {
+	// TODO(perrito666) This should filter adding users to controllers.
+	// TODO(perrito666) Add an exaustive list of facades/methods that are
+	// admin only and put them in an authoritative source to be re-used.
+	// TODO(perrito666) This is a stub, the idea is to maintain the current
+	// status of permissions until we decide what goes to admin only.
+	return false
+}

apiserver/client_auth_root_test.go

@@ -43,17 +43,17 @@ func (s *clientAuthRootSuite) AssertCallErrPerm(c *gc.C, client *clientAuthRoot,
 }
 
 func (s *clientAuthRootSuite) TestNormalUser(c *gc.C) {
-	envUser := s.Factory.MakeModelUser(c, nil)
-	client := newClientAuthRoot(&fakeFinder{}, envUser)
+	modelUser := s.Factory.MakeModelUser(c, nil)
+	client := newClientAuthRoot(&fakeFinder{}, modelUser)
 	s.AssertCallGood(c, client, "Application", 1, "Deploy")
 	s.AssertCallGood(c, client, "UserManager", 1, "UserInfo")
 	s.AssertCallNotImplemented(c, client, "Client", 1, "Unknown")
 	s.AssertCallNotImplemented(c, client, "Unknown", 1, "Method")
 }
 
 func (s *clientAuthRootSuite) TestReadOnlyUser(c *gc.C) {
-	envUser := s.Factory.MakeModelUser(c, &factory.ModelUserParams{Access: state.ModelReadAccess})
-	client := newClientAuthRoot(&fakeFinder{}, envUser)
+	modelUser := s.Factory.MakeModelUser(c, &factory.ModelUserParams{Access: state.ReadAccess})
+	client := newClientAuthRoot(&fakeFinder{}, modelUser)
 	// deploys are bad
 	s.AssertCallErrPerm(c, client, "Application", 1, "Deploy")
 	// read only commands are fine

apiserver/common/modeluser.go

@@ -16,20 +16,17 @@ import (
 // ModelUser defines the subset of the state.ModelUser type
 // that we require to convert to a params.ModelUserInfo.
 type ModelUser interface {
-	Access() state.ModelAccess
 	DisplayName() string
 	LastConnection() (time.Time, error)
 	UserName() string
 	UserTag() names.UserTag
+	IsReadOnly() bool
+	IsReadWrite() bool
+	IsAdmin() bool
 }
 
 // ModelUserInfo converts *state.ModelUser to params.ModelUserInfo.
 func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
-	access, err := StateToParamsModelAccess(user.Access())
-	if err != nil {
-		return params.ModelUserInfo{}, errors.Trace(err)
-	}
-
 	var lastConn *time.Time
 	userLastConn, err := user.LastConnection()
 	if err == nil {
@@ -38,6 +35,14 @@ func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
 		return params.ModelUserInfo{}, errors.Trace(err)
 	}
 
+	access := params.ModelReadAccess
+	switch {
+	case user.IsAdmin():
+		access = params.ModelAdminAccess
+	case user.IsReadWrite():
+		access = params.ModelWriteAccess
+	}
+
 	userInfo := params.ModelUserInfo{
 		UserName:       user.UserName(),
 		DisplayName:    user.DisplayName(),
@@ -47,13 +52,15 @@ func ModelUserInfo(user ModelUser) (params.ModelUserInfo, error) {
 	return userInfo, nil
 }
 
-// StateToParamsModelAccess converts state.ModelAccess to params.ModelAccessPermission.
-func StateToParamsModelAccess(stateAccess state.ModelAccess) (params.ModelAccessPermission, error) {
+// StateToParamsModelAccess converts state.Access to params.AccessPermission.
+func StateToParamsModelAccess(stateAccess state.Access) (params.ModelAccessPermission, error) {
 	switch stateAccess {
-	case state.ModelReadAccess:
+	case state.ReadAccess:
 		return params.ModelReadAccess, nil
-	case state.ModelAdminAccess:
+	case state.WriteAccess:
 		return params.ModelWriteAccess, nil
+	case state.AdminAccess:
+		return params.ModelAdminAccess, nil
 	}
 	return "", errors.Errorf("invalid model access permission %q", stateAccess)
 }

apiserver/modelmanager/modelinfo_test.go

@@ -57,15 +57,15 @@ func (s *modelInfoSuite) SetUpTest(c *gc.C) {
 		},
 		users: []*mockModelUser{{
 			userName: "admin",
-			access:   state.ModelAdminAccess,
+			access:   state.AdminAccess,
 		}, {
 			userName:    "bob@local",
 			displayName: "Bob",
-			access:      state.ModelReadAccess,
+			access:      state.ReadAccess,
 		}, {
 			userName:    "charlotte@local",
 			displayName: "Charlotte",
-			access:      state.ModelReadAccess,
+			access:      state.ReadAccess,
 		}},
 	}
 	var err error
@@ -82,7 +82,8 @@ func (s *modelInfoSuite) setAPIUser(c *gc.C, user names.UserTag) {
 
 func (s *modelInfoSuite) TestModelInfo(c *gc.C) {
 	s.st.model.users[1].SetErrors(
-		nil, state.NeverConnectedError("never connected"),
+		state.NeverConnectedError("never connected"),
+		nil, nil, nil, nil,
 	)
 	info := s.getModelInfo(c)
 	c.Assert(info, jc.DeepEquals, params.ModelInfo{
@@ -102,7 +103,7 @@ func (s *modelInfoSuite) TestModelInfo(c *gc.C) {
 		Users: []params.ModelUserInfo{{
 			UserName:       "admin",
 			LastConnection: &time.Time{},
-			Access:         params.ModelWriteAccess,
+			Access:         params.ModelAdminAccess,
 		}, {
 			UserName:       "bob@local",
 			DisplayName:    "Bob",
@@ -353,14 +354,26 @@ type mockModelUser struct {
 	gitjujutesting.Stub
 	userName       string
 	displayName    string
-	access         state.ModelAccess
 	lastConnection time.Time
+	access         state.Access
 }
 
-func (u *mockModelUser) Access() state.ModelAccess {
-	u.MethodCall(u, "Access")
+func (u *mockModelUser) IsAdmin() bool {
+	u.MethodCall(u, "IsAdmin")
 	u.PopNoErr()
-	return u.access
+	return u.access == state.AdminAccess
+}
+
+func (u *mockModelUser) IsReadOnly() bool {
+	u.MethodCall(u, "IsReadOnly")
+	u.PopNoErr()
+	return u.access == state.ReadAccess
+}
+
+func (u *mockModelUser) IsReadWrite() bool {
+	u.MethodCall(u, "IsReadWrite")
+	u.PopNoErr()
+	return u.access == state.WriteAccess
 }
 
 func (u *mockModelUser) DisplayName() string {

apiserver/modelmanager/modelmanager.go

@@ -425,34 +425,18 @@ func (m *ModelManagerAPI) ModifyModelAccess(args params.ModifyModelAccessRequest
 
 // resolveStateAccess returns the state representation of the logical model
 // access type.
-func resolveStateAccess(access permission.ModelAccess) (state.ModelAccess, error) {
-	var fail state.ModelAccess
+func resolveStateAccess(access permission.ModelAccess) (state.Access, error) {
+	var fail state.Access
 	switch access {
 	case permission.ModelReadAccess:
-		return state.ModelReadAccess, nil
+		return state.ReadAccess, nil
 	case permission.ModelWriteAccess:
-		// TODO: Initially, we'll map "write" access to admin-level access.
-		// Post Juju-2.0, support for more nuanced access will be added to the
-		// permission business logic and state model.
-		return state.ModelAdminAccess, nil
+		return state.WriteAccess, nil
 	}
 	logger.Errorf("invalid access permission: %+v", access)
 	return fail, errors.Errorf("invalid access permission")
 }
 
-// isGreaterAccess returns whether the new access provides more permissions
-// than the current access.
-// TODO(cmars): If/when more access types are implemented in state,
-//   the implementation of this function will certainly need to change, and it
-//   should be abstracted away to juju/permission as pure business logic
-//   instead of operating on state values.
-func isGreaterAccess(currentAccess, newAccess state.ModelAccess) bool {
-	if currentAccess == state.ModelReadAccess && newAccess == state.ModelAdminAccess {
-		return true
-	}
-	return false
-}
-
 func userAuthorizedToChangeAccess(st Backend, userIsAdmin bool, userTag names.UserTag) error {
 	if userIsAdmin {
 		// Just confirm that the model that has been given is a valid model.
@@ -473,7 +457,7 @@ func userAuthorizedToChangeAccess(st Backend, userIsAdmin bool, userTag names.Us
 		}
 		return errors.Annotate(err, "could not retrieve user")
 	}
-	if currentUser.Access() != state.ModelAdminAccess {
+	if !currentUser.IsAdmin() {
 		return common.ErrPerm
 	}
 	return nil
@@ -511,34 +495,42 @@ func ChangeModelAccess(accessor Backend, modelTag names.ModelTag, apiUser, targe
 			}
 
 			// Only set access if greater access is being granted.
-			if isGreaterAccess(modelUser.Access(), stateAccess) {
+			if modelUser.IsGreaterAccess(stateAccess) {
 				err = modelUser.SetAccess(stateAccess)
 				if err != nil {
 					return errors.Annotate(err, "could not set model access for user")
 				}
 			} else {
-				return errors.Errorf("user already has %q access", modelUser.Access())
+				return errors.Errorf("user already has %q access or greater", stateAccess)
 			}
 			return nil
 		}
 		return errors.Annotate(err, "could not grant model access")
 
 	case params.RevokeModelAccess:
-		if stateAccess == state.ModelReadAccess {
+		switch stateAccess {
+		case state.ReadAccess:
 			// Revoking read access removes all access.
 			err := st.RemoveModelUser(targetUserTag)
 			return errors.Annotate(err, "could not revoke model access")
-
-		} else if stateAccess == state.ModelAdminAccess {
-			// Revoking admin access sets read-only.
+		case state.WriteAccess:
+			// Revoking write access sets read-only.
 			modelUser, err := st.ModelUser(targetUserTag)
 			if err != nil {
 				return errors.Annotate(err, "could not look up model access for user")
 			}
-			err = modelUser.SetAccess(state.ModelReadAccess)
+			err = modelUser.SetAccess(state.ReadAccess)
 			return errors.Annotate(err, "could not set model access to read-only")
+		case state.AdminAccess:
+			// Revoking admin access sets read-write.
+			modelUser, err := st.ModelUser(targetUserTag)
+			if err != nil {
+				return errors.Annotate(err, "could not look up model access for user")
+			}
+			err = modelUser.SetAccess(state.WriteAccess)
+			return errors.Annotate(err, "could not set model access to read-write")
 
-		} else {
+		default:
 			return errors.Errorf("don't know how to revoke %q access", stateAccess)
 		}
 
@@ -555,6 +547,8 @@ func FromModelAccessParam(paramAccess params.ModelAccessPermission) (permission.
 		return permission.ModelReadAccess, nil
 	case params.ModelWriteAccess:
 		return permission.ModelWriteAccess, nil
+	case params.ModelAdminAccess:
+		return permission.ModelAdminAccess, nil
 	}
 	return fail, errors.Errorf("invalid model access permission %q", paramAccess)
 }