Auth: Implement the SSO Settings GET endpoint (#79144)

* Return data in camelCase from the OAuth fb strategy

* changes

* wip

* Add defaults for oauth fb strategy

* revert other changes

* basic includeDefaults query param implementation

* basic secret removal and etag implementation

* correct imports

* rebase

* move default settings filter to models

* only replace ClientSecret value if set

* first GetForProvider test & use FNV for ETag to avoid Blocklisted import error

* add tests

* add annotation for the openapi spec & generate spec

* remove TODO

* use IsSecret, improve tests, remove DefaultOAuthSettings

* add comment explaining generateFNVETag

* add error handling for generateFNVETag

* run go generate

* Update pkg/services/ssosettings/api/api.go

Co-authored-by: Mihai Doarna <mihai.doarna@grafana.com>

* move isSecret to service, create GetForProviderWithRedactedSecrets func

* add unit test for GetForProviderWithRedactedSecrets & remove duplicated code

* regen openapi/swagger

* revert dependency bumps

---------

Co-authored-by: Mihaly Gyongyosi <mgyongyosi@users.noreply.github.com>
Co-authored-by: Mihai Doarna <mihai.doarna@grafana.com>
This commit is contained in:
colin-stuart
2024-01-08 09:35:14 -05:00
committed by GitHub
co-authored by Mihai Doarna Mihaly Gyongyosi
parent 505196bcd5
commit 062e772bb2
11 changed files with 440 additions and 6 deletions
+50 -3
View File
@@ -2,7 +2,10 @@ package api
import (
"context"
"encoding/json"
"errors"
"fmt"
"hash/fnv"
"net/http"
"github.com/grafana/grafana/pkg/api/response"
@@ -40,6 +43,22 @@ func ProvideApi(
return api
}
// generateFNVETag computes a FNV hash-based ETag for the SSOSettings struct
func generateFNVETag(SSOSettings *models.SSOSettings) (string, error) {
hasher := fnv.New64()
data, err := json.Marshal(SSOSettings)
if err != nil {
return "", err
}
_, err = hasher.Write(data)
if err != nil {
return "", err
}
return fmt.Sprintf("%x", hasher.Sum(nil)), nil
}
// RegisterAPIEndpoints Registers Endpoints on Grafana Router
func (api *Api) RegisterAPIEndpoints() {
api.RouteRegister.Group("/api/v1/sso-settings", func(router routing.RouteRegister) {
@@ -91,18 +110,39 @@ func (api *Api) getAuthorizedList(ctx context.Context, identity identity.Request
return authorizedProviders, nil
}
// swagger:route GET /v1/sso-settings/{key} sso_settings getProviderSettings
//
// # Get an SSO Settings entry by Key
//
// You need to have a permission with action `settings:read` with scope `settings:auth.<provider>:*`.
//
// Responses:
// 200: okResponse
// 400: badRequestError
// 401: unauthorisedError
// 403: forbiddenError
// 404: notFoundError
func (api *Api) getProviderSettings(c *contextmodel.ReqContext) response.Response {
key, ok := web.Params(c.Req)[":key"]
if !ok {
return response.Error(http.StatusBadRequest, "Missing key", nil)
}
settings, err := api.SSOSettingsService.GetForProvider(c.Req.Context(), key)
provider, err := api.SSOSettingsService.GetForProviderWithRedactedSecrets(c.Req.Context(), key)
if err != nil {
return response.Error(http.StatusNotFound, "The provider was not found", err)
if errors.Is(err, ssosettings.ErrNotFound) {
return response.Error(http.StatusNotFound, "The provider was not found", err)
}
return response.Error(http.StatusInternalServerError, "Failed to get provider settings", err)
}
return response.JSON(http.StatusOK, settings)
etag, err := generateFNVETag(provider)
if err != nil {
return response.Error(http.StatusInternalServerError, "Failed to get provider settings", err)
}
return response.JSON(http.StatusOK, provider).SetHeader("ETag", etag)
}
// swagger:route PUT /v1/sso-settings/{key} sso_settings updateProviderSettings
@@ -172,6 +212,13 @@ func (api *Api) removeProviderSettings(c *contextmodel.ReqContext) response.Resp
return response.JSON(http.StatusNoContent, nil)
}
// swagger:parameters getProviderSettings
type GetProviderSettingsWrapper struct {
// in:path
// required:true
Provider string `json:"key"`
}
// swagger:parameters updateProviderSettings
type UpdateProviderSettingsParams struct {
// in:path
+114
View File
@@ -7,6 +7,7 @@ import (
"fmt"
"net/http"
"testing"
"time"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@@ -253,6 +254,119 @@ func TestSSOSettingsAPI_Delete(t *testing.T) {
}
}
func TestSSOSettingsAPI_GetForProvider(t *testing.T) {
type TestCase struct {
desc string
key string
action string
scope string
expectedResult *models.SSOSettings
expectedError error
expectedServiceCall bool
expectedStatusCode int
}
tests := []TestCase{
{
desc: "successfully gets SSO settings",
key: "azuread",
action: "settings:read",
scope: "settings:auth.azuread:*",
expectedResult: &models.SSOSettings{
ID: "1",
Provider: "azuread",
Settings: make(map[string]interface{}),
Created: time.Now(),
Updated: time.Now(),
IsDeleted: false,
Source: models.DB,
},
expectedError: nil,
expectedServiceCall: true,
expectedStatusCode: http.StatusOK,
},
{
desc: "fails when action doesn't match",
key: "azuread",
action: "settings:write",
scope: "settings:auth.azuread:*",
expectedResult: nil,
expectedError: nil,
expectedServiceCall: false,
expectedStatusCode: http.StatusForbidden,
},
{
desc: "fails when scope doesn't match",
key: "azuread",
action: "settings:read",
scope: "settings:auth.azuread:write",
expectedResult: nil,
expectedError: nil,
expectedServiceCall: false,
expectedStatusCode: http.StatusForbidden,
},
{
desc: "fails when scope contains another provider",
key: "azuread",
action: "settings:read",
scope: "settings:auth.github:*",
expectedResult: nil,
expectedError: nil,
expectedServiceCall: false,
expectedStatusCode: http.StatusForbidden,
},
{
desc: "fails with not found when key was not found",
key: "nonexistant",
action: "settings:read",
scope: "settings:auth.nonexistant:*",
expectedResult: nil,
expectedError: ssosettings.ErrNotFound,
expectedServiceCall: true,
expectedStatusCode: http.StatusNotFound,
},
{
desc: "fails with internal server error when service returns an error",
key: "azuread",
action: "settings:read",
scope: "settings:auth.azuread:*",
expectedResult: nil,
expectedError: errors.New("something went wrong"),
expectedServiceCall: true,
expectedStatusCode: http.StatusInternalServerError,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service := ssosettingstests.NewMockService(t)
if tt.expectedServiceCall {
service.On("GetForProviderWithRedactedSecrets", mock.AnythingOfType("*context.valueCtx"), tt.key).Return(tt.expectedResult, tt.expectedError).Once()
}
server := setupTests(t, service)
path := fmt.Sprintf("/api/v1/sso-settings/%s", tt.key)
req := server.NewRequest(http.MethodGet, path, nil)
webtest.RequestWithSignedInUser(req, &user.SignedInUser{
OrgRole: org.RoleEditor,
OrgID: 1,
Permissions: getPermissionsForActionAndScope(tt.action, tt.scope),
})
res, err := server.SendJSON(req)
require.NoError(t, err)
require.Equal(t, tt.expectedStatusCode, res.StatusCode)
if tt.expectedError == nil {
var data models.SSOSettings
require.NoError(t, json.NewDecoder(res.Body).Decode(&data))
}
require.NoError(t, res.Body.Close())
})
}
}
func getPermissionsForActionAndScope(action, scope string) map[int64]map[string][]string {
return map[int64]map[string][]string{
1: accesscontrol.GroupScopesByAction([]accesscontrol.Permission{{
+17
View File
@@ -26,6 +26,23 @@ func (s SettingsSource) MarshalJSON() ([]byte, error) {
}
}
func (s *SettingsSource) UnmarshalJSON(data []byte) error {
var source string
if err := json.Unmarshal(data, &source); err != nil {
return err
}
switch source {
case "database":
*s = DB
case "system":
*s = System
default:
return fmt.Errorf("unknown source: %s", source)
}
return nil
}
type SSOSettings struct {
ID string `xorm:"id pk" json:"id"`
Provider string `xorm:"provider" json:"provider"`
+2
View File
@@ -23,6 +23,8 @@ type Service interface {
List(ctx context.Context) ([]*models.SSOSettings, error)
// GetForProvider returns the SSO settings for a given provider (DB or config file)
GetForProvider(ctx context.Context, provider string) (*models.SSOSettings, error)
// GetForProviderWithRedactedSecrets returns the SSO settings for a given provider (DB or config file) with secret values redacted
GetForProviderWithRedactedSecrets(ctx context.Context, provider string) (*models.SSOSettings, error)
// Upsert creates or updates the SSO settings for a given provider
Upsert(ctx context.Context, settings models.SSOSettings) error
// Delete deletes the SSO settings for a given provider (soft delete)
@@ -84,6 +84,21 @@ func (s *SSOSettingsService) GetForProvider(ctx context.Context, provider string
return storeSettings, nil
}
func (s *SSOSettingsService) GetForProviderWithRedactedSecrets(ctx context.Context, provider string) (*models.SSOSettings, error) {
storeSettings, err := s.GetForProvider(ctx, provider)
if err != nil {
return nil, err
}
for k, v := range storeSettings.Settings {
if isSecret(k) && v != "" {
storeSettings.Settings[k] = "*********"
}
}
return storeSettings, nil
}
func (s *SSOSettingsService) List(ctx context.Context) ([]*models.SSOSettings, error) {
result := make([]*models.SSOSettings, 0, len(ssosettings.AllOAuthProviders))
storedSettings, err := s.store.List(ctx)
@@ -103,6 +103,99 @@ func TestSSOSettingsService_GetForProvider(t *testing.T) {
}
}
func TestSSOSettingsService_GetForProviderWithRedactedSecrets(t *testing.T) {
testCases := []struct {
name string
setup func(env testEnv)
want *models.SSOSettings
wantErr bool
}{
{
name: "should return successfully and redact secrets",
setup: func(env testEnv) {
env.store.ExpectedSSOSetting = &models.SSOSettings{
Provider: "github",
Settings: map[string]any{
"enabled": true,
"secret": "secret",
"client_secret": "client_secret",
"client_id": "client_id",
},
Source: models.DB,
}
},
want: &models.SSOSettings{
Provider: "github",
Settings: map[string]any{
"enabled": true,
"secret": "*********",
"client_secret": "*********",
"client_id": "client_id",
},
},
wantErr: false,
},
{
name: "should return error if store returns an error different than not found",
setup: func(env testEnv) { env.store.ExpectedError = fmt.Errorf("error") },
want: nil,
wantErr: true,
},
{
name: "should fallback to strategy if store returns not found",
setup: func(env testEnv) {
env.store.ExpectedError = ssosettings.ErrNotFound
env.fallbackStrategy.ExpectedIsMatch = true
env.fallbackStrategy.ExpectedConfig = map[string]any{"enabled": true}
},
want: &models.SSOSettings{
Provider: "github",
Settings: map[string]any{"enabled": true},
Source: models.System,
},
wantErr: false,
},
{
name: "should return error if the fallback strategy was not found",
setup: func(env testEnv) {
env.store.ExpectedError = ssosettings.ErrNotFound
env.fallbackStrategy.ExpectedIsMatch = false
},
want: nil,
wantErr: true,
},
{
name: "should return error if fallback strategy returns error",
setup: func(env testEnv) {
env.store.ExpectedError = ssosettings.ErrNotFound
env.fallbackStrategy.ExpectedIsMatch = true
env.fallbackStrategy.ExpectedError = fmt.Errorf("error")
},
want: nil,
wantErr: true,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
env := setupTestEnv(t)
if tc.setup != nil {
tc.setup(env)
}
actual, err := env.service.GetForProviderWithRedactedSecrets(context.Background(), "github")
if tc.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tc.want, actual)
})
}
}
func TestSSOSettingsService_List(t *testing.T) {
testCases := []struct {
name string
@@ -1,4 +1,4 @@
// Code generated by mockery v2.37.1. DO NOT EDIT.
// Code generated by mockery v2.38.0. DO NOT EDIT.
package ssosettingstests
@@ -18,6 +18,10 @@ type MockReloadable struct {
func (_m *MockReloadable) Reload(ctx context.Context, settings models.SSOSettings) error {
ret := _m.Called(ctx, settings)
if len(ret) == 0 {
panic("no return value specified for Reload")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings) error); ok {
r0 = rf(ctx, settings)
@@ -32,6 +36,10 @@ func (_m *MockReloadable) Reload(ctx context.Context, settings models.SSOSetting
func (_m *MockReloadable) Validate(ctx context.Context, settings models.SSOSettings) error {
ret := _m.Called(ctx, settings)
if len(ret) == 0 {
panic("no return value specified for Validate")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings) error); ok {
r0 = rf(ctx, settings)
@@ -1,4 +1,4 @@
// Code generated by mockery v2.37.1. DO NOT EDIT.
// Code generated by mockery v2.38.0. DO NOT EDIT.
package ssosettingstests
@@ -20,6 +20,10 @@ type MockService struct {
func (_m *MockService) Delete(ctx context.Context, provider string) error {
ret := _m.Called(ctx, provider)
if len(ret) == 0 {
panic("no return value specified for Delete")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string) error); ok {
r0 = rf(ctx, provider)
@@ -34,6 +38,40 @@ func (_m *MockService) Delete(ctx context.Context, provider string) error {
func (_m *MockService) GetForProvider(ctx context.Context, provider string) (*models.SSOSettings, error) {
ret := _m.Called(ctx, provider)
if len(ret) == 0 {
panic("no return value specified for GetForProvider")
}
var r0 *models.SSOSettings
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, string) (*models.SSOSettings, error)); ok {
return rf(ctx, provider)
}
if rf, ok := ret.Get(0).(func(context.Context, string) *models.SSOSettings); ok {
r0 = rf(ctx, provider)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).(*models.SSOSettings)
}
}
if rf, ok := ret.Get(1).(func(context.Context, string) error); ok {
r1 = rf(ctx, provider)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// GetForProviderWithRedactedSecrets provides a mock function with given fields: ctx, provider
func (_m *MockService) GetForProviderWithRedactedSecrets(ctx context.Context, provider string) (*models.SSOSettings, error) {
ret := _m.Called(ctx, provider)
if len(ret) == 0 {
panic("no return value specified for GetForProviderWithRedactedSecrets")
}
var r0 *models.SSOSettings
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, string) (*models.SSOSettings, error)); ok {
@@ -60,6 +98,10 @@ func (_m *MockService) GetForProvider(ctx context.Context, provider string) (*mo
func (_m *MockService) List(ctx context.Context) ([]*models.SSOSettings, error) {
ret := _m.Called(ctx)
if len(ret) == 0 {
panic("no return value specified for List")
}
var r0 []*models.SSOSettings
var r1 error
if rf, ok := ret.Get(0).(func(context.Context) ([]*models.SSOSettings, error)); ok {
@@ -86,6 +128,10 @@ func (_m *MockService) List(ctx context.Context) ([]*models.SSOSettings, error)
func (_m *MockService) Patch(ctx context.Context, provider string, data map[string]interface{}) error {
ret := _m.Called(ctx, provider, data)
if len(ret) == 0 {
panic("no return value specified for Patch")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string, map[string]interface{}) error); ok {
r0 = rf(ctx, provider, data)
@@ -110,6 +156,10 @@ func (_m *MockService) Reload(ctx context.Context, provider string) {
func (_m *MockService) Upsert(ctx context.Context, settings models.SSOSettings) error {
ret := _m.Called(ctx, settings)
if len(ret) == 0 {
panic("no return value specified for Upsert")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings) error); ok {
r0 = rf(ctx, settings)
@@ -1,4 +1,4 @@
// Code generated by mockery v2.37.1. DO NOT EDIT.
// Code generated by mockery v2.38.0. DO NOT EDIT.
package ssosettingstests
@@ -18,6 +18,10 @@ type MockStore struct {
func (_m *MockStore) Delete(ctx context.Context, provider string) error {
ret := _m.Called(ctx, provider)
if len(ret) == 0 {
panic("no return value specified for Delete")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string) error); ok {
r0 = rf(ctx, provider)
@@ -32,6 +36,10 @@ func (_m *MockStore) Delete(ctx context.Context, provider string) error {
func (_m *MockStore) Get(ctx context.Context, provider string) (*models.SSOSettings, error) {
ret := _m.Called(ctx, provider)
if len(ret) == 0 {
panic("no return value specified for Get")
}
var r0 *models.SSOSettings
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, string) (*models.SSOSettings, error)); ok {
@@ -58,6 +66,10 @@ func (_m *MockStore) Get(ctx context.Context, provider string) (*models.SSOSetti
func (_m *MockStore) List(ctx context.Context) ([]*models.SSOSettings, error) {
ret := _m.Called(ctx)
if len(ret) == 0 {
panic("no return value specified for List")
}
var r0 []*models.SSOSettings
var r1 error
if rf, ok := ret.Get(0).(func(context.Context) ([]*models.SSOSettings, error)); ok {
@@ -84,6 +96,10 @@ func (_m *MockStore) List(ctx context.Context) ([]*models.SSOSettings, error) {
func (_m *MockStore) Patch(ctx context.Context, provider string, data map[string]interface{}) error {
ret := _m.Called(ctx, provider, data)
if len(ret) == 0 {
panic("no return value specified for Patch")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, string, map[string]interface{}) error); ok {
r0 = rf(ctx, provider, data)
@@ -98,6 +114,10 @@ func (_m *MockStore) Patch(ctx context.Context, provider string, data map[string
func (_m *MockStore) Upsert(ctx context.Context, settings models.SSOSettings) error {
ret := _m.Called(ctx, settings)
if len(ret) == 0 {
panic("no return value specified for Upsert")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, models.SSOSettings) error); ok {
r0 = rf(ctx, settings)
+33
View File
@@ -11351,6 +11351,39 @@
}
},
"/v1/sso-settings/{key}": {
"get": {
"description": "You need to have a permission with action `settings:read` with scope `settings:auth.\u003cprovider\u003e:*`.",
"tags": [
"sso_settings"
],
"summary": "Get an SSO Settings entry by Key",
"operationId": "getProviderSettings",
"parameters": [
{
"type": "string",
"name": "key",
"in": "path",
"required": true
}
],
"responses": {
"200": {
"$ref": "#/responses/okResponse"
},
"400": {
"$ref": "#/responses/badRequestError"
},
"401": {
"$ref": "#/responses/unauthorisedError"
},
"403": {
"$ref": "#/responses/forbiddenError"
},
"404": {
"$ref": "#/responses/notFoundError"
}
}
},
"put": {
"description": "Inserts or updates the SSO Settings for a provider.\n\nYou need to have a permission with action `settings:write` and scope `settings:auth.\u003cprovider\u003e:*`.",
"tags": [
+35
View File
@@ -24969,6 +24969,41 @@
"sso_settings"
]
},
"get": {
"description": "You need to have a permission with action `settings:read` with scope `settings:auth.\u003cprovider\u003e:*`.",
"operationId": "getProviderSettings",
"parameters": [
{
"in": "path",
"name": "key",
"required": true,
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"$ref": "#/components/responses/okResponse"
},
"400": {
"$ref": "#/components/responses/badRequestError"
},
"401": {
"$ref": "#/components/responses/unauthorisedError"
},
"403": {
"$ref": "#/components/responses/forbiddenError"
},
"404": {
"$ref": "#/components/responses/notFoundError"
}
},
"summary": "Get an SSO Settings entry by Key",
"tags": [
"sso_settings"
]
},
"put": {
"description": "Inserts or updates the SSO Settings for a provider.\n\nYou need to have a permission with action `settings:write` and scope `settings:auth.\u003cprovider\u003e:*`.",
"operationId": "updateProviderSettings",