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

Side effects in Auth0 constructor #543

Closed
X-Coder264 opened this issue Aug 20, 2021 · 3 comments · Fixed by #550
Closed

Side effects in Auth0 constructor #543

X-Coder264 opened this issue Aug 20, 2021 · 3 comments · Fixed by #550
Assignees
Milestone

Comments

@X-Coder264
Copy link

X-Coder264 commented Aug 20, 2021

SDK Version

8.0.0-BETA2

PHP Version

PHP 8.0

Composer Version

2.x

What happened?

The Auth0 class restores the session state in its constructor. This causes a lot of problems and debugging headaches.

In any scenarios when the session has not been started yet and the Auth0 class is being instantiated the data is wrongly restored in the state (it gets all null values) which then causes infinite redirect loops between the login and callback endpoints (so we have login route-> callback route -> route which requires the user to be logged in redirects the user again to the login route because it thinks there's no user because \Auth0\SDK\Auth0::getCredentials returns null because the session state has a null value for the user which happened due to the the state being wrongly restored as the session hasn't been started yet).

How can we reproduce this issue?

For example this happens if you typehint the Auth0 service (or any other service which has that class in its dependencies graph) in a Laravel controller constructor which gets instantiated from the Laravel container before Laravel runs its route middleware (and the start session middleware is one of those middleware).

<?php

declare(strict_types=1);

namespace App\Http\Controllers;

use Auth0\SDK\Auth0;

final class FooController
{
    public function __construct(
        private Auth0 $auth0
    ) {
    }

    public function __invoke(): Response
    {
        var_dump($this->auth0->getCredentials()); // it's null, even though \stdClass object is expected
    }
}

Additional context

This problems affects both the v7 and v8 version of this SDK and ideally the fix should be applied to both versions. And by fix I mean restoring the state at the last possible moment, just before the session data actually needs to be accessed (in a similar way as it was done in #528).

A workaround in the meantime is to use method injection instead of constructor injection so that the service gets resolved from the container after the session has been started.

@evansims evansims self-assigned this Aug 21, 2021
@evansims
Copy link
Member

evansims commented Aug 21, 2021

Hey @X-Coder264 👋 Thanks for your report! I don't think we've ever had this suggestion raised before, which surprises me a bit, now that you bring it up. I can see where this could be a beneficial change. I'm a bit uneasy about backporting this to v7 as I could see some instances where it might be considered a breaking change, but we can definitely make this happen for v8.

@evansims evansims added this to the 8.0.0 milestone Aug 21, 2021
@X-Coder264
Copy link
Author

Thank you @evansims. Since nobody reported this before I agree that it's fine to fix this just in v8 to be on the safe side regarding any possible BC.

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants