Merge pull request #30235 from nextcloud/bugfix/noid/primary-key-for-…

…ratelimit-table

Add primary key for ratelimit table

core/Migrations/Version23000Date20210906132259.php

@@ -11,33 +11,36 @@
 use OCP\Migration\SimpleMigrationStep;
 
 class Version23000Date20210906132259 extends SimpleMigrationStep {
-	private const TABLE_NAME = 'ratelimit_entries';
-
 	/**
 	 * @param IOutput $output
 	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
 	 * @param array $options
 	 * @return null|ISchemaWrapper
 	 */
 	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
-		/** @var ISchemaWrapper $schema */
-		$schema = $schemaClosure();
-
-		$hasTable = $schema->hasTable(self::TABLE_NAME);
-
-		if (!$hasTable) {
-			$table = $schema->createTable(self::TABLE_NAME);
-			$table->addColumn('hash', Types::STRING, [
-				'notnull' => true,
-				'length' => 128,
-			]);
-			$table->addColumn('delete_after', Types::DATETIME, [
-				'notnull' => true,
-			]);
-			$table->addIndex(['hash'], 'ratelimit_hash');
-			$table->addIndex(['delete_after'], 'ratelimit_delete_after');
-			return $schema;
-		}
+		/**
+		 * Table was missing a primary key
+		 * Therefore it was dropped with Version24000Date20211213081506
+		 * and then recreated with a primary key in Version24000Date20211213081604
+		 */
+//		/** @var ISchemaWrapper $schema */
+//		$schema = $schemaClosure();
+//
+//		$hasTable = $schema->hasTable(self::TABLE_NAME);
+//
+//		if (!$hasTable) {
+//			$table = $schema->createTable(self::TABLE_NAME);
+//			$table->addColumn('hash', Types::STRING, [
+//				'notnull' => true,
+//				'length' => 128,
+//			]);
+//			$table->addColumn('delete_after', Types::DATETIME, [
+//				'notnull' => true,
+//			]);
+//			$table->addIndex(['hash'], 'ratelimit_hash');
+//			$table->addIndex(['delete_after'], 'ratelimit_delete_after');
+//			return $schema;
+//		}
 
 		return null;
 	}

core/Migrations/Version24000Date20211213081506.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Core\Migrations;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version24000Date20211213081506 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$hasTable = $schema->hasTable('ratelimit_entries');
+		if ($hasTable) {
+			$schema->dropTable('ratelimit_entries');
+			return $schema;
+		}
+
+		return null;
+	}
+}

core/Migrations/Version24000Date20211213081604.php

@@ -0,0 +1,66 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2021 Joas Schilling <coding@schilljs.com>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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 Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Core\Migrations;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version24000Date20211213081604 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$hasTable = $schema->hasTable('ratelimit_entries');
+
+		if (!$hasTable) {
+			$table = $schema->createTable('ratelimit_entries');
+			$table->addColumn('id', Types::BIGINT, [
+				'autoincrement' => true,
+				'notnull' => true,
+			]);
+			$table->addColumn('hash', Types::STRING, [
+				'notnull' => true,
+				'length' => 128,
+			]);
+			$table->addColumn('delete_after', Types::DATETIME, [
+				'notnull' => true,
+			]);
+			$table->setPrimaryKey(['id']);
+			$table->addIndex(['hash'], 'ratelimit_hash');
+			$table->addIndex(['delete_after'], 'ratelimit_delete_after');
+			return $schema;
+		}
+
+		return null;
+	}
+}

lib/composer/composer/autoload_classmap.php

@@ -992,6 +992,8 @@
     'OC\\Core\\Migrations\\Version23000Date20211203110726' => $baseDir . '/core/Migrations/Version23000Date20211203110726.php',
     'OC\\Core\\Migrations\\Version23000Date20211213203940' => $baseDir . '/core/Migrations/Version23000Date20211213203940.php',
     'OC\\Core\\Migrations\\Version24000Date20211210141942' => $baseDir . '/core/Migrations/Version24000Date20211210141942.php',
+    'OC\\Core\\Migrations\\Version24000Date20211213081506' => $baseDir . '/core/Migrations/Version24000Date20211213081506.php',
+    'OC\\Core\\Migrations\\Version24000Date20211213081604' => $baseDir . '/core/Migrations/Version24000Date20211213081604.php',
     'OC\\Core\\Migrations\\Version24000Date20211222112246' => $baseDir . '/core/Migrations/Version24000Date20211222112246.php',
     'OC\\Core\\Migrations\\Version24000Date20211230140012' => $baseDir . '/core/Migrations/Version24000Date20211230140012.php',
     'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php',

lib/composer/composer/autoload_static.php

@@ -1021,6 +1021,8 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Core\\Migrations\\Version23000Date20211203110726' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20211203110726.php',
         'OC\\Core\\Migrations\\Version23000Date20211213203940' => __DIR__ . '/../../..' . '/core/Migrations/Version23000Date20211213203940.php',
         'OC\\Core\\Migrations\\Version24000Date20211210141942' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20211210141942.php',
+        'OC\\Core\\Migrations\\Version24000Date20211213081506' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20211213081506.php',
+        'OC\\Core\\Migrations\\Version24000Date20211213081604' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20211213081604.php',
         'OC\\Core\\Migrations\\Version24000Date20211222112246' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20211222112246.php',
         'OC\\Core\\Migrations\\Version24000Date20211230140012' => __DIR__ . '/../../..' . '/core/Migrations/Version24000Date20211230140012.php',
         'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php',

lib/private/Security/RateLimiting/Backend/DatabaseBackend.php

@@ -82,9 +82,6 @@ private function getExistingAttemptCount(
 			->from(self::TABLE_NAME)
 			->where(
 				$qb->expr()->eq('hash', $qb->createNamedParameter($identifier, IQueryBuilder::PARAM_STR))
-			)
-			->andWhere(
-				$qb->expr()->gte('delete_after', $qb->createNamedParameter($currentTime, IQueryBuilder::PARAM_DATE))
 			);
 
 		$cursor = $qb->executeQuery();