Allow the usso token only for the Submitqueue priority requests

src/__phutil_library_map__.php

@@ -484,6 +484,9 @@
     'PrettierLinter' => 'lint/linter/PrettierLinter.php',
     'PytestTestEngine' => 'unit/engine/PytestTestEngine.php',
     'SubmitQueueMockClient' => 'land/__tests__/mocks/MockSubmitQueueClient.php',
+    'SubmitQueueMockClientWithUSSOTracking' => 'land/__tests__/mocks/MockSubmitQueueClient.php',
+    'SubmitQueueMockFuture' => 'land/__tests__/mocks/MockSubmitQueueClient.php',
+    'SubmitQueueMockHTTPSFuture' => 'land/__tests__/mocks/MockSubmitQueueClient.php',
     'SubmitQueueMockRepositoryAPI' => 'land/__tests__/mocks/MockRepositoryAPI.php',
     'SubmitQueueMockWorkflow' => 'land/__tests__/mocks/MockWorkflow.php',
     'TAPTestEngine' => 'unit/engine/TAPTestEngine.php',
@@ -1016,6 +1019,9 @@
     'PrettierLinter' => 'ArcanistExternalLinter',
     'PytestTestEngine' => 'ArcanistUnitTestEngine',
     'SubmitQueueMockClient' => 'stdClass',
+    'SubmitQueueMockClientWithUSSOTracking' => 'UberSubmitQueueClient',
+    'SubmitQueueMockFuture' => 'Phobject',
+    'SubmitQueueMockHTTPSFuture' => 'Phobject',
     'SubmitQueueMockRepositoryAPI' => 'ArcanistRepositoryAPI',
     'SubmitQueueMockWorkflow' => 'ArcanistWorkflow',
     'TAPTestEngine' => 'ArcanistUnitTestEngine',

src/land/UberSubmitQueueClient.php

@@ -1,11 +1,11 @@
 <?php
 
-final class UberSubmitQueueClient extends Phobject {
+class UberSubmitQueueClient extends Phobject {
 
     private $uri;
     private $host;
     private $conduitToken;
-    private $timeout;
+    protected $timeout;
 
     public function __construct($uri, $conduitToken, $timeout=10) {
         $this->uri = new PhutilURI($uri);
@@ -42,7 +42,7 @@ public function submitPriorityMergeRequest($revisionId) {
         'revisionId' => $revisionId,
         'conduitToken' => $this->conduitToken,
       );
-      return $this->callMethodSynchronous("POST", "/v2/priority_merge_request", $params);
+      return $this->callMethodSynchronous("POST", "/v2/priority_merge_request", $params, true);
   }
 
   public function submitMergeStackRequest($remoteUrl, $stack, $shouldShadow, $targetOnto) {
@@ -58,25 +58,27 @@ public function submitMergeStackRequest($remoteUrl, $stack, $shouldShadow, $targ
     return $this->callMethodSynchronous("POST", "/merge_requests", $params);
   }
 
-    private function callMethodSynchronous($method, $api, array $params) {
-        return $this->callMethod($method, $api, $params)->resolve();
+    private function callMethodSynchronous($method, $api, array $params, $use_usso_token = false) {
+        return $this->callMethod($method, $api, $params, $use_usso_token)->resolve();
     }
 
-    private function callMethod($method, $api, array $params) {
+    protected function callMethod($method, $api, array $params, $use_usso_token = false) {
         $req = id(clone $this->uri)->setPath('/api'.$api.'?'.http_build_query($params));
         // Always use the cURL-based HTTPSFuture, for proxy support and other
         // protocol edge cases that HTTPFuture does not support.
         $core_future = new HTTPSFuture($req);
         $core_future->addHeader('Host', $this->getHost());
 
-        // Add uSSO token to the request
-        $usso = new UberUSSO();
-        $hostname = parse_url($this->uri, PHP_URL_HOST);
-        $token = $usso->maybeUseUSSOToken($hostname);
-        if (!$token) {
-          $token = $usso->getUSSOToken($hostname);
+        // Add uSSO token to the request only when using --skip-submitqueue-checks flag
+        if ($use_usso_token) {
+          $usso = new UberUSSO();
+          $hostname = parse_url($this->uri, PHP_URL_HOST);
+          $token = $usso->maybeUseUSSOToken($hostname);
+          if (!$token) {
+            $token = $usso->getUSSOToken($hostname);
+          }
+          $core_future->addHeader('Authorization', "Bearer {$token}");
         }
-        $core_future->addHeader('Authorization', "Bearer {$token}");
 
         $core_future->setMethod($method);
         $core_future->setTimeout($this->timeout);

src/land/__tests__/UberArcanistSubmitQueueEngineTestCase.php

@@ -154,4 +154,61 @@ public function testPushChangeToSubmitQueue_PriorityMerge() {
     $this->assertEqual('111', $mock_client->last_call_params['revisionId']);
     $this->assertEqual(1, count($mock_client->last_call_params), 'Should only have one parameter');
   }
+
+  /**
+   * Test that submitMergeRequest does NOT add USSO token.
+   */
+  public function testSubmitMergeRequest_NoUSSOToken() {
+    $client = new SubmitQueueMockClientWithUSSOTracking(
+      'https://submit-queue.example.com',
+      'test-conduit-token'
+    );
+
+    // Call submitMergeRequest (regular merge, no --skip-submitqueue-checks flag)
+    $client->submitMergeRequest(
+      'git@github.com:test/repo.git',
+      '123',
+      '456',
+      false,
+      'master'
+    );
+
+    // Verify that use_usso_token was false
+    $this->assertEqual(false, $client->last_use_usso_token,
+      'Regular merge requests should NOT use USSO token');
+    $this->assertEqual(0, count($client->added_headers),
+      'No Authorization headers should be added for regular merge requests');
+  }
+
+  /**
+   * Test that submitPriorityMergeRequest DOES add USSO token.
+   */
+  public function testSubmitPriorityMergeRequest_WithUSSOToken() {
+    $client = new SubmitQueueMockClientWithUSSOTracking(
+      'https://submit-queue.example.com',
+      'test-conduit-token'
+    );
+
+    // Call submitPriorityMergeRequest (with --skip-submitqueue-checks flag)
+    $client->submitPriorityMergeRequest('789');
+
+    // Verify that use_usso_token was true
+    $this->assertEqual(true, $client->last_use_usso_token,
+      'Priority merge requests should use USSO token');
+    $this->assertEqual(1, count($client->added_headers),
+      'Authorization header should be added for priority merge requests');
+
+    // Verify Authorization header was added with correct format
+    $auth_header_found = false;
+    foreach ($client->added_headers as $header) {
+      if (strpos($header[0], 'Authorization') === 0) {
+        $auth_header_found = true;
+        $this->assertEqual('Authorization', $header[0]);
+        $this->assertTrue(strpos($header[1], 'Bearer ') === 0,
+          'Authorization header should start with "Bearer "');
+      }
+    }
+    $this->assertTrue($auth_header_found,
+      'Authorization header should be present for priority merge requests');
+  }
 }

src/land/__tests__/mocks/MockSubmitQueueClient.php

@@ -27,3 +27,89 @@ public function submitPriorityMergeRequest($revisionId) {
     return 'http://submit-queue.example.com/priority/456';
   }
 }
+
+/**
+ * Testable version of UberSubmitQueueClient that tracks USSO token usage.
+ */
+class SubmitQueueMockClientWithUSSOTracking extends UberSubmitQueueClient {
+  public $last_use_usso_token = null;
+  public $added_headers = array();
+
+  protected function callMethod($method, $api, array $params, $use_usso_token = false) {
+    // Track whether USSO token was requested
+    $this->last_use_usso_token = $use_usso_token;
+    $this->added_headers = array();
+
+    // Create a mock future
+    $mock_future = new SubmitQueueMockHTTPSFuture($this);
+
+    // Simulate adding headers
+    $mock_future->addHeader('Host', $this->getHost());
+
+    // Simulate USSO token logic
+    if ($use_usso_token) {
+      // For testing, use a simple mock token
+      $token = 'mock-usso-token';
+
+      // Check if environment variable is set (matching real behavior)
+      $env_token = getenv('ARC_USSO_TOKEN');
+      if ($env_token) {
+        $token = $env_token;
+      }
+
+      $mock_future->addHeader('Authorization', "Bearer {$token}");
+    }
+
+    $mock_future->setMethod($method);
+    $mock_future->setTimeout($this->timeout);
+
+    // Return a mock future that resolves to a success response
+    return new SubmitQueueMockFuture($mock_future);
+  }
+}
+
+/**
+ * Mock HTTPSFuture for testing.
+ */
+class SubmitQueueMockHTTPSFuture {
+  private $client;
+
+  public function __construct($client) {
+    $this->client = $client;
+  }
+
+  public function addHeader($name, $value) {
+    // Track added headers in the testable client
+    if ($name === 'Authorization') {
+      $this->client->added_headers[] = array($name, $value);
+    }
+    return $this;
+  }
+
+  public function setMethod($method) {
+    return $this;
+  }
+
+  public function setTimeout($timeout) {
+    return $this;
+  }
+}
+
+/**
+ * Mock UberSubmitQueueFuture for testing.
+ */
+class SubmitQueueMockFuture {
+  private $future;
+
+  public function __construct($future) {
+    $this->future = $future;
+  }
+
+  public function isReady() {
+    return true;
+  }
+
+  public function resolve() {
+    return 'http://submit-queue.example.com/status/mock';
+  }
+}