Bug #32726

Cryptography Strategies don't have Singleton annotation

Added by Adrian Föder almost 10 years ago. Updated almost 10 years ago.

Status:
Closed
Priority:
Must have
Assignee:
Category:
Security
Start date:
2011-12-20
Due date:
% Done:

100%

Estimated time:
PHP Version:
Has patch:
No
Complexity:

Description

e.g. \TYPO3\FLOW3\Security\Cryptography\Pbkdf2HashingStrategy doesn't have @FLOW3\Scope("singleton") annotation which leads to

#1265370540: Cannot set instance of object "TYPO3\FLOW3\Security\Cryptography\PasswordHashingStrategyInterface" because it is of scope prototype. Only session and singleton instances can be set. 

Adding that annotation fixes the issue.

Due to the fact that Pbkdf2 isn't default anymore it's likely that it hasn't been discovered earlier, but I wonder BCrypt also hasn't that annotation set and it seems to work?

#1

Updated by Gerrit Code Review almost 10 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/7436

#2

Updated by Adrian Föder almost 10 years ago

add. information:
Objects.yaml is set to

TYPO3\FLOW3\Security\Cryptography\PasswordHashingStrategyInterface:
  className: TYPO3\FLOW3\Security\Cryptography\Pbkdf2HashingStrategy
#3

Updated by Karsten Dambekalns almost 10 years ago

Ok, what you want to do is set the default hashing strategy. But your approach is wrong. Change the default instead in Settings.yaml:

TYPO3:
  FLOW3:
    security:
      cryptography:
        hashingStrategies:
          default: pbkdf2

The fact that the strategies miss the singleton scope annotation is something that is still a valid issue, though.

#4

Updated by Adrian Föder almost 10 years ago

ah, ok, thanks a lot... That's funny because my "solution" also works... but thanks; I'll change it.

Do you need further information regarding the scope annotation?

#5

Updated by Karsten Dambekalns almost 10 years ago

  • Target version set to 1.0.2
#6

Updated by Gerrit Code Review almost 10 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/7436

#7

Updated by Karsten Dambekalns almost 10 years ago

  • Target version changed from 1.0.2 to 1.0.3
#8

Updated by Sebastian Kurfuerst almost 10 years ago

Karsten Dambekalns wrote:

Ok, what you want to do is set the default hashing strategy. But your approach is wrong. Change the default instead in Settings.yaml:
[...]
The fact that the strategies miss the singleton scope annotation is something that is still a valid issue, though.

Hey Karsten,

can you point me to the place where this is evaluated? I just grepped the full FLOW3 source for "hashingStrategies", and did not find anything!

IMHO the strategies should be allowed to be prototypes...

Greets,
Sebastian

#9

Updated by Christopher Hlubek almost 10 years ago

This should work without changing the scope annotation since it is prototype by intent (to allow different configurations). The strategy interface should not be used directly as a singleton anymore (see HashService for example). If a direct access is needed use the specific strategy (which is still configured in Objects.yaml) or introduce a custom marker interface or factory.

#10

Updated by Karsten Dambekalns almost 10 years ago

  • Status changed from Under Review to Closed
  • % Done changed from 0 to 100
#11

Updated by Karsten Dambekalns almost 10 years ago

Sebastian Kurfuerst wrote:

can you point me to the place where this is evaluated? I just grepped the full FLOW3 source for "hashingStrategies", and did not find anything!

It is used in HashingService.

Also available in: Atom PDF