diff options
author | Nick Thomas <nick@gitlab.com> | 2021-09-23 07:48:16 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2021-09-23 07:48:16 +0000 |
commit | 882c55eabf74eb5996098c9898045099927803a1 (patch) | |
tree | d3e5382e29ba9633cc01e3ae79a56b1fd1f786a3 | |
parent | a7c424fe96f18ac18b454bd734d9be99c78e452e (diff) | |
parent | d2f64237fc08116695d690c3b264c0d106a93ec5 (diff) | |
download | gitlab-shell-882c55eabf74eb5996098c9898045099927803a1.tar.gz |
Merge branch 'sh-fix-issue-529' into 'main'
Only validate SSL cert file exists if a value is supplied
See merge request gitlab-org/gitlab-shell!527
-rw-r--r-- | client/httpclient.go | 22 | ||||
-rw-r--r-- | client/httpsclient_test.go | 24 |
2 files changed, 34 insertions, 12 deletions
diff --git a/client/httpclient.go b/client/httpclient.go index 72238f8..cdf5665 100644 --- a/client/httpclient.go +++ b/client/httpclient.go @@ -54,6 +54,22 @@ func WithClientCert(certPath, keyPath string) HTTPClientOpt { } } +func validateCaFile(filename string) error { + if filename == "" { + return nil + } + + if _, err := os.Stat(filename); err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("cannot find cafile '%s': %w", filename, ErrCafileNotFound) + } + + return err + } + + return nil +} + // Deprecated: use NewHTTPClientWithOpts - https://gitlab.com/gitlab-org/gitlab-shell/-/issues/484 func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient { c, err := NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath, selfSignedCert, readTimeoutSeconds, nil) @@ -73,10 +89,8 @@ func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath stri } else if strings.HasPrefix(gitlabURL, httpProtocol) { transport, host = buildHttpTransport(gitlabURL) } else if strings.HasPrefix(gitlabURL, httpsProtocol) { - if _, err := os.Stat(caFile); err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("cannot find cafile '%s': %w", caFile, ErrCafileNotFound) - } + err = validateCaFile(caFile) + if err != nil { return nil, err } diff --git a/client/httpsclient_test.go b/client/httpsclient_test.go index d2c2293..debe1bd 100644 --- a/client/httpsclient_test.go +++ b/client/httpsclient_test.go @@ -66,10 +66,11 @@ func TestSuccessfulRequests(t *testing.T) { func TestFailedRequests(t *testing.T) { testCases := []struct { - desc string - caFile string - caPath string - expectedError string + desc string + caFile string + caPath string + expectedCaFileNotFound bool + expectedError string }{ { desc: "Invalid CaFile", @@ -77,18 +78,25 @@ func TestFailedRequests(t *testing.T) { expectedError: "Internal API unreachable", }, { - desc: "Invalid CaPath", - caPath: path.Join(testhelper.TestRoot, "certs/invalid"), + desc: "Missing CaFile", + caFile: path.Join(testhelper.TestRoot, "certs/invalid/missing.crt"), + expectedCaFileNotFound: true, }, { - desc: "Empty config", + desc: "Invalid CaPath", + caPath: path.Join(testhelper.TestRoot, "certs/invalid"), + expectedError: "Internal API unreachable", + }, + { + desc: "Empty config", + expectedError: "Internal API unreachable", }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { client, err := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false) - if tc.caFile == "" { + if tc.expectedCaFileNotFound { require.Error(t, err) require.ErrorIs(t, err, ErrCafileNotFound) } else { |