View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
811 | RackTables | default | public | 2013-03-27 13:33 | 2013-04-15 10:51 |
Reporter | mvdschoot | Assigned To | infrastation | ||
Priority | normal | Severity | feature | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Target Version | 0.20.4 | Fixed in Version | 0.20.4 | ||
Summary | 811: Please add SAML support for authentication | ||||
Description | Besides the current build in authentication LDAP, DATABASE and HTTP it would be more then welcome to add SAML support for Racktables. Using the simplesaml API, it's just a few lines. | ||||
Steps To Reproduce | Change inc/auth.php: Search for switch ($user_auth_src) and add the following lines just before 'default:' -> case 'saml': require_once('../simplesaml/lib/_autoload.php'); $as = new SimpleSAML_Auth_Simple('default-sp'); if (!$as->isAuthenticated()) { $as->requireAuth(); } $attributes = $as->getAttributes(); $remote_username = $attributes['eduPersonPrincipalName'][0]; break; Search for switch (TRUE) and add the following lines just before 'default:' -> case ('saml' == $user_auth_src): $remote_displayname = strlen ($userinfo['user_realname']) ? $userinfo['user_realname'] : $remote_username; return; And you're ready to go! | ||||
Additional Information | To enable SAML support, just change "$user_auth_src = 'saml'" in secret.php. We assume the following: - a valid simplesaml SP installation is available, located in ../simplesaml - the attribute 'eduPersonPrincipalName' contains the uid Those values could be made configurable. | ||||
Tags | No tags attached. | ||||
Would you like to prepare a patch for SAML in RackTables, update the documentation and maintain this feature? | |
diff file for auth.php has been attached to case. Things to do: configuration has now to be done hardcoded in auth.php; Need to make some changes where configuration will be configurable from inside e.g. secret.php. We assume simplesaml is installed in ../simplesaml; We assume service provider default-sp has to be used; We assume SAML attribute eduPersonPrincipalName is used for $remote_username attribute We assume SAML attribute fullname is used for $remote_displayname |
|
Could you add the parameters below to secret.php (through install.php) like below and update the code to use these? Also the ChangeLog and the release notes should be updated. $saml_dir = '../simplesaml'; $saml_sp = 'default-sp'; $saml_remusername = 'eduPersonPrincipalName'; $saml_remdispname = 'fullname'; Also please mind the code style: if (! file_exists ('../simplesaml/lib/_autoload.php')) throw new RackTablesError ('Configured for SAML authentication, but simplesaml is not found.', RackTablesError::MISCONFIGURED); require_once ('../simplesaml/lib/_autoload.php'); $as = new SimpleSAML_Auth_Simple ('default-sp'); if (! $as->isAuthenticated()) $as->requireAuth(); $attributes = $as->getAttributes(); $remote_username = $attributes['eduPersonPrincipalName'][0]; I can accept this through a GitHub pull request, if you would like. |
|
Removed original auth.diff and recreated a new diff file. Updated with new code style and rewrote code to make it configurable from inside secret.php; Based on how LDAP is configured, to configure SAML you can add the following to secret.php (With your permission): $SAML_options = array ( 'simplesamlphp_basedir' => '../simplesaml', 'sp_profile' => 'default-sp', 'usernameAttribute' => 'eduPersonPrincipName', 'fullnameAttribute' => 'fullName', ); To activate, change $user_auth_src = 'saml'; in secret.php. Update: code has been implemented in install.php. See patch. |
|
This is better, although the very first case block is taken wrong and file_exists() still tests the hardcoded path. $SAML_options is a good idea. | |
auth-and-install.diff (3,935 bytes)
diff -Naur org//ChangeLog new//ChangeLog --- org//ChangeLog 2013-04-04 13:29:21.028703623 +0200 +++ new//ChangeLog 2013-04-04 13:30:18.601660237 +0200 @@ -33,6 +33,7 @@ new feature: configurable top-to-bottom units order in particular racks (#601) new feature: overridable tag names display style (via CSS and plug-ins) + new feature: SAML is now supported, using SimpleSAMLphp API (ticket:811) 0.20.3 2012-12-19 bugfix: DB exception on ports linking (#699) 0.20.2 2012-12-19 diff -Naur org//wwwroot/inc/auth.php new//wwwroot/inc/auth.php --- org//wwwroot/inc/auth.php 2013-04-04 11:41:34.620994648 +0200 +++ new//wwwroot/inc/auth.php 2013-04-04 13:58:10.877447038 +0200 @@ -54,6 +54,13 @@ throw new RackTablesError ('The web-server didn\'t authenticate the user, although ought to do.', RackTablesError::MISCONFIGURED); $remote_username = $_SERVER['REMOTE_USER']; break; + case 'saml': + $saml_username = ''; + $saml_success = authenticated_via_saml ( $saml_username ); + if (!$saml_success) + break; //failure + $remote_username = $saml_username; + break; default: throw new RackTablesError ('Invalid authentication source!', RackTablesError::MISCONFIGURED); die; @@ -90,6 +97,16 @@ $userinfo['user_realname'] : (strlen ($ldap_dispname) ? $ldap_dispname : $remote_username); // then one from LDAP return; // success + case ('saml' == $user_auth_src): + $saml_username = ''; + $saml_dispname = ''; + $saml_success = authenticated_via_saml ( $saml_username, $saml_dispname); + if (!$saml_success) + break; //failure + $remote_displayname = strlen($saml_dispname) ? + $saml_dispname : + $saml_username; + return; // success default: throw new RackTablesError ('Invalid authentication source!', RackTablesError::MISCONFIGURED); } @@ -244,6 +261,35 @@ return $didChanges; } +// a wrapper for SAML auth method +function authenticated_via_saml (&$saml_username = NULL, &$saml_displayname = NULL ) +{ + global $SAML_options, $debug_mode; + if (! file_exists ($SAML_options['simplesamlphp_basedir'] . '/lib/_autoload.php')) + throw new RackTablesError ('Configured for SAML authentication, but simplesaml is not found.', RackTablesError::MISCONFIGURED); + require_once($SAML_options['simplesamlphp_basedir'] . '/lib/_autoload.php'); + $as = new SimpleSAML_Auth_Simple($SAML_options['sp_profile']); + if (!$as->isAuthenticated()) + $as->requireAuth(); + $attributes = $as->getAttributes(); + $saml_username = saml_getAttributeValue($attributes, $SAML_options['usernameAttribute']); + $saml_displayname = saml_getAttributeValue($attributes, $SAML_options['fullnameAttribute']); + if ($as->isAuthenticated()) + return true; + return false; +} + +function saml_getAttributeValue($attributes, $name) { + if (isset($attributes[$name])) { + if (is_array($attributes[$name])) + return $attributes[$name][0]; + else + return $attributes[$name]; + } + return ''; +} + + // a wrapper for two LDAP auth methods below function authenticated_via_ldap ($username, $password, &$ldap_displayname) { diff -Naur org//wwwroot/inc/install.php new//wwwroot/inc/install.php --- org//wwwroot/inc/install.php 2013-04-04 13:10:04.873511050 +0200 +++ new//wwwroot/inc/install.php 2013-04-04 13:10:42.850141319 +0200 @@ -275,6 +275,17 @@ # 'use_tls' => 2, // 0 == don't attempt, 1 == attempt, 2 == require #); +# For SAML configuration details: +# http://wiki.racktables.org/index.php?title=SAML + +#\$SAML_options = array +#( +# 'simplesamlphp_basedir' => '../simplesaml', +# 'sp_profile' => 'default-sp', +# 'usernameAttribute' => 'eduPersonPrincipName', +# 'fullnameAttribute' => 'fullName', +#); + # This HTML banner is intended to assist users in dispatching their issues # to the local tech support service. Its text (in its verbatim form) will # be appended to assorted error messages visible in user's browser (including |
|
Fixed file_exists check. Changed some code in the first case block. Problem occurs due to the fact the first case block assumes there allready is a valid username available (Otherwise $userinfo = constructUserCell ($remote_username); will fail). Therefore full authorization already has to be done in the first case block. Now, in the first case block $remote_username is resolved. In the second case block $remote_displayname is additional requested. |
|
Thank you. I will review more thoroughly later today and let you know. | |
Well... It is almost complete. The authenticate() function implements the following generic steps: 1. Assert basic configuration variables (not method-specific). 2. Initialize $remote_username from the method-specific source, possibly involving password verification. 3. Satisfy the $require_local_account condition, initialize $user_given_tags and merge user-specific autotags (not method-specific). 4. Initialize $remote_displayname from method-specific source, possibly involving password verification if not done above. This approach was introduced long time ago to justify the three currently implemented authentication methods and it can also work for SAML. The first case block should be the only calling authenticated_via_saml() to set $remote_username immediately and keep the value of the displayed name. The second case block should set $remote_displayname. This may look complicated, but the code should be as uniform as possible to ease future auditing of this core function. |
|
I can make the remaining fixes myself. | |
Your code and the associated fixes are now in the master branch, please test. | |
I close the issue as fixed, please make any future fixes/improvements against the released tar.gz source code or the git master branch. Thank you for the contribution! | |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-03-27 13:33 | mvdschoot | New Issue | |
2013-03-29 11:44 | infrastation | Note Added: 0001273 | |
2013-04-04 12:04 | mvdschoot | File Added: auth.diff | |
2013-04-04 12:08 | mvdschoot | Note Added: 0001289 | |
2013-04-04 12:23 | infrastation | Note Added: 0001291 | |
2013-04-04 12:24 | infrastation | Assigned To | => infrastation |
2013-04-04 12:24 | infrastation | Status | new => assigned |
2013-04-04 12:52 | mvdschoot | File Deleted: auth.diff | |
2013-04-04 13:00 | mvdschoot | Note Added: 0001293 | |
2013-04-04 13:00 | mvdschoot | File Added: auth.diff | |
2013-04-04 13:12 | mvdschoot | File Added: auth-and-install.diff | |
2013-04-04 13:12 | mvdschoot | File Deleted: auth.diff | |
2013-04-04 13:12 | mvdschoot | Note Edited: 0001293 | |
2013-04-04 13:34 | infrastation | Note Added: 0001295 | |
2013-04-04 13:59 | mvdschoot | File Deleted: auth-and-install.diff | |
2013-04-04 13:59 | mvdschoot | File Added: auth-and-install.diff | |
2013-04-04 14:03 | mvdschoot | Note Added: 0001297 | |
2013-04-04 14:04 | mvdschoot | File Added: saml.wiki | |
2013-04-04 16:24 | infrastation | Note Added: 0001299 | |
2013-04-04 22:11 | infrastation | Note Added: 0001301 | |
2013-04-05 11:10 | infrastation | Note Added: 0001305 | |
2013-04-07 15:21 | infrastation | Note Added: 0001319 | |
2013-04-07 15:21 | infrastation | Status | assigned => feedback |
2013-04-09 20:24 | infrastation | Target Version | => 0.20.4 |
2013-04-15 10:51 | infrastation | Note Added: 0001323 | |
2013-04-15 10:51 | infrastation | Status | feedback => closed |
2013-04-15 10:51 | infrastation | Resolution | open => fixed |
2013-04-15 10:51 | infrastation | Fixed in Version | => 0.20.4 |