Problem/Motivation

Deprecated function: Creation of dynamic property Drupal\tfa\Plugin\Tfa\TfaTotp::$isValid is deprecated in Drupal\tfa\Plugin\Tfa\TfaTotp->validate() (line 318 of modules/contrib/tfa/src/Plugin/Tfa/TfaTotp.php).

In PHP 8.2 and later, setting a value to an undeclared class property is deprecated, and emits a deprecation notice the first time the property is set during the lifetime of the application execution.

Proposed resolution

Add property to TfaBasePlugin class.

CommentFileSizeAuthor
#8 3371146-8.patch1.88 KBleo liao

Issue fork tfa-3371146

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:

About issue forks

Comments

markdorison created an issue. See original summary.

markdorison opened merge request !27

markdorison’s picture

markdorison
he/him
commented
Issue summary: View changes
Status: Active » Needs review
barry_fisher’s picture

barry_fisher commented

Hi Mark,

Patch is good and applies cleanly. It's a simple and logical fix, so hopefully this will get accepted by the maintainer.

Thanks!

barry_fisher’s picture

barry_fisher commented
Status: Needs review » Reviewed & tested by the community
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

It fixes the problem, but this is not the correct way to fix it.

The isValid property is only ever used inside the protected validate() function in the TfaRecoveryCode, TfaHotp and TfaTotp plugins, and can be trivially simplified by the direct return of the boolean value instead of the assignment to the dynamic property followed by the return, or by an assignment to a function-scope variable (i.e. $is_valid instead of $this->isValid).

I think we can all agree that promoting an internal temporary value of the validate() function to be a property of the plugin base class is not the best way to solve this.

markdorison’s picture

markdorison
he/him
commented
Version: 2.0.0-alpha2 » 2.x-dev
Status: Needs work » Needs review

I was just assuming that the implementation was intentional, but I am happy to make the change as suggested!

leo liao’s picture

leo liao commented
StatusFileSize
new 3371146-8.patch1.88 KB

We will use this patch in drupal 10 until it is officially released.

  • jcnventura authored ee617059 on 2.x 
    Issue #3371146 by markdorison, leo liao, Barry_Fisher, jcnventura:...
jcnventura’s picture

jcnventura
Primary language English
commented
Title: Deprecated function: Creation of dynamic property Drupal\tfa\Plugin\Tfa\TfaTotp::$isValid is deprecated » Creation of dynamic property Drupal\tfa\Plugin\Tfa\TfaTotp::$isValid is deprecated
Status: Needs review » Fixed

jcnventura closed merge request !27

Status: Fixed » Closed (fixed)

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

holo96’s picture

holo96 commented

Can we get this in next release?

apotek’s picture

apotek commented

Unfortunately, this did not get ported to the 8x-1.7 release and there is no stable 2 release even a year after this was merged. So while this is closed, it isn't really "fixed" for any sites which require stable releases.

Is there a plan to make a 2.x release? As more and more people update to PHP 8.2/8.3 (Acquia is moving everyone onto those versions now), this issue will only get more widespread.

rupertj’s picture

rupertj commented

@apotek This issue isn't present in the 8.x-1.x branch, so there's nothing to port. You can find the roadmap for a 2.x release here: https://www.drupal.org/project/tfa/issues/3374845