OAuth: Removes send_client_credentials_via_post setting (grafana#20044)

Removes send_client_credentials_via_post oauth setting and 
use auto-detect mechanism instead.
By these changes also fixes statichcheck errors

Ref grafana#8968

Author: LK4D4

conf/defaults.ini

@@ -387,7 +387,6 @@ tls_skip_verify_insecure = false
 tls_client_cert =
 tls_client_key =
 tls_client_ca =
-send_client_credentials_via_post = false
 
 #################################### SAML Auth ###########################
 [auth.saml] # Enterprise only

conf/sample.ini

@@ -378,10 +378,6 @@
 ;tls_client_key =
 ;tls_client_ca =
 
-; Set to true to enable sending client_id and client_secret via POST body instead of Basic authentication HTTP header
-; This might be required if the OAuth provider is not RFC6749 compliant, only supporting credentials passed via POST payload
-;send_client_credentials_via_post = false
-
 #################################### SAML Auth ###########################
 [auth.saml] # Enterprise only
 # Defaults to false. If true, the feature is enabled.

docs/sources/auth/generic-oauth.md

@@ -219,20 +219,6 @@ allowed_organizations =
     auth_url = https://<your domain>.my.centrify.com/OAuth2/Authorize/<Application ID>
     token_url = https://<your domain>.my.centrify.com/OAuth2/Token/<Application ID>
     api_url = https://<your domain>.my.centrify.com/OAuth2/UserInfo/<Application ID>
-    ```
-
-## Set up OAuth2 with non-compliant providers
-
-> Only available in Grafana v6.0 and above.
-
-Some OAuth2 providers might not support `client_id` and `client_secret` passed via Basic Authentication HTTP header, which
-results in `invalid_client` error. To allow Grafana to authenticate via these type of providers, the client identifiers must be
-send via POST body, which can be enabled via the following settings:
-
-```bash
-[auth.generic_oauth]
-send_client_credentials_via_post = true
-```
 
 ## JMESPath examples
 

docs/sources/installation/upgrading.md

@@ -85,7 +85,7 @@ sudo apt-get update
 
 #### Upgrade from binary .tar file
 
-If you downloaded the binary `.tar.gz` package, then you can just download and extract the new package and overwrite all your existing files. However, this might overwrite your config changes. 
+If you downloaded the binary `.tar.gz` package, then you can just download and extract the new package and overwrite all your existing files. However, this might overwrite your config changes.
 
 We recommend that you save your custom config changes in a file named `<grafana_install_dir>/conf/custom.ini`.
 This allows you to upgrade Grafana without risking losing your configuration changes.
@@ -222,3 +222,7 @@ Pre Grafana 6.5.0, the CloudWatch datasource used the GetMetricStatistics API fo
 Each request to the GetMetricData API can include 100 queries. This means that each panel in Grafana will only issue one GetMetricData request, regardless of the number of query rows that are present in the panel. Consequently as it is no longer possible to set `HighRes` on a per query level anymore, this switch is now removed from the query editor. High resolution can still be achieved by choosing a smaller minimum period in the query editor.
 
 The handling of multi-valued template variables in dimension values has been changed in Grafana 6.5. When a multi template variable is being used, Grafana will generate a search expression. In the GetMetricData API, expressions are limited to 1024 characters, so it might be the case that this limit is reached when a multi-valued template variable that has a lot of values is being used. If this is the case, we suggest you start using `*` wildcard as dimension value instead of a multi-valued template variable.
+
+## Upgrading to v6.6
+
+The Generic OAuth setting `send_client_credentials_via_post`, used for supporting non-compliant providers, has been removed. From now on, Grafana will automatically detect if credentials should be sent as part of the URL or request body for a specific provider. The result will be remembered and used for additional OAuth requests for that provider.

pkg/login/social/social.go

@@ -65,37 +65,30 @@ func NewOAuthService() {
 	for _, name := range allOauthes {
 		sec := setting.Raw.Section("auth." + name)
 		info := &setting.OAuthInfo{
-			ClientId:                     sec.Key("client_id").String(),
-			ClientSecret:                 sec.Key("client_secret").String(),
-			Scopes:                       util.SplitString(sec.Key("scopes").String()),
-			AuthUrl:                      sec.Key("auth_url").String(),
-			TokenUrl:                     sec.Key("token_url").String(),
-			ApiUrl:                       sec.Key("api_url").String(),
-			Enabled:                      sec.Key("enabled").MustBool(),
-			EmailAttributeName:           sec.Key("email_attribute_name").String(),
-			EmailAttributePath:           sec.Key("email_attribute_path").String(),
-			RoleAttributePath:            sec.Key("role_attribute_path").String(),
-			AllowedDomains:               util.SplitString(sec.Key("allowed_domains").String()),
-			HostedDomain:                 sec.Key("hosted_domain").String(),
-			AllowSignup:                  sec.Key("allow_sign_up").MustBool(),
-			Name:                         sec.Key("name").MustString(name),
-			TlsClientCert:                sec.Key("tls_client_cert").String(),
-			TlsClientKey:                 sec.Key("tls_client_key").String(),
-			TlsClientCa:                  sec.Key("tls_client_ca").String(),
-			TlsSkipVerify:                sec.Key("tls_skip_verify_insecure").MustBool(),
-			SendClientCredentialsViaPost: sec.Key("send_client_credentials_via_post").MustBool(),
+			ClientId:           sec.Key("client_id").String(),
+			ClientSecret:       sec.Key("client_secret").String(),
+			Scopes:             util.SplitString(sec.Key("scopes").String()),
+			AuthUrl:            sec.Key("auth_url").String(),
+			TokenUrl:           sec.Key("token_url").String(),
+			ApiUrl:             sec.Key("api_url").String(),
+			Enabled:            sec.Key("enabled").MustBool(),
+			EmailAttributeName: sec.Key("email_attribute_name").String(),
+			EmailAttributePath: sec.Key("email_attribute_path").String(),
+			RoleAttributePath:  sec.Key("role_attribute_path").String(),
+			AllowedDomains:     util.SplitString(sec.Key("allowed_domains").String()),
+			HostedDomain:       sec.Key("hosted_domain").String(),
+			AllowSignup:        sec.Key("allow_sign_up").MustBool(),
+			Name:               sec.Key("name").MustString(name),
+			TlsClientCert:      sec.Key("tls_client_cert").String(),
+			TlsClientKey:       sec.Key("tls_client_key").String(),
+			TlsClientCa:        sec.Key("tls_client_ca").String(),
+			TlsSkipVerify:      sec.Key("tls_skip_verify_insecure").MustBool(),
 		}
 
 		if !info.Enabled {
 			continue
 		}
 
-		// handle the clients that do not properly support Basic auth headers and require passing client_id/client_secret via POST payload
-		if info.SendClientCredentialsViaPost {
-			// TODO: Fix the staticcheck error
-			oauth2.RegisterBrokenAuthHeaderProvider(info.TokenUrl) //nolint:staticcheck
-		}
-
 		if name == "grafananet" {
 			name = grafanaCom
 		}
@@ -106,8 +99,9 @@ func NewOAuthService() {
 			ClientID:     info.ClientId,
 			ClientSecret: info.ClientSecret,
 			Endpoint: oauth2.Endpoint{
-				AuthURL:  info.AuthUrl,
-				TokenURL: info.TokenUrl,
+				AuthURL:   info.AuthUrl,
+				TokenURL:  info.TokenUrl,
+				AuthStyle: oauth2.AuthStyleAutoDetect,
 			},
 			RedirectURL: strings.TrimSuffix(setting.AppUrl, "/") + SocialBaseUrl + name,
 			Scopes:      info.Scopes,
@@ -181,8 +175,9 @@ func NewOAuthService() {
 				ClientID:     info.ClientId,
 				ClientSecret: info.ClientSecret,
 				Endpoint: oauth2.Endpoint{
-					AuthURL:  setting.GrafanaComUrl + "/oauth2/authorize",
-					TokenURL: setting.GrafanaComUrl + "/api/oauth2/token",
+					AuthURL:   setting.GrafanaComUrl + "/oauth2/authorize",
+					TokenURL:  setting.GrafanaComUrl + "/api/oauth2/token",
+					AuthStyle: oauth2.AuthStyleInHeader,
 				},
 				RedirectURL: strings.TrimSuffix(setting.AppUrl, "/") + SocialBaseUrl + name,
 				Scopes:      info.Scopes,

pkg/services/provisioning/dashboards/file_reader.go

@@ -54,11 +54,10 @@ func NewDashboardFileReader(cfg *DashboardsAsConfig, log log.Logger) (*fileReade
 // pollChanges periodically runs startWalkingDisk based on interval specified in the config.
 func (fr *fileReader) pollChanges(ctx context.Context) {
 
-	// TODO: Fix the staticcheck error
-	ticker := time.Tick(time.Duration(int64(time.Second) * fr.Cfg.UpdateIntervalSeconds)) //nolint:staticcheck
+	ticker := time.NewTicker(time.Duration(int64(time.Second) * fr.Cfg.UpdateIntervalSeconds))
 	for {
 		select {
-		case <-ticker:
+		case <-ticker.C:
 			if err := fr.startWalkingDisk(); err != nil {
 				fr.log.Error("failed to search for dashboards", "error", err)
 			}

pkg/setting/setting_oauth.go

@@ -1,23 +1,22 @@
 package setting
 
 type OAuthInfo struct {
-	ClientId, ClientSecret       string
-	Scopes                       []string
-	AuthUrl, TokenUrl            string
-	Enabled                      bool
-	EmailAttributeName           string
-	EmailAttributePath           string
-	RoleAttributePath            string
-	AllowedDomains               []string
-	HostedDomain                 string
-	ApiUrl                       string
-	AllowSignup                  bool
-	Name                         string
-	TlsClientCert                string
-	TlsClientKey                 string
-	TlsClientCa                  string
-	TlsSkipVerify                bool
-	SendClientCredentialsViaPost bool
+	ClientId, ClientSecret string
+	Scopes                 []string
+	AuthUrl, TokenUrl      string
+	Enabled                bool
+	EmailAttributeName     string
+	EmailAttributePath     string
+	RoleAttributePath      string
+	AllowedDomains         []string
+	HostedDomain           string
+	ApiUrl                 string
+	AllowSignup            bool
+	Name                   string
+	TlsClientCert          string
+	TlsClientKey           string
+	TlsClientCa            string
+	TlsSkipVerify          bool
 }
 
 type OAuther struct {