[glom/glom-1-20] Allow user and group names to have spaces and other special characters.

[Murray Cumming <murrayc src gnome org>]

commit bd49369d61cb9904ef46bb293ad0d8c80750ee49
Author: Murray Cumming <murrayc murrayc com>
Date:   Wed Feb 1 22:09:14 2012 +0100

    Allow user and group names to have spaces and other special characters.
    
    * glom/libglom/privs.cc: get_table_privileges(): Instead of parsing the
    relacl.pg_class field, use the PostgreSQL has_table_privilege()
    function, though it needs some strange quoting (see comments).
    This code is much simpler now.
    * tests/test_selfhosting_new_empty_then_users.cc: Add various other
    table, group, and user names, to excercise the code.
    This now passes.

 ChangeLog                                      |   12 ++
 glom/libglom/db_utils.cc                       |    1 +
 glom/libglom/privs.cc                          |  162 ++++++++++++------------
 tests/test_selfhosting_new_empty_then_users.cc |   43 +++++--
 4 files changed, 128 insertions(+), 90 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 71d6093..581c482 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
 2012-02-01  Murray Cumming  <murrayc murrayc com>
 
+	Allow user and group names to have spaces and other special characters.
+
+	* glom/libglom/privs.cc: get_table_privileges(): Instead of parsing the
+	relacl.pg_class field, use the PostgreSQL has_table_privilege() 
+	function, though it needs some strange quoting (see comments).
+	This code is much simpler now. 
+	* tests/test_selfhosting_new_empty_then_users.cc: Add various other
+	table, group, and user names, to excercise the code.
+	This now passes.
+
+2012-02-01  Murray Cumming  <murrayc murrayc com>
+
 	test_selfhosting_new_empty_then_users: Show the problem with spaces.
 
 	* tests/test_selfhosting_new_empty_then_users.cc: 
diff --git a/glom/libglom/db_utils.cc b/glom/libglom/db_utils.cc
index 391c3fc..450581e 100644
--- a/glom/libglom/db_utils.cc
+++ b/glom/libglom/db_utils.cc
@@ -2028,6 +2028,7 @@ bool add_group(const Document* document, const Glib::ustring& group)
   }
  
   const Glib::ustring strQuery = DbUtils::build_query_create_group(group);
+  //std::cout << "DEBUGCREATE: " << strQuery << std::endl;
   const bool test = DbUtils::query_execute_string(strQuery);
   if(!test)
   {
diff --git a/glom/libglom/privs.cc b/glom/libglom/privs.cc
index 6d65171..5d95cbf 100644
--- a/glom/libglom/privs.cc
+++ b/glom/libglom/privs.cc
@@ -257,6 +257,29 @@ bool Privs::set_table_privileges(const Glib::ustring& group_name, const Glib::us
   return true;
 }
 
+static Glib::RefPtr<Gnome::Gda::Connection> get_connection()
+{
+  sharedptr<SharedConnection> sharedconnection;
+  try
+  {
+     sharedconnection = ConnectionPool::get_and_connect();
+  }
+  catch (const Glib::Error& error)
+  {
+    std::cerr << G_STRFUNC << ": " << error.what() << std::endl;
+  }
+
+  if(!sharedconnection)
+  {
+    std::cerr << G_STRFUNC << ": No connection yet." << std::endl;
+    return Glib::RefPtr<Gnome::Gda::Connection>(0);
+  }
+
+  Glib::RefPtr<Gnome::Gda::Connection> gda_connection = sharedconnection->get_gda_connection();
+
+  return gda_connection;
+}
+
 Privileges Privs::get_table_privileges(const Glib::ustring& group_name, const Glib::ustring& table_name)
 {
   Privileges result;
@@ -274,95 +297,70 @@ Privileges Privs::get_table_privileges(const Glib::ustring& group_name, const Gl
   //Get the permissions:
   Glib::RefPtr<Gnome::Gda::SqlBuilder> builder =
     Gnome::Gda::SqlBuilder::create(Gnome::Gda::SQL_STATEMENT_SELECT);
-  builder->select_add_field("relacl", "pg_class");
-  builder->select_add_target("pg_class");
-  builder->set_where(
-    builder->add_cond(Gnome::Gda::SQL_OPERATOR_TYPE_EQ,
-      builder->add_field_id("relname", "pg_class"),
-      builder->add_expr(table_name)));
-  Glib::RefPtr<Gnome::Gda::DataModel> data_model = DbUtils::query_execute_select(builder);
-  if(data_model && data_model->get_n_rows())
-  {
-    const Gnome::Gda::Value value = data_model->get_value_at(0, 0);
 
-    Glib::ustring access_details;
-    if(!value.is_null())
-      access_details = value.get_string();
+  const Glib::ustring function_name = "has_table_privilege";
+  std::vector<Gnome::Gda::SqlBuilder::Id> args_base;
 
-    //std::cout << "DEBUG: access_details:" << access_details << std::endl;
-
-    //Parse the strange postgres permissions format:
-    //For instance, "{murrayc=arwdxt/murrayc,operators=r/murrayc}" (Postgres 8.x)
-    //For instance, "{murrayc=arwdxt/murrayc,group operators=r/murrayc}" (Postgres <8.x)
-    const type_vec_strings vecItems = pg_list_separate(access_details);
-    for(type_vec_strings::const_iterator iterItems = vecItems.begin(); iterItems != vecItems.end(); ++iterItems)
-    {
-      Glib::ustring item = *iterItems;
-      //std::cout << "DEBUG: item:" << item << std::endl;
-
-      item = Utils::string_trim(item, "\""); //Remove quotes from front and back.
-
-      //std::cout << "DEBUG: item without quotes:" << item << std::endl;
-
-      //Find group permissions, ignoring user permissions:
-      //We need to find the role by name.
-      // Previous versions of Postgres (8.1, or maybe 7.4) prefixed group names by "group ",
-      // but that doesn't work for recent versions of Postgres,
-      // probably because the user and group concepts have been combined into "roles".
-      //
-      //const Glib::ustring strgroup = "group ";
-      const Glib::ustring strgroup = group_name + "=";
-      Glib::ustring::size_type posFind = item.find(strgroup);
-      if(posFind != Glib::ustring::npos)
-      {
-        //It is the needed group permision:
+  Glib::RefPtr<Gnome::Gda::Connection> connection = get_connection();
+  if(!connection)
+  {
+    std::cerr << ": Could not get a connection." << std::endl;
+    return result;
+  }
 
-        //Remove the "group " prefix (not needed for Postgres 8.x):
-        //item = item.substr(strgroup.size());
-        item = item.substr(posFind);
-        //std::cout << "DEBUG: user permissions:" << item << std::endl;
+  //TODO: Why doesn't this need to be quoted like table_name.
+  //The group names are quoted when we create them:
+  //For some reason, this is what PostgreSQL wants: SELECT has_table_privilege('Some Group', '"Some Table"', 'SELECT');
+  //However, note that libgda does seem to escape characters here (for instance, changing ' to '').
+  //const Glib::ustring group_name_for_arg = connection->quote_sql_identifier(group_name);
+  const Glib::ustring group_name_for_arg = group_name;
+  args_base.push_back(builder->add_expr(group_name_for_arg));
+
+  //The table name must be quoted if it needs to be quoted,
+  //but not quoted if it is does not need to be quoted,
+  //because it must match how it was created, and libgda probably did not quote it unless necessary.
+  const Glib::ustring table_name_for_arg = connection->quote_sql_identifier(table_name);
+  args_base.push_back(builder->add_expr(table_name_for_arg));
+
+  std::vector<Gnome::Gda::SqlBuilder::Id> args = args_base;
+  args.push_back(builder->add_expr("SELECT"));
+  builder->add_field_value_id(builder->add_function(function_name, args));
+  args = args_base;
+  args.push_back(builder->add_expr("UPDATE"));
+  builder->add_field_value_id(builder->add_function(function_name, args));
+  args = args_base;
+  args.push_back(builder->add_expr("INSERT"));
+  builder->add_field_value_id(builder->add_function(function_name, args));
+  args = args_base;
+  args.push_back(builder->add_expr("DELETE"));
+  builder->add_field_value_id(builder->add_function(function_name, args));
+
+  //const Glib::ustring sql_debug = Utils::sqlbuilder_get_full_query(builder);
+  //std::cout << "DEBUG: " << sql_debug << std::endl;
 
-        //Get the parts before and after the =:
-        const type_vec_strings vecParts = Utils::string_separate(item, "=");
-        if(vecParts.size() == 2)
-        {
-          const Glib::ustring this_group_name = vecParts[0];
-          if(this_group_name == group_name) //Only look at permissions for the requested group->
-          {
-            Glib::ustring group_permissions = vecParts[1];
-
-            //Get the part before the /user_who_granted_the_privileges:
-            const type_vec_strings vecParts = Utils::string_separate(group_permissions, "/");
-            if(!vecParts.empty())
-              group_permissions = vecParts[0];
-
-            //g_warning("  group=%s", group_name.c_str());
-            //g_warning("  permisisons=%s", group_permissions.c_str());
-
-            //Iterate through the characters:
-            for(Glib::ustring::iterator iter = group_permissions.begin(); iter != group_permissions.end(); ++iter)
-            {
-              gunichar chperm = *iter;
-              Glib::ustring perm(1, chperm);
-
-              //See http://www.postgresql.org/docs/8.0/interactive/sql-grant.html
-              if(perm == "r")
-                result.m_view = true;
-              else if(perm == "w")
-                result.m_edit = true;
-              else if(perm == "a")
-                result.m_create = true;
-              else if(perm == "d")
-                result.m_delete = true;
-            }
-          }
-        }
-      }
+  Glib::RefPtr<Gnome::Gda::DataModel> data_model = DbUtils::query_execute_select(builder);
+  if(!data_model || (data_model->get_n_rows() <= 0))
+  {
+    std::cerr << G_STRFUNC << ": The query returned no data." << std::endl;
+    return result;
+  }
 
-    }
+  if(data_model->get_n_columns() < 4)
+  {
+    std::cerr << G_STRFUNC << ": The query did not return enough columns." << std::endl;
+    return result;
   }
 
-  //g_warning("get_table_privileges(group_name=%s, table_name=%s) returning: %d", group_name.c_str(), table_name.c_str(), result.m_create);
+  Gnome::Gda::Value value = data_model->get_value_at(0, 0);
+  result.m_view = value.get_boolean();
+  value = data_model->get_value_at(1, 0);
+  result.m_edit = value.get_boolean();
+  value = data_model->get_value_at(2, 0);
+  result.m_create = value.get_boolean();
+  value = data_model->get_value_at(3, 0);
+  result.m_delete = value.get_boolean();
+  
+  //std::cout << G_STRFUNC << ": group_name=" << group_name << ", table_name=" << table_name << ", returning create=" << result.m_create << std::endl;
   return result;
 }
 
diff --git a/tests/test_selfhosting_new_empty_then_users.cc b/tests/test_selfhosting_new_empty_then_users.cc
index 9bba432..c55bae3 100644
--- a/tests/test_selfhosting_new_empty_then_users.cc
+++ b/tests/test_selfhosting_new_empty_then_users.cc
@@ -98,9 +98,11 @@ static bool test(Glom::Document::HostingMode hosting_mode)
 
   typedef std::vector<Glib::ustring> type_vec_strings;
   type_vec_strings table_names;
-  table_names.push_back("sometable1");
+  table_names.push_back("sometable");
+  table_names.push_back("SomeTableWithUpperCase");
   table_names.push_back("sometable with space characters");
-  table_names.push_back("sometable with a \" quote character");
+  table_names.push_back("sometable with a \" doublequote character");
+  table_names.push_back("sometable with a ' quote character");
   table_names.push_back("sometablewithaverylongnameyaddayaddayaddayaddayaddyaddayaddayaddayaddayaddayaddayaddayaddayaddayaddayaddayadda");
 
   //Add some tables, for the groups to have rights for:
@@ -133,7 +135,8 @@ static bool test(Glom::Document::HostingMode hosting_mode)
   type_vec_strings group_names;
   group_names.push_back("somegroup1");
   group_names.push_back("somegroup with space characters");
-  group_names.push_back("somegroup with a \" quote character");
+  group_names.push_back("somegroup with a \" doublequote character");
+  group_names.push_back("somegroup with a ' quote character");
   group_names.push_back("somegroupwithaverylongnameyaddayaddayaddayaddayaddyaddayaddayad"); //Almost too big.
   //We expect this to fail because of an apparently-undocumented max pg_user size of 63 characters in PostgreSQL:
   //group_names.push_back("somegroupwithaverylongnameyaddayaddayaddayaddayaddyaddayaddayadd");
@@ -152,7 +155,8 @@ static bool test(Glom::Document::HostingMode hosting_mode)
   type_vec_strings user_names;
   user_names.push_back("someuser1");
   user_names.push_back("someuser with space characters");
-  user_names.push_back("someuser with a \" quote character");
+  user_names.push_back("someuser with a \" doublequote character");
+  user_names.push_back("someuser with a ' quote character");
   user_names.push_back("someuserwithaverylongnameyaddayaddayaddayaddayaddyaddayadda"); //Almost too big, with space for the numeric suffix below.
   //We expect this to fail because of an apparently-undocumented max pg_user size of 63 characters in PostgreSQL:
   //user_names.push_back("someuserwithaverylongnameyaddayaddayaddayaddayaddyaddayaddayadd");
@@ -183,19 +187,42 @@ static bool test(Glom::Document::HostingMode hosting_mode)
           return false;
         }
 
-	/*
-        if(!privs.m_create)
+        //We default to create and delete being false:
+        /*
+        if(privs.m_create)
         {
           std::cerr << "Privs::get_table_privileges() returned an unexpected create privilege for group=" << group_name << ", table_name=" << table_name << std::endl;
           return false;
         }
 
-        if(!privs.m_delete)
+        if(privs.m_delete)
         {
           std::cerr << "Privs::get_table_privileges() returned an unexpected delete privilege for group=" << group_name << ", table_name=" << table_name << std::endl;
           return false;
         }
-	*/
+        */
+
+        //Change the privs and make sure that it worked:
+        Glom::Privileges privs_new;
+        privs_new.m_view = true;
+        privs_new.m_edit = true;
+        privs_new.m_create = true;
+        privs_new.m_delete = false;
+        if(!Glom::Privs::set_table_privileges(group_name, table_name, privs_new, false))
+        {
+          std::cerr << "Privs::set_table_privileges() failed for group=" << group_name << ", table_name=" << table_name << std::endl;
+          return false;
+        }
+
+        const Glom::Privileges privs_changed = Glom::Privs::get_table_privileges(group_name, table_name);
+        if( (privs_changed.m_view != privs_new.m_view) ||
+            (privs_changed.m_edit != privs_new.m_edit) ||
+            (privs_changed.m_create != privs_new.m_create) ||
+            (privs_changed.m_delete != privs_new.m_delete) )
+        {
+          std::cerr << "Changing and re-reading privileges failed for group=" << group_name << ", table_name=" << table_name << std::endl;
+          return false;
+        }
       }
 
       ++i;