8000 implement add and delete method by fabpot · Pull Request #17679 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

implement add and delete method #17679

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

Closed
wants to merge 1 commit into from
Closed

implement add and delete method #17679

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
61 changes: 46 additions & 15 deletions src/Symfony/Component/Ldap/LdapClient.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ public function __construct($host = null, $port = 389, $version = 3, $useSsl = f
$this->useSsl = (bool) $useSsl;
$this->useStartTls = (bool) $useStartTls;
$this->optReferrals = (bool) $optReferrals;

$this->connect();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's a good idea to open the connection in the constructor.

Copy link

Choose a reason for hiding this comment

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

I don't know, the returned resource by ldap_connect() function is required by 99% of ldap_* function.

If you want, i can put as was.

Copy link
Contributor

Choose a reason for hiding this comment

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

I almost think there should be a config class or an array of options passed to the constructor. That way you could have something like an auto-connect option that could default to true, but could still be changed if someone wanted to.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ChadSikorra PR #17560 adresses this issue of the constructor.

@L0rD59, the connection should not be done in the constructor. Indeed, services which depend on the Ldap service may actually want to do checks before even trying to connect to the Ldap server. Like checking whether the user's password is empty or not before trying to bind against the server.

}

public function __destruct()
Expand All @@ -66,10 +68,6 @@ public function __destruct()
*/
public function bind($dn = null, $password = null)
{
if (!$this->connection) {
$this->connect();
}

if (false === @ldap_bind($this->connection, $dn, $password)) {
throw new ConnectionException(ldap_error($this->connection));
}
Expand Down Expand Up @@ -115,23 +113,56 @@ public function escape($subject, $ignore = '', $flags = 0)
return $value;
}

/**
* Add entry to the ldap.
*
* @param string $dn A LDAP dn
* @param array $entry key:value
*
* @throws \Exception.
*/
public function add($dn, $entry)
{
if(ldap_add($this->connection, $dn, $entry) === false)
{
throw new \Exception('Cannot add to ldap : '.ldap_error($this->connection));
}

return true;
}

/**
* Delete entry to the ldap.
*
* @param string $dn A LDAP dn
*
* @throws \Exception.
*/
public function delete($dn)
{
if(ldap_delete($this->connection, $dn) === false)
{
throw new \Exception('Cannot delete to ldap : '.ldap_error($this->connection));
}

return true;
}

private function connect()
{
if (!$this->connection) {
$host = $this->host;
$host = $this->host;

if ($this->useSsl) {
$host = 'ldaps://'.$host;
}
if ($this->useSsl) {
$host = 'ldaps://'.$host;
}

$this->connection = ldap_connect($host, $this->port);
$this->connection = ldap_connect($host, $this->port);

ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, $this->version);
ldap_set_option($this->connection, LDAP_OPT_REFERRALS, $this->optReferrals);
ldap_set_option($this->connection, LDAP_OPT_PROTOCOL_VERSION, $this->version);
ldap_set_option($this->connection, LDAP_OPT_REFERRALS, $this->optReferrals);

if ($this->useStartTls) {
ldap_start_tls($this->connection);
}
if ($this->useStartTls) {
ldap_start_tls($this->connection);
}
}

Expand Down
0