Improve oauth2 database migration from ownCloud

[julien-nc]

• Drop the oauth2_clients trusted column
• Delete unsupported clients and their access tokens
  The redirect_uri set by OC can be like oc://android.owncloud.com or http://host/* and we don't support those.

I tested it by first putting those changes in a migration step in the oauth2 app. Worked fine.

[artonge]

Does this work? I though getSQL didn't replace the parameters.

[julien-nc]

That works. Maybe because the expression is built with the same query builder.
$qbSelectClientId->expr() and $qbSelectClientId->getSQL()

[artonge]

If this can help, I kept a script to test the initial version of the migration: #30276

[juliusknorr]

Do we know if the column is indexed on the owncloud side?

[AndyScherzinger]

We don't but I guess @pmarini-nc can get that information for us

[pmarini-nc]

I understand the column that we want to know whether it is indexed is oauth2_clients.id. I asked the prospect

[julien-nc]

I installed ownCloud 10.12.1. The only oauth2 related index is on the token column of oauth2_access_tokens.

[pmarini-nc]

oC table definition:

MySQL [ucprod]> show create table oc_oauth2_clients\G
*************************** 1. row ***************************
       Table: oc_oauth2_clients
Create Table: CREATE TABLE `oc_oauth2_clients` (
  `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `identifier` varchar(64) COLLATE utf8_bin NOT NULL,
  `secret` varchar(64) COLLATE utf8_bin NOT NULL,
  `redirect_uri` varchar(2000) COLLATE utf8_bin NOT NULL,
  `name` varchar(80) COLLATE utf8_bin NOT NULL,
  `allow_subdomains` tinyint(1) NOT NULL DEFAULT 0,
  `trusted` tinyint(1) NOT NULL DEFAULT 0,
  PRIMARY KEY (`id`),
  UNIQUE KEY `name_idx` (`name`)
) ENGINE=InnoDB AUTO_INCREMENT=11 DEFAULT CHARSET=utf8 COLLATE=utf8_bin

[juliusknorr]

All right, then this would require a full table scan for the like statements, which could take longer depending on the size of the table, but I'd say since this is a onetime migration this should be fine.

[julien-nc]

Potential problem: The name column of oauth2_clients is a VARCHAR(80) in ownCloud and a VARCHAR(64) in Nextcloud.
No problem for the migration, the DB will keep the column with a length of 80.
But what if an existing stored value's length is more than 64 and later we, for some reason, insert it somewhere else, expecting it to be 64 max?

Do you think we should cut the values during the migration?

[AndyScherzinger]

Do you think we should cut the values during the migration?

I think we should just bump the field length to VARCHAR(64) during/after migration, so it matches our Nextcloud-length. We don't need to cut anything during migration since the original oC length is shorter than ours however any newly added row after the migration to Nextcloud would expect 64 and might fail. Hence the DB structure needs to be updated to VARCHAR(64).

To support future migrations it be code if this is done in code by us and not purely via DBA cli else we might miss it in future migration scenarios.

Wat do you think @julien-nc ?

[julien-nc]

@AndyScherzinger Actually we already set the name column length to 64 in this ownCloud oauth2 repair step: https:/nextcloud/server/blob/master/lib/private/Repair/Owncloud/MigrateOauthTables.php#L79-L81

I wonder what happens for rows having a name which is longer than 64.
I think running UPDATE oc_oauth2_clients SET name=substr(name, 1, 64); before changing the column size is safer.

[julien-nc]

@AndyScherzinger 745b408 shortens the values before resizing the column.

[AndyScherzinger]

Sounds/looks good to me @julien-nc 👍

[julien-nc]

/backport to stable27

[julien-nc]

/backport to stable26

[julien-nc]

/backport to stable25

[AndyScherzinger]

LGTM 👍
...as per our discussions within the PR :)

[julien-nc]

CI failures are not related (Cypress jobs)

[backportbot-nextcloud]

The backport to stable25 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable25
git pull origin/stable25

# Create the new backport branch
git checkout -b fix/foo-stable25

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable25

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport