View Issue Details

This bug affects 1 person(s).
 6
IDProjectCategoryView StatusLast Update
19686Bug reportsOtherpublic2024-09-18 17:04
Reportertibor.pacalat Assigned Totibor.pacalat  
PrioritynoneSeverityminor 
Status closedResolutionwon't fix 
Product Version6.6.x 
Summary19686: php8.3 compatibility - robthree/twofactorauth
Description

Main ticket https://bugs.limesurvey.org/view.php?id=19632

robthree/twofactorauth
FILE: /var/www/html/limesurvey/application/core/plugins/TwoFactorAdminLogin/vendor/robthree/twofactorauth/lib/TwoFactorAuth.php
FILE: /var/www/html/limesurvey/application/core/plugins/TwoFactorAdminLogin/vendor/robthree/twofactorauth/lib/Providers/Rng/
FILE: /var/www/html/limesurvey/application/core/plugins/TwoFactorAdminLogin/vendor/robthree/twofactorauth/tests/TwoFactorAuthTest.php

Steps To Reproduce

Steps to reproduce

(Replace this text with detailed step-by-step instructions on how to reproduce the issue)

Expected result

(Write here what you expected to happen)

Actual result

(Write here what happened instead)

TagsNo tags attached.
Bug heat6
Complete LimeSurvey version number (& build)6.6.1+240806
I will donate to the project if issue is resolvedNo
Browser
Database type & version.
Server OS (if known)
Webserver software & version (if known)
PHP Version.

Relationships

child of 19632 assignedgabrieljenik Make Limesurvey compatible with php8.3 

Users monitoring this issue

There are no users monitoring this issue.

Activities

DenisChenu

DenisChenu

2024-09-12 11:18

developer   ~80982

See https://github.com/LimeSurvey/LimeSurvey/pull/3960/files#r1756475043

gabrieljenik

gabrieljenik

2024-09-12 21:54

manager   ~81008

Last edited: 2024-09-12 22:02

Fixes to this are already included in 19685.

The TwoFactor lib has been updated by its author. But the newest version doesn't support versions older than 8.2.
So we have 2 alternatives:
1) Just use the one we patched (and move it outside of the vendor directory)

2) Adapt the plugin so:

  • it includes one version or the other of the lib (this would require to move out the lib from the vendor and composer).
  • it can deal with the new version of the library (huge change between the two versions)

3) Use a different TwoFactorAuth lib

Either way, we would need to move the chnage we did in 19685 and include them in this PR.

I would say go with Alternative #1. (technical solution for this PHP upgrade)
And then approach on another ticket alternative #3. (feature update)

DenisChenu

DenisChenu

2024-09-13 08:57

developer   ~81009

I would say go with Alternative #1. (technical solution for this PHP upgrade)
And then approach on another ticket alternative #3. (feature update)

:+1: for me here !

tibor.pacalat

tibor.pacalat

2024-09-13 12:04

administrator   ~81011

@c_schmitz what do you think what would be the best solution here? I agree to go with #1 proposed solution, but I am not sure if there are some additional implications for our software if we do it this way that I am not aware of.

gabrieljenik

gabrieljenik

2024-09-13 22:37

manager   ~81026

Quick update. Nothing is needed here. Although there is impact, those are for deprecations.
The issue is thatthose deprectations are since 7.1... so if we got this far with that, means that code is not used.

If that part of the code is not used, shall we still update it?
Make sense?
According to the previous comments, maybe is best to not update anything and later on refactor the whole plugin to use a different library.

As per the impacts on the library that we commented, they are on the demo folder, so not real impact.
I would just don't do anything.
Test. If something popsup, we review it.

Thoughts?

DenisChenu

DenisChenu

2024-09-16 08:36

developer   ~81027

We don't have a test for 2FA ?
I know it's complex to do one since we can not really validate it work …
But adding some test seems cleaner :)

tibor.pacalat

tibor.pacalat

2024-09-18 15:39

administrator   ~81043

Ok, is there some work to be done for this ticket, or we can close it?

gabrieljenik

gabrieljenik

2024-09-18 15:55

manager   ~81046

I think you can close it. If we found an issue when testing, we will re open it.

Issue History

Date Modified Username Field Change
2024-08-08 14:09 tibor.pacalat New Issue
2024-09-11 12:34 tibor.pacalat Assigned To => gabrieljenik
2024-09-11 12:34 tibor.pacalat Status new => assigned
2024-09-11 12:34 tibor.pacalat Assigned To gabrieljenik => DenisChenu
2024-09-12 11:17 DenisChenu Relationship added child of 19632
2024-09-12 11:17 DenisChenu Assigned To DenisChenu => tibor.pacalat
2024-09-12 11:17 DenisChenu Assigned To tibor.pacalat => gabrieljenik
2024-09-12 11:18 DenisChenu Note Added: 80982
2024-09-12 11:18 DenisChenu Bug heat 0 => 2
2024-09-12 21:54 gabrieljenik Note Added: 81008
2024-09-12 21:54 gabrieljenik Bug heat 2 => 4
2024-09-12 21:54 gabrieljenik Assigned To gabrieljenik => DenisChenu
2024-09-12 21:54 gabrieljenik Status assigned => ready for code review
2024-09-12 21:59 gabrieljenik Note Edited: 81008
2024-09-12 22:00 gabrieljenik Assigned To DenisChenu => gabrieljenik
2024-09-12 22:00 gabrieljenik Status ready for code review => assigned
2024-09-12 22:00 gabrieljenik Note Edited: 81008
2024-09-12 22:01 gabrieljenik Note Edited: 81008
2024-09-12 22:02 gabrieljenik Note Edited: 81008
2024-09-13 08:57 DenisChenu Note Added: 81009
2024-09-13 12:04 tibor.pacalat Note Added: 81011
2024-09-13 12:04 tibor.pacalat Bug heat 4 => 6
2024-09-13 22:37 gabrieljenik Note Added: 81026
2024-09-13 22:37 gabrieljenik Assigned To gabrieljenik => tibor.pacalat
2024-09-13 22:37 gabrieljenik Status assigned => ready for testing
2024-09-16 08:36 DenisChenu Note Added: 81027
2024-09-18 15:39 tibor.pacalat Note Added: 81043
2024-09-18 15:55 gabrieljenik Note Added: 81046
2024-09-18 17:04 tibor.pacalat Status ready for testing => closed
2024-09-18 17:04 tibor.pacalat Resolution open => won't fix