Skip to content
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.

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

YOURLS version 1.8.1 functionality #28

Closed
wants to merge 1 commit into from
Closed

Conversation

mattv8
Copy link
Owner

@mattv8 mattv8 commented May 24, 2021

Updated so yourls-ldap-plugin now functions with latest YOURLS version (1.8.1 as of May 24, 2021).

  • LDAPAUTH_ADD_NEW' flag working again with latest YOURLS config, so users can be cached in the config.

Updated so this plugin functions with latest YOURLS version
@k3a
Copy link
Collaborator

k3a commented May 27, 2021

Thanks for the contribution! Are you sure it will be backward-compatible with older YOURLS? Especially renaming is_valid_user to shunt_is_valid_user. I don't use YOURLS actively anymore so it would be difficult for me to properly test.

@mattv8
Copy link
Owner Author

mattv8 commented Jun 14, 2021

Hi k3a, apologies for the delayed response. I can't speak for certain that this is backward-compatible. Would it be ideal to just branch my PR? In my opinion, branching would be the path of least resistance. If backwards compatibility is important enough to you, I can dig deeper and run some tests with older versions, however this will take time.

We might be able to get backwards compatibility with some version-logic. Something like if (version < 1.8.1) use is_valid_user; else shunt_is_valid_user might do the trick.

From my personal testing of is_valid... to shunt_is_valid... I could not get is_valid_user to properly function with the latest version of YourLS and documentation on API filters is sparse. I'm not overly familiar with YourLS API, so it could just be due to a limitation of my understanding of the API. Am I missing some better documentation somewhere?

Let me know what you think the best course of action is.

@IMIjrebassa
Copy link

Hello, I'have installed yourls 1.9.2 and your ldap plugin, I'have modified the ldap plugin because I could not search in ldap, now I can bind to ldap and search in ldap, but now appears a new error when I login with a new user : Couldn't add user, plugin may not be compatible with YourLS version. When you will upload a new ldap plugin version compatible with Yourls last version. Thanks.

@IMIjrebassa
Copy link

Hello, I'have could login in Yourls with ldap user setting LDAPAUTH_NEW_USER to false and LDAPAUTH_ALL_USERS_ADMIN to true. Now my doubt is: only can login admin users in Yourls?

@IMIjrebassa
Copy link

don't worry I got it working

@mattv8
Copy link
Owner Author

mattv8 commented Jul 19, 2023

Hey glad you got it working. What was the fix?

@IMIjrebassa
Copy link

IMIjrebassa commented Jul 19, 2023

Hello, finally the change that I made in plugin.php is not necessary, putting the corrects values for alls the define( 'LDAPAUTH_xxxxx in config.php it works. Basically at the begining the plugin failed because I set LDAPAUTH_NEW_USER to true and when the plugin tries to modify the config.php file to add the ldap users to this file the plugin fails, but I don't need that ldap users be added to the config.php.

@k3a k3a mentioned this pull request Dec 19, 2023
@mattv8 mattv8 closed this Apr 24, 2024
mattv8 added a commit that referenced this pull request May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants