Squashed 'src/SfsClient/sfs-client/' changes from be733af9..44d81dc8

44d81dc8 Using early-return in UrlBuilder::GetQuery() 9e9b27e3 Fixing "Use of string after lifetime ends" 21bf6b98 Adding functional proxy tests 46388eaa Adding a ProxyServer implementation 7a0bfbec Exposing Path and Query via UrlBuilder b2014df3 Adding unit tests for proxy validation cdd840b0 Adding proxy input string support 216210ab Adding required permissions to enable uploading of CodeQL results (microsoft#214) fb953d6e Bump github/codeql-action from 2 to 3 (microsoft#215) 52af7124 Enabling CodeQL scanning (microsoft#211) e555d764 Bump clang-format from 18.1.5 to 19.1.1 (microsoft#210) ab8f0e72 Setup: improving build tools installation (microsoft#207) git-subtree-dir: src/SfsClient/sfs-client git-subtree-split: 44d81dc8e7614c0be8777db22431e5065aa7a6b8
nidietr-MSFT · Dec 6, 2024 · ac2ecc7 · ac2ecc7
1 parent d5ae2a9
commit ac2ecc7
Show file tree

Hide file tree

Showing 21 changed files with 584 additions and 173 deletions.
diff --git a/.github/workflows/initialize-codeql/action.yml b/.github/workflows/initialize-codeql/action.yml
@@ -0,0 +1,12 @@
+name: Initialize CodeQL
+
+description: Initializes CodeQL action to be used in build workflows
+
+runs:
+  using: "composite"
+
+  steps:
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v3
+      with:
+          languages: cpp
diff --git a/.github/workflows/main-build-ubuntu.yml b/.github/workflows/main-build-ubuntu.yml
@@ -5,8 +5,10 @@ on:
     branches: [ "main" ]
 
 # Permissions and environment values to be able to update the dependency graph with vcpkg information
+# and to enable the writing/uploading of CodeQL scan results
 permissions:
   contents: write
+  security-events: write
 
 env:
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -19,6 +21,9 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: Initialize CodeQL
+        uses: ./.github/workflows/initialize-codeql
+
       - name: Setup
         run: source ./scripts/setup.sh
 
@@ -36,3 +41,6 @@ jobs:
         run: |
           ./scripts/build.sh --build-type Release
           ./scripts/test.sh --output-on-failure
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3
diff --git a/.github/workflows/main-build-windows.yml b/.github/workflows/main-build-windows.yml
@@ -5,8 +5,10 @@ on:
     branches: [ "main" ]
 
 # Permissions and environment values to be able to update the dependency graph with vcpkg information
+# and to enable the writing/uploading of CodeQL scan results
 permissions:
   contents: write
+  security-events: write
 
 env:
   GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
@@ -19,12 +21,15 @@ jobs:
     steps:
       - uses: actions/checkout@v4
 
+      - name: Initialize CodeQL
+        uses: ./.github/workflows/initialize-codeql
+
       - name: Install Winget
         uses: ./.github/workflows/install-winget
 
       - name: Setup
         shell: pwsh
-        run: .\scripts\Setup.ps1 -NoBuildTools
+        run: .\scripts\Setup.ps1
 
       - name: Build and Test (no test overrides)
         shell: pwsh
@@ -43,3 +48,6 @@ jobs:
         run: |
             .\scripts\Build.ps1 -BuildType Release
             .\scripts\Test.ps1 -OutputOnFailure
+      
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3
diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml
@@ -22,12 +22,15 @@ jobs:
             core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');
             core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');
 
+      - name: Initialize CodeQL
+        uses: ./.github/workflows/initialize-codeql
+
       - name: Install Winget
         uses: ./.github/workflows/install-winget
 
       - name: Setup
         shell: pwsh
-        run: .\scripts\Setup.ps1 -NoBuildTools
+        run: .\scripts\Setup.ps1
 
       - name: Check formatting
         shell: pwsh
@@ -45,6 +48,9 @@ jobs:
           .\scripts\Build.ps1 -EnableTestOverrides
           .\scripts\Test.ps1 -OutputOnFailure
 
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3
+
   build-ubuntu:
     runs-on: ubuntu-latest
 
@@ -58,6 +64,9 @@ jobs:
             core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');
             core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');
 
+      - name: Initialize CodeQL
+        uses: ./.github/workflows/initialize-codeql
+
       - name: Setup
         run: source ./scripts/setup.sh
 
@@ -73,3 +82,6 @@ jobs:
         run: |
           ./scripts/build.sh --enable-test-overrides
           ./scripts/test.sh --output-on-failure
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3
diff --git a/client/include/sfsclient/RequestParams.h b/client/include/sfsclient/RequestParams.h
@@ -35,6 +35,11 @@ struct RequestParams
     /// @note If not provided, a new CorrelationVector will be generated
     std::optional<std::string> baseCV;
 
+    /// @brief Proxy setting which can be used to establish connections with the server (optional)
+    /// @note The string can be a hostname or dotted numerical IP address. It can be suffixed with the port number
+    /// like :[port], and can be prefixed with [scheme]://. If not provided, no proxy will be used.
+    std::optional<std::string> proxy;
+
     /// @brief Retry for a web request after a failed attempt. If true, client will retry up to c_maxRetries times
     bool retryOnError{true};
 };

diff --git a/client/src/details/UrlBuilder.cpp b/client/src/details/UrlBuilder.cpp
@@ -66,6 +66,31 @@ std::string UrlBuilder::GetUrl() const
     return urlPtr;
 }
 
+std::string UrlBuilder::GetPath() const
+{
+    CurlCharPtr path;
+    char* pathPtr = path.get();
+    THROW_IF_CURL_URL_SETUP_ERROR(curl_url_get(m_handle, CURLUPART_PATH, &pathPtr, 0 /*flags*/));
+    return pathPtr;
+}
+
+std::string UrlBuilder::GetQuery() const
+{
+    CurlCharPtr query;
+    char* queryPtr = query.get();
+    const auto queryResult = curl_url_get(m_handle, CURLUPART_QUERY, &queryPtr, 0 /*flags*/);
+    switch (queryResult)
+    {
+    case CURLUE_OK:
+        return queryPtr;
+    case CURLUE_NO_QUERY:
+        return {};
+    default:
+        THROW_IF_CURL_URL_SETUP_ERROR(queryResult);
+    }
+    return {};
+}
+
 UrlBuilder& UrlBuilder::SetScheme(Scheme scheme)
 {
     switch (scheme)

diff --git a/client/src/details/UrlBuilder.h b/client/src/details/UrlBuilder.h
@@ -39,6 +39,9 @@ class UrlBuilder
 
     std::string GetUrl() const;
 
+    std::string GetPath() const;
+    std::string GetQuery() const;
+
     /**
      * @brief Set the scheme for the URL
      * @param scheme The scheme to set for the URL Ex: Https

diff --git a/client/src/details/connection/ConnectionConfig.cpp b/client/src/details/connection/ConnectionConfig.cpp
@@ -11,5 +11,6 @@ using namespace SFS::details;
 ConnectionConfig::ConnectionConfig(const SFS::RequestParams& requestParams)
     : maxRetries(requestParams.retryOnError ? c_maxRetries : 0)
     , baseCV(requestParams.baseCV)
+    , proxy(requestParams.proxy)
 {
 }
diff --git a/client/src/details/connection/ConnectionConfig.h b/client/src/details/connection/ConnectionConfig.h
@@ -22,6 +22,9 @@ struct ConnectionConfig
 
     /// @brief The correlation vector to use for requests
     std::optional<std::string> baseCV;
+
+    /// @brief Proxy setting which can be used to establish connections with the server
+    std::optional<std::string> proxy;
 };
 } // namespace details
 } // namespace SFS
diff --git a/client/src/details/connection/CurlConnection.cpp b/client/src/details/connection/CurlConnection.cpp
@@ -284,6 +284,11 @@ CurlConnection::CurlConnection(const ConnectionConfig& config, const ReportingHa
                           m_handler,
                           "Failed to set up curl");
 
+    if (config.proxy)
+    {
+        THROW_IF_CURL_SETUP_ERROR(curl_easy_setopt(m_handle, CURLOPT_PROXY, config.proxy->c_str()));
+    }
+
     // TODO #41: Pass AAD token in the header if it is available
     // TODO #42: Cert pinning with service
 }

diff --git a/client/tests/CMakeLists.txt b/client/tests/CMakeLists.txt
@@ -17,6 +17,8 @@ target_sources(
             functional/details/SFSClientImplTests.cpp
             functional/SFSClientTests.cpp
             mock/MockWebServer.cpp
+            mock/ProxyServer.cpp
+            mock/ServerCommon.cpp
             unit/AppContentTests.cpp
             unit/AppFileTests.cpp
             unit/ApplicabilityDetailsTests.cpp

diff --git a/client/tests/functional/SFSClientTests.cpp b/client/tests/functional/SFSClientTests.cpp
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 #include "../mock/MockWebServer.h"
+#include "../mock/ProxyServer.h"
 #include "../util/TestHelper.h"
 #include "TestOverride.h"
 #include "sfsclient/SFSClient.h"
@@ -112,6 +113,19 @@ TEST("Testing SFSClient::GetLatestDownloadInfo()")
             CheckMockContent(contents[0], c_version);
         }
 
+        SECTION("No attributes + proxy")
+        {
+            test::ProxyServer proxy;
+
+            params.productRequests = {{c_productName, {}}};
+            params.proxy = proxy.GetBaseUrl();
+            REQUIRE(sfsClient->GetLatestDownloadInfo(params, contents) == Result::Success);
+            REQUIRE(contents.size() == 1);
+            CheckMockContent(contents[0], c_version);
+
+            REQUIRE(proxy.Stop() == Result::Success);
+        }
+
         SECTION("With attributes")
         {
             const TargetingAttributes attributes{{"attr1", "value"}};
@@ -143,6 +157,8 @@ TEST("Testing SFSClient::GetLatestDownloadInfo()")
             CheckMockContent(contents[0], c_nextVersion);
         }
     }
+
+    REQUIRE(server.Stop() == Result::Success);
 }
 
 TEST("Testing SFSClient::GetLatestAppDownloadInfo()")
@@ -244,6 +260,8 @@ TEST("Testing SFSClient::GetLatestAppDownloadInfo()")
                 "Unexpected content type [Generic] returned by the service does not match the expected [App]");
         REQUIRE(contents.empty());
     }
+
+    REQUIRE(server.Stop() == Result::Success);
 }
 
 TEST("Testing SFSClient retry behavior")
@@ -437,4 +455,6 @@ TEST("Testing SFSClient retry behavior")
             }
         }
     }
+
+    REQUIRE(server.Stop() == Result::Success);
 }
diff --git a/client/tests/functional/details/CurlConnectionTests.cpp b/client/tests/functional/details/CurlConnectionTests.cpp
@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 #include "../../mock/MockWebServer.h"
+#include "../../mock/ProxyServer.h"
 #include "../../util/SFSExceptionMatcher.h"
 #include "../../util/TestHelper.h"
 #include "ReportingHandler.h"
@@ -102,20 +103,46 @@ TEST("Testing CurlConnection()")
     {
         const std::string url = urlBuilder.GetSpecificVersionUrl(c_productName, c_version);
 
-        // Before registering the product, the URL returns 404 Not Found
-        REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound);
+        json expectedResponse;
+        expectedResponse["ContentId"] = {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_version}};
 
-        // Register the product
-        server.RegisterProduct(c_productName, c_version);
+        SECTION("Direct connection")
+        {
+            // Before registering the product, the URL returns 404 Not Found
+            REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound);
 
-        // After registering the product, the URL returns 200 OK
-        std::string out;
-        REQUIRE_NOTHROW(out = connection->Get(url));
+            // Register the product
+            server.RegisterProduct(c_productName, c_version);
 
-        json expectedResponse;
-        expectedResponse["ContentId"] = {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_version}};
+            // After registering the product, the URL returns 200 OK
+            std::string out;
+            REQUIRE_NOTHROW(out = connection->Get(url));
+
+            REQUIRE(json::parse(out) == expectedResponse);
+        }
+
+        SECTION("With proxy")
+        {
+            test::ProxyServer proxy;
+
+            ConnectionConfig config;
+            config.proxy = proxy.GetBaseUrl();
+            connection = connectionManager.MakeConnection(config);
+
+            // Before registering the product, the URL returns 404 Not Found
+            REQUIRE_THROWS_CODE(connection->Get(url), HttpNotFound);
+
+            // Register the product
+            server.RegisterProduct(c_productName, c_version);
+
+            // After registering the product, the URL returns 200 OK
+            std::string out;
+            REQUIRE_NOTHROW(out = connection->Get(url));
 
-        REQUIRE(json::parse(out) == expectedResponse);
+            REQUIRE(json::parse(out) == expectedResponse);
+
+            REQUIRE(proxy.Stop() == Result::Success);
+        }
     }
 
     SECTION("Testing CurlConnection::Post()")
@@ -153,6 +180,21 @@ TEST("Testing CurlConnection()")
             expectedResponse.push_back(
                 {{"ContentId", {{"Namespace", "default"}, {"Name", c_productName}, {"Version", c_nextVersion}}}});
             REQUIRE(json::parse(out) == expectedResponse);
+
+            SECTION("Testing with proxy")
+            {
+                test::ProxyServer proxy;
+
+                ConnectionConfig config;
+                config.proxy = proxy.GetBaseUrl();
+                connection = connectionManager.MakeConnection(config);
+
+                REQUIRE_NOTHROW(out = connection->Post(url, body.dump()));
+
+                REQUIRE(json::parse(out) == expectedResponse);
+
+                REQUIRE(proxy.Stop() == Result::Success);
+            }
         }
 
         SECTION("With GetDownloadInfo mock")
@@ -187,6 +229,21 @@ TEST("Testing CurlConnection()")
                 {"IntegrityCheckInfo", {{"PiecesHashFileUrl", "http://localhost/2.bin"}, {"HashOfHashes", "abcd"}}}};
 
             REQUIRE(json::parse(out) == expectedResponse);
+
+            SECTION("Testing with proxy")
+            {
+                test::ProxyServer proxy;
+
+                ConnectionConfig config;
+                config.proxy = proxy.GetBaseUrl();
+                connection = connectionManager.MakeConnection(config);
+
+                REQUIRE_NOTHROW(out = connection->Post(url));
+
+                REQUIRE(json::parse(out) == expectedResponse);
+
+                REQUIRE(proxy.Stop() == Result::Success);
+            }
         }
     }