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

FreeIPA Auth Adaptor for LibreTime #136

Merged
merged 3 commits into from
Apr 3, 2017

Conversation

hairmare
Copy link
Member

Allow delegating user authentication to FreeIPA rather than having it be checked against the database.

With this feature I've ported everything I need to switch to LibreTime in my infrastructure. I did a ubuntu re-install to check if I don't break anything and have also been testing this on CentOS against a real FreeIPA server.

I've already started cleaning up the v3.0.0.alpha.1 Milestone and plan on doing another release soonish (ie. over the next couple of days unless lots of new bugs show up).

Allow delegating user authentication to FreeIPA rather than having it be checked against the database.
Since conf assumes the ldap stuff to exist it also needs to be in the tests :(
@Robbt
Copy link
Member

Robbt commented Mar 31, 2017

Ok, so I tested this out and I got a WSOD because there was a new ldap index required in airtime.conf

The installation process will create this but I think we might want to figure out a way to automatically add this via an upgrade script so that when people upgrade to a new alpha they don't end up with broken installs.

It probably isn't a big deal at this point but I think it might become an issue when we have more users so we might as well think about it now and see if there are any easy solutions that come to mind.

Here's the error - Uncaught exception 'ErrorException' with message 'Undefined index: ldap' in /vagrant/airtime_mvc/application/configs/conf.php:101\nStack trace:\n#0 /vagrant/airtime_mvc/application/configs/conf.php(101):

@hairmare
Copy link
Member Author

Yeah, I know the error (and the point you raise is valid). I'm not quite sure on how to proceed with new such values in such cases. This is the same issue we were facing after I refactored the soundcloud stuff. Once we reach 3.0.0 this kind of changes should either be automated or warrant a 4.x release as per SemVer.

@Robbt
Copy link
Member

Robbt commented Apr 2, 2017

Well I can commit this if we don't have an easy fix for the modified configuration. I'm sure we could hack something together but I'm trying to figure out what the best practices in this regard are.

This is a workaround to make updating easier for folks who do not re-install. A proper solution would get rid of most of the Config class and use something based on Zend_Config_Ini instead. It would also have some sensible defaults in the code and nor error when new values get added.
@hairmare
Copy link
Member Author

hairmare commented Apr 3, 2017

IMHO the best practices would be to use Zend_Config_Ini and not have a Config class that looks like it's mostly a case of the "not-invented-here" antipattern.

I added a check for the ldap section to the code only loads it if it's there. While it's not the nicest solution it does make updating without changing the config not lead to a WSOD.

@Robbt Robbt merged commit 8f372f5 into libretime:master Apr 3, 2017
@Robbt
Copy link
Member

Robbt commented Apr 3, 2017

Sounds good. It does seem like it would make sense to switch over to Zend_Config_Ini, but not sure how high a priority it is at this point.

@hairmare hairmare deleted the feature/freeipa-auth branch April 4, 2017 00:53
@hairmare
Copy link
Member Author

hairmare commented Apr 4, 2017

Shouldn't be that much of a prio considering we also need so switch whats happening in python...

@lock
Copy link

lock bot commented Oct 20, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please chat to us on discourse or ask for help on our chat if you have any questions or need further assistance with this issue.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants