Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(database): Fix public_key_credential_id length #36769

Closed
wants to merge 1 commit into from
Closed

fix(database): Fix public_key_credential_id length #36769

wants to merge 1 commit into from

Conversation

zevlee
Copy link

@zevlee zevlee commented Feb 18, 2023

Summary

Increases length of column for security key so that it fits. This is essentially an identical migration step to nextcloud/twofactor_webauthn#323.

Checklist

@szaimen szaimen added the 3. to review Waiting for reviews label Feb 18, 2023
@szaimen szaimen requested review from a team, ArtificialOwl, icewind1991 and blizzz and removed request for a team February 18, 2023 02:23
@szaimen szaimen added this to the Nextcloud 26 milestone Feb 18, 2023
* @param array $options
* @return null|ISchemaWrapper
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {

Check failure

Code scanning / Psalm

UndefinedClass Error

Class, interface or enum named OC\Core\Migrations\ISchemaWrapper does not exist
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 7, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@szaimen szaimen requested a review from a team May 26, 2023 14:15
@szaimen szaimen requested a review from ChristophWurst June 15, 2023 21:12
class Version25000Date20230217195657 extends SimpleMigrationStep {
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

Check failure

Code scanning / Psalm

UndefinedDocblockClass Error

Docblock-defined class, interface or enum named OC\Core\Migrations\ISchemaWrapper does not exist
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

$registrationsTable = $schema->getTable('webauthn');

Check failure

Code scanning / Psalm

UndefinedDocblockClass Error

Docblock-defined class, interface or enum named OC\Core\Migrations\ISchemaWrapper does not exist
->getColumn('public_key_credential_id')
->setLength(512);

return $schema;

Check failure

Code scanning / Psalm

InvalidReturnStatement Error

The inferred type 'OC\Core\Migrations\ISchemaWrapper' does not match the declared return type 'OCP\DB\ISchemaWrapper&OC\Core\Migrations\ISchemaWrapper|null' for OC\Core\Migrations\Version25000Date20230217195657::changeSchema
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
@blizzz blizzz mentioned this pull request Nov 14, 2023
@blizzz blizzz mentioned this pull request Nov 20, 2023
5 tasks
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Please also adjust the original migration that created the column so that new instances have the correct length by default. This should be quicker than creating the wrong width and changing the length right away.

This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@skjnldsv
Copy link
Member

Solved by #47240 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passwordless WebAuthn with Nitrokey 3 does not work
5 participants