Comments

• 

  Dan Marsden

  Tue, 27 Sep 2016, 4:55 AM

  we already have a lot of different saml plugins within the plugins repo - what features/changes have you made here compared with the other plugins? - it looks like you have copied a bit from the auth_saml2 plugin - is there any particular reason you didn't try to provide the plugin developers of auth_saml2 with the improvements and created your own version?
  
  I'm not convinced (at least by your description here) that we need another saml plugin to confuse people even further - can you please update the description and make it really clear how your version differs from the existing plugins that are available? - or would you consider providing a patch to one of the other plugin developers to make the features you have developed available?
• 

  Plugins bot

  Tue, 27 Sep 2016, 2:53 PM

  Prechecker results: 112 errors/17 warnings
  https://integration.moodle.org/job/Precheck%20remote%20branch/27630/artifact/work/smurf.html
• 

  Daniel Miranda

  Tue, 27 Sep 2016, 7:58 PM

  I'll try to explain one by one.
  Firstly, in case of auth_saml (https://moodle.org/plugins/auth_saml), it is a obsolete plugin, code is a little bit confuse and in my opnion the maintainer went beyond the purpose of a authentication plugin. It has a mix of a auth and enrol plugin. Mine is only for authentication in a already working simplesamlphp service provider. I saw a fewer users on comments session of auth_saml requesting updates for Moodle 3.0 and the maintainer just complain about donation.
  
  And about auth_saml2 (https://moodle.org/plugins/auth_saml2), it's a complete solution to users that has not a working service provider (SP) and a solution for those that needs a quickly solution, but, for every single instance of moodle (in my environment has at least 8 now) the plugin creates a different service provider and the metadata needs to be exchanged with every identity provider (IdP). In cases where there are more than one identity provider, this is unpractical.
  
  So, to suit my needs, I develop another SAML auth plugin that just do authentication in a existing service provider.
  
  I'll improve the docs to better explain the plugin solution
• 

  David Mudrák

  Tue, 27 Sep 2016, 11:04 PM

  Thanks Daniel. It sounds like valid and legitimate reasons to me. It will be only fair if this reasoning is part of the plugin description here. That will help the administrators to choose the best plugin to suite their needs.

• 

  ivan matviyuk

  Wed, 28 Sep 2016, 12:22 AM

  Hi Daniel,
  
  There are two things i find confusing in the screenshot 1 you posted.
  One: what is, and what for is the "IdP to Moodle mapping" option? There is a separate page for data mapping, given on the screenshot 2 and it should go there maybe? If this is the username mapping, what is the purpose of the dropdown?
  Two: SimpleSAMLphp has a pretty correctly working logout mechanism. Why do you exclude the option of using it and instead give only the external logout URL option?
  
  Best regards,
  Ivan
• 

  Daniel Miranda

  Wed, 28 Sep 2016, 3:17 AM

  Hi Ivan, the "IdP to Moodle Mapping" is where we tell to the plugin which field (the field name) returned from IdP will be used to verify if the user exists in Moodle database. Lines 142 and 145 in the current version.
  [https://github.com/dmirandaa/moodle-auth_saml2_auth/blob/master/auth.php#L142]
  For example, in my environment, every user has to have a brazilian social security number (oid name brPersonCPF). I set it in this field as brPersonCPF and map with ID number from Moodle. But I can change this in another Moodle instance instead to validate a user account from email address. I hope I can explain.
  
  About the second question, I think your question is about the logout URL redirect in config. This is just where users will be redirected after they hit the logout button in Moodle. The method for logout [logoutpage_hook()] in line 101 [https://github.com/dmirandaa/moodle-auth_saml2_auth/blob/master/auth.php#L101] do exactly I think you are saying. Before close the Moodle session, we ensure that the user's session in IdP is finished too, getting the proper URL from IdP and then redirect back to Moodle.
• 

  ivan matviyuk

  Wed, 28 Sep 2016, 5:48 PM

  Hi Daniel,
  
  Let me explain my concerns first. I am an active user of the auth_saml plugin and i can see a situation then for a next installation i would prefer a plugin that is compatible with Moodle 3.1, so probably your plugin. I am familiar with the auth_saml principles and configurations and now when i come to configure auth_saml2_auth i am confused - especially with those two places i mentioned before. My point here is this is most probably the case for many other people running SAML SSO
  
  In auth_saml there is a clear username mapping policy instead of "IdP to Moodle Mapping" configuration and this is a good thing: it is transparent how the username is created in Moodle and so transparent what the username will be. If we choose idnumber in "IdP to Moodle Mapping", how username is created?
  Would be it possible that we leave only username option there and move the logic to verify if the user exists in the DB somewhere else?
  
  And, still leave the option from auth_saml to enable or disable Single Sign Off (a checkbox)? The mechanism you described seems OK to me, but now i'm definitely missing the option not to trigger Single Sign Off, as i used it a few times in my Moodles before
  
  Thanks and regards,
  Ivan
• 

  Daniel Miranda

  Wed, 28 Sep 2016, 10:28 PM

  Hi Ivan
  
  I understand all your concerns and did some changes in v3.0-r3.
  Let me know if it is clear now about the username field.
  Also, I add the Single Sign Off option
  
  Thanks for your concerns,
  Daniel
• 

  Dan Marsden

  Thu, 29 Sep 2016, 5:33 AM

  the description is looking a lot better here thanks.
  
  I'm slightly concerned about the name you've assigned this plugin "auth_saml2_auth" - it's probably not good practice for independent plugins to use another plugin name as the prefix "auth_saml2" - Does this actually pass our Frankenstyle coding guidelines? - it might be better to use "auth_saml3" or "auth_samlauth" or maybe "auth_samlsharedidp" ?