Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make OAuth 2.0 client_id and client_secret mandatory #294

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config/http_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,9 @@ func (c *HTTPClientConfig) Validate() error {
if c.BasicAuth != nil {
return fmt.Errorf("at most one of basic_auth, oauth2 & authorization must be configured")
}
if len(c.OAuth2.ClientID) < 1 || (len(c.OAuth2.ClientSecret) < 1 || len(c.OAuth2.ClientSecretFile) < 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally use == 0 to check this.

Can we have 2 distincts errors? This would be more readable and more direct for the user.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need the token url mandatory.

return fmt.Errorf("the oauth2 client_id and either the client_secret or client_secret_file must be configured")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I would still prefer to use == 0 like elsewhere and have 2 different errors for better readability.

}
if len(c.OAuth2.ClientSecret) > 0 && len(c.OAuth2.ClientSecretFile) > 0 {
return fmt.Errorf("at most one of oauth2 client_secret & client_secret_file must be configured")
}
Expand Down
8 changes: 8 additions & 0 deletions config/http_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,14 @@ var invalidHTTPClientConfigs = []struct {
httpClientConfigFile: "testdata/http.conf.oauth2-secret-and-file-set.bad.yml",
errMsg: "at most one of oauth2 client_secret & client_secret_file must be configured",
},
{
httpClientConfigFile: "testdata/http.conf.oauth2-no-client-id.bad.yaml",
errMsg: "the oauth2 client_id and either the client_secret or client_secret_file must be configured",
},
{
httpClientConfigFile: "testdata/http.conf.oauth2-no-client-secret.bad.yaml",
errMsg: "the oauth2 client_id and either the client_secret or client_secret_file must be configured",
},
}

func newTestServer(handler func(w http.ResponseWriter, r *http.Request)) (*httptest.Server, error) {
Expand Down
2 changes: 2 additions & 0 deletions config/testdata/http.conf.oauth2-no-client-id.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
oauth2:
client_secret: "mysecret"
2 changes: 2 additions & 0 deletions config/testdata/http.conf.oauth2-no-client-secret.bad.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
oauth2:
client_id: "myclientid"
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
oauth2:
client_id: "myclient"
client_secret: "mysecret"
client_secret_file: "mysecret"