Skip to content

perf: move remember login tokens out of oc_preferences - #64203

Open
cristianscheid wants to merge 3 commits into
masterfrom
perf/61728/remember-login-token
Open

perf: move remember login tokens out of oc_preferences#64203
cristianscheid wants to merge 3 commits into
masterfrom
perf/61728/remember-login-token

Conversation

@cristianscheid

@cristianscheid cristianscheid commented Sep 10, 2026

Copy link
Copy Markdown
Member

Summary

Currently, when logging in with "Remember me" selected, a token is stored in oc_preferences table:

image
MariaDB [nextcloud]> SELECT * FROM oc_preferences WHERE appId="login_token";
+--------+-------------+----------------------------------+-------------+------+------+-------+---------+
| userid | appid       | configkey                        | configvalue | lazy | type | flags | indexed |
+--------+-------------+----------------------------------+-------------+------+------+-------+---------+
| john   | login_token | eqqbZqE6U1t7GsYyd/hCNtMQdGXDq1k0 | 1789069718  |    0 |    0 |     0 |         |
+--------+-------------+----------------------------------+-------------+------+------+-------+---------+
1 row in set (0.001 sec)

Changes introduced by this PR:

  • new dedicated table for this purporse: oc_remember_login_tokens
MariaDB [nextcloud]> SELECT * FROM oc_remember_login_tokens;
+----+------+----------------------+------------+
| id | uid  | token                | created    |
+----+------+----------------------+------------+
|  1 | john | <hashed-token-here>  | 1789070212 |
+----+------+----------------------+------------+
1 row in set (0.000 sec)
  • mechanism to migrate old remember login tokens from oc_preferences to oc_remember_login_tokens
    • first look for token on oc_remember_login_tokens, if not there, look at oc_preferences
    • if found in oc_preferences, insert token on oc_remember_login_tokens and remove from oc_preferences
    • old tokens from oc_preferences should all be eventually removed either by mechanism above or by OC\User\BackgroundJobs\CleanupLoginTokens
    • OC\User\BackgroundJobs\CleanupLoginTokens cleans stale tokens, by default the ones created > 15 days

Note 1

While brainstorming how to implement this, one suggested approach was to use OCP\Security\ICredentialsManager to store the tokens.

  • credentials are stored in oc_storages_credentials (columns: id, user, identifier, credentials)
  • credentials column value is encrypted like $this->crypto->encrypt(json_encode($credentials))
  • to store the timestamp, as is currently done in oc_preferences, the token would need to be set as the identifier (to be able to search by token), with credentials holding the timestamp:
$this->credentialsManager->store($userId, $token, $timestamp);
  • this would store the token in plain text. If we wanted to hash it instead (e.g. hash('sha512', $token)), the 128-character string would be too large for the column, since identifier is defined as:
$table->addColumn('identifier', Types::STRING, [
    'notnull' => true,
    'length' => 64,
]);
  • even without hash, there's still OC\User\BackgroundJobs\CleanupLoginTokens, which removes all login tokens older than a certain threshold. Since credentials column value is encrypted before being stored, cleanup would require decrypting the timestamp for each one just to determine whether an entry is stale

For these reasons, I went with a dedicated table instead, since we can store the hashed value of the token, and easily delete stale records with a direct query, since the timestamp itself is not hashed.

Note 2

After this get merged in master, existing dev instances will need to apply new migration to avoid table not found error:

# to run pending migrations for core
occ migrations:migrate core

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

@cristianscheid
cristianscheid requested a review from a team as a code owner September 10, 2026 21:05
@cristianscheid
cristianscheid requested review from come-nc, icewind1991, leftybournes and provokateurin and removed request for a team September 10, 2026 21:05
@cristianscheid cristianscheid self-assigned this Sep 10, 2026
@cristianscheid
cristianscheid force-pushed the perf/61728/remember-login-token branch 4 times, most recently from 2968e13 to 34462cd Compare September 11, 2026 13:17
@solracsf solracsf added this to the Nextcloud 36 milestone Sep 12, 2026
@CarlSchwan

Copy link
Copy Markdown
Member

After this get merged in master, existing dev instances will need to apply new migration to avoid table not found error:

Bump the version in version.php

@come-nc come-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would make sense to update the entity when rotating the token, rather than deleting+inserting.
I’m also wondering whether we could use snowflake ids instead of numeric id + created at. The snowflake contains the creation timestamp.

@come-nc

come-nc commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Also it would be better to switch to the new entity system.
See #63288 for an example.

Comment thread lib/private/Authentication/RememberLogin/RememberLoginToken.php Outdated
@cristianscheid

cristianscheid commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

I think it would make sense to update the entity when rotating the token, rather than deleting+inserting. I’m also wondering whether we could use snowflake ids instead of numeric id + created at. The snowflake contains the creation timestamp.

I think we cannot have both things together:

  • if we rotate the token by updating the entry, same snowflake ID would be kept, meaning timestamp would still hold creation time
  • CleanupLoginTokens cron job would eventually cleanup this entry from DB, even if it was updated, since timestamp is still from creation time

So I think we have three options:

option 1

  • use snowflake ID and use it's own timestamp to check when token was generated instead of created column
  • delete + insert entry when rotating token, to generate correct timestamp

option 2

  • use regular numeric ID + dedicated timestamp column
  • update entry instead of delete + insert when rotating token

option 3

  • use snowflake ID + dedicated timestamp column
  • update entry instead of delete + insert when rotating token

cc @come-nc

@come-nc

come-nc commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

@cristianscheid Is it not possible to update the id as well?

Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
@cristianscheid
cristianscheid force-pushed the perf/61728/remember-login-token branch from 34462cd to 7eade0a Compare September 14, 2026 17:50
@cristianscheid

Copy link
Copy Markdown
Member Author

Bump the version in version.php

@CarlSchwan Thanks for the tip! Should the commit below be enough?
commit: d7aa083

@cristianscheid

cristianscheid commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

@cristianscheid Is it not possible to update the id as well?

@come-nc I did not consider this approach at first since EntityManager::update() does not allow updating ID, but looking at codebase again I think we can follow a similar approach to AccessTokenMapper::rotateToken(), which does a more direct update using query builder instead of EntityManager::update().

We could do something similar to update both the token and the snowflake ID, refreshing it's timestamp. I implemented this on this commit: 7eade0a

There's one caveat with this approach: when migrating from oc_preferences, the old token's creation timestamp gets reset, since the row is recreated in the new table with snowflake id, encoding current timestamp instead of the original one:
SnowflakeGenerator::nextId()

edit: caveat above (strikethrough text) isn't actually a problem. Before this change, token was already regenerated at that same point in loginWithCookie(), which produces a fresh timestamp.

@cristianscheid
cristianscheid force-pushed the perf/61728/remember-login-token branch from 7eade0a to 2fd0a09 Compare September 14, 2026 18:22
$table->addUniqueIndex(['token'], 'remember_login_tokens_token');
$table->addIndex(['uid'], 'remember_login_tokens_uid');
// Makes sure there is no auto-increment in Oracle
$schema->dropAutoincrementColumn('remember_login_tokens', 'id');

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

technically not needed as this was never merged as a autoincremented column but this also doesn't hurt

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

you think it's safe to remove the dropAutoincrementColumn() then?

…ake ids

Signed-off-by: Cristian Scheid <cristianscheid@gmail.com>
@cristianscheid
cristianscheid force-pushed the perf/61728/remember-login-token branch from 2fd0a09 to 41d9870 Compare September 14, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move remember me token out of oc_preferences

4 participants