Closed sjpadgett closed 5 years ago
What do you mean?
If anything, I would like to take charge of the LDAP integration with OpenEMR / figure out all the documentation.
I’ve done it before with Sambda on Linux.
On Apr 5, 2019, at 6:11 AM, Jerry Padgett notifications@github.com wrote:
Why are we forcing LDAP now, Why?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.
what in openemr is now requiring ldap to be installed and running?
I dunno u tell me
I just see people ask on the forums sometimes about integrating OpenEMR with their Active Directory
On Apr 5, 2019, at 10:12 AM, Jerry Padgett notifications@github.com wrote:
what in openemr is now requiring ldap to be installed and running?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
So then they can install LDAP just don't force on everyone.
On Fri, Apr 5, 2019 at 1:16 PM Dan E notifications@github.com wrote:
I dunno u tell me
I just see people ask on the forums sometimes about integrating OpenEMR with their Active Directory
On Apr 5, 2019, at 10:12 AM, Jerry Padgett notifications@github.com wrote:
what in openemr is now requiring ldap to be installed and running?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openemr/openemr/issues/2362#issuecomment-480353806, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxhkXkYVqxlC8HeE8UiDPWRGbygxXKXks5vd4TygaJpZM4ce5GO .
-- Jerry Padgett Padgett's Consulting Brandon, Florida 33511 sjpadgett@gmail.com sjpadgett@gmailo.com
No we are going to force it on everyone. That’s why we’re including an Oracle EE database and Windows IIS 7.0 with OpenEMR starting next month
Sent from my iPhone
On Apr 5, 2019, at 10:19 AM, Jerry Padgett notifications@github.com wrote:
So then they can install LDAP just don't force on everyone.
On Fri, Apr 5, 2019 at 1:16 PM Dan E notifications@github.com wrote:
I dunno u tell me
I just see people ask on the forums sometimes about integrating OpenEMR with their Active Directory
On Apr 5, 2019, at 10:12 AM, Jerry Padgett notifications@github.com wrote:
what in openemr is now requiring ldap to be installed and running?
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openemr/openemr/issues/2362#issuecomment-480353806, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxhkXkYVqxlC8HeE8UiDPWRGbygxXKXks5vd4TygaJpZM4ce5GO .
-- Jerry Padgett Padgett's Consulting Brandon, Florida 33511 sjpadgett@gmail.com sjpadgett@gmailo.com — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
That's all fine and more power but it appears LDAP is now a dependency in composer? That's why I question this.
Oh yeah that’s weird. I dunno. Seems like a security issue too if anything since we want the minimum amount of software (I.e. vulnerabilities) in composer
On Apr 5, 2019, at 10:29 AM, Jerry Padgett notifications@github.com wrote:
That's all fine and more power but it appears LDAP is now a dependency in composer? That's why I question this.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
Hi,
LDAP is a supported option by OpenEMR out of the box: https://www.open-emr.org/wiki/index.php/Administration_Globals#Use_Active_Directory https://community.open-emr.org/t/active-directory-authentication-in-openemr-5/8243
Matrix contributed this feature and maintain it (relatively recently updated to support a recent version of the adldap2/adldap2 package, although version 10 is now out :) ).
as an aside, looks like it's time for a package update run (looks like at least adodb/adodb-php also needs an update)
Brady:
Where is the LDAP documentation / AD integration?
Do we not have this (I’ve seen people ask for it before)?
If so I will create an issue and work on this. Give me 6 weeks along with the logs issue.
On Apr 5, 2019, at 6:15 PM, Brady Miller notifications@github.com wrote:
as an aside, looks like it's time for a package update run (looks like at least adodb/adodb-php also needs an update)
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
Sounds good,
See above 2 links for what documentation we even have.
Hopefully @sharonco-matrix can provide some more details.
Can grep/search for active_directory_validation
to see how used in codebase.
Here we go again. If Active Directory is wanted, then those wanting it can install the dependencies to support the feature. Not force the other 95% of us into installing an unused potential security hole. An unsetup LDAP can be dangerous.
I can add the dependency installation to the guide instead of bundling it with OpenEMR when I write the docs.
Don’t know if removing it from composer will break other stuff tho.
Any other unnecessary libraries you see in composer? Would be good to remove also
On Apr 5, 2019, at 9:52 PM, Jerry Padgett notifications@github.com wrote:
Here we go again. If Active Directory is wanted, then those wanting it can install the dependencies to support the feature. Not force the other 95% of us into installing an unused potential security hole. An unsetup LDAP can be dangerous.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.
hi, The LDAP package is just some classes. And is only used if the user turns on this feature. Making end users try to use composer is probably more insecure than simply including this package (ie. who knows what bad things they may do learning how to use composer on their production instance); even many devs have a tough time with composer. Am I missing something? I can move it to the bottom of the list in composer.json :) -brady
Not talking about our class support but the composer run requirement that requires LDAP to be installed and running.
What do you mean 'installed and running'? which application do you need install? I don't find any LDAP application on my local machine and the composer runs well..
Probably because you already have the ldap extension installed. I tried to install openemr on a fresh LAMP and had to install LDAP for composer to complete. Figured it was a new dependency added recently, noted, installed LDAP and moved on.
On Sun, Apr 7, 2019 at 1:52 AM Amiel Elboim (Matrix) < notifications@github.com> wrote:
What do you mean 'installed and running'? which application do you need install? I don't find any LDAP application on my local machine and the composer runs well..
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/openemr/openemr/issues/2362#issuecomment-480561604, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxhkcodRkESDRf7TuRXDCTPnP5tm2hpks5veYehgaJpZM4ce5GO .
-- Jerry Padgett Padgett's Consulting Brandon, Florida 33511 sjpadgett@gmail.com sjpadgett@gmailo.com
Ok.. agree.. The composer.json of adldap2 require extension of ldap.
"require": {
"php": ">=7.0",
"ext-ldap": "*",
"psr/log": "~1.1",
"tightenco/collect": "~5.0",
"illuminate/contracts": "~5.0"
},
The solution If you don't want to install that extension is use --ignore-platform-reqs
option
composer install --ignore-platform-reqs=ext-ldap
Hopefully it will help.
FIX.
composer install --ignore-platform-reqs
This is the right command.
Not my point really and not looking for work arounds. My point is the vast majority of installs will not need directory services therefore, the requirement should not be in a default install. For the few that wish ldap support they can install as needed. So the fix should be, remove the req.
@bradymiller Please do as you think better. I agree that we must have the basic stuff and then add it according to our needs, but that means that we need to add a mechanism that let us do that without doing conflicts every time. SO just let us know what is your final call before removing it, please.
hi @sjpadgett and @sharonco-matrix , Let me do some research on this. Off the top of my head, ldap just requires php-ldap library on the OS end but I could be wrong. -brady
@sjpadgett and @danehrlich1 ,
Looking into security issues of the php-ldap php extension: https://www.php.net/manual/en/book.ldap.php
I have found no reported security issues with this extension. Do you know of any?
My concern was for running an unconfigured LDAP service. There are certainly security concerns for a web facing app using LDAP but those, i'm sure, are being addressed in the application layer. My main issue here is the additional install requirement and because this is a rarely used requirement to run OpenEMR, is it necessary to include in default dependencies? Bottom line is that i'm trying to head off implementation issues for 502 release where I see them. Or I can just go back to bed. :) I think if we ensure this requirement is part of our install doc and you security guys don't see issue with running an unconfigured LDAP extension then, i'm okay leaving as is...
Is there anyway this can be adjusted / migrated to a module? This seems the perfect kind of functionality you would want to enable / disable as a module extension instead of something core to the system.
It would have to be a module with composer dependencies so perhaps the module system needs to be adjusted to allow modules to be added by composer with their relevant composer dependencies. This would be similar to symfony's framework of adding bundles.
Hi,
Turns out a LDAP solution was in OpenEMR previously: https://community.open-emr.org/t/ldap/3852
This kind of stuff (incompletely done code missing lots of elements that only one person knew how to use) was not very uncommon in OpenEMR's remote past. Then for OpenEMR 5.0.1 release, Matrix formalized a integrated LDAP feature that worked by just configuring it in globals: https://www.open-emr.org/wiki/index.php/Administration_Globals#Use_Active_Directory
The php-ldap extension was then also added at that time to list of OpenEMR's php dependencies: https://www.open-emr.org/wiki/index.php/OpenEMR_System_Architecture#Ubuntu_16.04_and_Mint_18_and_Debian_9_and_greater_versions_.28PHP7.29_.28With_MySQL_or_MariaDB.29
And this feature is definitely useful in enterprise setting and does get some chatter on the forums (ie. it is being used; agree not a big percentage of userbase, but it is being used).
Composer is a relatively recent addition into OpenEMR, and the official build steps in composer involve the commands that approximate the following (ie. not simple stuff):
composer install
composer global require phing/phing
/root/.composer/vendor/bin/phing vendor-clean
composer global remove phing/phing
composer dump-autoload -o
And if end users don't do the above correctly, then there will be a bunch more files in the packages (ie. such as testing/examples directories) than should be.
I could find no security issues with including php-ldap extension since it's really just a library along with the composer adldap2 composer package.
In the end, it seems like having end users try to mess with composer on their productions instances would be far more insecure than keeping the ldap dependencies. A formal module system for things would be nice but that has eluded us so far.
One more point I should of added. Having the composer package visible in composer.json also ensures that it remains based on cutting edge versions of the adldap2 package (since we basically routinely update all those packages couple times per year). This prevents it from turning into broken code as ldap feature was back in the 2015 forum thread posted above :)
Wait a minute! You can run an enterprise but not know how to add a composer require without blowing up composer. Yet we want to send a user through additional hoops to ensure LDAP is installed. How many internet guides are there to install a LAMP that instruct, oh yes, install LDAP just in case. It's arduous enough now having users install composer and npm(plus getting permissions correct for same) to then finally run composer to get a required error. What's LDAP is their first thought!
Prior to 502 yes, the adldap libraries was included however, they were included with download and LDAP requirement was left to the user that wish to take advantage of the feature. Big difference. And the one or two issues on the forum tend to deal with how to configure the application for Active Directory and not the fact one needs to have LDAP present.
As to security. The project is consistently plugging potential and real security exploits to LDAP but like virus scanners, they're only good after the original exploit has happened to prevent future exploits. You just never know and if the temptation is not there, all the better.
Furthermore, a user that wishes to use Active Directory is going to know at install time that fact. Chances of a user suddenly deciding to use the feature and then enjoy the convenience of just going to globals to turn on the feature are remote.
Come on folks, let's be honest. The forced install of LDAP is just a convenience for the few. I made a compromise a few posts back but please, don't try to justify this as anything but that, a convenience. We should be striving to make it easier to install OpenEMR for the normal use cases and not harder.
hi @sjpadgett ,
Couple quick points/clarifications :
This is not new to 5.0.2; this feature and php-ldap dependency were in the 5.0.1 production release: https://github.com/openemr/openemr/blob/rel-501/contrib/util/ubuntu_package_scripts/production/control-php7#L11
This php-ldap dependency in 5.0.1 has not been reported as a stumbing block by user(s) for openemr install
If we remove it for 5.0.2, then it will break for those users during upgrade to 5.0.2 (btw, got no access to github, in case I need to reply, until this evening; my work proxy actually blocks github... :) )
@bradymiller 1) I pointed out in previous post that the reason this is not an issue in 501 is because we do not install adldap dependency via command line composer run. It is downloaded in the install vendor directory tree where the LDAP os requirement is fulfilled on master build.
2). Refer to 1.
3). Turn it around. So now we're willing to break it for all the other users who don't have LDAP installed. Because, that is exactly what will happen.
Again, I pursue this issue as a reminder of the little gotca's that are heading our way and we should be firmly focused on what that install and upgrade process is going to look like. We also have to take into account simplisticity for the majority.
Hopefully this gets through (email reply to github works for me on and off)
Sounds like the issue stems from confusion (either me or you :) ) between version 5.0.1 and 5.0.2 . Quick question so I can figure this this out. Will there now be differences (related to ldap) for end users when installing the standard 5.0.1 zip package and the 5.0.2 zip package at https://www.open-emr.org/wiki/index.php/OpenEMR_Downloads#Daily_snapshots (this is a daily build from the dev branch; ignore the cvs misnomer). Both these packages include the adldap2 composer package. Why would one break for users and the other not? -brady
On Tue, Apr 9, 2019 at 8:41 AM Jerry Padgett notifications@github.com wrote:
@bradymiller https://github.com/bradymiller
- I pointed out in previous post that the reason this is not an issue in 501 is because we do not install adldap dependency via command line composer run. It is downloaded in the install vendor directory tree where the LDAP os requirement is fulfilled on master build.
2). Refer to 1.
3). Turn it around. So now we're willing to break it for all the other users who don't have LDAP installed. Because, that is exactly what will happen.
Again, I pursue this issue as a reminder of the little gotca's that are heading our way and we should be firmly focused on what that install and upgrade process is going to look like. We also have to take into account simplisticity for the majority.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openemr/openemr/issues/2362#issuecomment-481306731, or mute the thread https://github.com/notifications/unsubscribe-auth/AARBuJiS-2rIy-GXRoJFzgvQt4m61QIHks5vfLTHgaJpZM4ce5GO .
It won't. I was under the impression user was building to their os on site. Guess not so this is moot.
closed due to authors senility!
Why are we forcing LDAP now, Why?