Read about our upcoming Code of Conduct on this issue

Commit da6ed6ed authored by Nick Thomas's avatar Nick Thomas
Browse files

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
parent 854da4efdb45
......@@ -55,6 +55,22 @@
}
}
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)
......@@ -74,10 +90,8 @@
} 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
}
......
......@@ -66,10 +66,11 @@
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,7 +78,8 @@
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,
},
{
......@@ -82,9 +84,15 @@
},
{
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)
......@@ -85,10 +93,10 @@
},
}
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 {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment