Skip to content

Conversation

@Ioannis
Copy link
Contributor

@Ioannis Ioannis commented Mar 17, 2026

No description provided.

@Ioannis Ioannis requested a review from arlen March 17, 2026 14:12
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch from c991c60 to cd5cdbd Compare March 17, 2026 14:16
@Ioannis
Copy link
Contributor Author

Ioannis commented Mar 17, 2026

Screenshot from 2026-03-17 16-17-57
Screenshot from 2026-03-17 16-19-01

@Ioannis Ioannis requested a review from benno March 17, 2026 14:22
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch from cd5cdbd to c31a514 Compare March 19, 2026 14:45
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 2 times, most recently from 7a63755 to 05b735c Compare April 7, 2026 07:06
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 2 times, most recently from af9faba to ea8ac7d Compare April 19, 2026 04:29
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 4 times, most recently from dd66827 to 4293765 Compare May 7, 2026 16:09
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 3 times, most recently from 1b2ec09 to bf616a2 Compare May 10, 2026 04:00
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 2 times, most recently from 179d6da to 66621fa Compare May 20, 2026 06:05
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch 4 times, most recently from 4fe0400 to ef65098 Compare June 8, 2026 07:33
@Ioannis Ioannis requested a review from satkinson June 8, 2026 07:36
@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch from ef65098 to d9b0ace Compare June 23, 2026 03:29
*
* @param EventInterface $event Cake Event
* @return \Cake\Http\Response|null|void
* @since COmanage Registry v5.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

v5.3.0

return true;
}
} catch (\Exception $e) {
Log::error(__METHOD__ . "::Connection test '{$name}' failed: " . $e->getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

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

If you use LabeledLogTrait::llog it'll automatically insert function call information in a standard way

* @throws \InvalidArgumentException If non-delete and $newDn is missing.
* @throws \RuntimeException On LDAP failures not handled by fallback.
*/
public function applyProvisioningPlan(
Copy link
Contributor

Choose a reason for hiding this comment

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

I realize this is a bit of a grey area, but I think there's a question as to how much of this belongs here and how much belongs in the LDAP Provisioner.

For comparison, the other Server models tend to be pretty thin, providing basic configuration and connectivity, and maybe functions that map pretty close to the wire protocol. For example, SmtpServer doesn't have any support functions (those are implemented in DeliveryUtilities), while MatchServer basically has function calls that perform the Match API requests.

* Performs a case-insensitive suffix match. If $baseDn is empty or not a suffix
* of $newDn, returns $newDn unchanged.
*/
protected function computeRelativeRdn(string $newDn, string $baseDn): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

"Relative RDN" is like "ATM Machine" :)

$context['diagnostic_message'] = $err;
}

Log::error(__METHOD__ . "::LDAP error during $functionName", $context);
Copy link
Contributor

Choose a reason for hiding this comment

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

Like above, maybe use LabeledLogTrait?

if ($connection) {
$this->Flash->success(__d('operation', 'rs.test.ok'));
Log::debug("Successfully connected to server " . $serverId);
Copy link
Contributor

Choose a reason for hiding this comment

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

LabeledLogTrait?

try {
if (method_exists($pluginTable, 'checkConnectivity')) {
// Set the Connection Manager config
Copy link
Contributor

Choose a reason for hiding this comment

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

Connection Manager?

Comment on lines +87 to +96
'controller' => 'Servers',
'action' => 'index',
'?' => [
'co_id' => $serverObj->co_id
]
];

// Redirect to the referring page, with a fallback to the Servers index
return $this->redirect($this->referer($fallbackRedirectToIndexPage));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use generateRedirect()?

[
'action' => 'testconnection',
'label' => __d('operation', 'connection.test'),
'icon' => 'fact_check'
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I think we're going to need to have a meeting to discuss icon selection

Comment on lines +166 to +179
msgstr "Failed to connect to LDAP server {0}"

msgid "error.LdapServers.credentials"
msgstr "Missing authentication credentials for LDAP server {0}"

msgid "error.LdapServers.serverurl.valid"
msgstr "Please enter a valid ldap or ldaps URL (e.g., ldaps://server:port)"

msgid "field.LdapServers.serverurl"
msgstr "Server URL"

msgid "field.LdapServers.serverurl.desc"
msgstr "LDAP server URL, e.g. ldap://ldap.example.org or ldaps://ldap.example.org"
Copy link
Contributor

Choose a reason for hiding this comment

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

The messages should be grouped together as the rest of the file is grouped, so error messages with other error messages, field messages with other field messages, etc.

@Ioannis Ioannis force-pushed the CFM-103_LDAP_server_mvc branch from d9b0ace to fdaf7c3 Compare July 5, 2026 05:56
Sign in to join this conversation on GitHub.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants