diff --git a/pkg/services/ldap/ldap.go b/pkg/services/ldap/ldap.go index b400fafa815..d079d632f25 100644 --- a/pkg/services/ldap/ldap.go +++ b/pkg/services/ldap/ldap.go @@ -46,6 +46,9 @@ type Server struct { // Bind authenticates the connection with the LDAP server // - with the username and password setup in the config // - or, anonymously +// +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) Bind() error { if server.shouldAdminBind() { if err := server.AdminBind(); err != nil { @@ -139,6 +142,8 @@ func (server *Server) Dial() error { } // Close closes the LDAP connection +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) Close() { server.Connection.Close() } @@ -158,6 +163,9 @@ func (server *Server) Close() { // user without login/password binding with LDAP server, in such case // we will perform "unauthenticated bind", then search for the // targeted user and then perform the bind with passed login/password. +// +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) Login(query *models.LoginUserQuery) ( *models.ExternalUserInfo, error, ) { @@ -231,6 +239,8 @@ func (server *Server) shouldSingleBind() bool { } // Users gets LDAP users by logins +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) Users(logins []string) ( []*models.ExternalUserInfo, error, @@ -414,6 +424,8 @@ func (server *Server) buildGrafanaUser(user *ldap.Entry) (*models.ExternalUserIn } // UserBind binds the user with the LDAP server +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) UserBind(username, password string) error { err := server.userBind(username, password) if err != nil { @@ -429,6 +441,8 @@ func (server *Server) UserBind(username, password string) error { } // AdminBind binds "admin" user with LDAP +// Dial() sets the connection with the server for this Struct. Therefore, we require a +// call to Dial() before being able to execute this function. func (server *Server) AdminBind() error { err := server.userBind(server.Config.BindDN, server.Config.BindPassword) if err != nil { diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 9f89bf43833..ea1fd049bf3 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -4,10 +4,9 @@ import ( "errors" "testing" + "github.com/grafana/grafana/pkg/infra/log" . "github.com/smartystreets/goconvey/convey" "gopkg.in/ldap.v3" - - "github.com/grafana/grafana/pkg/infra/log" ) func TestPublicAPI(t *testing.T) { @@ -22,6 +21,34 @@ func TestPublicAPI(t *testing.T) { }) }) + Convey("Close()", t, func() { + Convey("Should close the connection", func() { + connection := &MockConnection{} + + server := &Server{ + Config: &ServerConfig{ + Attr: AttributeMap{}, + SearchBaseDNs: []string{"BaseDNHere"}, + }, + Connection: connection, + } + + So(server.Close, ShouldNotPanic) + So(connection.CloseCalled, ShouldBeTrue) + }) + + Convey("Should panic if no connection is established", func() { + server := &Server{ + Config: &ServerConfig{ + Attr: AttributeMap{}, + SearchBaseDNs: []string{"BaseDNHere"}, + }, + Connection: nil, + } + + So(server.Close, ShouldPanic) + }) + }) Convey("Users()", t, func() { Convey("Finds one user", func() { MockConnection := &MockConnection{} diff --git a/pkg/services/ldap/testing.go b/pkg/services/ldap/testing.go index 7e0b33fcbd5..8bad83a2d92 100644 --- a/pkg/services/ldap/testing.go +++ b/pkg/services/ldap/testing.go @@ -19,6 +19,8 @@ type MockConnection struct { DelParams *ldap.DelRequest DelCalled bool + CloseCalled bool + UnauthenticatedBindCalled bool BindCalled bool @@ -49,7 +51,9 @@ func (c *MockConnection) UnauthenticatedBind(username string) error { } // Close mocks Close connection function -func (c *MockConnection) Close() {} +func (c *MockConnection) Close() { + c.CloseCalled = true +} func (c *MockConnection) setSearchResult(result *ldap.SearchResult) { c.SearchResult = result diff --git a/pkg/services/multildap/multildap.go b/pkg/services/multildap/multildap.go index 953654ff483..b79a21e0417 100644 --- a/pkg/services/multildap/multildap.go +++ b/pkg/services/multildap/multildap.go @@ -85,13 +85,12 @@ func (multiples *MultiLDAP) Ping() ([]*ServerStatus, error) { if err == nil { status.Available = true serverStatuses = append(serverStatuses, status) + server.Close() } else { status.Available = false status.Error = err serverStatuses = append(serverStatuses, status) } - - defer server.Close() } return serverStatuses, nil diff --git a/pkg/services/multildap/multildap_test.go b/pkg/services/multildap/multildap_test.go index 89877320847..6554bdbaabf 100644 --- a/pkg/services/multildap/multildap_test.go +++ b/pkg/services/multildap/multildap_test.go @@ -40,11 +40,12 @@ func TestMultiLDAP(t *testing.T) { So(statuses[0].Port, ShouldEqual, 361) So(statuses[0].Available, ShouldBeFalse) So(statuses[0].Error, ShouldEqual, expectedErr) + So(mock.closeCalledTimes, ShouldEqual, 0) teardown() }) - Convey("Shoudl get the LDAP server statuses", func() { - setup() + Convey("Should get the LDAP server statuses", func() { + mock := setup() multi := New([]*ldap.ServerConfig{ {Host: "10.0.0.1", Port: 361}, @@ -57,6 +58,7 @@ func TestMultiLDAP(t *testing.T) { So(statuses[0].Port, ShouldEqual, 361) So(statuses[0].Available, ShouldBeTrue) So(statuses[0].Error, ShouldBeNil) + So(mock.closeCalledTimes, ShouldEqual, 1) teardown() })