pitbulk / moodle_saml Goto Github PK

In index.php we do not use Moodle-sessions since this fucks Simplesamls session handling up. However, since we're not using sessions we loose the info about where the user should be redirected after the login has been completed, since this is part of $SESSION->wantsurl.

A solution is to change session-store for simplesamlphp, so it uses either memcache or sql - but it would be nice if we could figure a way to redirect users to the correct place, without requiring users to setup simplesaml.

I've tried different approaches, but nothing has worked out so far.

Hi,

Would like to know does the current version 2013110701 support Moodle 2.6?

Thanks

Theoretically we could do this with the composer installer, however this package provides both an auth and enrol plugin. Perhaps we could whip up a quick custom installer?

When saving settings in the auth/saml module, users will get a warning with a permission denied when trying to write the config to the saml_config.json file in the plugin-root. Instead this file should be saved in Moodledata, allowing the webserver to write to the file. Furthermore this error will make the plugin fail on clustered setup, since the file has to be saved on each frontend, hence the placement of the config file.

Right now, only absolute paths will work for the setting samlhookfile. That is because when the setting is used it's used from moodle/auth/saml directory, but when the input is validated, it's validated from moodle/admin. So, my value of "../../local/myplugin/auth_hooks.php" is valid and will work, but fails validation on the settings form.

I placed my hook file in a custom local plugin. The fact that I have to use an absolute path ties my hands in my development and staging environments. I have to install Moodle in the exact same place.

To fix this, maybe the plugin could assume that if the path does not begin with a "/" (or "" on Windows?) then the path is relative to dirroot.

Or it could be more explicit and use a token. Like a valid value could be "[dirroot]/local/myplugin/auth_hooks.php" and that's expanded to $CFG->dirroot . '/local/myplugin/auth_hooks.php' for use and for validation.

What do you think?

When auto login is enabled, is there a way to bypass this with a known manual username? For example, with the CAS plugin, you can append the url as such: https:///login/index.php?username=SomeManualUser and bypass the CAS redirect.

Having the auth_saml and enrol_saml plugins in separate git repositories would allow installing them by cloning the git repository directly either to auth/ or enrol/ directory. This would make installing and testing much easier from developer's point of view.

See http://blogs.atlassian.com/2014/04/tear-apart-repository-git-way/

I have simplesamlphp installed and have configured the moodle saml module to look at it in moodle 2.5.
When I hit the login button I see 'webpage could not be found'

in the apache logs I see

script '/var/www/moodle25/simplesamlphp/module.php' not found or unable to stat, referer: https://sso.midkent.ac.uk/adfs/ls/?SAMLRequest= - blah blah key here - Fmoodledev.midkent.ac.uk%2Fauth%2Fsaml%2Findex.php

So it looks like my request is getting to the ADFS server, authenticating and a token is being sent back. I can confirm this be changing the settings and making it error on the ADFS server.

any ideas please?

With so many people (apparently) depending on this module, and there being a few PR's and substantial issues pending, we should probably add a CI test process to the project.

Travis CI seems to be the most obvious choice. I don't have time to work on this just this second, so putting up this issue to see if there are any strong thoughts on another platform (e.g., CircleCI) before moving forward with Travis.

@pitbulk I'd probably need you to set up webhooks in the project settings when we make a move.

Using auth/saml version 2013110701 on latest Moodle 2.7 install, I get:

Strict standards: Declaration of auth_plugin_saml::validate_form() 
should be compatible with auth_plugin_base::validate_form($form, &$err) in 
/var/www/html/moodle27/auth/saml/auth.php on line 623

https://github.com/pitbulk/moodle_saml/blob/master/auth/saml/auth.php#L191
Removing "&" from "&$form" solves it. Which seems not to cause any functionality regression. Can you validate this fix?

We have received a request from Moodle to upgrade the plugin to support their guidelines, I'm opening several issues here to track getting these adjusted:

3.1.2 Use the proper Moodle global variables.
• Description: Keeps with Moodle coding guidelines.
• Example(s):
o File: auth.php, index.php
o auth.php 37:
require_once($GLOBALS['CFG']-

libdir.'/authlib.php'); 39:
require_once($GLOBALS['CFG']->
libdir.'\authlib.php'); 69: if
(isset($GLOBALS['onelogin_saml_login_attributes'])) {
83: $saml_attributes =$GLOBALS['onelogin_saml_login_attributes'];
84: $nameID =$GLOBALS['onelogin_saml_nameID'];
153:$saml_attributes = $GLOBALS['onelogin_saml_login_attributes'];
235:$GLOBALS['CFG']->nolastloggedin = true;
238:$init_sso_url = $GLOBALS['CFG']-
wwwroot.'/auth/onelogin_saml/index.php';
246: $logout_url = $GLOBALS['CFG']-
wwwroot.'/auth/onelogin_saml/index.php?logo
ut=1'; index.php 134:
$GLOBALS['onelogin_saml_nameID'] =
$onelogin_saml_nameId = $auth->getNameId();
135:
$GLOBALS['onelogin_saml_login_attributes'] =
$saml_attributes = $auth->getAttributes();
158: $GLOBALS['onelogin_saml_login'] = TRUE;

• Suggested Fix: See REF: Core global variables in Moodle are identified
using uppercase variables (ie $CFG, $SESSION, $USER, $COURSE,
$SITE, $PAGE, $DB and $THEME).
• Reason for Fix: Coding guidelines
(https://docs.moodle.org/dev/Coding_style)

To test this:

1. Make sure you have a good saml_config.php in dataroot
2. Edit saml_config.php and change the sp source to "foobar"
3. Try to log in

You will see the error:
Invalid authentication source: foobar

The expected behavior is that /auth/saml/saml_config.php should be ignored if /saml_config.php exists.

I think this is because index.php line 13 is accessing $CFG->dataroot, but config.php has not been included. Adding "require_once('../../config.php');" to the top of the file should fix it.

Add a license?

Use plugin related prefixes on defines.
• Description: All defined values should have plugin name related prefixes.
• Example(s):
o File: index.php
o 36: define(‘SAML_INTERNAL’, 1);
37: define(‘SAML_RETIRES’, 10);
• Suggested Fix: Append a ONELOGIN_SAML_ prefix, etc.
• Reason for Fix: Prevent namespace collision in codebase..

There is a new api hook in moodle to avoid 2 extra redirects in the whole saml auth flow which can make login dramatically quicker.

See this moodle tracker:
https://tracker.moodle.org/browse/MDL-48887

API docs:
https://docs.moodle.org/dev/Authentication_plugins#pre_loginpage_hook.28.29

A working example (from a forked version of the saml auth plugin):
brendanheywood/moodle-auth_saml@b6b11b9

Thanks for writing this helpful module! There are a few outstanding PR's and issues; would you be open to adding additional maintainers to help work through the backlog? (Yes, I'm volunteering.)

Thanks!

Using auth/saml version 2013110701 on latest Moodle 2.7 install, I get:

Strict standards: Declaration of auth_plugin_saml::config_form() should be 
compatible with auth_plugin_base::config_form($config, $err, $user_fields) in 
/var/www/html/moodle27/auth/saml/auth.php on line 623

https://github.com/pitbulk/moodle_saml/blob/master/auth/saml/auth.php#L157
Removing "&" from "&$err" solves it. Which seems not to cause any functionality regression. Can you validate this fix?

Remove the end PHP tags.
• Description: The end PHP tags are left off files as specified in the Moodle
coding style guide. This helps prevent the accidental sending of headers
when whitespace gets set after the end flag.
• Example(s):
o File: Numerous files in the code.
• Suggested Fix: Remove the ?> php flag.
• Reason for Fix: Moodle Core style and prevention of premature header
sending.

Using Moodle 2.9 got the following error:

The enrolment plugin 'saml' should override the function can_hide_show_instance().

• line 1571 of /lib/enrollib.php: call to debugging()
• line 249 of /enrol/instances.php: call to enrol_plugin->can_hide_show_instance()

When upgrading moodle to 3.0, the following error is returned:
Plugin "auth_saml" is defective or outdated, can not continue, sorry.

More information about this error
Debug info: Missing $plugin->component declaration in version.php.
Error code: detectedbrokenplugin
Stack trace:

line 459 of \lib\upgradelib.php: plugin_defective_exception thrown
line 1647 of \lib\upgradelib.php: call to upgrade_plugins()
line 677 of \admin\index.php: call to upgrade_noncore()

Hello,

We are in the process of setting up SAML integration with Shibboleth for our Moodle site and have installed the SAML plugin version 2015072901 on our development server (Moodle 2.8.6+ Build 20150521).
Our set up has one group of users authenticating with SAML and another population using manual accounts.

During testing we have noticed that if a user enters a wrong username or password for the manual login, there is no error message to inform the user what happened. (e.g. "Invalid username or password.") The page just reloads and presents the login page as though nothing happened.

Inspection of the login.php and index.php pages in the SAML plugin has revealed that there is no code handling display of error messages to the user if their log in fails.

Is there any chance this could be an easy and quick fix?
We need to deploy this solution the first week of August.

Thanks for your attention and help.

Best Regards,
Sarah Ashley.
Instructional Designer
Office of Instructional Technology and eLearning Solutions (ITeS)
RUTGERS/OIT
RSDM Bldg, Suite B725
110 Bergen St., Newark, NJ
Office: 973-972-8238
Cell: 973-289-7415
Email: [email protected]
http://ites.rutgers.edu

should include something like this to remove the SAML Token process for logout, unfortunately adding this to the logout_hook won't work as the cookies recreate themselves as the user is not logged out yet.

unset($_COOKIE['SimpleSAMLAuthToken']);
setcookie('SimpleSAMLAuthToken', null, -1, '/');

To reproduce, turn on error display and visit auth/saml/login.php.

2 Notices appear:

Notice: Undefined variable: show_instructions in /vagrant/auth/saml/login.php on line 41
Notice: Undefined variable: show_instructions in /vagrant/auth/saml/login.php on line 136

The login page in login.php appears to reproduce the older 2.x login form. But this was revised in Moodle 2.6 and the "loginusing" string is no longer defined in the language files. So you end up with a form with "[[loginusing]]" above the username and password fields when this plugin is enabled with versions 2.6 or above.

This is pretty much the same as issue #30
but the fix there does not cater the case that the datadir contains "." (dot)

line 14 of index.php should change to
if (preg_match("/[$]CFG->dataroot[\s][=][\s]'([.\w/-_]*)'/i", $config_content, $matches)) {

(note that I added .)

moodle_saml/auth/saml/login.php line 3 is loading config from /var/www/onelogindemo/moodle29/config.php

• Make a note on the repo with pointer to new organization
• Move remaining PRs and issues to appropriate new repos
• Turn off PR and Issue features on this repo
• Redirect moodle.org plugin entries to new repos
• Tag releases on new repos

Assigning to @pitbulk since he has admin privileges on this repo. I've been working on moving the issues and PRs.

Anything else?

As per IDP documentation, steps 7 shows how the metadata should be configured for instance in the metadata/saml20-sp-remote.php to add the SP details, like

<?php
$metadata['https://sp.example.org/simplesaml/module.php/saml/sp/metadata.php/default-sp'] = array(
    'AssertionConsumerService' => 'https://sp.example.org/simplesaml/module.php/saml/sp/saml2-acs.php/default-sp',
    'SingleLogoutService'      => 'https://sp.example.org/simplesaml/module.php/saml/sp/saml2-logout.php/default-sp',
);

My understanding is that this module wraps around the simple saml library and should handle the Assertions and Logouts in some sort of internal url?

maybe something like http://myMoodleDomain.com/auth/saml/index.php/something ?

Could you suggest how the metadata should be added here?

So far I get redirected by moodle to the IDP and after the login is successful in the IDP I end up in a page that moodle does not know/incorrect (404).

When you turn on error display, there are warning s that $frm is not defined:

Notice: Trying to get property of non-object in /vagrant/auth/saml/login.php on line 101
Notice: Trying to get property of non-object in /vagrant/auth/saml/login.php on line 113
Notice: Undefined variable: frm in /vagrant/auth/saml/login.php on line 112

I am running SimpleSAMLphp and pulling attributes from a Drupal instance. The attributes are being pulled out of Drupal fine because they appear in the SimpleSAMLphp authentication test page just fine.

The user is authenticated and the Firstname and Lastname attributes ("givenName" and "sn") are even being used, however the email address ("mail") and other details such as the Organisational Unit ("ou") are not being mapped into the Moodle user profile.

auth_saml_moodle25_2013110701

Moodle version 2013111801.06
Moodle release 2.6.1+ (Build: 20140207)

SimpleSAMLphp 1.11.0

I have set all of the attributes to be updated on login and this has not helped.

At this momment this parameter hardcoded as the userStatus urn.

Let's do it more flexible.

We have received a request from Moodle to upgrade the plugin to support their guidelines, I'm opening several issues here to track getting these adjusted:

Major Issues
Below is a list of major code issues and concerns that prevent the plug-in from passing the
Code Certification Process.
3.1 CODE CHANGE ISSUES:
3.1.1 Unprefixed functions included.
• Description: Functions in use in the Moodle system should have either a
namespace, or prefixes that prevent fatal namespace collisions.
• Example(s):
o File: functions.php
o function deleteLocalSession() {
function logoutpage_hook() {
function get_saml_settings() {
• Suggested Fix: Append auth_onelogin_saml_prefix (to match the other
functions in that file) and change all instances of the functions elsewhere
then retest the plugin.
• Reason for Fix: Namespace collisions

On the configuration page for the plugin, the Reset values to factory settings option doesn't work. You just get a message saying "Error reseting the saml plugin values".

I get this warning when trying to log out:

Class 'textlib' has been renamed for the autoloader and is now deprecated. Please use 'core_text' instead.

The reference is on line 131 of /auth/saml/index.php

Hi pitbulk, thanks for creating this plugin.

I am trying to get your plugin to work however I am having issues with the logging before I ask you questions.

Moodle > Site Administration > Advanced Features > Plugins > Authentication > SAML Authentication

Log file path: /opt/moodle/auth/saml/moodle_saml_auth.log

touch /opt/moodle/auth/saml/moodle_saml_auth.log && chown -v apache.apache /opt/moodle/auth/saml/moodle_saml_auth.log

service httpd restart

Nothing is writing to this logfile I have also checked /var/log/messages for information, any ideas?

Thanks