|
View:
New views
8 Messages
—
Rating Filter:
Alert me
|
|
|
Re: Supressing passwords in debug messagesHi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff.
This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): Debug: redact_parameters: - password and the resulting log will look like: [debug] Query Parameters are: .-------------------------------------+--------------------------------------. | Parameter | Value | +-------------------------------------+--------------------------------------+ | password | (redacted by config) | | username | some_user | '-------------------------------------+--------------------------------------' There are two patches attached - redact-patch.diff - contains patch and test - cookbook-patch.diff - patch for cookbook entry about this Thanks to J Shirley for help with this. Thanks Byron Byron Young wrote on 2009-01-16: > -----Original Message----- > From: Byron Young [mailto:Byron.Young@...] > Sent: Friday, January 16, 2009 6:39 PM > To: The elegant MVC web framework > Subject: RE: [Catalyst] Re: Supressing passwords in debug messages > > Byron Young wrote on 2009-01-12: >> >> J. Shirley wrote on 2009-01-12: >>> On Mon, Jan 12, 2009 at 2:35 PM, Byron Young >>> <Byron.Young@...> wrote: >>>> J. Shirley wrote on 2009-01-12: >>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young >>>>> <Byron.Young@...> wrote: >> >> [snip] >> >>>>> The patch I'm creating needs to be configured in some way, I am >>>>> thinking at this point it can be configured as follows: >>>>> >>>>> package MyApp; >>>>> >>>>> __PACKAGE__->config( >>>>> 'Debug' => { >>>>> skip_dump_parameters => 1, # Simply don't render the >>>>> parameters incoming, very shotgunny skip_dump_parameters => >>>>> [ qw/password/ ], # Show '(redacted >>>>> by >>>>> config)' as the value of these fields >>>>> } >>>>> ); >>>>> >>>>> I'll need to bake tests for this, which there are currently no tests >>>>> for handling the dumping of parameters so it will be a bit more. If >>>>> someone wants to help with that, let me know and I can help guide. >>>>> >>>>> -J >>>>> >>>> >>>> I'd be happy to write some unit tests. I haven't worked with >> any >>> of the Catalyst unit tests before so I'm not sure what the process is >>> like for getting the code, setting up the test environment, making and >>> submitting changes and unit tests, etc. Is there a doc you can point >>> me to? I don't see anything in the manual or wiki. >>>> >>>> Byron >>>> >>>> Mostly it is just checking out the code from svn and starting. >> The >>> patch that I've started is at http://scsys.co.uk:8001/22410 - you can >>> apply this to a svn checkout of >>> http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70 >>> >>> It doesn't have the actual testing part, just a stub. I'll be working >>> on it more over today and tomorrow when I get free moments, but >>> they're few and far between. >>> >> Ditto on the lack of free time. I'll check it out and let you know >> what I come up with. >> >> byron >> > > J Shirley - I finally got a chance to look at this today. You did > most of the work for me. I just updated the unit test, changed the > 'skip_dump_parameters' parameter to 'redact_parameters', and > expanded the log_parameters() documentation a bit. I also added a > section to the cookbook explaining how to use the parameter. > > Attached are two patches: > redact-patch.diff - patch containing the new unit test and changes to > Catalyst.pm. cookbook-patch.diff - patch containing a new cookbook > section on > this feature, for the Catalyst-Manual repository > > Anything else I need to do? > > Byron _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesOn Thu, Jan 29, 2009 at 10:53 AM, Byron Young <Byron.Young@...> wrote:
> Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. > > This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): > > Debug: > redact_parameters: > - password > > and the resulting log will look like: > > [debug] Query Parameters are: > .-------------------------------------+--------------------------------------. > | Parameter | Value | > +-------------------------------------+--------------------------------------+ > | password | (redacted by config) | > | username | some_user | > '-------------------------------------+--------------------------------------' > > There are two patches attached > - redact-patch.diff - contains patch and test > - cookbook-patch.diff - patch for cookbook entry about this > > Thanks to J Shirley for help with this. > > Thanks > Byron > > > Byron Young wrote on 2009-01-16: >> -----Original Message----- >> From: Byron Young [mailto:Byron.Young@...] >> Sent: Friday, January 16, 2009 6:39 PM >> To: The elegant MVC web framework >> Subject: RE: [Catalyst] Re: Supressing passwords in debug messages >> >> Byron Young wrote on 2009-01-12: >>> >>> J. Shirley wrote on 2009-01-12: >>>> On Mon, Jan 12, 2009 at 2:35 PM, Byron Young >>>> <Byron.Young@...> wrote: >>>>> J. Shirley wrote on 2009-01-12: >>>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young >>>>>> <Byron.Young@...> wrote: >>> >>> [snip] >>> >>>>>> The patch I'm creating needs to be configured in some way, I am >>>>>> thinking at this point it can be configured as follows: >>>>>> >>>>>> package MyApp; >>>>>> >>>>>> __PACKAGE__->config( >>>>>> 'Debug' => { >>>>>> skip_dump_parameters => 1, # Simply don't render the >>>>>> parameters incoming, very shotgunny skip_dump_parameters => >>>>>> [ qw/password/ ], # Show '(redacted >>>>>> by >>>>>> config)' as the value of these fields >>>>>> } >>>>>> ); >>>>>> >>>>>> I'll need to bake tests for this, which there are currently no tests >>>>>> for handling the dumping of parameters so it will be a bit more. If >>>>>> someone wants to help with that, let me know and I can help guide. >>>>>> >>>>>> -J >>>>>> >>>>> >>>>> I'd be happy to write some unit tests. I haven't worked with >>> any >>>> of the Catalyst unit tests before so I'm not sure what the process is >>>> like for getting the code, setting up the test environment, making and >>>> submitting changes and unit tests, etc. Is there a doc you can point >>>> me to? I don't see anything in the manual or wiki. >>>>> >>>>> Byron >>>>> >>>>> Mostly it is just checking out the code from svn and starting. >>> The >>>> patch that I've started is at http://scsys.co.uk:8001/22410 - you can >>>> apply this to a svn checkout of >>>> http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70 >>>> >>>> It doesn't have the actual testing part, just a stub. I'll be working >>>> on it more over today and tomorrow when I get free moments, but >>>> they're few and far between. >>>> >>> Ditto on the lack of free time. I'll check it out and let you know >>> what I come up with. >>> >>> byron >>> >> >> J Shirley - I finally got a chance to look at this today. You did >> most of the work for me. I just updated the unit test, changed the >> 'skip_dump_parameters' parameter to 'redact_parameters', and >> expanded the log_parameters() documentation a bit. I also added a >> section to the cookbook explaining how to use the parameter. >> >> Attached are two patches: >> redact-patch.diff - patch containing the new unit test and changes to >> Catalyst.pm. cookbook-patch.diff - patch containing a new cookbook >> section on >> this feature, for the Catalyst-Manual repository >> >> Anything else I need to do? >> >> Byron > > > > _______________________________________________ > List: Catalyst@... > Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst > Searchable archive: http://www.mail-archive.com/catalyst@.../ > Dev site: http://dev.catalyst.perl.org/ > > Hi Byron, Just my fault -- been busy and then sick, I'll try to get to it in the next few days. -J _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesOn Thu, Jan 29, 2009 at 12:30 PM, J. Shirley <jshirley@...> wrote:
> On Thu, Jan 29, 2009 at 10:53 AM, Byron Young <Byron.Young@...> wrote: >> Hi - I'm not sure what the repost policy on patches, but I have the feeling this one slipped through the cracks. Let me know if it's generally annoying to repost stuff. >> >> This is a patch that allows you to suppress printing the value of certain query or body parameters when running Catalyst in debug mode - For example, if you want to hide passwords sent from the login page, you can put this in your app config (yaml): >> >> Debug: >> redact_parameters: >> - password >> >> and the resulting log will look like: >> >> [debug] Query Parameters are: >> .-------------------------------------+--------------------------------------. >> | Parameter | Value | >> +-------------------------------------+--------------------------------------+ >> | password | (redacted by config) | >> | username | some_user | >> '-------------------------------------+--------------------------------------' >> >> There are two patches attached >> - redact-patch.diff - contains patch and test >> - cookbook-patch.diff - patch for cookbook entry about this >> >> Thanks to J Shirley for help with this. >> >> Thanks >> Byron >> >> >> Byron Young wrote on 2009-01-16: >>> -----Original Message----- >>> From: Byron Young [mailto:Byron.Young@...] >>> Sent: Friday, January 16, 2009 6:39 PM >>> To: The elegant MVC web framework >>> Subject: RE: [Catalyst] Re: Supressing passwords in debug messages >>> >>> Byron Young wrote on 2009-01-12: >>>> >>>> J. Shirley wrote on 2009-01-12: >>>>> On Mon, Jan 12, 2009 at 2:35 PM, Byron Young >>>>> <Byron.Young@...> wrote: >>>>>> J. Shirley wrote on 2009-01-12: >>>>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young >>>>>>> <Byron.Young@...> wrote: >>>> >>>> [snip] >>>> >>>>>>> The patch I'm creating needs to be configured in some way, I am >>>>>>> thinking at this point it can be configured as follows: >>>>>>> >>>>>>> package MyApp; >>>>>>> >>>>>>> __PACKAGE__->config( >>>>>>> 'Debug' => { >>>>>>> skip_dump_parameters => 1, # Simply don't render the >>>>>>> parameters incoming, very shotgunny skip_dump_parameters => >>>>>>> [ qw/password/ ], # Show '(redacted >>>>>>> by >>>>>>> config)' as the value of these fields >>>>>>> } >>>>>>> ); >>>>>>> >>>>>>> I'll need to bake tests for this, which there are currently no tests >>>>>>> for handling the dumping of parameters so it will be a bit more. If >>>>>>> someone wants to help with that, let me know and I can help guide. >>>>>>> >>>>>>> -J >>>>>>> >>>>>> >>>>>> I'd be happy to write some unit tests. I haven't worked with >>>> any >>>>> of the Catalyst unit tests before so I'm not sure what the process is >>>>> like for getting the code, setting up the test environment, making and >>>>> submitting changes and unit tests, etc. Is there a doc you can point >>>>> me to? I don't see anything in the manual or wiki. >>>>>> >>>>>> Byron >>>>>> >>>>>> Mostly it is just checking out the code from svn and starting. >>>> The >>>>> patch that I've started is at http://scsys.co.uk:8001/22410 - you can >>>>> apply this to a svn checkout of >>>>> http://dev.catalystframework.org/repos/Catalyst/Catalyst- Runtime/5.70 >>>>> >>>>> It doesn't have the actual testing part, just a stub. I'll be working >>>>> on it more over today and tomorrow when I get free moments, but >>>>> they're few and far between. >>>>> >>>> Ditto on the lack of free time. I'll check it out and let you know >>>> what I come up with. >>>> >>>> byron >>>> >>> >>> J Shirley - I finally got a chance to look at this today. You did >>> most of the work for me. I just updated the unit test, changed the >>> 'skip_dump_parameters' parameter to 'redact_parameters', and >>> expanded the log_parameters() documentation a bit. I also added a >>> section to the cookbook explaining how to use the parameter. >>> >>> Attached are two patches: >>> redact-patch.diff - patch containing the new unit test and changes to >>> Catalyst.pm. cookbook-patch.diff - patch containing a new cookbook >>> section on >>> this feature, for the Catalyst-Manual repository >>> >>> Anything else I need to do? >>> >>> Byron >> >> >> >> _______________________________________________ >> List: Catalyst@... >> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst >> Searchable archive: http://www.mail-archive.com/catalyst@.../ >> Dev site: http://dev.catalyst.perl.org/ >> >> > > Hi Byron, > > Just my fault -- been busy and then sick, I'll try to get to it in the > next few days. > > -J > Actually, scratch that. I don't have the tuits or desire to cat herd this out. Someone on the core team can finish this up with you, I'm out. -J _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesOn 29 Jan 2009, at 18:53, Byron Young wrote: > Hi - I'm not sure what the repost policy on patches, but I have the > feeling this one slipped through the cracks. Let me know if it's > generally annoying to repost stuff. No, reposting if things get dropped on the floor good :) If you have time, then arriving on #catalyst-dev and making noise also gets stuff done. > This is a patch that allows you to suppress printing the value of > certain query or body parameters when running Catalyst in debug > mode - For example, if you want to hide passwords sent from the > login page, you can put this in your app config (yaml): Having been discussed in #catalyst-dev, we think that the patch could be made both more generic, and more elegant. The key thing is to split the table drawing, and the data filtering into separate methods (maybe filter_debug_data?). This would then allow you to filter per-type, and support things such as redact_parameters (all), redact_body_parameters, redact_query_parameters, and even potentially to add support for filtering things like the URI (I can see use-cases where that'd be significant - e.g. not wanting to log session IDs which are in URIs).. Have a look at the way the debug screen stuff works (in Catalyst::Engine), this is more elegant and would also benefit from being able to have things redacted I guess - as with the current patch, you're going to display the things you're redacting in the logs to the end user... Cheers t0m _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
RE: Re: Supressing passwords in debug messagesTomas Doran wrote on 2009-01-29:
> > On 29 Jan 2009, at 18:53, Byron Young wrote: > >> Hi - I'm not sure what the repost policy on patches, but I have the >> feeling this one slipped through the cracks. Let me know if it's >> generally annoying to repost stuff. > > No, reposting if things get dropped on the floor good :) > > If you have time, then arriving on #catalyst-dev and making noise > also gets stuff done. > >> This is a patch that allows you to suppress printing the value of >> certain query or body parameters when running Catalyst in debug >> mode - For example, if you want to hide passwords sent from the >> login page, you can put this in your app config (yaml): > Having been discussed in #catalyst-dev, we think that the patch could > be made both more generic, and more elegant. > > The key thing is to split the table drawing, and the data filtering > into separate methods (maybe filter_debug_data?). > > This would then allow you to filter per-type, and support things such as > redact_parameters (all), redact_body_parameters, > redact_query_parameters, and even potentially to add support for > filtering things like the URI (I can see use-cases where that'd be > significant - e.g. not wanting to log session IDs which are in URIs).. > > Have a look at the way the debug screen stuff works (in > Catalyst::Engine), this is more elegant and would also benefit from > being able to have things redacted I guess - as with the current > patch, you're going to display the things you're redacting in the > logs to the end user... > > Cheers > t0m > Tom, Thanks for the feedback. I think you're referring to $c->dump_these() and it's usage in finalize_error(). I'll refactor log_parameters() to call a separate method that will return the params to log, akin to dump_these(). Not sure when I'll have time for it since my current solution is working for me and I have some big deadlines coming up. Hopefully within the next month. Thanks byron _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesI've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome.
On Fri, Jan 30, 2009 at 5:20 PM, Byron Young <Byron.Young@...> wrote: Tomas Doran wrote on 2009-01-29: _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesSorry for the repost but I realized I hadn't updated to svn HEAD before making the patch file. In case that matters, the attached should apply cleanly against r10759.
Thanks!
On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips <bpphillips%2Bml@...> wrote: I've taken a stab at implementing this as I recently was wanting this functionality. See attached for the patch (including docs and unit tests). Feedback welcome. _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
|
|
Re: Re: Supressing passwords in debug messagesOK, I'm seriously ruining my first attempt at submitting a patch. :-) I changed the capitalization of a config key without updating the unit tests so <blush> here's another patch file.
Apologies for the spam. I think this is the last one I'll need to post on this issue ... hopefully ... :-)
On Wed, Jul 1, 2009 at 11:16 AM, Brian Phillips <bpphillips%2Bml@...> wrote: Sorry for the repost but I realized I hadn't updated to svn HEAD before making the patch file. In case that matters, the attached should apply cleanly against r10759. _______________________________________________ List: Catalyst@... Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst Searchable archive: http://www.mail-archive.com/catalyst@.../ Dev site: http://dev.catalyst.perl.org/ |
| Free embeddable forum powered by Nabble | Forum Help |