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

[Bug]: Redirecting to wrong panel #2

Closed
CodeWithDennis opened this issue Aug 19, 2024 · 8 comments · Fixed by #3
Closed

[Bug]: Redirecting to wrong panel #2

CodeWithDennis opened this issue Aug 19, 2024 · 8 comments · Fixed by #3
Assignees
Labels
bug Something isn't working

Comments

@CodeWithDennis
Copy link
Contributor

CodeWithDennis commented Aug 19, 2024

What happened?

When adding the 2FA on a panel that isn't the main one, the user will be redirected to the main panel instead of the one where 2FA is set up. For example, if the panel is /customer/login, the user will be redirected to /employees/login, as that's the main panel.

How to reproduce the bug

Set up two panels: one for customers and one for employees, and place the login on the employees panel and try logging in (with code).

Package Version

1.0.0

PHP Version

8.2

Laravel Version

10.10

Which operating systems does with happen with?

macOS

Notes

(It likely has to do with the response. You might want to retrieve the Filament::getCurrentPanel() and then get the route from there.)

@CodeWithDennis CodeWithDennis added the bug Something isn't working label Aug 19, 2024
@Baspa Baspa self-assigned this Aug 19, 2024
@Baspa
Copy link
Member

Baspa commented Aug 19, 2024

Thank you for your clear explanation of the bug! I'm very sure it has something to do with the response. Tomorrow I have time to take a look at it, I will let you know when I fixed it :) @CodeWithDennis

@CodeWithDennis
Copy link
Contributor Author

CodeWithDennis commented Aug 19, 2024

Thank you for your clear explanation of the bug! I'm very sure it has something to do with the response. Tomorrow I have time to take a look at it, I will let you know when I fixed it :) @CodeWithDennis

No problem! Here is some more information. The method store on the TwoFactorAuthenticatedSessionController class returns the wrong response.

// Currently
public function store(TwoFactorLoginRequest $request)
{
    ...    
    return app(TwoFactorLoginResponse::class);
}
// Should be
public function store(TwoFactorLoginRequest $request)
{
    ...
    return app(\Vormkracht10\TwoFactorAuth\Http\Responses\TwoFactorLoginResponse::class);
}

The only issue now is that the response is returning the main panel route instead of the panel using 2FA. (Which is the main issue)

return redirect()->intended(Filament::getCurrentPanel()->getUrl());

That's as far as I've gotten.

@Baspa
Copy link
Member

Baspa commented Aug 19, 2024

I've made some changes in the 2-bug-redirecting-to-wrong-panel branch but in my local repository it still doesn't redirect to the correct panel. Can you verify that it isn't working correctly when checking out that branch? I'm not sure if I have setup the panels correctly. @CodeWithDennis

@CodeWithDennis
Copy link
Contributor Author

Yes it still redirects to the main panel. @Baspa

@CodeWithDennis
Copy link
Contributor Author

CodeWithDennis commented Aug 19, 2024

If you use Filament::getCurrentPanel() in the response or the class, it will return the main panel. It looks like it can't fetch the original panel because the 2FA page is likely under the default "login" panel. We need to find a way to grab the correct panel or pass it along somehow.

@Baspa
Copy link
Member

Baspa commented Aug 19, 2024

Alright, thanks for checking. I think I've built something like that in a project, will check it later today / tomorrow.

@Baspa Baspa linked a pull request Aug 19, 2024 that will close this issue
@Baspa Baspa closed this as completed in #3 Aug 19, 2024
@Baspa
Copy link
Member

Baspa commented Aug 19, 2024

It should be fixed now, can you double check it for me please? Tagged it with tag v1.0.3 @CodeWithDennis

@CodeWithDennis
Copy link
Contributor Author

It should be fixed now, can you double check it for me please? Tagged it with tag v1.0.3 @CodeWithDennis

Yes! Works perfectly, nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants