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

Session data, user agents, and CCs. #26

Closed
501st-alpha1 opened this issue Sep 1, 2015 · 10 comments
Closed

Session data, user agents, and CCs. #26

501st-alpha1 opened this issue Sep 1, 2015 · 10 comments

Comments

@501st-alpha1
Copy link
Contributor

I have a couple Laravel 4 projects that implement similar functionality as this package, so I'm looking into switching to this package to make things easier to maintain.

There are three pieces of functionality that are implemented in those projects that this package does not provide:

  • Including a dump of the session, by taking Session::all() and then unseting any sensitive data.
  • Including the user agent of the client that triggered the error.
  • Sending the error notifications to an additional email address as a CC.

Would you be open to adding these items? If so, I can probably submit PRs for all three (though I may have questions about how best to implement them).

@anlutro
Copy link
Owner

anlutro commented Sep 2, 2015

I think all of those points seem sensible.

On Wed, 2 Sep 2015 01:50 Scott Weldon notifications@github.com wrote:

I have a couple Laravel 4 projects that implement similar functionality as
this package, so I'm looking into switching to this package to make things
easier to maintain.

There are three pieces of functionality that are implemented in those
projects that this package does not provide:

  • Including a dump of the session, by taking Session::all() and then
    unseting any sensitive data.
  • Including the user agent of the client that triggered the error.
  • Sending the error notifications to an additional email address as a
    CC.

Would you be open to adding these items? If so, I can probably submit PRs
for all three (though I may have questions about how best to implement
them).


Reply to this email directly or view it on GitHub
#26.

@anlutro
Copy link
Owner

anlutro commented Sep 2, 2015

I'm not sure how you would determine sensitive session data, though. With
input I took the naive method of just checking if the string "password" was
in the input data array key, I don't think guessing sensitive session data
is as easy.

On Wed, 2 Sep 2015 07:01 Andreas Lutro anlutro@gmail.com wrote:

I think all of those points seem sensible.

On Wed, 2 Sep 2015 01:50 Scott Weldon notifications@github.com wrote:

I have a couple Laravel 4 projects that implement similar functionality
as this package, so I'm looking into switching to this package to make
things easier to maintain.

There are three pieces of functionality that are implemented in those
projects that this package does not provide:

  • Including a dump of the session, by taking Session::all() and then
    unseting any sensitive data.
  • Including the user agent of the client that triggered the error.
  • Sending the error notifications to an additional email address as a
    CC.

Would you be open to adding these items? If so, I can probably submit PRs
for all three (though I may have questions about how best to implement
them).


Reply to this email directly or view it on GitHub
#26.

@501st-alpha1
Copy link
Contributor Author

Cool, I'll get started on some PRs. My idea was to add another config option so that the user may define an array of keys that they want wiped.

@anlutro
Copy link
Owner

anlutro commented Sep 2, 2015

Sounds good.

@anlutro
Copy link
Owner

anlutro commented Sep 11, 2015

Should be completed in 089deb4 - @501st-alpha1 do you want to change your version requirement to dev-master and test it for a bit?

The changes will be released as 2.5.0 when I'm confident everything works as it's supposed to.

@501st-alpha1
Copy link
Contributor Author

I have this running in production for one of the projects. Just got the first notification in, and it seems to be working fine.

@501st-alpha1
Copy link
Contributor Author

I've gotten some more errors in over the past few days. I have verified that the session wiping works as expected, and the user agent is included in the email.

@anlutro
Copy link
Owner

anlutro commented Sep 17, 2015

Awesome - I'm out traveling so it may be a few days until I can tag a
release.

On Thu, 17 Sep 2015 18:54 Scott Weldon notifications@github.com wrote:

I've gotten some more errors in over the past few days. I have verified
that the session wiping works as expected, and the user agent is included
in the email.


Reply to this email directly or view it on GitHub
#26 (comment)
.

@501st-alpha1
Copy link
Contributor Author

No prob.

@anlutro
Copy link
Owner

anlutro commented Nov 19, 2015

Completely forgot about this! I just tagged 2.5.0.

@anlutro anlutro closed this as completed Nov 19, 2015
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

2 participants