second pass

SCIM-add-reloadable-api-config
colin-stuart 3 months ago
parent 50e533ca16
commit 37e6f7fdfe
  1. 588
      pkg/api/scim_settings_api_test.go
  2. 174
      pkg/api/scim_settings_integration_test.go
  3. 8
      pkg/server/wire.go
  4. 1
      pkg/services/featuremgmt/featuremgmttest/feature_toggle.go
  5. 31
      pkg/services/scimsettings/database/database.go
  6. 3
      pkg/services/scimsettings/models/scim_settings.go
  7. 7
      pkg/services/scimsettings/scimsettingsimpl/scim_settings.go
  8. 32
      pkg/services/scimsettings/scimsettingsimpl/scim_settings_test.go
  9. 37
      pkg/services/scimsettings/scimsettingstest/mock_service.go
  10. 18
      pkg/services/social/social.go
  11. 11
      pkg/services/sqlstore/migrations/9999999999_create_scim_settings_table.mysql.sql
  12. 11
      pkg/services/sqlstore/migrations/9999999999_create_scim_settings_table.postgres.sql
  13. 11
      pkg/services/sqlstore/migrations/9999999999_create_scim_settings_table.sql

@ -2,297 +2,74 @@ package api
import (
"bytes"
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"gopkg.in/ini.v1"
"gopkg.in/macaron.v1"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/db/dbtest"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/infra/log/logtest"
"github.com/grafana/grafana/pkg/middleware"
"github.com/grafana/grafana/pkg/models" // Alias required due to scimmodels
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/scimsettings"
scimmodels "github.com/grafana/grafana/pkg/services/scimsettings/models"
"github.com/grafana/grafana/pkg/services/scimsettings/scimsettingstest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
"github.com/grafana/grafana/pkg/web/webtest"
)
const (
testOrgID = 1
testUserID = 1
scimAPIPath = "/api/scim/settings"
)
// setupTestServer initializes the API with mocks and sets up a test server.
func setupTestServer(t *testing.T, cfg *setting.Cfg, features *featuremgmt.FeatureManager) *SCIMSettingsAPI {
func setupSCIMSettingsAPITest(t *testing.T, scimEnabled bool, mockService *scimsettingstest.MockService) *webtest.Server {
t.Helper()
routeRegister := routing.NewRouteRegister()
// Mock middleware dependencies if needed, here we just use a basic logger
cfg.Logger = log.NewNopLogger()
middleware.Provide(cfg) // Initialize necessary middleware
api := ProvideSCIMSettingsAPI(cfg, routeRegister, features)
api.RegisterAPIEndpoints()
// We return the API instance directly for handler testing
// Route registration testing needs the routeRegister
return api
}
// newTestContext creates a new ReqContext for testing handlers.
func newTestContext(method, path string, body []byte) *contextmodel.ReqContext {
req := httptest.NewRequest(method, path, bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
resp := httptest.NewRecorder()
mac := web.NewMacaron() // Use web.NewMacaron() or appropriate context setup
// Setup routing context if needed by handlers or middleware
mac.Use(middleware.ReqSignedIn) // Basic middleware for context setup
mac.Use(middleware.ReqGrafanaAdmin) // Ensure admin check is part of context setup for testing
// Create a base context with necessary values
baseCtx := &contextmodel.ReqContext{
Context: context.Background(),
Req: req,
Resp: web.NewResponseWriter(method, resp),
SignedInUser: &contextmodel.SignedInUser{UserID: testUserID, OrgID: testOrgID, IsGrafanaAdmin: true}, // Assume admin for most tests initially
AllowAnonymous: false,
}
// If routing parameters are needed, they would be set here via web.Params or context
// Example: req = req.WithContext(context.WithValue(req.Context(), web.ParamsKey, map[string]string{"id": "1"}))
return baseCtx
features := featuremgmt.WithFeatures()
if scimEnabled {
features = featuremgmt.WithFeatures(featuremgmt.FlagEnableSCIM)
}
func TestRegisterAPIEndpoints(t *testing.T) {
t.Run("should not register routes if enableSCIM feature toggle is disabled", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: false})
routeRegister := routing.NewRouteRegister()
middleware.Provide(cfg)
api := ProvideSCIMSettingsAPI(cfg, routeRegister, features)
api.RegisterAPIEndpoints()
routes := routeRegister.GetRoutes()
assert.Len(t, routes, 0, "Expected no routes to be registered")
})
t.Run("should register routes if enableSCIM feature toggle is enabled", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: true})
routeRegister := routing.NewRouteRegister()
middleware.Provide(cfg)
api := ProvideSCIMSettingsAPI(cfg, routeRegister, features)
api := ProvideSCIMSettingsAPI(cfg, routeRegister, features.(*featuremgmt.FeatureManager), mockService)
api.RegisterAPIEndpoints()
routes := routeRegister.GetRoutes()
require.Len(t, routes, 1, "Expected one route group to be registered")
group := routes[0]
require.Len(t, group.Routes, 2, "Expected two routes within the group")
getRoute := group.Routes[0]
putRoute := group.Routes[1]
assert.Equal(t, http.MethodGet, getRoute.Method)
assert.Equal(t, scimAPIPath, getRoute.Pattern)
// Note: Checking exact middleware function pointers can be brittle.
// We assume ReqGrafanaAdmin is correctly applied if routes are registered.
assert.Equal(t, http.MethodPut, putRoute.Method)
assert.Equal(t, scimAPIPath, putRoute.Pattern)
})
}
func TestGetSCIMSettings(t *testing.T) {
t.Run("when user is Grafana Admin", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: true})
api := setupTestServer(t, cfg, features)
t.Run("should return settings from config when section and keys exist", func(t *testing.T) {
// Prepare INI data
iniData := `
[auth.scim]
user_sync_enabled = true
group_sync_enabled = true
`
rawIni, err := ini.Load([]byte(iniData))
require.NoError(t, err)
api.cfg.Raw = rawIni
c := newTestContext(http.MethodGet, scimAPIPath, nil)
c.SignedInUser.IsGrafanaAdmin = true
resp := api.getSCIMSettings(c)
require.Equal(t, http.StatusOK, resp.Status())
var result scimSettingsDTO
require.NoError(t, json.Unmarshal(resp.Body(), &result))
assert.True(t, result.UserSyncEnabled)
assert.True(t, result.GroupSyncEnabled)
})
t.Run("should return default false when section or keys are missing", func(t *testing.T) {
// Prepare empty INI data
rawIni, err := ini.Load([]byte(""))
require.NoError(t, err)
api.cfg.Raw = rawIni
c := newTestContext(http.MethodGet, scimAPIPath, nil)
c.SignedInUser.IsGrafanaAdmin = true
resp := api.getSCIMSettings(c)
require.Equal(t, http.StatusOK, resp.Status())
var result scimSettingsDTO
require.NoError(t, json.Unmarshal(resp.Body(), &result))
assert.False(t, result.UserSyncEnabled)
assert.False(t, result.GroupSyncEnabled)
})
})
t.Run("when user is not Grafana Admin", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: true})
api := setupTestServer(t, cfg, features)
rawIni, err := ini.Load([]byte("")) // Config content doesn't matter here
require.NoError(t, err)
api.cfg.Raw = rawIni
c := newTestContext(http.MethodGet, scimAPIPath, nil)
c.SignedInUser.IsGrafanaAdmin = false // Set user as non-admin
// We expect the ReqGrafanaAdmin middleware (simulated in newTestContext setup)
// to handle the authorization before the handler is called.
// In a real HTTP test, we'd check the status code directly from the response recorder.
// Since we test the handler directly, we rely on the test setup assuming middleware ran.
// A more robust test would involve a full HTTP server test.
// For now, let's just assert that if the handler *was* called (it shouldn't be),
// it would still work, but the critical part is the middleware check.
// This test mainly documents the non-admin scenario.
// If we were doing a full HTTP test:
// recorder := httptest.NewRecorder()
// req := httptest.NewRequest(http.MethodGet, scimAPIPath, nil)
// // Setup non-admin user in request context/session
// server.ServeHTTP(recorder, req)
// assert.Equal(t, http.StatusForbidden, recorder.Code)
// Direct handler call test (less ideal for middleware checks):
resp := api.getSCIMSettings(c) // This wouldn't actually be called if middleware blocks
assert.Equal(t, http.StatusOK, resp.Status()) // Shows handler logic is ok, but middleware prevents access
})
return webtest.NewServer(t, routeRegister)
}
func TestUpdateSCIMSettings(t *testing.T) {
t.Run("when user is Grafana Admin", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: true})
api := setupTestServer(t, cfg, features)
func TestSCIMSettingsAPI_RegisterAPIEndpoints(t *testing.T) {
t.Run("should not register routes if enableSCIM feature toggle is disabled", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
server := setupSCIMSettingsAPITest(t, false, mockService)
t.Run("should return success message for valid payload", func(t *testing.T) {
payload := scimSettingsDTO{
UserSyncEnabled: true,
GroupSyncEnabled: false,
}
bodyBytes, err := json.Marshal(payload)
req := server.NewGetRequest(scimAPIPath)
resp, err := server.Send(req)
require.NoError(t, err)
c := newTestContext(http.MethodPut, scimAPIPath, bodyBytes)
c.SignedInUser.IsGrafanaAdmin = true
resp := api.updateSCIMSettings(c)
require.Equal(t, http.StatusOK, resp.Status())
var result map[string]string
require.NoError(t, json.Unmarshal(resp.Body(), &result))
assert.Contains(t, result["message"], "Grafana restart is required")
// Optional: Check logs if a mock logger with capture capabilities was used
})
t.Run("should return bad request for invalid payload", func(t *testing.T) {
invalidBody := []byte(`{invalid json`) // Malformed JSON
c := newTestContext(http.MethodPut, scimAPIPath, invalidBody)
c.SignedInUser.IsGrafanaAdmin = true
resp := api.updateSCIMSettings(c)
require.Equal(t, http.StatusBadRequest, resp.Status())
// Check error message if needed
var result map[string]string
require.NoError(t, json.Unmarshal(resp.Body(), &result))
assert.Contains(t, result["message"], "Invalid request payload")
})
require.Equal(t, http.StatusNotFound, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})
t.Run("when user is not Grafana Admin", func(t *testing.T) {
cfg := setting.NewCfg()
features := fakes.NewFakeFeatureManager(map[string]bool{featuremgmt.FlagEnableSCIM: true})
api := setupTestServer(t, cfg, features)
t.Run("should register routes if enableSCIM feature toggle is enabled", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
server := setupSCIMSettingsAPITest(t, true, mockService)
payload := scimSettingsDTO{UserSyncEnabled: true, GroupSyncEnabled: true}
bodyBytes, err := json.Marshal(payload)
// When a route is registered but unauthorized, we get 401 instead of 404
req := server.NewGetRequest(scimAPIPath)
resp, err := server.Send(req)
require.NoError(t, err)
c := newTestContext(http.MethodPut, scimAPIPath, bodyBytes)
c.SignedInUser.IsGrafanaAdmin = false // Set user as non-admin
// Again, expecting middleware to block this.
// Direct handler call test:
resp := api.updateSCIMSettings(c) // This wouldn't actually be called
assert.Equal(t, http.StatusOK, resp.Status()) // Handler logic works, but access denied by middleware
// If we were doing a full HTTP test:
// recorder := httptest.NewRecorder()
// req := httptest.NewRequest(http.MethodPut, scimAPIPath, bytes.NewReader(bodyBytes))
// req.Header.Set("Content-Type", "application/json")
// // Setup non-admin user in request context/session
// server.ServeHTTP(recorder, req)
// assert.Equal(t, http.StatusForbidden, recorder.Code)
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})
}
func setupAPITest(t *testing.T) (*SCIMSettingsAPI, *scimsettings.MockService, *featuremgmt.FeatureManager) {
t.Helper()
mockService := scimsettings.NewMockService(t)
features := featuremgmt.WithFeatures(featuremgmt.FlagEnableSCIM) // Ensure feature is enabled
cfg := setting.NewCfg()
cfg.StaticRootPath = "../../public" // Adjust path if needed for renderer
cfg.Logger = logtest.NewFake(t) // Use a fake logger for API tests if needed
api := ProvideSCIMSettingsAPI(cfg, routing.NewRouteRegister(), features, mockService)
return api, mockService, features
}
func TestSCIMSettingsAPI_GetSettings(t *testing.T) {
api, mockService, features := setupAPITest(t)
server := setupTestServer(t, api, features, "/api/scim/settings", http.MethodGet, api.getSCIMSettings, middleware.ReqGrafanaAdmin)
now := time.Now()
expectedSettings := &scimmodels.ScimSettings{
ID: 1,
@ -308,92 +85,106 @@ func TestSCIMSettingsAPI_GetSettings(t *testing.T) {
t.Run("As Grafana Admin", func(t *testing.T) {
t.Run("should return settings when service returns them", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
mockService.On("Get", mock.Anything).Return(expectedSettings, nil).Once()
req, err := http.NewRequest(http.MethodGet, "/api/scim/settings", nil)
require.NoError(t, err)
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
server := setupSCIMSettingsAPITest(t, true, mockService)
req := server.NewGetRequest(scimAPIPath)
api.getSCIMSettings(sc.Context)
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
require.Equal(t, http.StatusOK, sc.Resp.Code)
var respDTO scimSettingsDTO
err = json.Unmarshal(sc.Resp.Body.Bytes(), &respDTO)
resp, err := server.Send(req)
require.NoError(t, err)
assert.Equal(t, expectedDTO, respDTO)
require.Equal(t, http.StatusOK, resp.StatusCode)
var result scimSettingsDTO
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.NoError(t, resp.Body.Close())
assert.Equal(t, expectedDTO, result)
mockService.AssertExpectations(t)
})
t.Run("should return 500 if service returns an error", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
testErr := errors.New("database broke")
mockService.On("Get", mock.Anything).Return(nil, testErr).Once()
req, err := http.NewRequest(http.MethodGet, "/api/scim/settings", nil)
require.NoError(t, err)
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
server := setupSCIMSettingsAPITest(t, true, mockService)
req := server.NewGetRequest(scimAPIPath)
api.getSCIMSettings(sc.Context)
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
resp, err := server.Send(req)
require.NoError(t, err)
require.Equal(t, http.StatusInternalServerError, resp.StatusCode)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusInternalServerError, sc.Resp.Code)
// Optionally check response body for error message
mockService.AssertExpectations(t)
})
t.Run("should return 500 if service returns ErrSettingsNotFound (should be handled by service/default)", func(t *testing.T) {
// Note: Depending on Get implementation, ErrSettingsNotFound might return default values instead of an error.
// Adjust this test based on the actual service behavior for ErrSettingsNotFound.
// Assuming Get propagates the error for this test case.
t.Run("should return 500 if service returns ErrSettingsNotFound", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
mockService.On("Get", mock.Anything).Return(nil, scimsettings.ErrSettingsNotFound).Once()
req, err := http.NewRequest(http.MethodGet, "/api/scim/settings", nil)
require.NoError(t, err)
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
server := setupSCIMSettingsAPITest(t, true, mockService)
req := server.NewGetRequest(scimAPIPath)
api.getSCIMSettings(sc.Context)
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
resp, err := server.Send(req)
require.NoError(t, err)
require.Equal(t, http.StatusInternalServerError, resp.StatusCode)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusInternalServerError, sc.Resp.Code) // Or check for default DTO with 200 OK if defaults are returned
mockService.AssertExpectations(t)
})
})
t.Run("As non-Admin", func(t *testing.T) {
// Mock service call to avoid nil pointer if handler is somehow reached
mockService.On("Get", mock.Anything).Return(expectedSettings, nil).Maybe()
mockService := scimsettingstest.NewMockService(t)
req, err := http.NewRequest(http.MethodGet, "/api/scim/settings", nil)
require.NoError(t, err)
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = false // Ensure not admin
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
server := setupSCIMSettingsAPITest(t, true, mockService)
req := server.NewGetRequest(scimAPIPath)
server.Mux.ServeHTTP(sc.Resp, sc.Req)
// Create non-admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleViewer,
IsGrafanaAdmin: false,
})
require.Equal(t, http.StatusForbidden, sc.Resp.Code)
resp, err := server.Send(req)
require.NoError(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})
}
func TestSCIMSettingsAPI_UpdateSettings(t *testing.T) {
api, mockService, features := setupAPITest(t)
server := setupTestServer(t, api, features, "/api/scim/settings", http.MethodPut, api.updateSCIMSettings, middleware.ReqGrafanaAdmin)
updateCmd := scimSettingsDTO{
UserSyncEnabled: true,
GroupSyncEnabled: true,
}
cmdBytes, err := json.Marshal(updateCmd)
require.NoError(t, err)
expectedModel := &scimmodels.ScimSettings{
UserSyncEnabled: updateCmd.UserSyncEnabled,
GroupSyncEnabled: updateCmd.GroupSyncEnabled,
@ -401,170 +192,119 @@ func TestSCIMSettingsAPI_UpdateSettings(t *testing.T) {
t.Run("As Grafana Admin", func(t *testing.T) {
t.Run("should update settings when service succeeds", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
mockService.On("Update", mock.Anything, mock.MatchedBy(func(s *scimmodels.ScimSettings) bool {
return s.UserSyncEnabled == expectedModel.UserSyncEnabled && s.GroupSyncEnabled == expectedModel.GroupSyncEnabled
return s.UserSyncEnabled == expectedModel.UserSyncEnabled &&
s.GroupSyncEnabled == expectedModel.GroupSyncEnabled
})).Return(nil).Once()
req, err := http.NewRequest(http.MethodPut, "/api/scim/settings", bytes.NewReader(cmdBytes))
server := setupSCIMSettingsAPITest(t, true, mockService)
bodyBytes, err := json.Marshal(updateCmd)
require.NoError(t, err)
req := server.NewRequest(http.MethodPut, scimAPIPath, bytes.NewReader(bodyBytes))
req.Header.Set("Content-Type", "application/json")
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
api.updateSCIMSettings(sc.Context)
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
require.Equal(t, http.StatusOK, sc.Resp.Code)
var resp map[string]string
err = json.Unmarshal(sc.Resp.Body.Bytes(), &resp)
resp, err := server.Send(req)
require.NoError(t, err)
assert.Equal(t, "SCIM settings updated successfully.", resp["message"])
require.Equal(t, http.StatusOK, resp.StatusCode)
var result map[string]string
require.NoError(t, json.NewDecoder(resp.Body).Decode(&result))
require.NoError(t, resp.Body.Close())
assert.Equal(t, "SCIM settings updated successfully.", result["message"])
mockService.AssertExpectations(t)
})
t.Run("should return 400 if request body is invalid", func(t *testing.T) {
req, err := http.NewRequest(http.MethodPut, "/api/scim/settings", bytes.NewReader([]byte("{invalid json")))
require.NoError(t, err)
mockService := scimsettingstest.NewMockService(t)
// No expectations since we shouldn't call the service
server := setupSCIMSettingsAPITest(t, true, mockService)
req := server.NewRequest(http.MethodPut, scimAPIPath, bytes.NewReader([]byte("{invalid json")))
req.Header.Set("Content-Type", "application/json")
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
api.updateSCIMSettings(sc.Context)
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
resp, err := server.Send(req)
require.NoError(t, err)
require.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.NoError(t, resp.Body.Close())
require.Equal(t, http.StatusBadRequest, sc.Resp.Code)
// No calls expected to the service
mockService.AssertNotCalled(t, "Update", mock.Anything, mock.Anything)
})
t.Run("should return 500 if service update fails", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
testErr := errors.New("database broke on update")
mockService.On("Update", mock.Anything, mock.Anything).Return(testErr).Once()
req, err := http.NewRequest(http.MethodPut, "/api/scim/settings", bytes.NewReader(cmdBytes))
server := setupSCIMSettingsAPITest(t, true, mockService)
bodyBytes, err := json.Marshal(updateCmd)
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = true
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
api.updateSCIMSettings(sc.Context)
req := server.NewRequest(http.MethodPut, scimAPIPath, bytes.NewReader(bodyBytes))
req.Header.Set("Content-Type", "application/json")
require.Equal(t, http.StatusInternalServerError, sc.Resp.Code)
mockService.AssertExpectations(t)
})
// Create admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
})
t.Run("As non-Admin", func(t *testing.T) {
// Mock service call to avoid nil pointer if handler is somehow reached
mockService.On("Update", mock.Anything, mock.Anything).Return(nil).Maybe()
req, err := http.NewRequest(http.MethodPut, "/api/scim/settings", bytes.NewReader(cmdBytes))
resp, err := server.Send(req)
require.NoError(t, err)
req.Header.Set("Content-Type", "application/json")
sc := setupScenarioContext(t, req.URL.String())
sc.Req = req
sc.IsGrafanaAdmin = false // Ensure not admin
sc.contextHandler.GetContextProvider().SetContext(sc.Req, sc.Context)
require.Equal(t, http.StatusInternalServerError, resp.StatusCode)
require.NoError(t, resp.Body.Close())
server.Mux.ServeHTTP(sc.Resp, sc.Req)
require.Equal(t, http.StatusForbidden, sc.Resp.Code)
// No calls expected to the service
mockService.AssertNotCalled(t, "Update", mock.Anything, mock.Anything)
mockService.AssertExpectations(t)
})
})
}
// --- Helper Functions (Potentially move to a shared test util package) ---
t.Run("As non-Admin", func(t *testing.T) {
mockService := scimsettingstest.NewMockService(t)
// Minimal server setup for handler tests
type testServer struct {
*httptest.Server
RouteRegister routing.RouteRegister
Mux *macaron.Macaron
}
server := setupSCIMSettingsAPITest(t, true, mockService)
func setupTestServer(t *testing.T, api *SCIMSettingsAPI, features *featuremgmt.FeatureManager, route string, method string, handler interface{}, middlewares ...web.Handler) *testServer {
t.Helper()
m := macaron.New()
// Use the context handler from the actual API package
contextHandler := ProvideContextHandler()
m.Use(contextHandler.Middleware)
rr := routing.NewRouteRegister()
// Need to re-assign routeRegister and features to the api instance used in tests
api.routeRegister = rr
api.features = features
// Mimic RegisterAPIEndpoints structure
api.RegisterAPIEndpoints() // Call the actual registration logic
// Setup renderer
m.Use(web.Renderer(macaron.RenderOptions{Directory: api.cfg.StaticRootPath}))
rr.Register(m) // Register routes collected by api.RegisterAPIEndpoints()
ts := httptest.NewServer(m)
t.Cleanup(ts.Close)
return &testServer{
Server: ts,
RouteRegister: rr,
Mux: m,
}
}
bodyBytes, err := json.Marshal(updateCmd)
require.NoError(t, err)
// setupScenarioContext creates a context for handler tests
func setupScenarioContext(t *testing.T, url string) *scenarioContext {
t.Helper()
contextHandler := ProvideContextHandler()
sc := &scenarioContext{
t: t,
url: url,
Resp: httptest.NewRecorder(),
Context: contextmodel.NewReqContext(nil), // Will be populated by middleware
unitOfWork: &dbtest.FakeUnitOfWork{}, // Fake UoW if needed
contextHandler: contextHandler, // Store the handler
}
req := server.NewRequest(http.MethodPut, scimAPIPath, bytes.NewReader(bodyBytes))
req.Header.Set("Content-Type", "application/json")
// Set default user/org info
sc.SignedInUser = &user.SignedInUser{
// Create non-admin user
req = webtest.RequestWithSignedInUser(req, &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: models.ROLE_VIEWER, // Default role
}
// Apply IsGrafanaAdmin to the SignedInUser struct
sc.SignedInUser.IsGrafanaAdmin = sc.IsGrafanaAdmin
if sc.IsGrafanaAdmin {
sc.SignedInUser.OrgRole = models.ROLE_ADMIN
}
OrgRole: org.RoleViewer,
IsGrafanaAdmin: false,
})
// We need to associate the base request with the context handler
// The actual macaron context gets created within the middleware chain
baseReq, _ := http.NewRequest("GET", url, nil) // Method doesn't matter much here
sc.Req = baseReq.WithContext(context.WithValue(baseReq.Context(), contextmodel.ReqContextKey{}, sc.Context))
sc.Context.Req = sc.Req
sc.Context.Logger = log.NewNopLogger()
sc.Context.SignedInUser = sc.SignedInUser
sc.Context.OrgID = sc.SignedInUser.OrgID
sc.Context.IsSignedIn = true
sc.Context.IsGrafanaAdmin = sc.IsGrafanaAdmin
return sc
}
resp, err := server.Send(req)
require.NoError(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
require.NoError(t, resp.Body.Close())
// scenarioContext holds state for a single API scenario test
type scenarioContext struct {
t *testing.T
Req *http.Request
Resp *httptest.ResponseRecorder
url string
Context *contextmodel.ReqContext
SignedInUser *user.SignedInUser
IsGrafanaAdmin bool // Control admin status easily
unitOfWork db.UnitOfWork
contextHandler contextmodel.ContextHandler // Store the context handler
mockService.AssertNotCalled(t, "Update", mock.Anything, mock.Anything)
})
}

@ -0,0 +1,174 @@
package api
import (
"bytes"
"encoding/json"
"net/http"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/routing"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/org"
scimmodels "github.com/grafana/grafana/pkg/services/scimsettings/models"
"github.com/grafana/grafana/pkg/services/scimsettings/scimsettingstest"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web/webtest"
)
const (
scimAPISettingsPath = "/api/scim/settings"
)
// setupMockedSCIMService creates a mock SCIM settings service with expectations for changing settings
func setupMockedSCIMService(t *testing.T) *scimsettingstest.MockService {
mockService := scimsettingstest.NewMockService(t)
// Setup initial state - SCIM disabled
mockService.On("Get", mock.Anything).Return(&scimmodels.ScimSettings{
UserSyncEnabled: false,
GroupSyncEnabled: false,
}, nil).Once()
// The settings will be updated to enable SCIM
mockService.On("Update", mock.Anything, mock.MatchedBy(func(s *scimmodels.ScimSettings) bool {
return s.UserSyncEnabled && s.GroupSyncEnabled
})).Return(nil).Once()
// Get will be called after update to verify - SCIM enabled
mockService.On("Get", mock.Anything).Return(&scimmodels.ScimSettings{
UserSyncEnabled: true,
GroupSyncEnabled: true,
}, nil).Once()
// The settings will be updated to disable SCIM
mockService.On("Update", mock.Anything, mock.MatchedBy(func(s *scimmodels.ScimSettings) bool {
return !s.UserSyncEnabled && !s.GroupSyncEnabled
})).Return(nil).Once()
// Get will be called after update to verify - SCIM disabled again
mockService.On("Get", mock.Anything).Return(&scimmodels.ScimSettings{
UserSyncEnabled: false,
GroupSyncEnabled: false,
}, nil).Once()
return mockService
}
// TestSCIMSettingsIntegration tests the SCIM settings API with mock service
func TestSCIMSettingsIntegration(t *testing.T) {
if testing.Short() {
t.Skip("skipping integration test")
}
// Create a mock service with expectations
mockService := setupMockedSCIMService(t)
// Set up the test server with SCIM settings API
features := featuremgmt.WithFeatures(featuremgmt.FlagEnableSCIM)
cfg := setting.NewCfg()
routeRegister := routing.NewRouteRegister()
api := ProvideSCIMSettingsAPI(cfg, routeRegister, features.(*featuremgmt.FeatureManager), mockService)
api.RegisterAPIEndpoints()
server := webtest.NewServer(t, routeRegister)
// Create admin user for authentication
adminUser := &user.SignedInUser{
UserID: 1,
OrgID: 1,
OrgRole: org.RoleAdmin,
IsGrafanaAdmin: true,
}
t.Run("SCIM settings API should allow enabling and disabling SCIM", func(t *testing.T) {
// First get the current settings - should be disabled
getSettingsReq := server.NewGetRequest(scimAPISettingsPath)
getSettingsReq = webtest.RequestWithSignedInUser(getSettingsReq, adminUser)
getSettingsResp, err := server.Send(getSettingsReq)
require.NoError(t, err)
require.Equal(t, http.StatusOK, getSettingsResp.StatusCode)
var settingsResp scimSettingsDTO
require.NoError(t, json.NewDecoder(getSettingsResp.Body).Decode(&settingsResp))
require.NoError(t, getSettingsResp.Body.Close())
assert.False(t, settingsResp.UserSyncEnabled, "User sync should initially be disabled")
assert.False(t, settingsResp.GroupSyncEnabled, "Group sync should initially be disabled")
// Update settings to enable SCIM
updateSettings := scimSettingsDTO{
UserSyncEnabled: true,
GroupSyncEnabled: true,
}
bodyBytes, err := json.Marshal(updateSettings)
require.NoError(t, err)
updateReq := server.NewRequest(http.MethodPut, scimAPISettingsPath, bytes.NewReader(bodyBytes))
updateReq.Header.Set("Content-Type", "application/json")
updateReq = webtest.RequestWithSignedInUser(updateReq, adminUser)
updateResp, err := server.Send(updateReq)
require.NoError(t, err)
require.Equal(t, http.StatusOK, updateResp.StatusCode, "Updating settings should succeed")
require.NoError(t, updateResp.Body.Close())
// Verify settings were updated successfully
getSettingsReq = server.NewGetRequest(scimAPISettingsPath)
getSettingsReq = webtest.RequestWithSignedInUser(getSettingsReq, adminUser)
getSettingsResp, err = server.Send(getSettingsReq)
require.NoError(t, err)
require.Equal(t, http.StatusOK, getSettingsResp.StatusCode)
require.NoError(t, json.NewDecoder(getSettingsResp.Body).Decode(&settingsResp))
require.NoError(t, getSettingsResp.Body.Close())
assert.True(t, settingsResp.UserSyncEnabled, "User sync should be enabled after update")
assert.True(t, settingsResp.GroupSyncEnabled, "Group sync should be enabled after update")
// Update settings again to disable SCIM
updateSettings = scimSettingsDTO{
UserSyncEnabled: false,
GroupSyncEnabled: false,
}
bodyBytes, err = json.Marshal(updateSettings)
require.NoError(t, err)
updateReq = server.NewRequest(http.MethodPut, scimAPISettingsPath, bytes.NewReader(bodyBytes))
updateReq.Header.Set("Content-Type", "application/json")
updateReq = webtest.RequestWithSignedInUser(updateReq, adminUser)
updateResp, err = server.Send(updateReq)
require.NoError(t, err)
require.Equal(t, http.StatusOK, updateResp.StatusCode, "Updating settings should succeed")
require.NoError(t, updateResp.Body.Close())
// Verify settings were updated
getSettingsReq = server.NewGetRequest(scimAPISettingsPath)
getSettingsReq = webtest.RequestWithSignedInUser(getSettingsReq, adminUser)
getSettingsResp, err = server.Send(getSettingsReq)
require.NoError(t, err)
require.Equal(t, http.StatusOK, getSettingsResp.StatusCode)
require.NoError(t, json.NewDecoder(getSettingsResp.Body).Decode(&settingsResp))
require.NoError(t, getSettingsResp.Body.Close())
assert.False(t, settingsResp.UserSyncEnabled, "User sync should be disabled after update")
assert.False(t, settingsResp.GroupSyncEnabled, "Group sync should be disabled after update")
// Verify all mock expectations were met
mockService.AssertExpectations(t)
t.Log("Note: This test verifies the SCIM settings API works correctly with the mock service.")
t.Log("To verify that SCIM settings actually affect the SCIM service endpoints,")
t.Log("we would need additional integration tests in pkg/extensions/apiserver/tests/scim/.")
})
}

@ -122,9 +122,6 @@ import (
"github.com/grafana/grafana/pkg/services/queryhistory"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/rendering"
scimsettings "github.com/grafana/grafana/pkg/services/scimsettings"
scimsettingsdb "github.com/grafana/grafana/pkg/services/scimsettings/database"
scimsettingsimpl "github.com/grafana/grafana/pkg/services/scimsettings/scimsettingsimpl"
"github.com/grafana/grafana/pkg/services/search"
"github.com/grafana/grafana/pkg/services/search/sort"
"github.com/grafana/grafana/pkg/services/searchV2"
@ -296,7 +293,6 @@ var wireBasicSet = wire.NewSet(
parca.ProvideService,
zipkin.ProvideService,
jaeger.ProvideService,
api.ProvideSCIMSettingsAPI,
datasourceservice.ProvideCacheService,
wire.Bind(new(datasources.CacheService), new(*datasourceservice.CacheServiceImpl)),
encryptionservice.ProvideEncryptionService,
@ -429,10 +425,6 @@ var wireBasicSet = wire.NewSet(
grafanaapiserver.WireSet,
apiregistry.WireSet,
appregistry.WireSet,
scimsettingsdb.ProvideStore,
scimsettingsimpl.ProvideService,
wire.Bind(new(scimsettings.Store), new(*scimsettingsdb.storeImpl)),
wire.Bind(new(scimsettings.Service), new(*scimsettingsimpl.ServiceImpl)),
)
var wireSet = wire.NewSet(

@ -10,9 +10,9 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/scimsettings"
"github.com/grafana/grafana/pkg/services/scimsettings/models"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
)
// ProvideStore creates a new database store for SCIM settings.
func ProvideStore(sqlStore db.DB) scimsettings.Store {
return &storeImpl{
sqlStore: sqlStore,
@ -27,8 +27,6 @@ type storeImpl struct {
var _ scimsettings.Store = (*storeImpl)(nil)
// Get retrieves the SCIM settings from the database.
// Returns scimsettings.ErrSettingsNotFound if no settings row exists.
func (s *storeImpl) Get(ctx context.Context) (*models.ScimSettings, error) {
settings := &models.ScimSettings{}
err := s.sqlStore.WithDbSession(ctx, func(sess *db.Session) error {
@ -37,32 +35,25 @@ func (s *storeImpl) Get(ctx context.Context) (*models.ScimSettings, error) {
return fmt.Errorf("database error fetching scim settings: %w", err)
}
if !has {
// Explicitly return ErrSettingsNotFound when no record exists
return scimsettings.ErrSettingsNotFound
}
return nil
})
if err != nil {
// Propagate the error (could be ErrSettingsNotFound or another DB error)
return nil, err
}
return settings, nil
}
// Update updates the SCIM settings in the database.
// It performs an Upsert operation based on whether settings already exist.
func (s *storeImpl) Update(ctx context.Context, settings *models.ScimSettings) error {
return s.sqlStore.WithTransactionalDbSession(ctx, func(sess *db.Session) error {
// Attempt to retrieve existing settings to determine if it's an insert or update.
existing := &models.ScimSettings{}
has, err := sess.OrderBy("id").Limit(1).Get(existing)
// Handle ErrSettingsNotFound specifically for the check, but don't error out
if err != nil && !errors.Is(err, scimsettings.ErrSettingsNotFound) {
return fmt.Errorf("failed to check for existing scim settings: %w", err)
}
// If settings were not found, 'has' will be false
if errors.Is(err, scimsettings.ErrSettingsNotFound) {
has = false
}
@ -70,8 +61,7 @@ func (s *storeImpl) Update(ctx context.Context, settings *models.ScimSettings) e
settings.UpdatedAt = time.Now()
if has {
// Update existing settings
settings.ID = existing.ID // Ensure we update the correct row
settings.ID = existing.ID
_, err = sess.ID(settings.ID).Update(settings)
if err != nil {
return fmt.Errorf("failed to update scim settings: %w", err)
@ -79,7 +69,7 @@ func (s *storeImpl) Update(ctx context.Context, settings *models.ScimSettings) e
s.log.Debug("Updated existing SCIM settings", "id", settings.ID)
} else {
// Insert new settings
settings.CreatedAt = settings.UpdatedAt // Set CreatedAt on first insert
settings.CreatedAt = settings.UpdatedAt
_, err = sess.Insert(settings)
if err != nil {
return fmt.Errorf("failed to insert scim settings: %w", err)
@ -89,3 +79,18 @@ func (s *storeImpl) Update(ctx context.Context, settings *models.ScimSettings) e
return nil
})
}
func AddMigration(mg *migrator.Migrator) {
scimSettingsV1 := migrator.Table{
Name: "scim_settings",
Columns: []*migrator.Column{
{Name: "id", Type: migrator.DB_BigInt, IsPrimaryKey: true, IsAutoIncrement: true},
{Name: "user_sync_enabled", Type: migrator.DB_Bool, Nullable: false},
{Name: "group_sync_enabled", Type: migrator.DB_Bool, Nullable: false},
{Name: "created_at", Type: migrator.DB_DateTime, Nullable: false},
{Name: "updated_at", Type: migrator.DB_DateTime, Nullable: false},
},
}
mg.AddMigration("create scim_settings table", migrator.NewAddTableMigration(scimSettingsV1))
}

@ -3,8 +3,7 @@ package models
import "time"
// ScimSettings represents the SCIM configuration stored in the database.
// Assuming these are global settings for now. If they need to be per-org,
// an OrgId field would be necessary.
// Assuming these are global settings, not per-org.
type ScimSettings struct {
ID int64 `xorm:"pk autoincr 'id'"`
UserSyncEnabled bool `xorm:"user_sync_enabled" json:"userSyncEnabled"`

@ -7,7 +7,9 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/scimsettings"
scimsettingsdb "github.com/grafana/grafana/pkg/services/scimsettings/database"
"github.com/grafana/grafana/pkg/services/scimsettings/models"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
)
type ServiceImpl struct {
@ -55,3 +57,8 @@ func (s *ServiceImpl) Update(ctx context.Context, settings *models.ScimSettings)
return nil
}
// AddMigration implements registry.DatabaseMigrator
func (s *ServiceImpl) AddMigration(mg *migrator.Migrator) {
scimsettingsdb.AddMigration(mg)
}

@ -7,6 +7,7 @@ import (
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log"
@ -14,14 +15,35 @@ import (
"github.com/grafana/grafana/pkg/services/scimsettings/models"
)
// mockStore is a manual mock for the scimsettings.Store interface.
type mockStore struct {
mock.Mock
}
// Get implements the scimsettings.Store interface.
func (m *mockStore) Get(ctx context.Context) (*models.ScimSettings, error) {
args := m.Called(ctx)
ret0 := args.Get(0)
if ret0 == nil {
return nil, args.Error(1)
}
return ret0.(*models.ScimSettings), args.Error(1)
}
// Update implements the scimsettings.Store interface.
func (m *mockStore) Update(ctx context.Context, settings *models.ScimSettings) error {
args := m.Called(ctx, settings)
return args.Error(0)
}
// setupTestEnv creates a test environment with mock dependencies.
func setupTestEnv(t *testing.T) (*ServiceImpl, *scimsettings.MockStore) {
func setupTestEnv(t *testing.T) (*ServiceImpl, *mockStore) {
t.Helper()
store := scimsettings.NewMockStore(t)
// reloadable := scimsettings.NewMockReloadable(t) // Removed
service := ProvideService(store).(*ServiceImpl) // Cast to ServiceImpl to access fields if needed
service.log = log.NewNopLogger() // Use NopLogger instead of Fake
store := &mockStore{}
store.Test(t)
service := ProvideService(store).(*ServiceImpl)
service.log = log.NewNopLogger()
return service, store
}

@ -0,0 +1,37 @@
package scimsettingstest
import (
"context"
"testing"
"github.com/stretchr/testify/mock"
"github.com/grafana/grafana/pkg/services/scimsettings/models"
)
// MockService is a mock implementation of scimsettings.Service
type MockService struct {
mock.Mock
}
// NewMockService creates a new mock instance
func NewMockService(t *testing.T) *MockService {
m := &MockService{}
m.Mock.Test(t)
return m
}
// Get implements Service.Get
func (m *MockService) Get(ctx context.Context) (*models.ScimSettings, error) {
args := m.Called(ctx)
if args.Get(0) == nil {
return nil, args.Error(1)
}
return args.Get(0).(*models.ScimSettings), args.Error(1)
}
// Update implements Service.Update
func (m *MockService) Update(ctx context.Context, settings *models.ScimSettings) error {
args := m.Called(ctx, settings)
return args.Error(0)
}

@ -1,18 +0,0 @@
// ... existing code ...
OAuthOIDCProviderName = "oidc"
// SAMLProviderName defines the name for the SAML provider
SAMLProviderName = "saml"
// ScimProviderName defines the name for the SCIM provider
SCIMProviderName = "scim"
// ProviderNotifiers defines the name for the provider notifiers
// ... existing code ...
var AllOAuthProviders = []string{
GenericOAuthProviderName,
// ... other providers ...
AzureADProviderName,
}
// AddProvider adds new provider to AllOAuthProviders slice
// ... existing code ...

@ -1,11 +0,0 @@
-- create table scim_settings
CREATE TABLE IF NOT EXISTS `scim_settings` (
`id` BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY
,`user_sync_enabled` TINYINT(1) NOT NULL
,`group_sync_enabled` TINYINT(1) NOT NULL
,`created_at` DATETIME NOT NULL
,`updated_at` DATETIME NOT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;
-- create index IDX_scim_settings_id
-- CREATE UNIQUE INDEX `IDX_scim_settings_id` ON `scim_settings` (`id`); -- PK is already unique index

@ -1,11 +0,0 @@
-- create table scim_settings
CREATE TABLE IF NOT EXISTS "scim_settings" (
"id" BIGSERIAL PRIMARY KEY
,"user_sync_enabled" BOOL NOT NULL
,"group_sync_enabled" BOOL NOT NULL
,"created_at" TIMESTAMP NOT NULL
,"updated_at" TIMESTAMP NOT NULL
);
-- create index IDX_scim_settings_id
CREATE UNIQUE INDEX "IDX_scim_settings_id" ON "scim_settings" ("id");

@ -1,11 +0,0 @@
-- create table scim_settings
CREATE TABLE IF NOT EXISTS "scim_settings" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL
,"user_sync_enabled" BOOL NOT NULL
,"group_sync_enabled" BOOL NOT NULL
,"created_at" DATETIME NOT NULL
,"updated_at" DATETIME NOT NULL
);
-- create index IDX_scim_settings_id - Usually not needed for PK in SQLite
-- CREATE UNIQUE INDEX "IDX_scim_settings_id" ON "scim_settings" ("id");
Loading…
Cancel
Save