From 875e0736ec990be6369de8e643bfcf4287a6e48a Mon Sep 17 00:00:00 2001 From: Selene Date: Tue, 1 Feb 2022 12:03:21 +0100 Subject: [PATCH] LDAP: Use an interface instead of a bus to get group teams (#42165) * Remove bus for GetTeams for LDAP * Fix lint --- pkg/api/common_test.go | 5 +++-- pkg/api/http_server.go | 5 ++++- pkg/api/ldap_debug.go | 7 ++----- pkg/api/ldap_debug_test.go | 9 ++------- pkg/models/user_auth.go | 5 ----- pkg/server/wireexts_oss.go | 3 +++ pkg/services/ldap/ldap_groups.go | 17 +++++++++++++++++ 7 files changed, 31 insertions(+), 20 deletions(-) create mode 100644 pkg/services/ldap/ldap_groups.go diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index 28b6781b5c6..ed2e956e096 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -11,8 +11,6 @@ import ( "path/filepath" "testing" - "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" @@ -31,6 +29,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/quota" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/services/searchusers" @@ -38,6 +37,7 @@ import ( "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" + "github.com/stretchr/testify/require" ) func loggedInUserScenario(t *testing.T, desc string, url string, routePattern string, fn scenarioFunc) { @@ -230,6 +230,7 @@ func setupAccessControlScenarioContext(t *testing.T, cfg *setting.Cfg, url strin RouteRegister: routing.NewRouteRegister(), AccessControl: accesscontrolmock.New().WithPermissions(permissions), searchUsersService: searchusers.ProvideUsersService(bus, filters.ProvideOSSSearchUserFilter()), + ldapGroups: ldap.ProvideGroupsService(), } sc := setupScenarioContext(t, url) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 114fa95141b..5a33efa7706 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -38,6 +38,7 @@ import ( "github.com/grafana/grafana/pkg/services/encryption" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/hooks" + "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/libraryelements" "github.com/grafana/grafana/pkg/services/librarypanels" "github.com/grafana/grafana/pkg/services/live" @@ -122,6 +123,7 @@ type HTTPServer struct { grafanaUpdateChecker *updatechecker.GrafanaService pluginsUpdateChecker *updatechecker.PluginsService searchUsersService searchusers.Service + ldapGroups ldap.Groups teamGuardian teamguardian.TeamGuardian queryDataService *query.Service serviceAccountsService serviceaccounts.Service @@ -152,7 +154,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi encryptionService encryption.Internal, grafanaUpdateChecker *updatechecker.GrafanaService, pluginsUpdateChecker *updatechecker.PluginsService, searchUsersService searchusers.Service, dataSourcesService *datasources.Service, secretsService secrets.Service, queryDataService *query.Service, - teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, + ldapGroups ldap.Groups, teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service, authInfoService authinfoservice.Service, resourcePermissionServices *resourceservices.ResourceServices) (*HTTPServer, error) { web.Env = cfg.Env m := web.New() @@ -207,6 +209,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi SecretsService: secretsService, DataSourcesService: dataSourcesService, searchUsersService: searchUsersService, + ldapGroups: ldapGroups, teamGuardian: teamGuardian, queryDataService: queryDataService, serviceAccountsService: serviceaccountsService, diff --git a/pkg/api/ldap_debug.go b/pkg/api/ldap_debug.go index c6a30b016da..ad223cdb66f 100644 --- a/pkg/api/ldap_debug.go +++ b/pkg/api/ldap_debug.go @@ -311,14 +311,11 @@ func (hs *HTTPServer) GetUserFromLDAP(c *models.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "An organization was not found - Please verify your LDAP configuration", err) } - cmd := &models.GetTeamsForLDAPGroupCommand{Groups: user.Groups} - err = bus.Dispatch(c.Req.Context(), cmd) - if err != nil && !errors.Is(err, bus.ErrHandlerNotFound) { + u.Teams, err = hs.ldapGroups.GetTeams(user.Groups) + if err != nil { return response.Error(http.StatusBadRequest, "Unable to find the teams for this user", err) } - u.Teams = cmd.Result - return response.JSON(200, u) } diff --git a/pkg/api/ldap_debug_test.go b/pkg/api/ldap_debug_test.go index c3f8dd9dc56..9cb6e339723 100644 --- a/pkg/api/ldap_debug_test.go +++ b/pkg/api/ldap_debug_test.go @@ -62,7 +62,7 @@ func getUserFromLDAPContext(t *testing.T, requestURL string) *scenarioContext { setting.LDAPEnabled = true t.Cleanup(func() { setting.LDAPEnabled = origLDAP }) - hs := &HTTPServer{Cfg: setting.NewCfg()} + hs := &HTTPServer{Cfg: setting.NewCfg(), ldapGroups: ldap.ProvideGroupsService()} sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response { sc.context = c @@ -274,11 +274,6 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { return nil }) - bus.AddHandler("test", func(ctx context.Context, cmd *models.GetTeamsForLDAPGroupCommand) error { - cmd.Result = []models.TeamOrgGroupDTO{} - return nil - }) - getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { return &ldap.Config{}, nil } @@ -310,7 +305,7 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) { "roles": [ { "orgId": 1, "orgRole": "Admin", "orgName": "Main Org.", "groupDN": "cn=admins,ou=groups,dc=grafana,dc=org" } ], - "teams": [] + "teams": null } ` diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go index c7f2b01e878..221b899c0e1 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -120,8 +120,3 @@ type TeamOrgGroupDTO struct { OrgName string `json:"orgName"` GroupDN string `json:"groupDN"` } - -type GetTeamsForLDAPGroupCommand struct { - Groups []string - Result []TeamOrgGroupDTO -} diff --git a/pkg/server/wireexts_oss.go b/pkg/server/wireexts_oss.go index 20a47214160..620c1b57107 100644 --- a/pkg/server/wireexts_oss.go +++ b/pkg/server/wireexts_oss.go @@ -21,6 +21,7 @@ import ( "github.com/grafana/grafana/pkg/services/encryption/ossencryption" "github.com/grafana/grafana/pkg/services/kmsproviders" "github.com/grafana/grafana/pkg/services/kmsproviders/osskmsproviders" + "github.com/grafana/grafana/pkg/services/ldap" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/authinfoservice" @@ -70,6 +71,8 @@ var wireExtsBasicSet = wire.NewSet( wire.Bind(new(accesscontrol.PermissionsProvider), new(*acdb.AccessControlStore)), osskmsproviders.ProvideService, wire.Bind(new(kmsproviders.Service), new(osskmsproviders.Service)), + ldap.ProvideGroupsService, + wire.Bind(new(ldap.Groups), new(*ldap.OSSGroups)), ) var wireExtsSet = wire.NewSet( diff --git a/pkg/services/ldap/ldap_groups.go b/pkg/services/ldap/ldap_groups.go new file mode 100644 index 00000000000..3406947660b --- /dev/null +++ b/pkg/services/ldap/ldap_groups.go @@ -0,0 +1,17 @@ +package ldap + +import "github.com/grafana/grafana/pkg/models" + +type Groups interface { + GetTeams(groups []string) ([]models.TeamOrgGroupDTO, error) +} + +type OSSGroups struct{} + +func ProvideGroupsService() *OSSGroups { + return &OSSGroups{} +} + +func (*OSSGroups) GetTeams(_ []string) ([]models.TeamOrgGroupDTO, error) { + return nil, nil +}