This patch allows a user to input a string as the port number, which is then used as a PHP constant when connecting to the LDAP server.

Image is attached.

Motivation

We want to provide Pantheon customers connectivity from their application servers to their internal LDAP servers via a secure tunnel. We provide a port number listening on localhost for this, and then tunnel the traffic to their server. We change this port number fairly frequently, so we want to provide the port number as an injected PHP constant.

This functionality should be useful in general for any enterprise Drupal site that would like this port configuration injected rather than stored in a database table.

Comments

Mark Theunissen’s picture

Issue summary: View changes
StatusFileSize
new26.26 KB
Mark Theunissen’s picture

Issue summary: View changes
Mark Theunissen’s picture

Issue summary: View changes
Mark Theunissen’s picture

Issue summary: View changes
Mark Theunissen’s picture

Status: Active » Needs review
johnbarclay’s picture

Aside from the bad coding practice of having a PHP constant that changes all the time and the unconventional act of connecting to an ldap server over constantly changing ports, this seems like an edge case. I'm open to any hooks that will make this possible. The goal of LDAP 7 is to move toward a release candidate.

Mark Theunissen’s picture

John, I've left out a lot of implementation details that would probably help explain why this architecture is necessary. Essentially, we provide a secure tunnel to our customer's LDAP server, and it's the port number of the tunnel that changes, rather than the port number of the LDAP server.

I disagree that it's bad coding practice. The concept of a PHP constant is that it must remain constant in a single request. Many constants change value from request-to-request and between code deploys. For example, the superglobals.

The principle is that we want to move configuration out of the database and into code. This is a Drupal 8 goal. I'm open to other suggestions - a Drupal variable perhaps, instead of a database entry, allowing us to override it in code? That seems like a worse solution, though.

johnbarclay’s picture

Your patch won't hurt anything, so I think we should just go with it as is. If you can review a couple other patches that would help the module out also.

Mark Theunissen’s picture

It's difficult for me to review patches as I don't have a production site or LDAP server to test with. I can't use our customer's sites to test random patches with, obviously this patch will be tested on customer sites, but I don't have control over much more than that. If you have some specific issues you'd like me to look at that don't require actually having a real-world installation of LDAP, I can look at them.

larowlan’s picture

Status: Needs review » Needs work

This would be better served with an alter hook that ran after loading the server from the DB.
Then you could implement a module that inspected an environment or $conf variable and dynamically set the port.

grahl’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, ldap_php-constant-port_1.patch, failed testing.

grahl’s picture

Status: Needs work » Closed (won't fix)

This patch won't get committed, since no one stepped up to get this in cleanly with an alter hook, I'm closing this issue.