search

nickw444 / flask-ldap3-login

LDAP3 Logins for Flask/Flask-Login
http://flask-ldap3-login.readthedocs.org/en/latest/
MIT License
73 stars 38 forks source link

Adding Mock LDAP Functionality for improved testing #103

Closed andrewjroth closed 2 years ago

andrewjroth commented 2 years ago

Hello,

As described in #28, I've added an option LDAP_MOCK_DATA that, when not None (default), will implement a Mock connection, instead of a real connection. This "mocking" is described in detail in the Python ldap3 package page on Mocking.

Minimal changes were made in the package code to setup the connection using a different strategy. Due to a problem with how the connection is setup with a ServerPool (see cannatag/ldap3#1007) a fake Server object is also created and used for the connection, ignoring the established ServerPool object for the extension.

Additional changes were needed in order to setup unit tests, including dumping the test "directory" to a JSON file to be used to load entries into the mock server DIT. There is also a difference between the schema used by the test directory information and what is used and returned by ldap3 during mocking, so a slight modification to the test directory was made. For a cleaner or more long term solution, the structure of the test directory might be modified to match the ldap3 mock implementation or the schema on the mock ldap3 implementation could be better modeled. In theory, all tests could start using the ldap3 mock instead of the existing mock.

I originally intended to use the Flask app instance directory for the relative import of the mock data, but found that adding this relative import required too much additional overhead. I believe that the import is relative to the current working directory of the Python thread, but unsure if this is always the case. Users of flask-ldap3-login should specify LDAP_MOCK_DATA using an absolute path or may want to do some trial and error to determine how to load data.

Thank you for considering this PR!!! I'm open to further modifications for improvements on this feature.

andrewjroth commented 2 years ago

Submitted cannatag/ldap3#1009 to be able to correctly address the Flask app instance directory relative import of the mock data, potentially for the future.

andrewjroth commented 2 years ago

There is no need to wait for the PR for ldap3. I have not submitted a PR to ldap3 that addresses cannatag/ldap3#1007 since it is too complex an issue for me to sort out. This PR works with what is currently published for ldap3. The PR I submitted for ldap3 (cannatag/ldap3#1009) would support Flask app instance directory relative import of mock data, which would require additional code if/when it is accepted. It would be a new future improvement.

andrewjroth commented 2 years ago

Sorry to ask, but I'm not sure what the procedure is. With multiple approvals, should this PR be merged? Or tagged to be included in a future release? Not trying to rush, but just curious what the timeline or next steps are.

Thank you!

nickw444 commented 2 years ago

@gmacon does this one need a release published (backport to 0.x)? Or will it be bundled into v1?

gmacon commented 2 years ago

I think it's probably OK to leave this until v1.