I've recently released the dvg_authentication project to the public. This project makes it easy to provide external authentication methods often used by Dutch citizens or organisations, which they use for managing government related stuff.

This module is part of the DvG ecosystem that I also actively maintain, that focuses on providing a complete package to build a website for a (Dutch) Municipality.

Current features

  • Support authentication method DigiD.
  • Support authentication method eHerkenning.
  • Support authentication method eIDAS.
  • Fully accessible conforming to the WCAG 2.0 standard.
  • Multilingual with included translations for Dutch.
  • Domain support.
  • Auto logout functionality, to limit the time user can be logged in. Configurable per authentication method and role.
  • Privacy management, automatically anonymize or remove webform submissions after a configurable time limit.
  • Servicedesk support, allow servicedesk staff to authorize citizens.
  • Dummy method, for testing authentication.

This project is already in use for our projects and currently is running on 8 production sites, for almost a year.
I've kept the full commit history, from before it was public, including the old version tags. Now with the public release I'm planning on creating the 1.0 release soon.

Local setup

To setup and test this site locally it is easiest to use the dummy authentication methods or the debug modes for the others. As it is quite hard to completely setup the main auth methods as it requires a SAML setup with a service which does not allow just anyone to connect. Since it is only permitted for government related activities.

Testing authentication for a webform

  1. Download and install the DvG distribution and add this project and xautoload to the modules.
  2. Enable some of the included modules you want to test, e.g. "dvg_authentication_digid".
  3. Configure one or more of the authentication levels at /admin/config/services/dvg-authentication/digid The "Dummy" source should be available if you've no SAML sources defined.
  4. Create a new user, give it the "external user" role and in the DvG Authentication settings specify the "DigiD" provider and one of the levels you've configured in step 2. The description suggests some possible values.
  5. To best test the functionality, create a new webform.
  6. Go to the webform settings of your new webform where you'll find the new "External authentication" fieldset.
  7. Enable "Require external authentication".
  8. Check "DigiD" (or any other auth method you want to allow) and select the minimum level you require for the authorization.
  9. Open your new webform in a fresh session/tab (incognito or something).
  10. You see you are now required to login before seeing the webform, login using your just created test account.
  11. After logging in you can now see and submit the form.

You can now also test that the minimum required level actually works. E.g. configure both the DigiD "basic" and "middle" levels in step 2. Create a test account for only level "basic". In your webform set the minimum level at "middle". If you now login using that test account, you will be denied access to the webform with an error message explaining your authorization level is too low.

Testing servicedesk functionality

  1. I assume you've the setup after doing the above testing scenario.
  2. Enable the module "dvg_authentication_servicedesk".
  3. Create a new user with the role "servicedesk" (and NO DvG Authentication settings).
  4. Login as your new servicedesk user, using the normal Drupal way.
  5. Go to the webform you've created earlier that requires authentication.
  6. You can now enter the BSN (Dutch SSN) of the user you are "helping at the servicedesk". Or choose to "skip prefill" if you want to enter all information manually. The servicedesk is trusted to know what (not) to do and always has this skip option. Also prefill refers to the dvg_stuf_bg module providing tokens for querying personal information using a BSN. But the endpoints required for such a service are obviously also restricted to government entities only. Just know that is the primary use for these forms with authentication.
  7. Now you can enter and submit the form like normal.

Project link

https://www.drupal.org/project/dvg_authentication

Git instructions

git clone --branch 7.x-1.x git@git.drupal.org:project/dvg_authentication.git

Other info

Besides just this project I've already got some experience with maintaining other (security advisory covered) projects like the mentioned DvG.
As well as having experience with releasing a security fix in cooperation with the security team.
The recent PSA about fixing a security issue in DvG: Link
The fix released in that security update: Link
The security issue (only accessible by Security team members or DvG maintainers): Link

Comments

paulvandenburg created an issue. See original summary.

avpaderno’s picture

Status: Needs review » Needs work

Thank you for applying! For these applications, we need a project/branch where you committed most of the code, if not all. In this case, there are more commits from other users.

Since the purpose of these applications is verifying what the user who applies understands about writing secure code that correctly uses the Drupal APIs and follow the Drupal coding standards, we cannot review code written from more users to understand what a single user understands of writing Drupal code.

paulvandenburg’s picture

Status: Needs work » Needs review

Hmm, so what would be the recommended approach to achieving security advisory coverage when most of my d.o work is in maintaining the DvG ecosystem (as the currently sole active maintainer for the past year)?
Because most of my work is on lots of things instead of 1 module/branch/thing. Where most of my work then again is in collaboration with others, but I as the current active maintainer have the final say in whether something is good enough for committing. Or handling security issues from past maintainers with the security team: https://www.drupal.org/sa-contrib-2019-031

But there are parts of the dvg_authentication project of which I did (almost) all work, which might be sufficient for this application.
The dvg_authentication_auto_logout module:
https://git.drupalcode.org/project/dvg_authentication/blame/7.x-1.x/modu...
https://git.drupalcode.org/project/dvg_authentication/blame/7.x-1.x/modu...
The dvg_authentication_servicedesk module:
https://git.drupalcode.org/project/dvg_authentication/blame/7.x-1.x/modu...

klausi’s picture

Status: Needs review » Needs work
StatusFileSize
new64.26 KB

Thanks for your contribution!

This automated report was generated with PAReview.sh, your friendly project application review script.

manual review:
* Why does this module not use the authmap table as other external login provider modules such as https://www.drupal.org/project/samlauth ?
* I also don't see any calls of user_external_login_register() or user_external_load(). How do you keep track of the mapping of external users?
* Where do you check if a user account is blocked before logging them in? Could not find that in AuthenticationManager. I think this is a security blocker, a module must never log in users that are blocked.

paulvandenburg’s picture

Status: Needs work » Needs review
StatusFileSize
new1.56 KB
new2.4 KB

Thanks for the feedback!

I've looked into all issues and solved most. See the recent commits:
https://git.drupalcode.org/project/dvg_authentication/commits/7.x-1.x

As a result there is an almost clean code sniffer log, see attached.

The only left over issues for the Drupal code style are:

  • The DIGID_SECTOR_BSN constant, which is merely kept for backwards compatibility reasons with dvg_digid from DvG
  • The naming of the condition_values function in AutoLogoutActiveContextCondition, but that is a function inherited from context_condition from the context module.

The only left over issues for the DrupalPractice code style are:

  • Redeclaration of $user global, but AFAIK this is a standard practice to user either the provider $user or use the logged in user.
  • Class name prefix for the auto logout context condition. While it would be better, it messes with backwards compatibility, which I don't think is worth it for this issue.
  • Various undefined variables in the service catalog. Those variables come from the simplesamlphp authsources. It is just the way simplesamlphp config works, can't be much better than this I'm afraid. I've added var docs so at least the expected type is clear.

Your review questions:

Why does this module not use the authmap table as other external login provider modules such as https://www.drupal.org/project/samlauth ?

I've never come across the need to define this additional metadata. Now that you pose the question, I've looked into it if it should be added. But I'm not convinced it is a necessary addition. I've found this explanation (by kiamlaluno) for why it would be needed, but besides the use as metadata there is no other mentioned use case. Also is mentioned that the authmap table is dropped in D8, which makes it seem not worth it to me to add it to this project now. The necessary metadata about where the authentication for a user comes from is currently stored in the "data" column of the users table. Any third party modules who would like to interact with this can find the necessary information there, or use any of the helper functions provided by the AuthenticationManager (prefered). Additionally the current strategy is similar to the predecessor dvg_digid module (see DvG). Any third parties already interacting with this functionality should be familiar with how it is currently done in this module.

I also don't see any calls of user_external_login_register() or user_external_load(). How do you keep track of the mapping of external users?

I've partially addressed this question in my previous answer. The registering or loading of Drupal users is currently ultimately managed by the \Drupal\dvg_authentication\AuthenticationManager::getDrupalUser() function. A similar construct was already used in dvg_digid and was kept for this project. If I were to redesign this module, I'd consider using those 2 functions. However I see those functions are mostly a thin wrapper around calling the correct user_load or user_save with the right parameters. By doing those inside this project we have additional control over the metadata and managing the privacy aspect of the authentication since the privacy is very important for our use case. Also this gives immediate control over which role(s) the user gets, which for our use case is very minimal. As the externally authenticated users get no additional permissions besides accessing specific forms requiring a specific authentication method.

Where do you check if a user account is blocked before logging them in? Could not find that in AuthenticationManager?

For the intended use case Drupal should only do the minimum possible managing of the users. The users' authentication comes from the (Dutch) government (DigiD/eHerkenning) or from the EU government (eIDAS), we are not providing any management tools for these users within Drupal, nor should anyone. These users are primarily for signing purposes as well as providing official information about citizens through the government. Any form submitted by such a user should be proof that it is a specific citizen. E.g. to tell their local government they've moved to a new home. There should never be a situation where Drupal is above what the external authentication method provides. As such there is no option of blocking users. Additionally all these externally authentication users have no additional permissions beyond what an anonymous user has, they mere get access to specific forms that require specific external authentication methods.

A security aspect that is important for this project, is whether a form, that requires external authentication, can only be submitted by those externally authenticated users. To which we have currently 2 exceptions: the servicedesk user or the dummy authentication method. Servicedesk users require a special role assigned to trained employees from the local government. The dummy authentication module has additional checks to make sure it is not enabled on production environments as it is only intended for testing purposes. And obviously the root user has access, but I don't think that requires explanation however DvG is not designed to have anyone other than the developers have access to the admin tools.

With that said I don't believe our implementation is a security blocker. Unless anyone finds an issue in the mentioned security aspect, which I'd make sure would gets the highest priority. As I've done before for the DvG sec issue.

I hope I've solved and/or answered all your questions.

klausi’s picture

Status: Needs review » Reviewed & tested by the community

I see, thanks for the explanation. I still think it is important that I as a site administrator can block users from logging in. Your module violates that basic assumption that is otherwise assumed behavior in a Drupal installation. Maybe warn users about that in the README or on the project page?

There is quite a lot of code and I did not look into all of it in detail, but otherwise I think this is good to go.

avpaderno’s picture

Assigned: Unassigned » avpaderno
Status: Reviewed & tested by the community » Fixed

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.

paulvandenburg’s picture

Thanks for the review!
I've mentioned the user management aspect in the readme.

ArvindV’s picture

Hello

I can still some of phpcs warnings. You can check the reviews here.
https://pareview.sh/pareview/https-git.drupal.org-project-dvg_authentica...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.