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

Include the passport guard when throwing exception #1796

Closed
wants to merge 2 commits into from

Conversation

willrowe
Copy link

This passes the guard used for passport when throwing an authentication exception.

When this exception is thrown, the default behavior is to redirect to the login page. This login page is usually associated with the default web guard. If a custom guard is used for Passport, you may want to redirect to a different URL. Including the guard itself in the exception allows the exception handler to detect it and change the redirect.

@taylorotwell
Copy link
Member

Tests are failing on this one.

@taylorotwell taylorotwell marked this pull request as draft October 31, 2024 18:10
@willrowe
Copy link
Author

willrowe commented Oct 31, 2024

Sorry, I got caught up with something and haven't been able to get back to this yet. Thanks for marking it as draft.

@hafezdivandari
Copy link
Contributor

hafezdivandari commented Nov 1, 2024

The guard on AuthenticationException not always equals passport.guard but depends on the context. For example this exception is thrown on CheckScopes middleware, but it has nothing to do with passport.guard. Actually this only makes sense on AuthorizationController:

// src/Http/Controllers/AuthorizationController.php

protected function promptForLogin(Request $request): never
{
    $request->session()->put('promptedForLogin', true);

-   throw new AuthenticationException;
+   throw new AuthenticationException(guards: isset($this->guard->name) ? [$this->guard->name] : []);
}

Note: SessionGuard is the only implementation of the StatefulGuard and it has name property.

@willrowe
Copy link
Author

willrowe commented Nov 1, 2024

The guard on AuthenticationException not always equals passport.guard but depends on the context. For example this exception is thrown on CheckScopes middleware, but it has nothing to do with passport.guard. Actually this only makes sense on AuthorizationController:

// src/Http/Controllers/AuthorizationController.php

protected function promptForLogin(Request $request): never
{
    $request->session()->put('promptedForLogin', true);

-   throw new AuthenticationException;
+   throw new AuthenticationException(guards: isset($this->guard->name) ? [$this->guard->name] : []);
}

Note: SessionGuard is the only implementation of the StatefulGuard and it has name property.

Good points, I was trying to make the least amount of changes, but you're right, it would not be accurate. I'm going to rethink the approach.

@willrowe willrowe closed this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants