Problem/Motivation

In Drupal 7, the services module was the de-facto way for providing REST API for Drupal sites. Drupal 8 provided REST services in core, and as such the presence of services_tfa is now a strange beast as it only provides TFA services for those few sites still using the services module (most probably don't actually need it, and have it only as a leftover of the D7->D8 migration).

The TFA module currently has no way to enforce TFA via the core REST services (see #3374260: Allow TFA authentication through REST routes), and the solution for that issue should also cover the sites using the services module. In any case, the use of the services_tfa module is entirely optional, as it wasn't actually preventing authentication via the services APIs without TFA, but simply providing a way to check it via services.

Proposed resolution

Deprecate the module in the 1.x version, and delete it from the 2.x codebase.

Issue fork tfa-3395250

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

jcnventura created an issue. See original summary.

jcnventura’s picture

Title: Deprecate the services_tfa module » Deprecate the tfa_services module
Issue summary: View changes
jcnventura’s picture

Title: Deprecate the tfa_services module » Deprecate the services_tfa module
Issue summary: View changes

cmlara made their first commit to this issue’s fork.

cmlara’s picture

Status: Active » Needs review
jcnventura’s picture

The 2.x change is RTBC, in my opinion :)

For 1.x, I'd prefer if the GenericValidation class would be annotated to indicate deprecation in addition to the lifecycle property in the .info.yml.. Not sure how well the static code analysers pick up the info.yml information, but I'm pretty sure they do pick up that type of information from code comments. I need to clean the spiderwebs out of my PhpStorm install, but until I do, this is one way of doing the annotation: https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...

cmlara’s picture

Status: Needs review » Needs work

Static analysis probably won't pick-up the info.yml so fair point, adding the deprecated flag to the class itself will help make sure anyone who is extending it without having the module itself enabled will get notified of the deprecation.

I'll add the deprecation flag to the docblock later today.

cmlara’s picture

Status: Needs work » Needs review

  • cmlara committed 727ac582 on 8.x-1.x
    Issue #3395250 by cmlara, jcnventura: Deprecate the services_tfa module
    
cmlara’s picture

Status: Needs review » Fixed

  • cmlara committed 4b8074ed on 2.x
    Issue #3395250 by cmlara, jcnventura: Remove the services_tfa module
    

Status: Fixed » Closed (fixed)

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