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.

Sign up for GitHub

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

Do not expose private key in ACP module (phpBB 4.0 consistent). #14

Merged
merged 2 commits into from
Jan 4, 2024
Merged

Do not expose private key in ACP module (phpBB 4.0 consistent). #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1889617
Do not expose private key in ACP module (phpBB 4.0 consistent).
rxu Jan 4, 2024
e0c2429
Fix test.
rxu Jan 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions acp/wpn_acp_module.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public function display_settings()
$this->template->assign_vars([
'S_WEBPUSH_ENABLE' => $this->config['wpn_webpush_enable'],
'WEBPUSH_VAPID_PUBLIC' => $this->config['wpn_webpush_vapid_public'],
'WEBPUSH_VAPID_PRIVATE' => $this->config['wpn_webpush_vapid_private'],
'WEBPUSH_VAPID_PRIVATE' => !$this->config['wpn_webpush_vapid_private'] ?: '********', // Replace private key with asterixes
'U_ACTION' => $this->u_action,
]);
}
Expand All @@ -117,6 +117,12 @@ public function save_settings()
'wpn_webpush_vapid_private'=> ['validate' => 'string:25:255', 'lang' => 'WEBPUSH_VAPID_PRIVATE'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you just set this to 'validate' => 'password:25:255' and that will handle everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be handy but there's no such a thing in config vars validator. build_cfg_template can handle displaying password as asterixes but preventing its updating and validation is still handled within dedicated xCP module.

];

// Do not validate and update private key field if the content is ******** and the key was already set
if ($config_array['wpn_webpush_vapid_private'] == '********' && $this->config['wpn_webpush_vapid_private'])
{
unset($display_settings['wpn_webpush_vapid_private'], $config_array['wpn_webpush_vapid_private']);
}

if ($config_array['wpn_webpush_enable'])
{
// Validate config values
Expand All @@ -135,9 +141,10 @@ public function save_settings()

$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'LOG_CONFIG_WEBPUSH');

$this->config->set('wpn_webpush_enable', $config_array['wpn_webpush_enable']);
$this->config->set('wpn_webpush_vapid_public', $config_array['wpn_webpush_vapid_public']);
$this->config->set('wpn_webpush_vapid_private', $config_array['wpn_webpush_vapid_private']);
foreach ($config_array as $config_name => $config_value)
{
$this->config->set($config_name, $config_value);
}

trigger_error($this->lang->lang('CONFIG_UPDATED') . adm_back_link($this->u_action), E_USER_NOTICE);
}
Expand Down
1 change: 1 addition & 0 deletions tests/functional/functional_test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public function test_acp_module()

foreach ($form_data as $config_name => $config_value)
{
$config_value = ($config_name === 'config[wpn_webpush_vapid_private]') ? '********' : $config_value;
$this->assertEquals($config_value, $crawler->filter('input[name="' . $config_name . '"]')->attr('value'));
}
}
Expand Down