diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index 435f1c209ff..6112de71dfe 100644 --- a/pkg/login/social/connectors/azuread_oauth.go +++ b/pkg/login/social/connectors/azuread_oauth.go @@ -23,6 +23,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -197,13 +198,19 @@ func (s *SocialAzureAD) Validate(ctx context.Context, settings ssoModels.SSOSett return err } + return validation.Validate(info, requester, + validateAllowedGroups, + validation.RequiredUrlValidator(info.AuthUrl, "Auth URL"), + validation.RequiredUrlValidator(info.TokenUrl, "Token URL")) +} + +func validateAllowedGroups(info *social.OAuthInfo, requester identity.Requester) error { for _, groupId := range info.AllowedGroups { _, err := uuid.Parse(groupId) if err != nil { return ssosettings.ErrInvalidOAuthConfig("One or more of the Allowed groups are not in the correct format. Allowed groups should be a list of Object Ids.") } } - return nil } diff --git a/pkg/login/social/connectors/azuread_oauth_test.go b/pkg/login/social/connectors/azuread_oauth_test.go index 39da9b7f76e..670426086cd 100644 --- a/pkg/login/social/connectors/azuread_oauth_test.go +++ b/pkg/login/social/connectors/azuread_oauth_test.go @@ -1003,10 +1003,14 @@ func TestSocialAzureAD_Validate(t *testing.T) { name: "SSOSettings is valid", settings: ssoModels.SSOSettings{ Settings: map[string]any{ - "client_id": "client-id", - "allowed_groups": "0bb9c9cc-4945-418f-9b6a-c1d3b81141b0, 6034d328-0e6a-4240-8d03-cb9f2c1f16e4", + "client_id": "client-id", + "allowed_groups": "0bb9c9cc-4945-418f-9b6a-c1d3b81141b0, 6034d328-0e6a-4240-8d03-cb9f2c1f16e4", + "allow_assign_grafana_admin": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, }, { name: "fails if settings map contains an invalid field", @@ -1040,6 +1044,8 @@ func TestSocialAzureAD_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allowed_groups": "abc, def", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, @@ -1051,9 +1057,12 @@ func TestSocialAzureAD_Validate(t *testing.T) { "client_id": "client-id", "allow_assign_grafana_admin": "true", "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -1064,6 +1073,52 @@ func TestSocialAzureAD_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "invalid_url", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "/path", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go index a8ddd13fd2a..413d8c0760f 100644 --- a/pkg/login/social/connectors/generic_oauth.go +++ b/pkg/login/social/connectors/generic_oauth.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -79,17 +80,33 @@ func (s *SocialGenericOAuth) Validate(ctx context.Context, settings ssoModels.SS return err } + err = validation.Validate(info, requester, + validation.UrlValidator(info.AuthUrl, "Auth URL"), + validation.UrlValidator(info.TokenUrl, "Token URL"), + validateTeamsUrlWhenNotEmpty) + + if err != nil { + return err + } + if info.Extra[teamIdsKey] != "" && (info.TeamIdsAttributePath == "" || info.TeamsUrl == "") { return ssosettings.ErrInvalidOAuthConfig("If Team Ids are configured then Team Ids attribute path and Teams URL must be configured.") } if info.AllowedGroups != nil && len(info.AllowedGroups) > 0 && info.GroupsAttributePath == "" { - return ssosettings.ErrInvalidOAuthConfig("If Allowed groups are configured then Groups attribute path must be configured.") + return ssosettings.ErrInvalidOAuthConfig("If Allowed groups is configured then Groups attribute path must be configured.") } return nil } +func validateTeamsUrlWhenNotEmpty(info *social.OAuthInfo, requester identity.Requester) error { + if info.TeamsUrl == "" { + return nil + } + return validation.UrlValidator(info.TeamsUrl, "Teams URL")(info, requester) +} + func (s *SocialGenericOAuth) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { newInfo, err := CreateOAuthInfoFromKeyValues(settings.Settings) if err != nil { diff --git a/pkg/login/social/connectors/generic_oauth_test.go b/pkg/login/social/connectors/generic_oauth_test.go index 3812a2d43bd..a7b64d7f20e 100644 --- a/pkg/login/social/connectors/generic_oauth_test.go +++ b/pkg/login/social/connectors/generic_oauth_test.go @@ -731,10 +731,25 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "teams_url": "https://example.com/teams", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, requester: &user.SignedInUser{IsGrafanaAdmin: true}, }, + { + name: "passes when team_url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + }, + }, + wantErr: nil, + }, { name: "fails if settings map contains an invalid field", settings: ssoModels.SSOSettings{ @@ -768,9 +783,12 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { "client_id": "client-id", "allow_assign_grafana_admin": "true", "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -782,6 +800,68 @@ func TestSocialGenericOAuth_Validate(t *testing.T) { "client_id": "client-id", "allow_assign_grafana_admin": "true", "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "https://example.com/teams", + "auth_url": "", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "https://example.com/teams", + "auth_url": "https://example.com/auth", + "token_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "https://example.com/teams", + "auth_url": "invalid_url", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "https://example.com/teams", + "auth_url": "https://example.com/auth", + "token_url": "/path", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if teams url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "teams_url": "file://teams", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go index 8e9bddfc316..cf346c7e4ec 100644 --- a/pkg/login/social/connectors/github_oauth.go +++ b/pkg/login/social/connectors/github_oauth.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/util/errutil" @@ -86,6 +87,14 @@ func (s *SocialGithub) Validate(ctx context.Context, settings ssoModels.SSOSetti return err } + return validation.Validate(info, requester, + validation.MustBeEmptyValidator(info.AuthUrl, "Auth URL"), + validation.MustBeEmptyValidator(info.TokenUrl, "Token URL"), + validation.MustBeEmptyValidator(info.ApiUrl, "API URL"), + teamIdsNumbersValidator) +} + +func teamIdsNumbersValidator(info *social.OAuthInfo, requester identity.Requester) error { teamIdsSplitted := util.SplitString(info.Extra[teamIdsKey]) teamIds := mustInts(teamIdsSplitted) diff --git a/pkg/login/social/connectors/github_oauth_test.go b/pkg/login/social/connectors/github_oauth_test.go index 9b27703da7d..e7c8edfa7e4 100644 --- a/pkg/login/social/connectors/github_oauth_test.go +++ b/pkg/login/social/connectors/github_oauth_test.go @@ -359,6 +359,9 @@ func TestSocialGitHub_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "auth_url": "", + "token_url": "", + "api_url": "", }, }, requester: &user.SignedInUser{IsGrafanaAdmin: true}, @@ -408,7 +411,8 @@ func TestSocialGitHub_Validate(t *testing.T) { "skip_org_role_sync": "true", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -419,7 +423,40 @@ func TestSocialGitHub_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", - "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "", + "api_url": "http://example.com/api", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/gitlab_oauth.go b/pkg/login/social/connectors/gitlab_oauth.go index 329b4d58c11..f082433c3d2 100644 --- a/pkg/login/social/connectors/gitlab_oauth.go +++ b/pkg/login/social/connectors/gitlab_oauth.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" ) @@ -76,7 +77,10 @@ func (s *SocialGitlab) Validate(ctx context.Context, settings ssoModels.SSOSetti return err } - return nil + return validation.Validate(info, requester, + validation.MustBeEmptyValidator(info.AuthUrl, "Auth URL"), + validation.MustBeEmptyValidator(info.TokenUrl, "Token URL"), + validation.MustBeEmptyValidator(info.ApiUrl, "API URL")) } func (s *SocialGitlab) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { diff --git a/pkg/login/social/connectors/gitlab_oauth_test.go b/pkg/login/social/connectors/gitlab_oauth_test.go index 926bccaae9f..a09944d29b3 100644 --- a/pkg/login/social/connectors/gitlab_oauth_test.go +++ b/pkg/login/social/connectors/gitlab_oauth_test.go @@ -477,6 +477,9 @@ func TestSocialGitlab_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "auth_url": "", + "token_url": "", + "api_url": "", }, }, requester: &user.SignedInUser{IsGrafanaAdmin: true}, @@ -514,9 +517,12 @@ func TestSocialGitlab_Validate(t *testing.T) { "client_id": "client-id", "allow_assign_grafana_admin": "true", "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -527,7 +533,42 @@ func TestSocialGitlab_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", - "skip_org_role_sync": "true", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "", + "api_url": "https://example.com/api", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + "api_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "https://example.com/token", + "api_url": "", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go index 6964a21dd6c..6709d4e05cf 100644 --- a/pkg/login/social/connectors/google_oauth.go +++ b/pkg/login/social/connectors/google_oauth.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" ) @@ -66,9 +67,10 @@ func (s *SocialGoogle) Validate(ctx context.Context, settings ssoModels.SSOSetti return err } - // add specific validation rules for Google - - return nil + return validation.Validate(info, requester, + validation.MustBeEmptyValidator(info.AuthUrl, "Auth URL"), + validation.MustBeEmptyValidator(info.TokenUrl, "Token URL"), + validation.MustBeEmptyValidator(info.ApiUrl, "API URL")) } func (s *SocialGoogle) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { diff --git a/pkg/login/social/connectors/google_oauth_test.go b/pkg/login/social/connectors/google_oauth_test.go index 055f416952f..95345c94e53 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -682,6 +682,9 @@ func TestSocialGoogle_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "auth_url": "", + "token_url": "", + "api_url": "", }, }, requester: &user.SignedInUser{IsGrafanaAdmin: true}, @@ -719,9 +722,12 @@ func TestSocialGoogle_Validate(t *testing.T) { "client_id": "client-id", "allow_assign_grafana_admin": "true", "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -732,7 +738,55 @@ func TestSocialGoogle_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", - "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "", + "api_url": "https://example.com/api", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + "api_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api token url is not empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "https://example.com/token", + "api_url": "", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/grafana_com_oauth.go b/pkg/login/social/connectors/grafana_com_oauth.go index 1db16e06c29..85928d8d77b 100644 --- a/pkg/login/social/connectors/grafana_com_oauth.go +++ b/pkg/login/social/connectors/grafana_com_oauth.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -64,9 +65,10 @@ func (s *SocialGrafanaCom) Validate(ctx context.Context, settings ssoModels.SSOS return err } - // add specific validation rules for GrafanaCom - - return nil + return validation.Validate(info, requester, + validation.MustBeEmptyValidator(info.AuthUrl, "Auth URL"), + validation.MustBeEmptyValidator(info.TokenUrl, "Token URL"), + validation.MustBeEmptyValidator(info.TeamsUrl, "Teams URL")) } func (s *SocialGrafanaCom) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { diff --git a/pkg/login/social/connectors/okta_oauth.go b/pkg/login/social/connectors/okta_oauth.go index 8354ab1a20f..3f1ab3e2c8c 100644 --- a/pkg/login/social/connectors/okta_oauth.go +++ b/pkg/login/social/connectors/okta_oauth.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/ssosettings" ssoModels "github.com/grafana/grafana/pkg/services/ssosettings/models" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" ) @@ -72,9 +73,10 @@ func (s *SocialOkta) Validate(ctx context.Context, settings ssoModels.SSOSetting return err } - // add specific validation rules for Okta - - return nil + return validation.Validate(info, requester, + validation.RequiredUrlValidator(info.AuthUrl, "Auth URL"), + validation.RequiredUrlValidator(info.TokenUrl, "Token URL"), + validation.RequiredUrlValidator(info.ApiUrl, "API URL")) } func (s *SocialOkta) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { diff --git a/pkg/login/social/connectors/okta_oauth_test.go b/pkg/login/social/connectors/okta_oauth_test.go index e390680a522..d3c44308342 100644 --- a/pkg/login/social/connectors/okta_oauth_test.go +++ b/pkg/login/social/connectors/okta_oauth_test.go @@ -150,6 +150,9 @@ func TestSocialOkta_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + "api_url": "https://example.com/api", }, }, requester: &user.SignedInUser{IsGrafanaAdmin: true}, @@ -189,7 +192,8 @@ func TestSocialOkta_Validate(t *testing.T) { "skip_org_role_sync": "true", }, }, - wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + requester: &user.SignedInUser{IsGrafanaAdmin: true}, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, }, { name: "fails if the user is not allowed to update allow assign grafana admin", @@ -200,7 +204,76 @@ func TestSocialOkta_Validate(t *testing.T) { Settings: map[string]any{ "client_id": "client-id", "allow_assign_grafana_admin": "true", - "skip_org_role_sync": "true", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if auth url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "invalid_url", + "token_url": "https://example.com/token", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if token url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "/path", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api url is empty", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "token_url": "https://example.com/token", + "api_url": "", + }, + }, + wantErr: ssosettings.ErrBaseInvalidOAuthConfig, + }, + { + name: "fails if api url is invalid", + settings: ssoModels.SSOSettings{ + Settings: map[string]any{ + "client_id": "client-id", + "auth_url": "https://example.com/auth", + "api_url": "/api", + "token_url": "https://example.com/token", }, }, wantErr: ssosettings.ErrBaseInvalidOAuthConfig, diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index afe64097c4e..a6579ea7b28 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -20,7 +20,7 @@ import ( "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" - "github.com/grafana/grafana/pkg/services/ssosettings" + "github.com/grafana/grafana/pkg/services/ssosettings/validation" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -223,17 +223,8 @@ func getRoleFromSearch(role string) (org.RoleType, bool) { } func validateInfo(info *social.OAuthInfo, requester identity.Requester) error { - if info.ClientId == "" { - return ssosettings.ErrInvalidOAuthConfig("Client Id is empty.") - } - - if info.AllowAssignGrafanaAdmin && !requester.GetIsGrafanaAdmin() { - return ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin can only be updated by Grafana Server Admins.") - } - - if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { - return ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin and Skip org role sync are both set thus Grafana Admin role will not be synced. Consider setting one or the other.") - } - - return nil + return validation.Validate(info, requester, + validation.RequiredValidator(info.ClientId, "Client Id"), + validation.AllowAssignGrafanaAdminValidator, + validation.SkipOrgRoleSyncAllowAssignGrafanaAdminValidator) } diff --git a/pkg/services/ssosettings/ssosettings.go b/pkg/services/ssosettings/ssosettings.go index be0bdaac718..0f5c92199e3 100644 --- a/pkg/services/ssosettings/ssosettings.go +++ b/pkg/services/ssosettings/ssosettings.go @@ -66,3 +66,5 @@ type Store interface { Upsert(ctx context.Context, settings *models.SSOSettings) error Delete(ctx context.Context, provider string) error } + +type ValidateFunc[T any] func(input *T, requester identity.Requester) error diff --git a/pkg/services/ssosettings/validation/oauth_validators.go b/pkg/services/ssosettings/validation/oauth_validators.go new file mode 100644 index 00000000000..23c8b256601 --- /dev/null +++ b/pkg/services/ssosettings/validation/oauth_validators.go @@ -0,0 +1,69 @@ +package validation + +import ( + "fmt" + "net/url" + "strings" + + "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ssosettings" +) + +func AllowAssignGrafanaAdminValidator(info *social.OAuthInfo, requester identity.Requester) error { + if info.AllowAssignGrafanaAdmin && !requester.GetIsGrafanaAdmin() { + return ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin can only be updated by Grafana Server Admins.") + } + return nil +} + +func SkipOrgRoleSyncAllowAssignGrafanaAdminValidator(info *social.OAuthInfo, requester identity.Requester) error { + if info.AllowAssignGrafanaAdmin && info.SkipOrgRoleSync { + return ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin and Skip org role sync are both set thus Grafana Admin role will not be synced. Consider setting one or the other.") + } + return nil +} + +func RequiredValidator(value string, name string) ssosettings.ValidateFunc[social.OAuthInfo] { + return func(info *social.OAuthInfo, requester identity.Requester) error { + if value == "" { + return ssosettings.ErrInvalidOAuthConfig(fmt.Sprintf("%s is required.", name)) + } + return nil + } +} + +func UrlValidator(value string, name string) ssosettings.ValidateFunc[social.OAuthInfo] { + return func(info *social.OAuthInfo, requester identity.Requester) error { + if !isValidUrl(value) { + return ssosettings.ErrInvalidOAuthConfig(fmt.Sprintf("%s is an invalid URL.", name)) + } + return nil + } +} + +func RequiredUrlValidator(value string, name string) ssosettings.ValidateFunc[social.OAuthInfo] { + return func(info *social.OAuthInfo, requester identity.Requester) error { + if err := RequiredValidator(value, name)(info, requester); err != nil { + return err + } + return UrlValidator(value, name)(info, requester) + } +} + +func MustBeEmptyValidator(value string, name string) ssosettings.ValidateFunc[social.OAuthInfo] { + return func(info *social.OAuthInfo, requester identity.Requester) error { + if value != "" { + return ssosettings.ErrInvalidOAuthConfig(fmt.Sprintf("%s must be empty.", name)) + } + return nil + } +} + +func isValidUrl(actual string) bool { + parsed, err := url.ParseRequestURI(actual) + if err != nil { + return false + } + return strings.HasPrefix(parsed.Scheme, "http") && parsed.Host != "" +} diff --git a/pkg/services/ssosettings/validation/oauth_validators_test.go b/pkg/services/ssosettings/validation/oauth_validators_test.go new file mode 100644 index 00000000000..1435fa110b9 --- /dev/null +++ b/pkg/services/ssosettings/validation/oauth_validators_test.go @@ -0,0 +1,154 @@ +package validation + +import ( + "testing" + + "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ssosettings" + "github.com/grafana/grafana/pkg/services/user" + "github.com/stretchr/testify/require" +) + +type testCase struct { + name string + input *social.OAuthInfo + requester identity.Requester + wantErr error +} + +func TestUrlValidator(t *testing.T) { + tc := []testCase{ + { + name: "passes when url is valid", + input: &social.OAuthInfo{ + AuthUrl: "https://example.com/auth", + }, + wantErr: nil, + }, + { + name: "fails when url is invalid", + input: &social.OAuthInfo{ + AuthUrl: "file://etc", + }, + wantErr: ssosettings.ErrInvalidOAuthConfig("Auth URL is an invalid URL."), + }, + } + + for _, tt := range tc { + t.Run(tt.name, func(t *testing.T) { + err := UrlValidator(tt.input.AuthUrl, "Auth URL")(tt.input, tt.requester) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} + +func TestRequiredValidator(t *testing.T) { + tc := []testCase{ + { + name: "passes when client id is not empty", + input: &social.OAuthInfo{ + ClientId: "client-id", + }, + wantErr: nil, + }, + { + name: "fails when client id is empty", + input: &social.OAuthInfo{ + ClientId: "", + }, + wantErr: ssosettings.ErrInvalidOAuthConfig("Client Id is required."), + }, + } + + for _, tt := range tc { + t.Run(tt.name, func(t *testing.T) { + err := RequiredValidator(tt.input.ClientId, "Client Id")(tt.input, tt.requester) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} + +func TestAllowAssignGrafanaAdminValidator(t *testing.T) { + tc := []testCase{ + { + name: "passes when user is grafana admin and allow assign grafana admin is true", + input: &social.OAuthInfo{ + AllowAssignGrafanaAdmin: true, + }, + requester: &user.SignedInUser{ + IsGrafanaAdmin: true, + }, + wantErr: nil, + }, + { + name: "fails when user is not grafana admin and allow assign grafana admin is true", + input: &social.OAuthInfo{ + AllowAssignGrafanaAdmin: true, + }, + requester: &user.SignedInUser{ + IsGrafanaAdmin: false, + }, + wantErr: ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin can only be updated by Grafana Server Admins."), + }, + } + + for _, tt := range tc { + t.Run(tt.name, func(t *testing.T) { + err := AllowAssignGrafanaAdminValidator(tt.input, tt.requester) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} + +func TestSkipOrgRoleSyncAllowAssignGrafanaAdminValidator(t *testing.T) { + tc := []testCase{ + { + name: "passes when allow assign grafana admin is set, but skip org role sync is not set", + input: &social.OAuthInfo{ + AllowAssignGrafanaAdmin: true, + SkipOrgRoleSync: false, + }, + wantErr: nil, + }, + { + name: "passes when allow assign grafana admin is not set, but skip org role sync is set", + input: &social.OAuthInfo{ + AllowAssignGrafanaAdmin: false, + SkipOrgRoleSync: true, + }, + wantErr: nil, + }, + { + name: "fails when both allow assign grafana admin and skip org role sync is set", + input: &social.OAuthInfo{ + AllowAssignGrafanaAdmin: true, + SkipOrgRoleSync: true, + }, + wantErr: ssosettings.ErrInvalidOAuthConfig("Allow assign Grafana Admin and Skip org role sync are both set thus Grafana Admin role will not be synced. Consider setting one or the other."), + }, + } + + for _, tt := range tc { + t.Run(tt.name, func(t *testing.T) { + err := SkipOrgRoleSyncAllowAssignGrafanaAdminValidator(tt.input, nil) + if tt.wantErr != nil { + require.ErrorIs(t, err, tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} diff --git a/pkg/services/ssosettings/validation/validator.go b/pkg/services/ssosettings/validation/validator.go new file mode 100644 index 00000000000..b235f7f41cb --- /dev/null +++ b/pkg/services/ssosettings/validation/validator.go @@ -0,0 +1,16 @@ +package validation + +import ( + "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/ssosettings" +) + +func Validate(info *social.OAuthInfo, requester identity.Requester, validators ...ssosettings.ValidateFunc[social.OAuthInfo]) error { + for _, validatorFunc := range validators { + if err := validatorFunc(info, requester); err != nil { + return err + } + } + return nil +} diff --git a/public/app/features/auth-config/fields.tsx b/public/app/features/auth-config/fields.tsx index 3f4a4f5abf1..6bc81af4b15 100644 --- a/public/app/features/auth-config/fields.tsx +++ b/public/app/features/auth-config/fields.tsx @@ -6,6 +6,7 @@ import { contextSrv } from 'app/core/core'; import { FieldData, SSOProvider, SSOSettingsField } from './types'; import { isSelectableValue } from './utils/guards'; +import { isUrlValid } from './utils/url'; /** Map providers to their settings */ export const fields: Record> = { @@ -139,7 +140,11 @@ export function fieldMap(provider: string): Record { type: 'text', description: 'The authorization endpoint of your OAuth2 provider.', validation: { - required: false, + required: true, + validate: (value) => { + return isUrlValid(value); + }, + message: 'This field is required and must be a valid URL.', }, }, authStyle: { @@ -159,7 +164,11 @@ export function fieldMap(provider: string): Record { type: 'text', description: 'The token endpoint of your OAuth2 provider.', validation: { - required: false, + required: true, + validate: (value) => { + return isUrlValid(value); + }, + message: 'This field is required and must be a valid URL.', }, }, scopes: { @@ -214,6 +223,18 @@ export function fieldMap(provider: string): Record { ), validation: { required: false, + validate: (value) => { + if (typeof value !== 'string') { + return false; + } + + if (value.length) { + return isUrlValid(value); + } + + return true; + }, + message: 'This field must be a valid URL if set.', }, }, roleAttributePath: { @@ -360,12 +381,17 @@ export function fieldMap(provider: string): Record { type: 'text', validation: { validate: (value, formValues) => { + let result = true; if (formValues.teamIds.length) { - return !!value; + result = !!value; } - return true; + + if (typeof value === 'string' && value.length) { + result = isUrlValid(value); + } + return result; }, - message: 'This field must be set if Team Ids are configured.', + message: 'This field must be set if Team Ids are configured and must be a valid URL.', }, }, teamIdsAttributePath: { diff --git a/public/app/features/auth-config/utils/url.ts b/public/app/features/auth-config/utils/url.ts index 089f72b7955..cf98cf311c2 100644 --- a/public/app/features/auth-config/utils/url.ts +++ b/public/app/features/auth-config/utils/url.ts @@ -4,3 +4,15 @@ import { AuthProviderInfo } from '../types'; export function getProviderUrl(provider: AuthProviderInfo) { return BASE_PATH + (provider.configPath || provider.id); } + +export const isUrlValid = (url: unknown): boolean => { + if (typeof url !== 'string') { + return false; + } + try { + const parsedUrl = new URL(url); + return parsedUrl.protocol.includes('http'); + } catch (_) { + return false; + } +};