Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication Error when no credentialCallable is given. #816

Closed
Kerion opened this issue Jul 31, 2023 · 5 comments · Fixed by #825
Closed

Authentication Error when no credentialCallable is given. #816

Kerion opened this issue Jul 31, 2023 · 5 comments · Fixed by #825
Labels
Milestone

Comments

@Kerion
Copy link

Kerion commented Jul 31, 2023

An credentialCallable is optional in the Authentication-Options

Trying to Auth without credentialCallable results in the following Error (php8.2)

Error: Typed property DoctrineModule\Options\Authentication::$credentialCallable must not be accessed before initialization

Please Change Line 83 in Class DoctrineModule\Options\Authentication

From

protected mixed $credentialCallable = null;

To

protected mixed $credentialCallable = null;

@curzio-della-santa
Copy link
Contributor

curzio-della-santa commented Sep 29, 2023

Same error here

Please do the suggested change in Class DoctrineModule\Options\Authentication on line 83

from

protected mixed $credentialCallable;

to:

protected mixed $credentialCallable = null;

as temporary bypass I'll set a wrapper function in my configuration

    // Until doctrine/DoctrineModule gets fixed, a wrapper function is needed
    // ref: https://github.com/doctrine/DoctrineModule/issues/816
    'credentialCallable' => static fn(mixed $identity, mixed $credentialValue): mixed => $credentialValue,

@TomHAnderson
Copy link
Member

I looked at this with one of the more recent releases and I did not find a use case where $credentialCallable would not be set. Setting this value is good security. Can you please expound on your reasons for not setting it?

@curzio-della-santa
Copy link
Contributor

@TomHAnderson My application integrates Doctrine in Laminas, and in this case is not set.

It is a simple update that:

  • fixes a default behavior that is wrong.
  • does not introduce any issue

@driehle
Copy link
Member

driehle commented Nov 4, 2023

@curzio-della-santa Please provide a PR with the suggested change and I'll happily review and merge it.

@curzio-della-santa
Copy link
Contributor

@driehle PR is ready, thanks for your attention.

@driehle driehle added this to the 6.0.6 milestone Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants