passbolt / passbolt_api

Passbolt Community Edition (CE) API. The JSON API for the open source password manager for teams!
https://passbolt.com
GNU Affero General Public License v3.0
4.73k stars 311 forks source link

[PB-35722] Timezone issue in GpgkeysIndexController.php #522

Open glingy opened 1 month ago

glingy commented 1 month ago

ISSUE NAME

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 https://github.com/passbolt/passbolt_api/blob/57e215097619e99e4dedd2f66b0e2acb846e29bd/src/Model/Table/GpgkeysTable.php#L182-L183

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: https://github.com/passbolt/passbolt_api/blob/57e215097619e99e4dedd2f66b0e2acb846e29bd/plugins/PassboltCe/MultiFactorAuthentication/src/Utility/MfaAccountSettings.php#L177-L178

glingy commented 1 month ago

Reported as abuse.

ishanvyas22 commented 1 month ago

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