Auth: Keep config in a separate struct in LDAP (#89149)

keep config in a separate struct in LDAP
This commit is contained in:
Mihai Doarna
2024-06-17 15:34:06 +03:00
committed by GitHub
parent 94e6bcd329
commit 32d21356b9
13 changed files with 157 additions and 126 deletions
+11 -9
View File
@@ -29,7 +29,8 @@ import (
)
type Service struct {
cfg *setting.Cfg
cfg *ldap.Config
adminUser string
userService user.Service
authInfoService login.AuthInfoService
ldapGroupsService ldap.Groups
@@ -47,7 +48,8 @@ func ProvideService(
sessionService auth.UserTokenService, bundleRegistry supportbundles.Service,
) *Service {
s := &Service{
cfg: cfg,
cfg: ldap.GetLDAPConfig(cfg),
adminUser: cfg.AdminUser,
userService: userService,
authInfoService: authInfoService,
ldapGroupsService: ldapGroupsService,
@@ -96,7 +98,7 @@ func ProvideService(
// 403: forbiddenError
// 500: internalServerError
func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response {
if !s.cfg.LDAPAuthEnabled {
if !s.cfg.Enabled {
return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil)
}
@@ -122,7 +124,7 @@ func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response {
// 403: forbiddenError
// 500: internalServerError
func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response {
if !s.cfg.LDAPAuthEnabled {
if !s.cfg.Enabled {
return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil)
}
@@ -169,7 +171,7 @@ func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response {
// 403: forbiddenError
// 500: internalServerError
func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Response {
if !s.cfg.LDAPAuthEnabled {
if !s.cfg.Enabled {
return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil)
}
@@ -206,7 +208,7 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp
userInfo, _, err := ldapClient.User(usr.Login)
if err != nil {
if errors.Is(err, multildap.ErrDidNotFindUser) { // User was not in the LDAP server - we need to take action:
if s.cfg.AdminUser == usr.Login { // User is *the* Grafana Admin. We cannot disable it.
if s.adminUser == usr.Login { // User is *the* Grafana Admin. We cannot disable it.
errMsg := fmt.Sprintf(`Refusing to sync grafana super admin "%s" - it would be disabled`, usr.Login)
s.log.Error(errMsg)
return response.Error(http.StatusBadRequest, errMsg, err)
@@ -250,7 +252,7 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp
// 403: forbiddenError
// 500: internalServerError
func (s *Service) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response {
if !s.cfg.LDAPAuthEnabled {
if !s.cfg.Enabled {
return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil)
}
@@ -330,8 +332,8 @@ func (s *Service) identityFromLDAPUser(user *login.ExternalUserInfo) *authn.Iden
SyncUser: true,
SyncTeams: true,
EnableUser: true,
SyncOrgRoles: !s.cfg.LDAPSkipOrgRoleSync,
AllowSignUp: s.cfg.LDAPAllowSignup,
SyncOrgRoles: !s.cfg.SkipOrgRoleSync,
AllowSignUp: s.cfg.AllowSignUp,
},
}
}
+12 -12
View File
@@ -95,7 +95,7 @@ func TestGetUserFromLDAPAPIEndpoint_UserNotFound(t *testing.T) {
ExpectedClient: &LDAPMock{
UserSearchResult: nil,
},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -160,7 +160,7 @@ func TestGetUserFromLDAPAPIEndpoint_OrgNotfound(t *testing.T) {
UserSearchResult: userSearchResult,
UserSearchConfig: userSearchConfig,
},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -229,7 +229,7 @@ func TestGetUserFromLDAPAPIEndpoint(t *testing.T) {
UserSearchResult: userSearchResult,
UserSearchConfig: userSearchConfig,
},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -314,7 +314,7 @@ func TestGetUserFromLDAPAPIEndpoint_WithTeamHandler(t *testing.T) {
UserSearchResult: userSearchResult,
UserSearchConfig: userSearchConfig,
},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -368,7 +368,7 @@ func TestGetLDAPStatusAPIEndpoint(t *testing.T) {
_, server := setupAPITest(t, func(a *Service) {
a.ldapService = &service.LDAPFakeService{
ExpectedClient: &LDAPMock{},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -407,7 +407,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_Success(t *testing.T) {
ExpectedClient: &LDAPMock{UserSearchResult: &login.ExternalUserInfo{
Login: "ldap-daniel",
}},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -442,7 +442,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotFound(t *testing.T) {
a.userService = userServiceMock
a.ldapService = &service.LDAPFakeService{
ExpectedClient: &LDAPMock{},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -475,10 +475,10 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenGrafanaAdmin(t *testing.T) {
_, server := setupAPITest(t, func(a *Service) {
a.userService = userServiceMock
a.cfg.AdminUser = "ldap-daniel"
a.adminUser = "ldap-daniel"
a.ldapService = &service.LDAPFakeService{
ExpectedClient: &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -511,7 +511,7 @@ func TestPostSyncUserWithLDAPAPIEndpoint_WhenUserNotInLDAP(t *testing.T) {
a.authInfoService = &authinfotest.FakeService{ExpectedExternalUser: &login.ExternalUserInfo{IsDisabled: true, UserId: 34}}
a.ldapService = &service.LDAPFakeService{
ExpectedClient: &LDAPMock{UserSearchError: multildap.ErrDidNotFindUser},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
@@ -641,12 +641,12 @@ search_base_dns = ["dc=grafana,dc=org"]`)
t.Run(tt.desc, func(t *testing.T) {
_, server := setupAPITest(t, func(a *Service) {
a.userService = &usertest.FakeUserService{ExpectedUser: &user.User{Login: "ldap-daniel", ID: 1}}
a.cfg.LDAPConfigFilePath = ldapConfigFile
a.cfg.ConfigFilePath = ldapConfigFile
a.ldapService = &service.LDAPFakeService{
ExpectedClient: &LDAPMock{UserSearchResult: &login.ExternalUserInfo{
Login: "ldap-daniel",
}},
ExpectedConfig: &ldap.Config{},
ExpectedConfig: &ldap.ServersConfig{},
}
})
// Add minimal setup to pass handler
+6 -6
View File
@@ -73,12 +73,12 @@ func (s *Service) supportBundleCollector(context.Context) (*supportbundles.Suppo
bWriter.WriteString("```ini\n")
bWriter.WriteString(fmt.Sprintf("enabled = %v\n", s.cfg.LDAPAuthEnabled))
bWriter.WriteString(fmt.Sprintf("config_file = %s\n", s.cfg.LDAPConfigFilePath))
bWriter.WriteString(fmt.Sprintf("allow_sign_up = %v\n", s.cfg.LDAPAllowSignup))
bWriter.WriteString(fmt.Sprintf("sync_cron = %s\n", s.cfg.LDAPSyncCron))
bWriter.WriteString(fmt.Sprintf("active_sync_enabled = %v\n", s.cfg.LDAPActiveSyncEnabled))
bWriter.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", s.cfg.LDAPSkipOrgRoleSync))
bWriter.WriteString(fmt.Sprintf("enabled = %v\n", s.cfg.Enabled))
bWriter.WriteString(fmt.Sprintf("config_file = %s\n", s.cfg.ConfigFilePath))
bWriter.WriteString(fmt.Sprintf("allow_sign_up = %v\n", s.cfg.AllowSignUp))
bWriter.WriteString(fmt.Sprintf("sync_cron = %s\n", s.cfg.SyncCron))
bWriter.WriteString(fmt.Sprintf("active_sync_enabled = %v\n", s.cfg.ActiveSyncEnabled))
bWriter.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", s.cfg.SkipOrgRoleSync))
bWriter.WriteString("```\n\n")