pkp#7366 user api key generation process update

classes/migration/upgrade/v3_4_0/I7366_UpdateUserAPIKeySettings.php

@@ -0,0 +1,52 @@
+<?php
+
+/**
+ * @file classes/migration/upgrade/v3_4_0/I7366_UpdateUserAPIKeySettings.php
+ *
+ * Copyright (c) 2014-2022 Simon Fraser University
+ * Copyright (c) 2000-2022 John Willinsky
+ * Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
+ *
+ * @class I7366_UpdateUserAPIKeySettings
+ * @brief Describe upgrade/downgrade for updating user API related settings
+ */
+
+namespace PKP\migration\upgrade\v3_4_0;
+
+use Illuminate\Support\Facades\DB;
+
+class I7366_UpdateUserAPIKeySettings extends \PKP\migration\Migration
+{
+    /**
+     * Run the migrations.
+     */
+    public function up(): void
+    {
+        $users = DB::select(
+            DB::raw(
+                "SELECT u.user_id FROM users u 
+                JOIN user_settings enabled_setting ON (enabled_setting.user_id = u.user_id AND enabled_setting.setting_name = 'apiKeyEnabled')
+                LEFT JOIN user_settings key_setting ON (key_setting.user_id = u.user_id AND key_setting.setting_name = 'apiKey') 
+                WHERE key_setting.user_id IS NULL"
+            )
+        );
+
+        collect($users)
+            ->pluck('user_id')
+            ->chunk(1000)
+            ->each(
+                fn ($ids) => DB::table('user_settings')
+                    ->where('setting_name', 'apiKeyEnabled')
+                    ->whereIn('user_id', $ids->toArray())
+                    ->delete()
+            );
+    }
+
+    /**
+     * Reverse the migration.
+     */
+    public function down(): void
+    {
+
+    }
+}

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([
-                'apiKey' => JWT::encode($user->getData('apiKey'), $secret, 'HS256'),
-            ]);
+            $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,45 @@ 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);
 
         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) {

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 "Are 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\")."
 

templates/user/apiProfileForm.tpl

@@ -23,21 +23,32 @@
 
 	{include file="controllers/notification/inPlaceNotification.tpl" notificationId="apiProfileNotification"}
 
-	{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 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 === \PKP\user\form\APIProfileForm::API_KEY_NEW) ? "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>