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

Lumen 5.1 Auth Check Can Break Exception Responses Unexpectedly #88

Closed
paulredmond opened this issue Jan 15, 2016 · 4 comments
Closed

Comments

@paulredmond
Copy link

While this might not be a direct Bugsnag issue, it is related to the way the Bugsnag service provider is resolved that triggers this potential issue.

I am running into an issue with the Bugsnag\BugsnagLaravel\BugsnagLumenServiceProvider provider in a Lumen 5.1 environment (see this block of code):

// Check if someone is logged in.
try {
    if ($app['auth']->check()) {
        // User is logged in.
        $user = $app['auth']->user();

        // If these attributes are available: pass them on.
        $client->setUser(array('id' => $user->getAuthIdentifier()));
    }
} catch (\Exception $e) {
    // Do nothing.
}

Out of the box in 5.1 $app['auth'] depends on the memcached extension and thus the Memcached class, yet the Lumen install documentation doesn't require the memcached extension.

You must ensure that you have the memcached extension installed or the block above breaks all app responses that result from an exception. Some apps might not use the auth provider at all and thus will never see this issue until they start trying to integrate bugsnag (or some other package that triggers auth checks).

See this example stack trace for details of what I am running into.

To summarize, I think at the very least it might be wise to point out in the documentation that you need to have the memcached extension installed if you are not using $app['auth'] in a project.

In my case I am using redis for queueing and caching so I needed these environment settings (specifically SESSION_DRIVER) set to overcome the issue even though I am not specifically using auth at all in my application.

CACHE_DRIVER=redis
SESSION_DRIVER=redis
@paulredmond
Copy link
Author

I am willing to write up a pull request with documentation or making the authorization block optional with a configuration when resolving the bugsnag service. Let me know which route (or any others) you prefer.

@paulredmond paulredmond changed the title Lumen 5.1 Issue Lumen 5.1 Auth Check Can Break Exception Responses Unexpectedly Jan 16, 2016
@kattrali
Copy link
Contributor

Hi @paulredmond, thank you for the very detailed report. It sounds like we need of both solutions, documenting the memcached requirement and perhaps making the block optional. Using auth sounds like it would be fairly standard, so the documentation fix is probably the priority.

@paulredmond
Copy link
Author

@kattrali thanks for getting back to me. Another thing I realized is that the latest Lumen 5.2 release doesn't support session state (see 5.2 Authentication), so I think this issue only affects <= Lumen 5.1. I will check closer.

@kattrali
Copy link
Contributor

Gotcha, thanks for the update, @paulredmond.

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

No branches or pull requests

3 participants