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

Authplugins and AD Improvements #141

Merged
merged 47 commits into from Feb 3, 2013
Merged

Authplugins and AD Improvements #141

merged 47 commits into from Feb 3, 2013

Conversation

splitbrain
Copy link
Collaborator

These are the merged changes of #132 #126 and #74

janschumann and others added 26 commits January 2, 2012 21:35
This changes how the AD auth backend handles multiple domains. It is now
possible to configure multiple authentication domains even when not
using SSO. USers can provide a domain in NTLM- and Kerberos-Style
(prepended with a backslash, appended with a @-char).

IMPORTANT: If you used AD auth before, you will need to adjust your ACLs
           and $conf['superuser'] settings.

This patch changes how user names are cleaned. Spaces and other special chars
are no longer removed. The only adjustment is lowercasing the username
and streamlining the domain handling.

User's login names will now contain the domain name in Kerberos style
(user@yourdomain.com) when they logged in a non-default domain. You need
to make sure your ACLs are setup accordingly.

Domain names are always lowercased and need to be specified lowercased
in the config.
* master:
  allow selects in block forms
The new version of adLDAP improves the speed in handling recursive group
memberships dramatically
* adldap4:
  make use of adLDAP 4.0.4 in AD backend
* multiad:
  lowercase groups
  make AD utilitiy functions public
  add user's domain to the list of groups
  fixed domain handling in user names
  some code cleanup/PHP5ization
  use UTF-8 aware lower casing
  Better support for multi domain AD setups

Conflicts:
	inc/auth/ad.class.php
This merge fixes all conflicts but is otherwise untested and might break
funktionality in the auth system somewhere. It NEEDS MAJOR TESTING!

Some refactoring of the auth plugins is still needed:

  * move to PHP5 style
  * fix comments
  * add plugin.info.txt

* janschumann/master:
  Refactored auth system: All auth methods are now introduced as plugins.
  Bugfix: auth types are now correcty added
  Setup auth system from plugins
  Added Auth-Plugin-Prototype to autoload
  Load auth types from plugins in settings_authtype class
  Added prototype for Auth-Plugins
  added plugin type 'auth'

Conflicts:
	inc/auth.php
	inc/auth/pgsql.class.php
	inc/init.php
	inc/load.php
	lib/plugins/auth.php
	lib/plugins/authad/auth.php
	lib/plugins/authldap/auth.php
	lib/plugins/authmysql/auth.php
	lib/plugins/authplain/auth.php
* master:
  Always load the parser in the test environment
  Escape filename in regex in search_index()
  Changed XHTML validator icon to HTML5 (FS#2619)
  fixed wrong apple-touch-icon extension (FS#2627)
  Revert "removed html dependency from media query detection JS in new template"
  removed html dependency from media query detection JS in new template
  de-couple mobile JS from widths to actual media query equivalent (fixes FS#2623)
We need to decide how to handle the renaming of the auth classes. Should
this be done automatically somehow? Or is an admin expected to fix this
manually when updating?
* master: (45 commits)
  TarLib code cleanup
  TarLib: fixed appending in non-dynamic mode
  fixed third method of adding files in TarLib
  fix lone zero block in TarLib created archives
  fix use of constructor in TarLib
  Slovak language update
  Korean language update
  Latvian language update
  removed redundant variables in tpl_include_page() (because of 3ff8773)
  added cut off points for mobile devices as parameters to style.ini
  Corrected typo: ruke -> rule
  Persian language update
  Spanish language update
  russian language update
  Kazach language update
  correctly check hash parameter in media dispatcher FS#2648
  avoid broken browser_uid on IE
  Removed acronyms for "Perl" and "PERL" as Perl is not an acronym. See http://learn.perl.org/faq/perlfaq1.html#Whats-the-difference-between-perl-and-Perl-
  Made striplangs.php executable
  release preparations
  ...
@splitbrain
Copy link
Collaborator Author

IRC Meeting decision: +1 or comments to this til Thursday the 8th of November. Merge on friday or delay for fixes

* fixes incorrect $this->adldap calls
* makes use of correct plugin config
* makes proper use of plugin config
* adds a few first defaults, but the whole config metadata is still
  missing
* proper PHP5 use and comments
* use proper plugin config
* code/PHP5 cleanup
* fixes merge messup (code was made up from parts of authad)
* code and PHP5 cleanup
* code/PHP5 cleanup
@splitbrain
Copy link
Collaborator Author

As you can see I pushed a whole bunch of cleanups, but everything else than authplain is still untested.

Configs are now pulled from their correct config locations with a fallback to the old configs if found.

We're still missing a whole bunch of config metadata (and language files) for the auth plugins. I'm also not sure how our config manager copes with multi line config strings as used in the *SQL auth plugins. Guess we need some fixes there, too.

Any help appreciated.

@splitbrain
Copy link
Collaborator Author

Multiline options should now be posible, however saving in the config manger is broken completely currently: trying to save will log you out.

This is unrelated to 23e8f02 but might be a side effect of moving the auth config to the proper plugin location. Not sure though.

@Chris--S I have no idea what causes this. Since you wrote the config manager, maybe you have an idea?

PS: @janschumann pinging you since you might want to help out here, too.

okay. I can't read. we already had a multiline config. It's the default.
So I reverted my change, except for making use of formText/cleanText for
proper line ending handling and I added made focused textareas a bit
larger.
Saving worked, but did you log out everytime. Now it is checked if the
auth mechanism was actually changed before assuming the login is
invalid.
@splitbrain
Copy link
Collaborator Author

okay, saving is fixed.

@janschumann
Copy link
Contributor

Hi! Sure. But I am quite bussy currently. Probably next week? Woud that be sufficient?

Am 10.11.2012 um 16:16 schrieb Andreas Gohr notifications@github.com:

Multiline options should now be posible, however saving in the config manger is broken completely currently: trying to save will log you out.

This is unrelated to 23e8f02 but might be a side effect of moving the auth config to the proper plugin location. Not sure though.

@Chris--S I have no idea what causes this. Since you wrote the config manager, maybe you have an idea?

PS: @janschumann pinging you since you might want to help out here, too.


Reply to this email directly or view it on GitHub.

* confmanager:
  added failing test for array type
  started to add some unit tests to config manager
  Revert "config manager: let PHP parse the config file"
  added 'array' type for config manager
  config manager: let PHP parse the config file
  config manager: removed dead/commented code
  added PCRE UTF-8 checks to do=check FS#2636
  avoid multiple paralell update checks
  fix regression bug in HTTPClient FS#2621
  changed PAGEUTILS_ID_HIDEPAGE to has BEFORE/AFTER
  added event PAGEUTILS_ID_HIDEPAGE
  added test for isHiddenPage()
  add index file similar to fileicons to show active smileys
  fix E_STRICT errors FS#2427
* confmanager:
  fixed problems with spaced arrays
  parse arrays from config file
@splitbrain
Copy link
Collaborator Author

I now added config meta data and descriptions for them to all the new auth plugins. But have two questions:

  • Should these options be translated? I'd say yes for consistency. But in reality nobody will be able to set these options up without reading some documentation anyway.
  • Should we deliver the auth plugins (except plain) disabled by default? I think it makes no sense to clutter the config interface with a bunch of options that aren't used anyway

@michitux
Copy link
Collaborator

michitux commented Feb 2, 2013

How is the transition going to work? I.e. if you had used a different auth backend till now, what will happen? I saw that there is something to pick up the old configuration option but does that work for the auth backend itself, too? I.e. could there be something that automatically enables the right auth plugin when you upgrade? I imagine it is kind of a bad user experience when you upgrade and suddenly you can't login anymore as you don't have any working authentication backend.

Concerning translations: why not? I think we have other options that you can't really use without reading some documentation and we still translate them.

@splitbrain
Copy link
Collaborator Author

The old config is picked up, your conf['auth'] setting needs to be updated manually (eg. from 'mysql' to 'authmysql'). If we disable the auth plugins by default, you need to enable the right plugin as well.

I would assume someone who does not use the plain backend has some minimal administrator skills to do this.

@splitbrain splitbrain merged commit 6cf2bbf into master Feb 3, 2013
@splitbrain splitbrain deleted the future branch February 3, 2013 21:00
michitux added a commit that referenced this pull request Feb 20, 2013
In the case of a failed authentication initialization, the
authentication setup was simply continued with an unset $auth object.
This restores the previous behavior (before merging #141) of simply
returning after unsetting $auth. Furthermore this re-introduces the
check if $auth is set before checking $auth and removes a useless
check if $auth is true (could never be false).
michitux added a commit that referenced this pull request Feb 24, 2013
In the case of a failed authentication initialization, the
authentication setup was simply continued with an unset $auth object.
This restores the previous behavior (before merging #141) of simply
returning after unsetting $auth. Furthermore this re-introduces the
check if $auth is set before checking $auth and removes a useless
check if $auth is true (could never be false).
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

6 participants