Beefy Boxes and Bandwidth Generously Provided by pair Networks
laziness, impatience, and hubris

Best practice with polymorphic constructors

by Ovid (Cardinal)
on Sep 03, 2001 at 01:08 UTC ( #109779=perlquestion: print w/replies, xml ) Need Help??
Ovid has asked for the wisdom of the Perl Monks concerning the following question:

I have a class, we'll call it Foo::Database, which is a base class for three other classes:

  • Foo::Security
  • Foo::Database::Select
  • Foo::Database::Modify

The Foo::Database class, amongst other things, connects to the database and controls all direct use of the DBI module. However, each of the child classes connects to the database as a different user. This is done for security reasons. I see no reason to give a Foo::Database::Select object the ability to modify the database if it doesn't need to. I view this as "defense in depth". If there's a security hole in another portion of code, hopefully the security built into the objects is a nice fallback.

Since I connect to the database in the constructor, the obvious idea is to make the constructor polymorphic by adding a unique constructor to each class. However, only the username and password for these constructors is unique and I didn't see the need to have repetitious code. This is what I used in the base class (well, there's more, but this is stripped down):

{ my $config_file = '/Inetpub/Secure/foo.dat'; my $data = do ( $config_file ) or die "Cannot process $conf +ig_file: $!"; sub new { my $class = shift; my ( $module ) = ( $class =~ /.*::(.*)$/ ); my $objref = { _dbh => _connect( data_source => $data->{ data_ +source }, user => $data->{ $modu +le }{ user }, pass => $data->{ $modu +le }{ pass } ), _error => 0 }; bless $objref, $class; } }

Note that all sensitive data is in a file (currently just a simple data structure) that's not Web-accessible. The problem that I don't like is that the config file now must have the module names hard-coded into it. As I wrote in a node about orthogonal code and security, I'm not wild about having to build something that requires different parts of a system to be synchronized. Any suggestions for improvement?


Vote for paco!

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

Replies are listed 'Best First'.
Re (tilly) 1: Best practice with polymorphic constructors
by tilly (Archbishop) on Sep 03, 2001 at 02:52 UTC
    Three suggestions.

    First of all if you have reason to force code to be kept in sync (and keeping names/passwords in a config file is good enough reason for me) then you can partially alleviate the problem by putting in run-time sanity checks. For instance use and confess away if someone tries to connect that you don't know about, or if your config file doesn't have all of the information that you expect. While having no synchronization is best, a close second is to have rapid and detailed feedback upon breakage.

    The second is about your indentation style. Yes, I know that a lot of people line up their indentation like that, but it is a maintanance hassle and comprehension is inhibited when indents go to 6 or more spaces in. My style is that if I am forced to move to another line, then I hit return and indent as I would for a block. YMMV on that, but consider how fast you indented way past 80 columns.

    I don't like having the config file hardcoded into the module. What I use at work is a (sorry, proprietary) configuration module that avoids having any hardcoded file names. Rather have an access method of some sort which decides where to get the configuration from dynamically. Why? Well that makes tracking down all of the configuration files that you are using a lot easier. It makes it easier to separate development from production. (Each just picks up configurations with slight but significant distinguishing features - like what database to connect to.) And if you code the configuration logic correctly, you can have it pick up tyops in your configuration file.

Re: Best practice with polymorphic constructors
by traveler (Parson) on Sep 03, 2001 at 02:15 UTC
    Please correct me if I am wrong or missing something. It seems to me that the module name in the config file is primarily a "key" to identify the particular user/password pair you are using. So why not structure the config file thus:
    <mydb> <config user="fooser" pass="foo" /> <modify user="barser" pass="bar" /> </mydb>
    (Or, you could specify a database or any other information in there.) Then you could retrieve the credentials based on the functionality, not the module name. Does that make sense?

    HTH, --traveler

Re: Best practice with polymorphic constructors
by Zaxo (Archbishop) on Sep 03, 2001 at 11:41 UTC

    How secure is /Inetpub/Secure/foo.dat from other users on the server? The thing I like about Apache suExec is that I can set 'chmod 600 /Inetpub/Secure/foo.dat' to protect the contents while retaining cgi access. I've been setting up private ~/lib directories that way.

    Can readers of the config be tricked into remembering too much? The polymorphisn you set up is a specialization to certain arguments of the stock DBI::connect method. Perhaps if your modules are sufficiently unreadable to the world, you can let each module take care of its own $dbuser and $dbpass using the same bareblocked closure you quote, but without the need for a synched config file. A module then knows its own secrets, and no other secrets are exposed to it.

    Security is pretty often at odds with maintainability, and I think your question is an example of that. Apache suExec requires extra care. I wouldn't develop for it without taint on.

    After Compline,

Re: Best practice with polymorphic constructors
by dragonchild (Archbishop) on Sep 04, 2001 at 18:24 UTC
    I think you're trying to build too much functionality into the base class's constructor. You're also hard-coding a lot of stuff into this random config file, which means that any usage of these classes has to be with the config files. You wouldn't be able to just pull this class hierarchy and use it in a completely different application.

    Why not do the following:

    package Class::Parent; sub new { my $class = shift; my ($args) = @_; my $self; unless (ref $class) { $self = {}; bless $self, $class; } else { $self = $class; } my @keys = keys %$args; @{$self}{@keys} = @{$args}{@keys}; # At this point, you have $self blessed into the right # class and with the right keys. Specifically, whatever # is needed for the DBI. # Do DBI thing. return $self; } package Class::Child; use Class::Parent; @ISA = qw(Class::Parent); sub new { my $class = shift; my ($args) = @_; my $self = {}; bless $self, ref$class || $class; $self->SUPER::new($args); # Do whatever else Class::Child needs return $self; }
    Thus, whenever you create a Foo::Database::Select, you're responsible for making sure it only goes and has select access. Of course, you could make sure that Foo::Database::Select only has a method to do selecting ...

    Now, if you wanted to get crazier, you could have each child initialize itself instead of in the parent. I originally thought of doing that, but decided to do it the simpler way, in case that was complex enough. You might want to move the hashslice assignment to the child and within the unless side (for the parent).

    We are the carpenters and bricklayers of the Information Age.

    Vote paco for President!

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://109779]
Approved by root
and all is quiet...

How do I use this? | Other CB clients
Other Users?
Others wandering the Monastery: (7)
As of 2018-06-20 04:35 GMT
Find Nodes?
    Voting Booth?
    Should cpanminus be part of the standard Perl release?

    Results (116 votes). Check out past polls.