Merge commit '9fe44fe' into 2.3-incremental

This brings in the changes to the RPC server to support a Recorder instead of an Observer, and changes to some of the audit logging. It does revert the changes to the machine agent for audit logging, but it doesn't appear to involve any test failures.
sdg0919 · Jan 10, 2018 · 70755fd · 70755fd
2 parents d6c6f44 + 9fe44fe
commit 70755fd
Show file tree

Hide file tree

Showing 23 changed files with 734 additions and 259 deletions.
diff --git a/api/apiclient.go b/api/apiclient.go
@@ -31,7 +31,6 @@ import (
 	"gopkg.in/retry.v1"
 
 	"github.com/juju/juju/api/base"
-	"github.com/juju/juju/apiserver/observer"
 	"github.com/juju/juju/apiserver/params"
 	"github.com/juju/juju/network"
 	"github.com/juju/juju/rpc"
@@ -197,7 +196,7 @@ func Open(info *Info, opts DialOpts) (Connection, error) {
 		return nil, errors.Trace(err)
 	}
 
-	client := rpc.NewConn(jsoncodec.New(dialResult.conn), observer.None())
+	client := rpc.NewConn(jsoncodec.New(dialResult.conn), nil)
 	client.Start(ctx)
 
 	bakeryClient := opts.BakeryClient

diff --git a/api/testing/fakeserver.go b/api/testing/fakeserver.go
@@ -18,7 +18,7 @@ func FakeAPIServer(root interface{}) net.Conn {
 	c0, c1 := net.Pipe()
 	serverCodec := jsoncodec.NewNet(c1)
 	serverRPC := rpc.NewConn(serverCodec, nil)
-	serverRPC.Serve(root, nil)
+	serverRPC.Serve(root, nil, nil)
 	serverRPC.Start(context.TODO())
 	go func() {
 		<-serverRPC.Dead()

diff --git a/apiserver/admin.go b/apiserver/admin.go
@@ -20,6 +20,7 @@ import (
 	"github.com/juju/juju/apiserver/facades/agent/presence"
 	"github.com/juju/juju/apiserver/observer"
 	"github.com/juju/juju/apiserver/params"
+	"github.com/juju/juju/core/auditlog"
 	"github.com/juju/juju/permission"
 	"github.com/juju/juju/rpc"
 	"github.com/juju/juju/rpc/rpcreflect"
@@ -51,12 +52,12 @@ func newAdminAPIV3(srv *Server, root *apiHandler, apiObserver observer.Observer)
 
 // Admin returns an object that provides API access to methods that can be
 // called even when not authenticated.
-func (r *admin) Admin(id string) (*admin, error) {
+func (a *admin) Admin(id string) (*admin, error) {
 	if id != "" {
 		// Safeguard id for possible future use.
 		return nil, common.ErrBadId
 	}
-	return r, nil
+	return a, nil
 }
 
 // Login logs in with the provided credentials.  All subsequent requests on the
@@ -135,7 +136,15 @@ func (a *admin) login(req params.LoginRequest, loginVersion int) (params.LoginRe
 		modelTag = a.root.model.Tag().String()
 	}
 
-	a.root.rpcConn.ServeRoot(apiRoot, serverError)
+	auditRecorder, err := a.getAuditRecorder(req, authResult)
+	if err != nil {
+		return fail, errors.Trace(err)
+	}
+
+	recorderFactory := observer.NewRecorderFactory(
+		a.apiObserver, auditRecorder)
+
+	a.root.rpcConn.ServeRoot(apiRoot, recorderFactory, serverError)
 	return params.LoginResult{
 		Servers:       params.FromNetworkHostsPorts(hostPorts),
 		ControllerTag: a.root.model.ControllerTag().String(),
@@ -147,6 +156,28 @@ func (a *admin) login(req params.LoginRequest, loginVersion int) (params.LoginRe
 	}, nil
 }
 
+func (a *admin) getAuditRecorder(req params.LoginRequest, authResult *authResult) (*auditlog.Recorder, error) {
+	if !authResult.userLogin || a.srv.auditLogger == nil {
+		return nil, nil
+	}
+	result, err := auditlog.NewRecorder(
+		a.srv.auditLogger,
+		a.srv.clock,
+		auditlog.ConversationArgs{
+			Who:          req.AuthTag,
+			What:         req.CLIArgs,
+			ModelName:    a.root.model.Name(),
+			ModelUUID:    a.root.model.UUID(),
+			ConnectionID: a.root.connectionID,
+		},
+	)
+	if err != nil {
+		logger.Errorf("couldn't add login to audit log: %+v", err)
+		return nil, errors.Trace(err)
+	}
+	return result, nil
+}
+
 type authResult struct {
 	tag                    names.Tag // nil if external user login
 	anonymousLogin         bool

diff --git a/apiserver/admin_test.go b/apiserver/admin_test.go
@@ -27,7 +27,9 @@ import (
 	"github.com/juju/juju/apiserver/common"
 	"github.com/juju/juju/apiserver/facades/client/controller"
 	"github.com/juju/juju/apiserver/params"
+	servertesting "github.com/juju/juju/apiserver/testing"
 	"github.com/juju/juju/constraints"
+	"github.com/juju/juju/core/auditlog"
 	jujutesting "github.com/juju/juju/juju/testing"
 	"github.com/juju/juju/network"
 	"github.com/juju/juju/permission"
@@ -41,12 +43,6 @@ type baseLoginSuite struct {
 	jujutesting.JujuConnSuite
 }
 
-type loginSuite struct {
-	baseLoginSuite
-}
-
-var _ = gc.Suite(&loginSuite{})
-
 func (s *baseLoginSuite) SetUpTest(c *gc.C) {
 	s.JujuConnSuite.SetUpTest(c)
 	loggo.GetLogger("juju.apiserver").SetLogLevel(loggo.TRACE)
@@ -62,6 +58,43 @@ func (s *baseLoginSuite) newMachineAndServer(c *gc.C) (*api.Info, *apiserver.Ser
 	return info, srv
 }
 
+func (s *baseLoginSuite) loginHostPorts(c *gc.C, info *api.Info) (connectedAddr string, hostPorts [][]network.HostPort) {
+	st, err := api.Open(info, fastDialOpts)
+	c.Assert(err, jc.ErrorIsNil)
+	defer st.Close()
+	return st.Addr(), st.APIHostPorts()
+}
+
+func (s *baseLoginSuite) addMachine(c *gc.C, job state.MachineJob) (*state.Machine, string) {
+	machine, err := s.State.AddMachine("quantal", job)
+	c.Assert(err, jc.ErrorIsNil)
+	password, err := utils.RandomPassword()
+	c.Assert(err, jc.ErrorIsNil)
+	err = machine.SetPassword(password)
+	c.Assert(err, jc.ErrorIsNil)
+	err = machine.SetProvisioned("foo", "fake_nonce", nil)
+	c.Assert(err, jc.ErrorIsNil)
+	return machine, password
+}
+
+func (s *baseLoginSuite) openAPIWithoutLogin(c *gc.C, info0 *api.Info) api.Connection {
+	info := *info0
+	info.Tag = nil
+	info.Password = ""
+	info.SkipLogin = true
+	info.Macaroons = nil
+	st, err := api.Open(&info, fastDialOpts)
+	c.Assert(err, jc.ErrorIsNil)
+	s.AddCleanup(func(*gc.C) { st.Close() })
+	return st
+}
+
+type loginSuite struct {
+	baseLoginSuite
+}
+
+var _ = gc.Suite(&loginSuite{})
+
 func (s *loginSuite) TestLoginWithInvalidTag(c *gc.C) {
 	info := s.APIInfo(c)
 	info.Tag = nil
@@ -210,13 +243,6 @@ func (s *loginSuite) TestLoginAddrs(c *gc.C) {
 	c.Assert(hostPorts, gc.DeepEquals, stateAPIHostPorts)
 }
 
-func (s *baseLoginSuite) loginHostPorts(c *gc.C, info *api.Info) (connectedAddr string, hostPorts [][]network.HostPort) {
-	st, err := api.Open(info, fastDialOpts)
-	c.Assert(err, jc.ErrorIsNil)
-	defer st.Close()
-	return st.Addr(), st.APIHostPorts()
-}
-
 func startNLogins(c *gc.C, n int, info *api.Info) (chan error, *sync.WaitGroup) {
 	errResults := make(chan error, 100)
 	var doneWG sync.WaitGroup
@@ -541,30 +567,6 @@ func (s *loginSuite) TestControllerMachineLoginDuringMaintenance(c *gc.C) {
 	c.Assert(st.Close(), jc.ErrorIsNil)
 }
 
-func (s *baseLoginSuite) addMachine(c *gc.C, job state.MachineJob) (*state.Machine, string) {
-	machine, err := s.State.AddMachine("quantal", job)
-	c.Assert(err, jc.ErrorIsNil)
-	password, err := utils.RandomPassword()
-	c.Assert(err, jc.ErrorIsNil)
-	err = machine.SetPassword(password)
-	c.Assert(err, jc.ErrorIsNil)
-	err = machine.SetProvisioned("foo", "fake_nonce", nil)
-	c.Assert(err, jc.ErrorIsNil)
-	return machine, password
-}
-
-func (s *baseLoginSuite) openAPIWithoutLogin(c *gc.C, info0 *api.Info) api.Connection {
-	info := *info0
-	info.Tag = nil
-	info.Password = ""
-	info.SkipLogin = true
-	info.Macaroons = nil
-	st, err := api.Open(&info, fastDialOpts)
-	c.Assert(err, jc.ErrorIsNil)
-	s.AddCleanup(func(*gc.C) { st.Close() })
-	return st
-}
-
 func (s *loginSuite) TestAnonymousModelLogin(c *gc.C) {
 	info, srv := newServer(c, s.StatePool)
 	defer assertStop(c, srv)
@@ -922,6 +924,71 @@ func (s *loginSuite) TestLoginUpdatesLastLoginAndConnection(c *gc.C) {
 	c.Assert(when.After(startTime), jc.IsTrue)
 }
 
+func (s *loginSuite) TestLoginAddsAuditConversation(c *gc.C) {
+	log := &servertesting.FakeAuditLog{}
+	cfg := defaultServerConfig(c)
+	cfg.AuditLogConfig.Enabled = true
+	cfg.AuditLog = log
+	info, srv := newServerWithConfig(c, s.StatePool, cfg)
+	defer assertStop(c, srv)
+
+	password := "shhh..."
+	user := s.Factory.MakeUser(c, &factory.UserParams{
+		Password: password,
+	})
+	conn := s.openAPIWithoutLogin(c, info)
+
+	var result params.LoginResult
+	request := &params.LoginRequest{
+		AuthTag:     user.Tag().String(),
+		Credentials: password,
+		CLIArgs:     "hey you guys",
+	}
+	err := conn.APICall("Admin", 3, "", "Login", request, &result)
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(result.UserInfo, gc.NotNil)
+
+	log.CheckCallNames(c, "AddConversation")
+	convo := log.Calls()[0].Args[0].(auditlog.Conversation)
+	// Blank out unknown fields.
+	convo.ConversationID = "0123456789abcdef"
+	convo.ConnectionID = "something"
+	c.Assert(convo, gc.Equals, auditlog.Conversation{
+		Who:            user.Tag().String(),
+		What:           "hey you guys",
+		When:           cfg.Clock.Now().Format(time.RFC3339),
+		ModelName:      s.IAASModel.Name(),
+		ModelUUID:      s.IAASModel.UUID(),
+		ConnectionID:   "something",
+		ConversationID: "0123456789abcdef",
+	})
+}
+
+func (s *loginSuite) TestAuditLoggingFailurePreventsLogin(c *gc.C) {
+	log := &servertesting.FakeAuditLog{}
+	log.SetErrors(errors.Errorf("bad news bears"))
+	cfg := defaultServerConfig(c)
+	cfg.AuditLogConfig.Enabled = true
+	cfg.AuditLog = log
+	info, srv := newServerWithConfig(c, s.StatePool, cfg)
+	defer assertStop(c, srv)
+
+	password := "shhh..."
+	user := s.Factory.MakeUser(c, &factory.UserParams{
+		Password: password,
+	})
+	conn := s.openAPIWithoutLogin(c, info)
+
+	var result params.LoginResult
+	request := &params.LoginRequest{
+		AuthTag:     user.Tag().String(),
+		Credentials: password,
+		CLIArgs:     "hey you guys",
+	}
+	err := conn.APICall("Admin", 3, "", "Login", request, &result)
+	c.Assert(err, gc.ErrorMatches, "bad news bears")
+}
+
 var _ = gc.Suite(&macaroonLoginSuite{})
 
 type macaroonLoginSuite struct {
@@ -1251,7 +1318,7 @@ func (s *migrationSuite) TestExportingModel(c *gc.C) {
 }
 
 type loginV3Suite struct {
-	loginSuite
+	baseLoginSuite
 }
 
 var _ = gc.Suite(&loginV3Suite{})