Skip to content

Commit e90d715

Browse files
test(OpenVPNClientExport): ensure model handles crtid ambiguity #756
This test ensures that the OpenVPNClientExport model correctly locates the target cert's 'crtid' before providing it to pfSense function. This is necessary as pfSense will change the meaning of crtid for various factors.
1 parent e392732 commit e90d715

2 files changed

Lines changed: 43 additions & 6 deletions

File tree

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Models/OpenVPNClientExport.inc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ class OpenVPNClientExport extends Model {
257257
return openvpn_client_export_config(
258258
srvid: $this->server->value,
259259
usrid: $this->username->value ? $this->username->get_related_model()->id : null,
260-
crtid: $this->__get_crtid_from_certref(),
260+
crtid: $this->locate_crtid(),
261261
useaddr: $this->useaddr_hostname->value ?? $this->useaddr->value,
262262
verifyservercn: $this->verifyservercn->value,
263263
blockoutsidedns: $this->blockoutsidedns->value,
@@ -291,7 +291,7 @@ class OpenVPNClientExport extends Model {
291291
return openvpn_client_export_installer(
292292
srvid: $this->server->value,
293293
usrid: $this->username->value ? $this->username->get_related_model()->id : null,
294-
crtid: $this->__get_crtid_from_certref(),
294+
crtid: $this->locate_crtid(),
295295
useaddr: $this->useaddr_hostnam->value ?? $this->useaddr->value,
296296
verifyservercn: $this->verifyservercn->value,
297297
blockoutsidedns: $this->blockoutsidedns->value,
@@ -319,7 +319,7 @@ class OpenVPNClientExport extends Model {
319319
return viscosity_openvpn_client_config_exporter(
320320
srvid: $this->server->value,
321321
usrid: $this->username->value ? $this->username->get_related_model()->id : null,
322-
crtid: $this->__get_crtid_from_certref(),
322+
crtid: $this->locate_crtid(),
323323
useaddr: $this->useaddr_hostname->value ?? $this->useaddr->value,
324324
verifyservercn: $this->verifyservercn->value,
325325
blockoutsidedns: $this->blockoutsidedns->value,
@@ -343,7 +343,7 @@ class OpenVPNClientExport extends Model {
343343
* 'system/user/{$usrid}/cert' config path.
344344
* @returns int|null The crtid value pfSense expects for a given certref, or null if no certref is set.
345345
*/
346-
private function __get_crtid_from_certref(): int|null {
346+
public function locate_crtid(): int|null {
347347
# If no certref is set, return null
348348
if (!$this->certref->value) {
349349
return null;
@@ -361,8 +361,8 @@ class OpenVPNClientExport extends Model {
361361
$ovpnsrv->mode->value === 'server_tls_user' and
362362
$ovpnsrv->authmode->value === ['Local Database']
363363
) {
364-
foreach ($user->cert->get_config() as $idx => $user_cert) {
365-
if ($user_cert['refid'] === $cert->id) {
364+
foreach ($user->cert->value as $idx => $user_cert) {
365+
if ($user_cert === $cert->refid->value) {
366366
return $idx;
367367
}
368368
}

pfSense-pkg-RESTAPI/files/usr/local/pkg/RESTAPI/Tests/APIModelsOpenVPNClientExportTestCase.inc

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,4 +441,41 @@ class APIModelsOpenVPNClientExportTestCase extends TestCase {
441441
$this->assert_is_true(!ctype_print($export->binary_data->value));
442442
$this->assert_is_false(file_exists("/tmp/{$export->filename->value}"));
443443
}
444+
445+
/**
446+
* Ensures we can exporta config for a server using the 'server_tls_user' mode, a user cert, and local database
447+
* auth. This is a regression test for issue #756. This is a necessary test distinction as pfSense will change the
448+
* meaning of the crtid passed into certain pfSense functions based on the server mode and authmode.
449+
*/
450+
public function test_create_with_server_tls_user_and_local_database(): void {
451+
# First, update the OpenVPNServer to use the 'server_tls_user' mode and local database auth
452+
$this->ovpns->mode->value = 'server_tls_user';
453+
$this->ovpns->authmode->value = ["Local Database"];
454+
$this->ovpns->update();
455+
456+
# Setup the export
457+
$export = new OpenVPNClientExport(
458+
id: $this->ovpnce->id,
459+
type: 'confinline',
460+
username: $this->user->name->value,
461+
certref: $this->user_cert->refid->value,
462+
);
463+
464+
# Ensure the certref's object ID does NOT match the determined crtid. These should not match because
465+
# pfSense now refers to the crtid as the index of the cert in the system/user config, NOT the cert config
466+
# like usual.
467+
$this->assert_not_equals(
468+
$this->user_cert->refid->get_related_model()->id,
469+
$export->locate_crtid()
470+
);
471+
472+
# Ensure we can complete the export as intended and that the embedded cert is correct
473+
$export->create();
474+
$this->assert_is_not_empty($export->binary_data->value);
475+
$this->assert_str_contains($export->binary_data->value, $this->user_cert->crt->value);
476+
477+
# Reset the server mode and authmode for other tests
478+
$this->ovpns->mode->value = 'server_tls';
479+
$this->ovpns->update();
480+
}
444481
}

0 commit comments

Comments
 (0)