* removed type field in IDF_Key on request of Loic and automatically detect

and validate raw key data
* reworked the parseMonotoneKeyData() function to parse ssh and monotone keys
* tweak help texts and exception strings

src/IDF/Form/Admin/UserCreate.php

@@ -89,16 +89,8 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form
                                             'widget_attrs' => array('rows' => 3,
                                                                     'cols' => 40),
                                             'widget' => 'Pluf_Form_Widget_TextareaInput',
-                                            'help_text' => __('Be careful to provide the public key and not the private key!')
+                                            'help_text' => __('Paste a SSH or monotone public key. Be careful to not provide your private key here!')
                                             ));
-        $this->fields['public_key_type'] = new Pluf_Form_Field_Varchar(
-                                   array('required' => true,
-                                         'label' => __('Key type'),
-                                         'initial' => 'ssh',
-                                         'widget_attrs' => array('choices' => IDF_Key::getAvailableKeyTypes()),
-                                         'widget' => 'Pluf_Form_Widget_SelectInput',
-                                         ));
-
     }
 
     /**
@@ -143,12 +135,11 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form
         $params = array('user' => $user);
         Pluf_Signal::send('Pluf_User::passwordUpdated',
                           'IDF_Form_Admin_UserCreate', $params);
-        // Create the ssh key as needed
+        // Create the public key as needed
         if ('' !== $this->cleaned_data['public_key']) {
             $key = new IDF_Key();
             $key->user = $user;
             $key->content = $this->cleaned_data['public_key'];
-            $key->type = $this->cleaned_data['public_key_type'];
             $key->create();
         }
         // Send an email to the user with the password
@@ -214,15 +205,11 @@ class IDF_Form_Admin_UserCreate extends Pluf_Form
         return $this->cleaned_data['login'];
     }
 
-    /**
+    public function clean_public_key()
-     * Checks whether any given public key is valid
-     */
-    public function clean()
     {
         $this->cleaned_data['public_key'] =
-            IDF_Form_UserAccount::checkPublicKey($this->cleaned_data['public_key'],
+            IDF_Form_UserAccount::checkPublicKey($this->cleaned_data['public_key']);
-                                                 $this->cleaned_data['public_key_type']);
 
-        return $this->cleaned_data;
+        return $this->cleaned_data['public_key'];
     }
 }

src/IDF/Form/UserAccount.php

@@ -99,16 +99,8 @@ class IDF_Form_UserAccount  extends Pluf_Form
                                             'widget_attrs' => array('rows' => 3,
                                                                     'cols' => 40),
                                             'widget' => 'Pluf_Form_Widget_TextareaInput',
-                                            'help_text' => __('Be careful to provide your public key and not your private key!')
+                                            'help_text' => __('Paste a SSH or monotone public key. Be careful to not provide your private key here!')
                                             ));
-        $this->fields['public_key_type'] = new Pluf_Form_Field_Varchar(
-                                           array('required' => true,
-                                                 'label' => __('Key type'),
-                                                 'initial' => 'ssh',
-                                                 'widget_attrs' => array('choices' => IDF_Key::getAvailableKeyTypes()),
-                                                 'widget' => 'Pluf_Form_Widget_SelectInput',
-                                                 ));
-
     }
 
     /**
@@ -161,7 +153,6 @@ class IDF_Form_UserAccount  extends Pluf_Form
             $key = new IDF_Key();
             $key->user = $this->user;
             $key->content = $this->cleaned_data['public_key'];
-            $key->type = $this->cleaned_data['public_key_type'];
             if ($commit) {
                 $key->create();
             }
@@ -203,50 +194,39 @@ class IDF_Form_UserAccount  extends Pluf_Form
      * validate the key.
      *
      * @param $key string The key
-     * @param $key string The type ('ssh' or 'mtn')
      * @param $user int The user id of the user of the key (0)
      * @return string The clean key
      */
-    public static function checkPublicKey($key, $type, $user=0)
+    public static function checkPublicKey($key, $user=0)
     {
         $key = trim($key);
         if (strlen($key) == 0) {
             return '';
         }
 
-        if ($type == 'ssh') {
+        if (preg_match('#^ssh\-[a-z]{3}\s\S+\s\S+$#', $key)) {
             $key = str_replace(array("\n", "\r"), '', $key);
-            if (!preg_match('#^ssh\-[a-z]{3}\s\S+\s\S+$#', $key)) {
+
-                throw new Pluf_Form_Invalid(
-                    __('The format of the key is not valid. It must start '.
-                       'with ssh-dss or ssh-rsa, a long string on a single '.
-                       'line and at the end a comment.')
-                );
-            }
             if (Pluf::f('idf_strong_key_check', false)) {
+
                 $tmpfile = Pluf::f('tmp_folder', '/tmp').'/'.$user.'-key';
                 file_put_contents($tmpfile, $key, LOCK_EX);
                 $cmd = Pluf::f('idf_exec_cmd_prefix', '').
                     'ssh-keygen -l -f '.escapeshellarg($tmpfile).' > /dev/null 2>&1';
                 exec($cmd, $out, $return);
                 unlink($tmpfile);
+
                 if ($return != 0) {
                     throw new Pluf_Form_Invalid(
-                        __('Please check the key as it does not appears '.
+                        __('Please check the key as it does not appear '.
-                           'to be a valid key.')
+                           'to be a valid SSH public key.')
                     );
                 }
             }
         }
-        else if ($type == 'mtn') {
+        else if (preg_match('#^\[pubkey [^\]]+\]\s*\S+\s*\[end\]$#', $key)) {
-            if (!preg_match('#^\[pubkey [^\]]+\]\s*\S+\s*\[end\]$#', $key)) {
-                throw new Pluf_Form_Invalid(
-                    __('The format of the key is not valid. It must start '.
-                       'with [pubkey KEYNAME], contain a long string on a single '.
-                       'line and end with [end] in the final third line.')
-                );
-            }
             if (Pluf::f('idf_strong_key_check', false)) {
+
                 // if monotone can read it, it should be valid
                 $mtn_opts = implode(' ', Pluf::f('mtn_opts', array()));
                 $cmd = Pluf::f('idf_exec_cmd_prefix', '').
@@ -258,22 +238,23 @@ class IDF_Form_UserAccount  extends Pluf_Form
 
                 if ($return != 0) {
                        throw new Pluf_Form_Invalid(
-                        __('Please check the key as it does not appears '.
+                        __('Please check the key as it does not appear '.
-                           'to be a valid key.')
+                           'to be a valid monotone public key.')
                     );
                 }
             }
         }
-        else
+        else {
-        {
+            throw new Pluf_Form_Invalid(
-            throw new Pluf_Form_Invalid(__('Unknown key type'));
+                __('Public key looks neither like a SSH '.
+                   'nor monotone public key.'));
         }
 
         // If $user, then check if not the same key stored
         if ($user) {
             $ruser = Pluf::factory('Pluf_User', $user);
             if ($ruser->id > 0) {
-                $sql = new Pluf_SQL('content=%s AND type=%s', array($key, $type));
+                $sql = new Pluf_SQL('content=%s', array($key));
                 $keys = Pluf::factory('IDF_Key')->getList(array('filter' => $sql->gen()));
                 if (count($keys) > 0) {
                     throw new Pluf_Form_Invalid(
@@ -317,9 +298,16 @@ class IDF_Form_UserAccount  extends Pluf_Form
         return $this->cleaned_data['email'];
     }
 
+    function clean_public_key()
+    {
+        $this->cleaned_data['public_key'] =
+                self::checkPublicKey($this->cleaned_data['public_key'],
+                                     $this->user->id);
+        return $this->cleaned_data['public_key'];
+    }
+
     /**
-     * Check to see if the 2 passwords are the same and if any
+     * Check to see if the 2 passwords are the same
-     * given public key is valid
      */
     public function clean()
     {
@@ -332,11 +320,6 @@ class IDF_Form_UserAccount  extends Pluf_Form
             }
         }
 
-        $this->cleaned_data['public_key'] =
-            self::checkPublicKey($this->cleaned_data['public_key'],
-                                 $this->cleaned_data['public_key_type'],
-                                 $this->user->id);
-
         return $this->cleaned_data;
     }
 }

src/IDF/Key.php

@@ -54,13 +54,6 @@ class IDF_Key extends Pluf_Model
                                   'blank' => false,
                                   'verbose' => __('public key'),
                                   ),
-                            'type' =>
-                            array(
-                                  'type' => 'Pluf_DB_Field_Varchar',
-                                  'size' => 3,
-                                  'blank' => false,
-                                  'verbose' => __('key type'),
-                                  ),
                             );
         // WARNING: Not using getSqlTable on the Pluf_User object to
         // avoid recursion.
@@ -82,27 +75,37 @@ class IDF_Key extends Pluf_Model
         return Pluf_Template::markSafe(Pluf_esc(substr($this->content, 0, 25)).' [...] '.Pluf_esc(substr($this->content, -55)));
     }
 
-    private function parseMonotoneKeyData()
+    private function parseContent()
     {
-        if ($this->type != "mtn")
+        if (preg_match('#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#', $this->content, $m)) {
-            throw new IDF_Exception("key is not a monotone key type");
+            return array('mtn', $m[1], $m[2]);
+        }
+        else if (preg_match('#^ssh\-[a-z]{3}\s(\S+)\s(\S+)$#', $this->content, $m)) {
+            return array('ssh', $m[2], $m[1]);
+        }
 
-        preg_match("#^\[pubkey ([^\]]+)\]\s*(\S+)\s*\[end\]$#", $this->content, $m);
+        throw new IDF_Exception('invalid or unknown key data detected');
-        if (count($m) != 3)
-            throw new IDF_Exception("invalid key data detected");
-
-        return array($m[1], $m[2]);
     }
 
     /**
-     * Returns the key name of the key, i.e. most of the time the email
+     * Returns the type of the public key
-     * address, which not neccessarily has to be unique across a project.
+     *
+     * @return string 'ssh' or 'mtn'
+     */
+    function getType()
+    {
+        list($type, , ) = $this->parseContent();
+        return $type;
+    }
+
+    /**
+     * Returns the key name of the key
      *
      * @return string
      */
-    function getMonotoneKeyName()
+    function getName()
     {
-        list($keyName, ) = $this->parseMonotoneKeyData();
+        list(, $keyName, ) = $this->parseContent();
         return $keyName;
     }
 
@@ -116,9 +119,11 @@ class IDF_Key extends Pluf_Model
      *
      * @return string
      */
-    function getMonotoneKeyId()
+    function getMtnId()
     {
-        list($keyName, $keyData) = $this->parseMonotoneKeyData();
+        list($type, $keyName, $keyData) = $this->parseContent();
+        if ($type != 'mtn')
+            throw new IDF_Exception('key is not a monotone public key');
         return sha1($keyName.":".$keyData);
     }
 
@@ -174,19 +179,4 @@ class IDF_Key extends Pluf_Model
         Pluf_Signal::send('IDF_Key::preDelete',
                           'IDF_Key', $params);
     }
-
-    /**
-     * Returns an associative array with available key types for this
-     * idf installation, ready for consumption for a <select> widget
-     *
-     * @return array
-     */
-    public static function getAvailableKeyTypes()
-    {
-        $key_types = array(__("SSH") => 'ssh');
-        if (array_key_exists('mtn', Pluf::f('allowed_scm', array()))) {
-            $key_types[__("monotone")] = 'mtn';
-        }
-        return $key_types;
-    }
 }

src/IDF/Migrations/16KeyType.php

@@ -1,55 +0,0 @@
-<?php
-/* -*- tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
-/*
-# ***** BEGIN LICENSE BLOCK *****
-# This file is part of InDefero, an open source project management application.
-# Copyright (C) 2008 Céondo Ltd and contributors.
-#
-# InDefero is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# InDefero is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
-#
-# ***** END LICENSE BLOCK ***** */
-
-/**
- * Add the type column in the keys table.
- */
-
-function IDF_Migrations_16KeyType_up($params=null)
-{
-    $table = Pluf::factory('IDF_Key')->getSqlTable();
-    $sql = array();
-    $sql['PostgreSQL'] = 'ALTER TABLE '.$table.' ADD COLUMN "type" VARCHAR(3) DEFAULT \'ssh\'';
-    $sql['MySQL'] = 'ALTER TABLE '.$table.' ADD COLUMN `type` VARCHAR(3) DEFAULT \'ssh\'';
-    $db = Pluf::db();
-    $engine = Pluf::f('db_engine');
-    if (!isset($sql[$engine])) {
-        throw new Exception('SQLite complex migration not supported.');
-    }
-    $db->execute($sql[$engine]);
-}
-
-function IDF_Migrations_16KeyType_down($params=null)
-{
-    $table = Pluf::factory('IDF_Key')->getSqlTable();
-    $sql = array();
-    $sql['PostgreSQL'] = 'ALTER TABLE '.$table.' DROP COLUMN "type"';
-    $sql['MySQL'] = 'ALTER TABLE '.$table.' DROP COLUMN `type`';
-    $db = Pluf::db();
-    $engine = Pluf::f('db_engine');
-    if (!isset($sql[$engine])) {
-        throw new Exception('SQLite complex migration not supported.');
-    }
-    $db->execute($sql[$engine]);
-
-}

src/IDF/Plugin/SyncGit/Cron.php

@@ -48,8 +48,7 @@ class IDF_Plugin_SyncGit_Cron
         $out = '';
         $keys = Pluf::factory('IDF_Key')->getList(array('view'=>'join_user'));
         foreach ($keys as $key) {
-            if ($key->type == 'ssh' && strlen($key->content) > 40 // minimal check
+            if ($key->getType() == 'ssh' and preg_match('/^[a-zA-Z][a-zA-Z0-9_.-]*(@[a-zA-Z][a-zA-Z0-9.-]*)?$/', $key->login)) {
-                and preg_match('/^[a-zA-Z][a-zA-Z0-9_.-]*(@[a-zA-Z][a-zA-Z0-9.-]*)?$/', $key->login)) {
                 $content = trim(str_replace(array("\n", "\r"), '', $key->content));
                 $out .= sprintf($template, $cmd, $key->login, $content)."\n";
             }

src/IDF/templates/idf/gadmin/users/create.html

@@ -46,11 +46,7 @@
 <th>{$form.f.public_key.labelTag}:</th>
 <td>{if $form.f.public_key.errors}{$form.f.public_key.fieldErrors}{/if}
 {$form.f.public_key|unsafe}<br />
-{$form.f.public_key_type.labelTag} {$form.f.public_key_type|unsafe}<br />
+<span class="helptext">{$form.f.public_key.help_text}</span>
-<span class="helptext">
-    {$form.f.public_key.help_text}
-    {$form.f.public_key_type.help_text}
-</span>
 </td>
 </tr>
 <tr>

src/IDF/templates/idf/user/myaccount.html

@@ -57,11 +57,7 @@
 <th>{$form.f.public_key.labelTag}:</th>
 <td>{if $form.f.public_key.errors}{$form.f.public_key.fieldErrors}{/if}
 {$form.f.public_key|unsafe}<br />
-{$form.f.public_key_type.labelTag} {$form.f.public_key_type|unsafe}<br />
+<span class="helptext">{$form.f.public_key.help_text}</span>
-<span class="helptext">
-    {$form.f.public_key.help_text}
-    {$form.f.public_key_type.help_text}
-</span>
 </td>
 </tr>
 <tr class="pass-info" id="extra-password">