pkp#7366 user api key generation process update

touhidurabir · Aug 23, 2022 · b98f04f · b98f04f
1 parent 1719e79
commit b98f04f
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 38 deletions.
diff --git a/classes/migration/upgrade/v3_4_0/I7366_UpdateUserAPIKeySettings.php b/classes/migration/upgrade/v3_4_0/I7366_UpdateUserAPIKeySettings.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace PKP\migration\upgrade\v3_4_0;
+
+use Illuminate\Support\Facades\DB;
+use PKP\install\DowngradeNotSupportedException;
+
+class I7366_UpdateUserAPIKeySettings extends \PKP\migration\Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        $userSettings = DB::table('user_settings')
+            ->select(['user_id', 'setting_name'])
+            ->where('setting_name', 'apiKeyEnabled')
+            ->distinct()
+            ->get();
+
+        $userSettingsWithApiKey = DB::table('user_settings')
+                ->select(['user_id', 'setting_name'])
+                ->where('user_id', $userSettings->pluck('user_id')->toArray())
+                ->where('setting_name', 'apiKey')
+                ->distinct()
+                ->get();
+
+        DB::table('user_settings')
+            ->whereIn(
+                'user_id', 
+                $userSettings
+                    ->pluck('user_id')
+                    ->diff($userSettingsWithApiKey->pluck('user_id'))
+                    ->toArray()
+            )
+            ->where('setting_name', 'apiKeyEnabled')
+            ->delete();
+    }
+
+    /**
+     * Reverse the migration.
+     */
+    public function down(): void
+    {
+        throw new DowngradeNotSupportedException('Downgrade unsupported due to data update');
+    }
+}
diff --git a/classes/user/form/APIProfileForm.php b/classes/user/form/APIProfileForm.php
@@ -17,15 +17,17 @@
 
 use APP\core\Application;
 use APP\notification\NotificationManager;
-
 use APP\template\TemplateManager;
 use Firebase\JWT\JWT;
 use PKP\config\Config;
-
 use PKP\notification\PKPNotification;
+use PKP\user\User;
 
 class APIProfileForm extends BaseProfileForm
 {
+    public const API_KEY_NEW = 1;
+    public const API_KEY_DELETE = 0;
+
     /**
      * Constructor.
      *
@@ -53,7 +55,9 @@ public function readInputData()
         parent::readInputData();
 
         $this->readUserVars([
-            'apiKeyEnabled', 'generateApiKey',
+            'apiKeyEnabled', 
+            'generateApiKey',
+            'apiKeyAction',
         ]);
     }
 
@@ -70,21 +74,23 @@ public function fetch($request, $template = null, $display = false)
     {
         $user = $request->getUser();
         $secret = Config::getVar('security', 'api_key_secret', '');
+        $templateMgr = TemplateManager::getManager($request);
+
         if ($secret === '') {
-            $notificationManager = new NotificationManager();
-            $notificationManager->createTrivialNotification(
-                $user->getId(),
-                PKPNotification::NOTIFICATION_TYPE_WARNING,
-                [
-                    'contents' => __('user.apiKey.secretRequired'),
-                ]
-            );
-        } elseif ($user->getData('apiKey')) {
-            $templateMgr = TemplateManager::getManager($request);
-            $templateMgr->assign([
+            $this->handleOnMissingAPISecret($templateMgr, $user);
+            return parent::fetch($request, $template, $display);
+        };
+
+        $templateMgr->assign($user->getData('apiKey') ? [
                 'apiKey' => JWT::encode($user->getData('apiKey'), $secret, 'HS256'),
-            ]);
-        }
+                'apiKeyAction' => self::API_KEY_DELETE,
+                'apiKeyActionTextKey' => 'user.apiKey.remove',
+            ] : [
+                'apiKeyAction' => self::API_KEY_NEW,
+                'apiKeyActionTextKey' => 'user.apiKey.generate',
+            ]
+        );
+
         return parent::fetch($request, $template, $display);
     }
 
@@ -95,25 +101,61 @@ public function execute(...$functionArgs)
     {
         $request = Application::get()->getRequest();
         $user = $request->getUser();
+        $templateMgr = TemplateManager::getManager($request);
 
-        $apiKeyEnabled = (bool) $this->getData('apiKeyEnabled');
-        $user->setData('apiKeyEnabled', $apiKeyEnabled);
-
-        // remove api key if exists
-        if (!$apiKeyEnabled) {
-            $user->setData('apiKeyEnabled', null);
+        if (Config::getVar('security', 'api_key_secret', '') === '') {
+            $this->handleOnMissingAPISecret($templateMgr, $user);
+            parent::execute(...$functionArgs);
         }
 
-        // generate api key
-        if ($apiKeyEnabled && !is_null($this->getData('generateApiKey'))) {
-            $secret = Config::getVar('security', 'api_key_secret', '');
-            if ($secret) {
-                $user->setData('apiKey', sha1(time()));
-            }
-        }
+        $apiKeyAction = (int)$this->getData('apiKeyAction');
+
+        $user->setData('apiKeyEnabled', $apiKeyAction === self::API_KEY_NEW ? 1 : null);
+        $user->setData('apiKey', $apiKeyAction === self::API_KEY_NEW ? sha1(time()) : null);
+
+        $this->setData('apiKeyAction', (int)!$apiKeyAction);
+
+        // $apiKeyEnabled = (bool) $this->getData('apiKeyEnabled');
+        // $user->setData('apiKeyEnabled', $apiKeyEnabled);
+
+        // // remove api key if exists
+        // if (!$apiKeyEnabled) {
+        //     $user->setData('apiKeyEnabled', null);
+        // }
+
+        // // generate api key
+        // if ($apiKeyEnabled && !is_null($this->getData('generateApiKey'))) {
+        //     $secret = Config::getVar('security', 'api_key_secret', '');
+        //     if ($secret) {
+        //         $user->setData('apiKey', sha1(time()));
+        //     }
+        // }
 
         parent::execute(...$functionArgs);
     }
+
+    /**
+     * Handle on missing API secret
+     * 
+     * @param TemplateManager $templateMgr
+     * @param User $user
+     *
+     * @return void
+     */
+    protected function handleOnMissingAPISecret(TemplateManager $templateMgr, User $user): void
+    {
+        $notificationManager = new NotificationManager();
+        $notificationManager->createTrivialNotification(
+            $user->getId(),
+            PKPNotification::NOTIFICATION_TYPE_WARNING,
+            [
+                'contents' => __('user.apiKey.secretRequired'),
+            ]
+        );
+        $templateMgr->assign([
+            'apiSecretMissing' => true,
+        ]);
+    }
 }
 
 if (!PKP_STRICT_MODE) {

diff --git a/locale/en_US/user.po b/locale/en_US/user.po
@@ -426,11 +426,20 @@ msgid "user.apiKeyEnabled"
 msgstr "Enable external applications with the API key to access this account"
 
 msgid "user.apiKey.generate"
-msgstr "Generate new API key"
+msgstr "Create API Key"
+
+msgid "user.apiKey.remove"
+msgstr "Delete"
 
 msgid "user.apiKey.generateWarning"
 msgstr "Generating a new API key will invalidate any existing key for this user."
 
+msgid "user.apiKey.removeWarning"
+msgstr "Deleting a key will revoke access to any application that uses it."
+
+msgid "user.apiKey.remove.confirmation.message"
+msgstr "You sure you want to delete this api key."
+
 msgid "user.apiKey.secretRequired"
 msgstr "Before generating an API key, your site administrator must set a secret in the config file (\"api_key_secret\")."
 

diff --git a/templates/user/apiProfileForm.tpl b/templates/user/apiProfileForm.tpl
@@ -23,21 +23,37 @@
 
 	{include file="controllers/notification/inPlaceNotification.tpl" notificationId="apiProfileNotification"}
 
-	{fbvFormSection list=true}
+	{* {fbvFormSection list=true}
 		{fbvElement id=apiKeyEnabled type="checkbox" label="user.apiKeyEnabled" checked=$apiKeyEnabled value=1}
 		{fbvElement id=generateApiKey type="checkbox" label="user.apiKey.generate" value=1}
-	{/fbvFormSection}
-	<p>{translate key="user.apiKey.generateWarning"}</p>
+	{/fbvFormSection} *}
 
-	{fbvFormSection}
+	{fbvFormSection title="user.apiKey"}
 		{if !$apiKey}{assign var=apiKey value="common.none"|translate}{/if}
-		{fbvElement id=apiKey type="text" label="user.apiKey" readonly="true" value=$apiKey size=$fbvStyles.size.MEDIUM}
+		{fbvElement id=apiKey type="text" readonly="true" inline=true value=$apiKey size=$fbvStyles.size.MEDIUM}
+		{if !$apiSecretMissing}
+			{fbvElement id=apiKeyAction type="hidden" readonly="true" value=$apiKeyAction}
+			<button
+				type="submit"
+				{if $apiKeyAction === \PKP\user\form\APIProfileForm::API_KEY_DELETE}
+					onClick="return confirm({translate|json_encode|escape key='user.apiKey.remove.confirmation.message'})"
+					class="pkpButton pkpButton--isWarnable"
+				{else}
+					class="pkp_button pkp_button_primary"
+				{/if}
+			>
+				{translate key=$apiKeyActionTextKey}
+			</button>
+		{/if}
+		<p>
+			{translate key=($apiKeyAction) ? "user.apiKey.generateWarning" : "user.apiKey.removeWarning"}
+		</p>
 	{/fbvFormSection}
 
 	<p>
-		{capture assign="privacyUrl"}{url router=\PKP\core\PKPApplication::ROUTE_PAGE page="about" op="privacy"}{/capture}
+		{capture assign="privacyUrl"}
+			{url router=\PKP\core\PKPApplication::ROUTE_PAGE page="about" op="privacy"}
+		{/capture}
 		{translate key="user.privacyLink" privacyUrl=$privacyUrl}
 	</p>
-
-	{fbvFormButtons hideCancel=true submitText="common.save"}
 </form>