Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Catalyst: Users

Supressing passwords in debug messages

  

 
Catalyst users RSS feed   Index | Next | Previous | View Threaded


Byron.Young at riverbed

Jan 7, 2009, 10:39 AM

Post #1 of 23 (2484 views)
Permalink
Supressing passwords in debug messages
I like the CATALYST_DEBUG mode for the test server - it's really nice to be able to see all the GET and POST params and requests as they happen. However, my app uses LDAP authentication and I really don't want people's LDAP passwords getting printed with the rest of the parameters.

Is there a way to suppress certain parameters from being printed? I didn't see anything in the docs about it, but thought I'd ask before jumping into the code.

Thanks,
Byron

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


jester at panix

Jan 7, 2009, 5:24 PM

Post #2 of 23 (2388 views)
Permalink
Re: Supressing passwords in debug messages [In reply to]
On Wed, Jan 07, 2009 at 10:39:34AM -0800, Byron Young wrote:
> I like the CATALYST_DEBUG mode for the test server - it's really nice to be able to see all the GET and POST params and requests as they happen. However, my app uses LDAP authentication and I really don't want people's LDAP passwords getting printed with the rest of the parameters.
>
> Is there a way to suppress certain parameters from being printed? I didn't see anything in the docs about it, but thought I'd ask before jumping into the code.
>

This is a FAQ:

http://dev.catalystframework.org/wiki/faq

"How do I hide certain variables (e.g. user/password) from the
debug screen?"

Jesse Sheidlower

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 9, 2009, 1:37 PM

Post #3 of 23 (2376 views)
Permalink
RE: Supressing passwords in debug messages [In reply to]
Jesse Sheidlower wrote on 2009-01-07:
> On Wed, Jan 07, 2009 at 10:39:34AM -0800, Byron Young wrote:
>> I like the CATALYST_DEBUG mode for the test server - it's really
> nice to be able to see all the GET and POST params and requests as
> they happen. However, my app uses LDAP authentication and I really
> don't want people's LDAP passwords getting printed with the rest of
> the parameters.
>>
>> Is there a way to suppress certain parameters from being printed?
> I didn't see anything in the docs about it, but thought I'd ask
> before jumping into the code.
>>
>
> This is a FAQ:
>
> http://dev.catalystframework.org/wiki/faq
>
> "How do I hide certain variables (e.g. user/password) from the
> debug screen?"
>
> Jesse Sheidlower
>

Jesse,

Thanks for the reply, but that doesn't quite do what I'm asking (or I'm using it wrong?). I mean the debug log that's prints request info when -Debug or CATALYST_DEBUG is turned on. For example:

[debug] Body Parameters are:
.-------------------------------------+--------------------------------------.
| Parameter | Value |
+-------------------------------------+--------------------------------------+
| password | REDACTED |
| submit | Go |
| username | youngb |
'-------------------------------------+--------------------------------------'
[debug] "POST" request for "login" from "10.16.5.10"
[debug] Path is "login"
[debug] Found sessionid "e4202e839e17004bc05baff653ad659f7b165ee7" in cookie
[debug] Restored session "e4202e839e17004bc05baff653ad659f7b165ee7"
[debug] Icebox::Controller::Login - Found username youngb, attempting login
[debug] Icebox::Controller::Login - LDAP login successful for youngb
[debug] Icebox::Controller::Login - Database login successful for youngb
[debug] ***Login::index - redirecting to http://icebox-dev.lab.nbttech.com:3000/
[debug] Redirecting to "http://icebox-dev.lab.nbttech.com:3000/"
[info] Request took 1.282297s (0.780/s)
.----------------------------------------------------------------+-----------.
| Action | Time |
+----------------------------------------------------------------+-----------+
| /auto | 0.000270s |
| /login/index | 0.078559s |
| /end | 0.000765s |
'----------------------------------------------------------------+-----------'


It's in that 'Body Parameters' section that I don't want the password to be displayed. It ends up there in plain text if debugging is turned on. Is there a simple way to remove or it or replace the value with '****'?


(but thanks for the link to the FAQ - I was only reading the POD. I have been using Catalyst for a while and have never seen a link to the Catalyst Wiki before - Maybe it would be a good idea to add a link to the Manual?)

Thanks,
Byron

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


jshirley at gmail

Jan 9, 2009, 2:13 PM

Post #4 of 23 (2374 views)
Permalink
Re: Supressing passwords in debug messages [In reply to]
On Fri, Jan 9, 2009 at 1:37 PM, Byron Young <Byron.Young[at]riverbed.com> wrote:
> Jesse Sheidlower wrote on 2009-01-07:
>> On Wed, Jan 07, 2009 at 10:39:34AM -0800, Byron Young wrote:
>>> I like the CATALYST_DEBUG mode for the test server - it's really
>> nice to be able to see all the GET and POST params and requests as
>> they happen. However, my app uses LDAP authentication and I really
>> don't want people's LDAP passwords getting printed with the rest of
>> the parameters.
>>>
>>> Is there a way to suppress certain parameters from being printed?
>> I didn't see anything in the docs about it, but thought I'd ask
>> before jumping into the code.
>>>
>>
>> This is a FAQ:
>>
>> http://dev.catalystframework.org/wiki/faq
>>
>> "How do I hide certain variables (e.g. user/password) from the
>> debug screen?"
>>
>> Jesse Sheidlower
>>
>
> Jesse,
>
> Thanks for the reply, but that doesn't quite do what I'm asking (or I'm using it wrong?). I mean the debug log that's prints request info when -Debug or CATALYST_DEBUG is turned on. For example:
>
> [debug] Body Parameters are:
> .-------------------------------------+--------------------------------------.
> | Parameter | Value |
> +-------------------------------------+--------------------------------------+
> | password | REDACTED |
> | submit | Go |
> | username | youngb |
> '-------------------------------------+--------------------------------------'
> [debug] "POST" request for "login" from "10.16.5.10"
> [debug] Path is "login"
> [debug] Found sessionid "e4202e839e17004bc05baff653ad659f7b165ee7" in cookie
> [debug] Restored session "e4202e839e17004bc05baff653ad659f7b165ee7"
> [debug] Icebox::Controller::Login - Found username youngb, attempting login
> [debug] Icebox::Controller::Login - LDAP login successful for youngb
> [debug] Icebox::Controller::Login - Database login successful for youngb
> [debug] ***Login::index - redirecting to http://icebox-dev.lab.nbttech.com:3000/
> [debug] Redirecting to "http://icebox-dev.lab.nbttech.com:3000/"
> [info] Request took 1.282297s (0.780/s)
> .----------------------------------------------------------------+-----------.
> | Action | Time |
> +----------------------------------------------------------------+-----------+
> | /auto | 0.000270s |
> | /login/index | 0.078559s |
> | /end | 0.000765s |
> '----------------------------------------------------------------+-----------'
>
>
> It's in that 'Body Parameters' section that I don't want the password to be displayed. It ends up there in plain text if debugging is turned on. Is there a simple way to remove or it or replace the value with '****'?
>
>
> (but thanks for the link to the FAQ - I was only reading the POD. I have been using Catalyst for a while and have never seen a link to the Catalyst Wiki before - Maybe it would be a good idea to add a link to the Manual?)
>
> Thanks,
> Byron
>

I think this is a valid feature request, but I don't think there is a
(simple) way to do it.

The methods in question are Catalyst->prepare_body and
prepare_query_parameters, which just dump all the parameters.

I think something like this would do the trick:
=== lib/Catalyst.pm
==================================================================
--- lib/Catalyst.pm (revision 18145)
+++ lib/Catalyst.pm (local)
@@ -1830,7 +1830,11 @@

if ( $c->debug && keys %{ $c->request->query_parameters } ) {
my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [ 36, 'Value' ] );
+ my %skip = map { $_ => $_ } @{
+ $c->config->{'Plugin::Debug'}->{'skip_dump_parameters'} || []
+ };
for my $key ( sort keys %{ $c->req->query_parameters } ) {
+ next if $skip{$key};
my $param = $c->req->query_parameters->{$key};
my $value = defined($param) ? $param : '';
$t->row( $key,


Then configure it via:

__PACKAGE__->config(
'Plugin::Debug' => {
skip_dump_parameters => [ qw/password/ /
}
);

Core devs around who want to look?

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


ansgar at 2008

Jan 11, 2009, 7:00 AM

Post #5 of 23 (2360 views)
Permalink
Re: Supressing passwords in debug messages [In reply to]
Hi,

"J. Shirley" <jshirley[at]gmail.com> writes:
> === lib/Catalyst.pm
> ==================================================================
> --- lib/Catalyst.pm (revision 18145)
> +++ lib/Catalyst.pm (local)
> @@ -1830,7 +1830,11 @@
>
> if ( $c->debug && keys %{ $c->request->query_parameters } ) {
> my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [ 36, 'Value' ] );
> + my %skip = map { $_ => $_ } @{
> + $c->config->{'Plugin::Debug'}->{'skip_dump_parameters'} || []
> + };
> for my $key ( sort keys %{ $c->req->query_parameters } ) {
> + next if $skip{$key};
> my $param = $c->req->query_parameters->{$key};
> my $value = defined($param) ? $param : '';
> $t->row( $key,

I think it would be better to show that the parameter was sent, but
Catalyst configured to not display its value. This can be done for
example by displaying a value of `(hidden)'.

If the parameter is simply skipped, it might be confusing if you forget
that you configured Catalyst to not display it.

Regards,
Ansgar

--
PGP: 1024D/595FAD19 739E 2D09 0969 BEA9 9797 B055 DDB0 2FF7 595F AD19

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 12, 2009, 10:45 AM

Post #6 of 23 (2333 views)
Permalink
RE: Re: Supressing passwords in debug messages [In reply to]
Ansgar Burchardt wrote on 2009-01-11:
> Hi,
>
> "J. Shirley" <jshirley[at]gmail.com> writes:
>> === lib/Catalyst.pm
>>
>> ================================================================== ---
>> lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
>> -1830,7 +1830,11 @@
>>
>> if ( $c->debug && keys %{ $c->request->query_parameters } )
> {
>> my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [
> 36, 'Value' ] );
>> + my %skip = map { $_ => $_ } @{
>> + $c->config->{'Plugin::Debug'}-
>> {'skip_dump_parameters'} || []
>> + };
>> for my $key ( sort keys %{ $c->req->query_parameters } )
> {
>> + next if $skip{$key};
>> my $param = $c->req->query_parameters->{$key};
>> my $value = defined($param) ? $param : '';
>> $t->row( $key,
>
> I think it would be better to show that the parameter was sent, but
> Catalyst configured to not display its value. This can be done for
> example by displaying a value of `(hidden)'.
>
> If the parameter is simply skipped, it might be confusing if you forget
> that you configured Catalyst to not display it.
>
> Regards,
> Ansgar
>

Yeah, I agree that the parameter should be shown as sent, but just not show the value.

J Shirley - Thanks for looking into it. Let me know if there's anything I can do to help.

Thanks,
Byron


_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


jshirley at gmail

Jan 12, 2009, 11:51 AM

Post #7 of 23 (2336 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
On Mon, Jan 12, 2009 at 10:45 AM, Byron Young <Byron.Young[at]riverbed.com> wrote:
> Ansgar Burchardt wrote on 2009-01-11:
>> Hi,
>>
>> "J. Shirley" <jshirley[at]gmail.com> writes:
>>> === lib/Catalyst.pm
>>>
>>> ================================================================== ---
>>> lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
>>> -1830,7 +1830,11 @@
>>>
>>> if ( $c->debug && keys %{ $c->request->query_parameters } )
>> {
>>> my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [
>> 36, 'Value' ] );
>>> + my %skip = map { $_ => $_ } @{
>>> + $c->config->{'Plugin::Debug'}-
>>> {'skip_dump_parameters'} || []
>>> + };
>>> for my $key ( sort keys %{ $c->req->query_parameters } )
>> {
>>> + next if $skip{$key};
>>> my $param = $c->req->query_parameters->{$key};
>>> my $value = defined($param) ? $param : '';
>>> $t->row( $key,
>>
>> I think it would be better to show that the parameter was sent, but
>> Catalyst configured to not display its value. This can be done for
>> example by displaying a value of `(hidden)'.
>>
>> If the parameter is simply skipped, it might be confusing if you forget
>> that you configured Catalyst to not display it.
>>
>> Regards,
>> Ansgar
>>
>
> Yeah, I agree that the parameter should be shown as sent, but just not show the value.
>
> J Shirley - Thanks for looking into it. Let me know if there's anything I can do to help.
>
> Thanks,
> Byron
>
>

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

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 12, 2009, 2:35 PM

Post #8 of 23 (2329 views)
Permalink
RE: Re: Supressing passwords in debug messages [In reply to]
J. Shirley wrote on 2009-01-12:
> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
> <Byron.Young[at]riverbed.com> wrote:
>> Ansgar Burchardt wrote on 2009-01-11:
>>> Hi,
>>>
>>> "J. Shirley" <jshirley[at]gmail.com> writes:
>>>> === lib/Catalyst.pm
>>>>
>>>>
> ==================================================================
> ---
>>>> lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
>>>> -1830,7 +1830,11 @@
>>>>
>>>> if ( $c->debug && keys %{ $c->request->query_parameters }
> )
>>> {
>>>> my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [
>>> 36, 'Value' ] );
>>>> + my %skip = map { $_ => $_ } @{
>>>> + $c->config->{'Plugin::Debug'}-
>>>> {'skip_dump_parameters'} || []
>>>> + };
>>>> for my $key ( sort keys %{ $c->req->query_parameters }
> )
>>> {
>>>> + next if $skip{$key};
>>>> my $param = $c->req->query_parameters->{$key};
>>>> my $value = defined($param) ? $param : '';
>>>> $t->row( $key,
>>> I think it would be better to show that the parameter was sent, but
>>> Catalyst configured to not display its value. This can be done for
>>> example by displaying a value of `(hidden)'.
>>>
>>> If the parameter is simply skipped, it might be confusing if you
>>> forget that you configured Catalyst to not display it.
>>>
>>> Regards,
>>> Ansgar
>>>
>> Yeah, I agree that the parameter should be shown as sent, but just not
>> show the value.
>>
>> J Shirley - Thanks for looking into it. Let me know if there's
>> anything I can do to help.
>>
>> Thanks,
>> Byron
>>
>>
>
> 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

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


jshirley at gmail

Jan 12, 2009, 2:55 PM

Post #9 of 23 (2331 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
On Mon, Jan 12, 2009 at 2:35 PM, Byron Young <Byron.Young[at]riverbed.com> wrote:
> J. Shirley wrote on 2009-01-12:
>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>> <Byron.Young[at]riverbed.com> wrote:
>>> Ansgar Burchardt wrote on 2009-01-11:
>>>> Hi,
>>>>
>>>> "J. Shirley" <jshirley[at]gmail.com> writes:
>>>>> === lib/Catalyst.pm
>>>>>
>>>>>
>> ==================================================================
>> ---
>>>>> lib/Catalyst.pm (revision 18145) +++ lib/Catalyst.pm (local) @@
>>>>> -1830,7 +1830,11 @@
>>>>>
>>>>> if ( $c->debug && keys %{ $c->request->query_parameters }
>> )
>>>> {
>>>>> my $t = Text::SimpleTable->new( [ 35, 'Parameter' ], [
>>>> 36, 'Value' ] );
>>>>> + my %skip = map { $_ => $_ } @{
>>>>> + $c->config->{'Plugin::Debug'}-
>>>>> {'skip_dump_parameters'} || []
>>>>> + };
>>>>> for my $key ( sort keys %{ $c->req->query_parameters }
>> )
>>>> {
>>>>> + next if $skip{$key};
>>>>> my $param = $c->req->query_parameters->{$key};
>>>>> my $value = defined($param) ? $param : '';
>>>>> $t->row( $key,
>>>> I think it would be better to show that the parameter was sent, but
>>>> Catalyst configured to not display its value. This can be done for
>>>> example by displaying a value of `(hidden)'.
>>>>
>>>> If the parameter is simply skipped, it might be confusing if you
>>>> forget that you configured Catalyst to not display it.
>>>>
>>>> Regards,
>>>> Ansgar
>>>>
>>> Yeah, I agree that the parameter should be shown as sent, but just not
>>> show the value.
>>>
>>> J Shirley - Thanks for looking into it. Let me know if there's
>>> anything I can do to help.
>>>
>>> Thanks,
>>> Byron
>>>
>>>
>>
>> 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.

-J

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 12, 2009, 4:15 PM

Post #10 of 23 (2331 views)
Permalink
RE: Re: Supressing passwords in debug messages [In reply to]
J. Shirley wrote on 2009-01-12:
> On Mon, Jan 12, 2009 at 2:35 PM, Byron Young
> <Byron.Young[at]riverbed.com> wrote:
>> J. Shirley wrote on 2009-01-12:
>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>>> <Byron.Young[at]riverbed.com> 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


_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 16, 2009, 6:38 PM

Post #11 of 23 (2247 views)
Permalink
RE: Re: Supressing passwords in debug messages [In reply to]
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[at]riverbed.com> wrote:
>>> J. Shirley wrote on 2009-01-12:
>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>>>> <Byron.Young[at]riverbed.com> 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
Attachments: redact-patch.diff (4.52 KB)
  cookbook-patch.diff (0.98 KB)


Byron.Young at riverbed

Jan 29, 2009, 10:53 AM

Post #12 of 23 (2094 views)
Permalink
Re: Supressing passwords in debug messages [In reply to]
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[at]riverbed.com]
> 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[at]riverbed.com> wrote:
>>>> J. Shirley wrote on 2009-01-12:
>>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>>>>> <Byron.Young[at]riverbed.com> 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
Attachments: redact-patch.diff (4.52 KB)
  cookbook-patch.diff (0.98 KB)


jshirley at gmail

Jan 29, 2009, 12:30 PM

Post #13 of 23 (2092 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
On Thu, Jan 29, 2009 at 10:53 AM, Byron Young <Byron.Young[at]riverbed.com> 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[at]riverbed.com]
>> 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[at]riverbed.com> wrote:
>>>>> J. Shirley wrote on 2009-01-12:
>>>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>>>>>> <Byron.Young[at]riverbed.com> 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[at]lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
> 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[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


jshirley at gmail

Jan 29, 2009, 12:41 PM

Post #14 of 23 (2090 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
On Thu, Jan 29, 2009 at 12:30 PM, J. Shirley <jshirley[at]gmail.com> wrote:
> On Thu, Jan 29, 2009 at 10:53 AM, Byron Young <Byron.Young[at]riverbed.com> 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[at]riverbed.com]
>>> 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[at]riverbed.com> wrote:
>>>>>> J. Shirley wrote on 2009-01-12:
>>>>>>> On Mon, Jan 12, 2009 at 10:45 AM, Byron Young
>>>>>>> <Byron.Young[at]riverbed.com> 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[at]lists.scsys.co.uk
>> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>> Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
>> 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[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


bobtfish at bobtfish

Jan 29, 2009, 12:53 PM

Post #15 of 23 (2091 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
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


_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


Byron.Young at riverbed

Jan 30, 2009, 2:20 PM

Post #16 of 23 (2080 views)
Permalink
RE: Re: Supressing passwords in debug messages [In reply to]
Tomas 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[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


bpphillips+ml at gmail

Jul 1, 2009, 9:03 AM

Post #17 of 23 (1383 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
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.
On Fri, Jan 30, 2009 at 5:20 PM, Byron Young <Byron.Young[at]riverbed.com>wrote:

> Tomas 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[at]lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
Attachments: 0001-param-filtering-docs-tests.patch (11.2 KB)


bpphillips+ml at gmail

Jul 1, 2009, 9:16 AM

Post #18 of 23 (1384 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
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.
Thanks!

On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips
<bpphillips+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> 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.
>
> On Fri, Jan 30, 2009 at 5:20 PM, Byron Young <Byron.Young[at]riverbed.com>wrote:
>
>> Tomas 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[at]lists.scsys.co.uk
>> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>> Searchable archive:
>> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
>> Dev site: http://dev.catalyst.perl.org/
>>
>
>
Attachments: 0001-param-filtering-docs-tests.patch (11.2 KB)


bpphillips+ml at gmail

Jul 1, 2009, 9:26 AM

Post #19 of 23 (1383 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
OK, 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+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> 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.
> Thanks!
>
>
> On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips <bpphillips+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> > 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.
>>
>> On Fri, Jan 30, 2009 at 5:20 PM, Byron Young <Byron.Young[at]riverbed.com>wrote:
>>
>>> Tomas 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[at]lists.scsys.co.uk
>>> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>>> Searchable archive:
>>> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
>>> Dev site: http://dev.catalyst.perl.org/
>>>
>>
>>
>
Attachments: 0001-param-filtering-docs-tests.patch (11.2 KB)


geoff.flarity at gmail

Nov 13, 2009, 1:03 PM

Post #20 of 23 (424 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
Bump.

Anyone know the status of this feature? Even if it was available only as
plugin it was would be incredibly useful.

Thanks,
GF

On Wed, Jul 1, 2009 at 11:26 AM, Brian Phillips
<bpphillips+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> wrote:

> OK, 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+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> > 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.
>>
>> Thanks!
>>
>>
>> On Wed, Jul 1, 2009 at 11:03 AM, Brian Phillips <bpphillips+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
>> > 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.
>>>
>>> On Fri, Jan 30, 2009 at 5:20 PM, Byron Young <Byron.Young[at]riverbed.com>wrote:
>>>
>>>> Tomas 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[at]lists.scsys.co.uk
>>>> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
>>>> Searchable archive:
>>>> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
>>>> Dev site: http://dev.catalyst.perl.org/
>>>>
>>>
>>>
>>
>
> _______________________________________________
> List: Catalyst[at]lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>
>


bobtfish at bobtfish

Nov 14, 2009, 8:59 AM

Post #21 of 23 (394 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
On 13 Nov 2009, at 21:03, Geoff Flarity wrote:

> Bump.
>
> Anyone know the status of this feature? Even if it was available
> only as plugin it was would be incredibly useful.

http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/param_filtering

I'm struggling to remember where this got to..

The patch looks ok, but not quite finished - IIRC I sent some feedback
after an initial review and it didn't go any further than that :(

Cheers
t0m


_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


bpphillips+ml at gmail

Nov 14, 2009, 2:03 PM

Post #22 of 23 (391 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
I dropped the ball on this. Here's my recollection of where things stand:
* I submitted an initial patch to the mailing list
* t0m gave feedback
* I refactored to allow configurable logging for several different
elements of the request/response
* t0m hooked me up with an SVN branch and commit bit and suggested
that once I committed, I should submit an RFC to the mailing list
* I committed the new functionality on the branch but neglected to
follow up as suggested

I don't recall any remaining changes that have been suggested that
weren't implemented. t0m, I'm guessing that branch is pretty stale by
now. Can we rebase or merge the current mainline to make sure it
still works with the current state of Catalyst? Do you have any
further suggestions on the branch in its current form? Should I just
submit it to the mailing list as you originally suggested?

Sorry for not following through on this. Real life took over and I
forgot about it.

Brian

On 11/14/09, Tomas Doran <bobtfish[at]bobtfish.net> wrote:
>
> On 13 Nov 2009, at 21:03, Geoff Flarity wrote:
>
>> Bump.
>>
>> Anyone know the status of this feature? Even if it was available
>> only as plugin it was would be incredibly useful.
>
> http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/param_filtering
>
> I'm struggling to remember where this got to..
>
> The patch looks ok, but not quite finished - IIRC I sent some feedback
> after an initial review and it didn't go any further than that :(
>
> Cheers
> t0m
>
>

_______________________________________________
List: Catalyst[at]lists.scsys.co.uk
Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
Searchable archive: http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
Dev site: http://dev.catalyst.perl.org/


geoff.flarity at gmail

Nov 16, 2009, 8:55 AM

Post #23 of 23 (331 views)
Permalink
Re: Re: Supressing passwords in debug messages [In reply to]
I'm just thankful someone else has taken the time to implement this
feature. Thanks!

GF

On Sat, Nov 14, 2009 at 5:03 PM, Brian Phillips
<bpphillips+ml[at]gmail.com<bpphillips%2Bml[at]gmail.com>
> wrote:

> I dropped the ball on this. Here's my recollection of where things stand:
> * I submitted an initial patch to the mailing list
> * t0m gave feedback
> * I refactored to allow configurable logging for several different
> elements of the request/response
> * t0m hooked me up with an SVN branch and commit bit and suggested
> that once I committed, I should submit an RFC to the mailing list
> * I committed the new functionality on the branch but neglected to
> follow up as suggested
>
> I don't recall any remaining changes that have been suggested that
> weren't implemented. t0m, I'm guessing that branch is pretty stale by
> now. Can we rebase or merge the current mainline to make sure it
> still works with the current state of Catalyst? Do you have any
> further suggestions on the branch in its current form? Should I just
> submit it to the mailing list as you originally suggested?
>
> Sorry for not following through on this. Real life took over and I
> forgot about it.
>
> Brian
>
> On 11/14/09, Tomas Doran <bobtfish[at]bobtfish.net> wrote:
> >
> > On 13 Nov 2009, at 21:03, Geoff Flarity wrote:
> >
> >> Bump.
> >>
> >> Anyone know the status of this feature? Even if it was available
> >> only as plugin it was would be incredibly useful.
> >
> >
> http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Runtime/5.80/branches/param_filtering
> >
> > I'm struggling to remember where this got to..
> >
> > The patch looks ok, but not quite finished - IIRC I sent some feedback
> > after an initial review and it didn't go any further than that :(
> >
> > Cheers
> > t0m
> >
> >
>
> _______________________________________________
> List: Catalyst[at]lists.scsys.co.uk
> Listinfo: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/catalyst
> Searchable archive:
> http://www.mail-archive.com/catalyst[at]lists.scsys.co.uk/
> Dev site: http://dev.catalyst.perl.org/
>

Catalyst users RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.