Use a unique random key to identify queries, not a sequential ID

Summary:
We save search information and then redirect to a "/search/<query_id>/" URI in
order to make search URIs short and bookmarkable, and save query data for
analysis/improvement of search results.

Currently, there's a vague object enumeration security issue with using
sequential IDs to identify searches, where non-admins can see searches other
users have performed. This isn't really too concerning but we lose nothing by
using random keys from a large ID space instead.

  - Drop 'authorPHID', which was unused anyway, so searches can not be
personally identified, even by admins.
  - Identify searches by random hash keys, not sequential IDs.
  - Map old queries' keys to their IDs so we don't break any existing bookmarked
URIs.

Test Plan: Ran several searches, got redirected to URIs with random hashes from
a large ID space rather than sequential integers.

Reviewers: arice, btrahan

Reviewed By: arice

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1587

resources/sql/patches/104.searchkey.sql

@@ -0,0 +1,12 @@
+ALTER TABLE phabricator_search.search_query
+  DROP authorPHID;
+
+ALTER TABLE phabricator_search.search_query
+  ADD queryKey VARCHAR(12) NOT NULL;
+
+/* Preserve URIs for old queries in case anyone has them bookmarked. */
+UPDATE phabricator_search.search_query
+  SET queryKey = id;
+
+ALTER TABLE phabricator_search.search_query
+  ADD UNIQUE KEY (queryKey);

src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php

@@ -192,7 +192,7 @@ class AphrontDefaultApplicationConfiguration
 
       '/search/' => array(
         '$' => 'PhabricatorSearchController',
-        '(?P<id>\d+)/$' => 'PhabricatorSearchController',
+        '(?P<key>[^/]+)/$' => 'PhabricatorSearchController',
         'attach/(?P<phid>[^/]+)/(?P<type>\w+)/(?:(?P<action>\w+)/)?$'
           => 'PhabricatorSearchAttachController',
         'select/(?P<type>\w+)/$'

src/applications/search/controller/search/PhabricatorSearchController.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,18 +21,20 @@
  */
 class PhabricatorSearchController extends PhabricatorSearchBaseController {
 
-  private $id;
+  private $key;
 
   public function willProcessRequest(array $data) {
-    $this->id = idx($data, 'id');
+    $this->key = idx($data, 'key');
   }
 
   public function processRequest() {
     $request = $this->getRequest();
     $user = $request->getUser();
 
-    if ($this->id) {
+    if ($this->key) {
-      $query = id(new PhabricatorSearchQuery())->load($this->id);
+      $query = id(new PhabricatorSearchQuery())->loadOneWhere(
+        'queryKey = %s',
+        $this->key);
       if (!$query) {
         return new Aphront404Response();
       }
@@ -64,7 +66,7 @@ class PhabricatorSearchController extends PhabricatorSearchBaseController {
 
         $query->save();
         return id(new AphrontRedirectResponse())
-          ->setURI('/search/'.$query->getID().'/');
+          ->setURI('/search/'.$query->getQueryKey().'/');
       }
     }
 

src/applications/search/storage/query/PhabricatorSearchQuery.php

@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,11 +19,11 @@
 /**
  * @group search
  */
-class PhabricatorSearchQuery extends PhabricatorSearchDAO {
+final class PhabricatorSearchQuery extends PhabricatorSearchDAO {
 
-  protected $authorPHID;
   protected $query;
   protected $parameters = array();
+  protected $queryKey;
 
   public function getConfiguration() {
     return array(
@@ -42,4 +42,11 @@ class PhabricatorSearchQuery extends PhabricatorSearchDAO {
     return idx($this->parameters, $parameter, $default);
   }
 
+  public function save() {
+    if (!$this->getQueryKey()) {
+      $this->setQueryKey(Filesystem::readRandomCharacters(12));
+    }
+    return parent::save();
+  }
+
 }

src/applications/search/storage/query/__init__.php

@@ -8,6 +8,7 @@
 
 phutil_require_module('phabricator', 'applications/search/storage/base');
 
+phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'utils');