[Jifty-commit] r808 - in jifty/trunk: lib lib/Jifty t

jifty-commit at lists.jifty.org jifty-commit at lists.jifty.org
Tue Apr 4 20:14:43 EDT 2006


Author: alexmv
Date: Tue Apr  4 20:14:42 2006
New Revision: 808

Added:
   jifty/trunk/lib/Jifty/API.pm
Modified:
   jifty/trunk/   (props changed)
   jifty/trunk/MANIFEST
   jifty/trunk/lib/Jifty.pm
   jifty/trunk/lib/Jifty/Dispatcher.pm
   jifty/trunk/lib/Jifty/Everything.pm
   jifty/trunk/lib/Jifty/Handler.pm
   jifty/trunk/lib/Jifty/Test.pm
   jifty/trunk/lib/Jifty/Util.pm
   jifty/trunk/lib/Jifty/Web.pm
   jifty/trunk/t/07-limit-actions.t

Log:
 r12142 at zoq-fot-pik:  chmrr | 2006-04-04 20:13:52 -0400
  * Move allow and deny'ing of actions into Jifty::API
  * Clean up MANIFEST
  * Don't allow applications to be named "Jifty" by default
  * Fix copy/paste bug in Jifty.pm's handler method
  * Remove unused Jifty->web->actions


Modified: jifty/trunk/MANIFEST
==============================================================================
--- jifty/trunk/MANIFEST	(original)
+++ jifty/trunk/MANIFEST	Tue Apr  4 20:14:42 2006
@@ -170,7 +170,6 @@
 share/web/static/js/key_bindings.js
 share/web/static/js/prototype.js
 share/web/static/js/rico.js
-share/web/static/js/rico.js.rej
 share/web/static/js/scriptaculous/builder.js
 share/web/static/js/scriptaculous/controls.js
 share/web/static/js/scriptaculous/dragdrop.js
@@ -213,8 +212,6 @@
 share/web/templates/helpers/calendar.html
 share/web/templates/index.html
 SIGNATURE
-svk-commitFyzdP.tmp
-svk-commitqPHs1.tmp
 t/00-load.t
 t/01-dependencies.t
 t/02-connect.t

Modified: jifty/trunk/lib/Jifty.pm
==============================================================================
--- jifty/trunk/lib/Jifty.pm	(original)
+++ jifty/trunk/lib/Jifty.pm	Tue Apr  4 20:14:42 2006
@@ -61,7 +61,7 @@
 use base qw/Jifty::Object/;
 use Jifty::Everything;
 
-use vars qw/$HANDLE $CONFIG $LOGGER $DISPATCHER/;
+use vars qw/$HANDLE $CONFIG $LOGGER $HANDLER $DISPATCHER $API/;
 
 =head1 METHODS
 
@@ -120,6 +120,7 @@
 
     __PACKAGE__->dispatcher(Jifty::Dispatcher->new());
     __PACKAGE__->handler(Jifty::Handler->new());
+    __PACKAGE__->api(Jifty::API->new());
 
 
    # Let's get the database rocking and rolling
@@ -162,8 +163,8 @@
 
 sub handler {
     my $class = shift;
-    $LOGGER = shift if (@_);
-    return $LOGGER;
+    $HANDLER = shift if (@_);
+    return $HANDLER;
 }
 
 =head2 handle
@@ -181,9 +182,8 @@
 
 =head2 dispatcher
 
-An accessor for the C<Jifty::Dispatcher> object that we use to make decisions about how
-to dispatch each request made by a web client.
-
+An accessor for the C<Jifty::Dispatcher> object that we use to make
+decisions about how to dispatch each request made by a web client.
 
 =cut
 
@@ -193,6 +193,19 @@
     return $DISPATCHER;
 }
 
+=head2 api
+
+An accessor for the L<Jifty::API> object that publishes and controls
+information about the application's L<Jifty::Action>s.
+
+=cut
+
+sub api {
+    my $class = shift;
+    $API = shift if (@_);
+    return $API;
+}
+
 =head2 web
 
 An accessor for the L<Jifty::Web> object that the web interface uses. 
@@ -207,7 +220,8 @@
 
 =head2 setup_database_connection
 
-Set up our database connection. Optionally takes a param hash with a single argument
+Set up our database connection. Optionally takes a param hash with a
+single argument.  This method is automatically called by L</new>.
 
 =over
 

Added: jifty/trunk/lib/Jifty/API.pm
==============================================================================
--- (empty file)
+++ jifty/trunk/lib/Jifty/API.pm	Tue Apr  4 20:14:42 2006
@@ -0,0 +1,202 @@
+use warnings;
+use strict;
+
+package Jifty::API;
+
+=head1 NAME
+
+Jifty::API - Manages and allow reflection on the Jifty::Actions that
+make up a Jifty application's API
+
+=cut
+
+use Jifty::Everything;
+use base qw/Class::Accessor Jifty::Object/;
+
+__PACKAGE__->mk_accessors(qw(action_limits));
+
+=head1 METHODS
+
+=head2 new
+
+Creates a new C<Jifty::API> object
+
+=cut
+
+sub new {
+    my $class = shift;
+    my $self  = bless {}, $class;
+
+    $self->reset;
+
+    return ($self);
+}
+
+=head2 qualify ACTIONNAME
+
+Returns the fully qualified package name for the given provided
+action.  If the C<ACTIONNAME> starts with C<Jifty::Action> or your
+application's C<ActionBasePath>, simply returns the given name;
+otherwise, it prefixes it with the C<ActionBasePath>.
+
+=cut
+
+sub qualify {
+    my $self = shift;
+    my $action = shift;
+
+    my $base_path = Jifty->config->framework('ActionBasePath');
+
+    return $action
+        if $action =~ /^Jifty::Action/
+        or $action =~ /^\Q$base_path\E/;
+
+    return $base_path . "::" . $action;
+}
+
+=head2 reset
+
+Resets which actions are allowed to the defaults; that is, all actions
+from the application's C<ActionBasePath>, and
+L<Jifty::Action::Autocomplete> and L<Jifty::Action::Redirect> are
+allowed; everything else is denied.  See L</restrict> for the details
+of how limits are processed.
+
+=cut
+
+sub reset {
+    my $self = shift;
+
+    # Set up defaults
+    my $app_actions = Jifty->config->framework('ActionBasePath');
+
+    $self->action_limits(
+        [   { deny => 1, restriction => qr/.*/ },
+            {   allow       => 1,
+                restriction => qr/^\Q$app_actions\E/,
+            },
+            { allow => 1, restriction => 'Jifty::Action::Autocomplete' },
+            { allow => 1, restriction => 'Jifty::Action::Redirect' },
+        ]
+    );
+}
+
+=head2 allow RESTRICTIONS
+
+Takes a list of strings or regular expressions, and adds them in order
+to the list of limits for the purposes of L</is_allowed>.  See
+L</restrict> for the details of how limits are processed.
+
+=cut
+
+sub allow {
+    my $self = shift;
+    $self->restrict( allow => @_ );
+}
+
+=head2 deny RESTRICTIONS
+
+Takes a list of strings or regular expressions, and adds them in order
+to the list of limits for the purposes of L</is_allowed>.  See
+L</restrict> for the details of how limits are processed.
+
+=cut
+
+sub deny {
+    my $self = shift;
+    $self->restrict( deny => @_ );
+}
+
+=head2 restrict POLARITY RESTRICTIONS
+
+Method that L</allow> and and L</deny> call internally; I<POLARITY> is
+either C<allow> or C<deny>.  Allow and deny limits are evaluated in
+the order they're called.  The last limit that applies will be the one
+which takes effect.  Regexes are matched against the class; strings
+are fully L</qualify|qualified> and used as an exact match against the
+class name.  The base set of restrictions (which is reset every
+request) is set in L</reset>, and usually modified by the
+application's L<Jifty::Dispatcher> if need be.
+
+If you call:
+
+    Jifty->api->deny  ( qr'Foo' );
+    Jifty->api->allow ( qr'FooBar' );
+    Jifty->api->deny  ( qr'FooBarDeleteTheWorld' );
+
+..then:
+
+    calls to MyApp::Action::Baz will succeed.
+    calls to MyApp::Action::Foo will fail.
+    calls to MyApp::Action::FooBar will pass.
+    calls to MyApp::Action::TrueFoo will fail.
+    calls to MyApp::Action::TrueFooBar will pass.
+    calls to MyApp::Action::TrueFooBarDeleteTheWorld will fail.
+    calls to MyApp::Action::FooBarDeleteTheWorld will fail.
+
+=cut
+
+sub restrict {
+    my $self         = shift;
+    my $polarity     = shift;
+    my @restrictions = @_;
+
+    die "Polarity must be 'allow' or 'deny'"
+        unless $polarity eq "allow"
+        or $polarity     eq "deny";
+
+    for my $restriction (@restrictions) {
+        # Don't let the user "allow .*"
+        die "For security reasons, Jifty won't let you allow all actions"
+            if $polarity eq "allow"
+            and ref $restriction
+            and $restriction =~ /^\(\?[-xism]*:\^?\.\*\$?\)$/;
+
+        # Fully qualify it if it's a string
+        $restriction = $self->qualify($restriction)
+            unless ref $restriction;
+
+        # Add to list of restrictions
+        push @{ $self->action_limits },
+            { $polarity => 1, restriction => $restriction };
+    }
+}
+
+=head2 is_allowed CLASS
+
+Returns false if the I<CLASS> name (which is fully qualified with the
+application's ActionBasePath if it is not already) is allowed to be
+executed.  See L</restrict> above for the rules that the class
+name must pass.
+
+=cut
+
+sub is_allowed {
+    my $self  = shift;
+    my $class = shift;
+
+    # Qualify the action
+    $class = $self->qualify($class);
+
+    # Assume that it doesn't pass; however, the real fallbacks are
+    # controlled by L</reset>, above.
+    my $allow = 0;
+
+    # Walk all of the limits
+    for my $limit ( @{ $self->action_limits } ) {
+
+        # Regexes are =~ matches, strigns are eq matches
+        if ( ( ref $limit->{restriction} and $class =~ $limit->{restriction} )
+            or ( $class eq $limit->{restriction} ) )
+        {
+
+            # If the restriction passes, set the current allow/deny
+            # bit according to if this was a positive or negative
+            # limit
+            $allow = $limit->{allow} ? 1 : 0;
+        }
+    }
+    return $allow;
+}
+
+1;

Modified: jifty/trunk/lib/Jifty/Dispatcher.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Dispatcher.pm	(original)
+++ jifty/trunk/lib/Jifty/Dispatcher.pm	Tue Apr  4 20:14:42 2006
@@ -66,10 +66,10 @@
 
 B<before> rules are run before Jifty evaluates actions. They're the
 perfect place to enable or disable L<Jifty::Action>s using
-L<Jifty::Web/allow_actions> and L<Jifty::Web/deny_actions> or to
-completely disallow user access to private I<component> templates such
-as the F<_elements> directory in a default Jifty application.  They're
-also the right way to enable L<Jifty::LetMe> actions.
+L<Jifty::API/allow> and L<Jifty::API/deny> or to completely disallow
+user access to private I<component> templates such as the F<_elements>
+directory in a default Jifty application.  They're also the right way
+to enable L<Jifty::LetMe> actions.
 
 You can entirely stop processing with the C<redirect> and C<abort>
 directives.

Modified: jifty/trunk/lib/Jifty/Everything.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Everything.pm	(original)
+++ jifty/trunk/lib/Jifty/Everything.pm	Tue Apr  4 20:14:42 2006
@@ -17,6 +17,7 @@
 use Jifty::Handle ();
 use Jifty::ClassLoader ();
 use Jifty::Util ();
+use Jifty::API ();
 
 use Jifty::Record ();
 use Jifty::Collection ();

Modified: jifty/trunk/lib/Jifty/Handler.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Handler.pm	(original)
+++ jifty/trunk/lib/Jifty/Handler.pm	Tue Apr  4 20:14:42 2006
@@ -166,6 +166,7 @@
     Jifty->web->response( Jifty::Response->new );
     Jifty->web->setup_session;
     Jifty->web->session->set_cookie;
+    Jifty->api->reset;
 
     Jifty->log->debug( "Received request for " . Jifty->web->request->path );
 

Modified: jifty/trunk/lib/Jifty/Test.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Test.pm	(original)
+++ jifty/trunk/lib/Jifty/Test.pm	Tue Apr  4 20:14:42 2006
@@ -8,7 +8,6 @@
 use Jifty::Script::Schema;
 use Email::LocalDelivery;
 use Email::Folder;
-use Hook::LexWrap;
 
 =head2 import_extra
 

Modified: jifty/trunk/lib/Jifty/Util.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Util.pm	(original)
+++ jifty/trunk/lib/Jifty/Util.pm	Tue Apr  4 20:14:42 2006
@@ -137,11 +137,13 @@
     my $self = shift;
     my @root = File::Spec->splitdir( Jifty::Util->app_root);
     my $name =  pop @root;
+
     # Jifty-0.10211 should become Jifty
-    if ($name =~ /^(.*?)-(.*\..*)$/) {
-        $name = $1;
+    $name = $1 if $name =~ /^(.*?)-(.*\..*)$/;
+
+    # But don't actually allow "Jifty" as the name
+    $name = "JiftyApp" if lc $name eq "jifty";
 
-    }
     return $name;
 }
 

Modified: jifty/trunk/lib/Jifty/Web.pm
==============================================================================
--- jifty/trunk/lib/Jifty/Web.pm	(original)
+++ jifty/trunk/lib/Jifty/Web.pm	Tue Apr  4 20:14:42 2006
@@ -20,7 +20,7 @@
 use vars qw/$SERIAL/;
 
 __PACKAGE__->mk_accessors(
-    qw(next_page request response session temporary_current_user action_limits)
+    qw(next_page request response session temporary_current_user)
 );
 
 =head1 METHODS
@@ -191,7 +191,7 @@
 Each action on the request is vetted in three ways -- first, it must
 be marked as C<active> by the L<Jifty::Request> (this is the default).
 Second, it must be in the set of allowed classes of actions (see
-L</is_allowed>, below).  Finally, the action must validate.  If it
+L<Jifty::API/is_allowed>).  Finally, the action must validate.  If it
 passes all of these criteria, the action is fit to be run.
 
 Before they are run, however, the request has a chance to be
@@ -216,7 +216,7 @@
     for my $request_action ( $self->request->actions ) {
         $self->log->debug("Found action ".$request_action->class . " " . $request_action->moniker);
         next unless $request_action->active;
-        unless ( $self->is_allowed( $request_action->class ) ) {
+        unless ( Jifty->api->is_allowed( $request_action->class ) ) {
             $self->log->warn( "Attempt to call denied action '"
                     . $request_action->class
                     . "'" );
@@ -246,7 +246,8 @@
                 # Pull the action out of the request (again, since
                 # mappings may have affected parameters).  This
                 # returns the cached version unless the request has
-                # changed
+                # been changed by argument mapping from previous
+                # actions (Jifty::Request::Mapper)
                 my $action = $self->new_action_from_request($request_action);
                 next unless $action;
                 if ($request_action->modified) {
@@ -294,149 +295,6 @@
 
 Gets or sets the current L<Jifty::Response> object.
 
-=head2 ACTIONS
-
-=head3 actions 
-
-Gets the actions that have been created with this framework with
-C<new_action> (whether automatically via a L<Jifty::Request>, or in
-Mason code), as L<Jifty::Action> objects.
-
-=cut
-
-sub actions {
-    my $self = shift;
-
-    $self->{'actions'} ||= {};
-    return
-        sort { ( $a->order || 0 ) <=> ( $b->order || 0 ) }
-        values %{ $self->{'actions'} };
-}
-
-sub _add_action {
-    my $self   = shift;
-    my $action = shift;
-
-    $self->{'actions'}{ $action->moniker } = $action;
-}
-
-=head3 allow_actions RESTRICTIONS
-
-Takes a list of strings or regular expressions, and adds them in order
-to the list of limits for the purposes of L</is_allowed>.  See
-L</restrict_actions> for the details of how limits are processed.
-
-=cut
-
-sub allow_actions {
-    my $self = shift;
-    $self->restrict_actions( allow => @_ );
-}
-
-=head3 deny_actions RESTRICTIONS
-
-Takes a list of strings or regular expressions, and adds them in order
-to the list of limits for the purposes of L</is_allowed>.  See
-L</restrict_actions> for the details of how limits are processed.
-
-=cut
-
-sub deny_actions {
-    my $self = shift;
-    $self->restrict_actions( deny => @_ );
-}
-
-=head3 restrict_actions POLARITY RESTRICTIONS
-
-Method that L</allow_actions> and and L</deny_actions> call
-internally; I<POLARITY> is either C<allow> or C<deny>.  Allow and deny
-limits are evaluated in the order they're called.  The last limit that
-applies will be the one which takes effect.  Regexes are matched
-against the class; strings are fully qualified with the application's
-I<ActionBasePath> (if they not already) and used as an exact match
-against the class name.
-
-If you call:
-
-    Jifty->web->allow_actions ( qr'.*' );
-    Jifty->web->deny_actions  ( qr'Foo' );
-    Jifty->web->allow_actions ( qr'FooBar' );
-    Jifty->web->deny_actions ( qr'FooBarDeleteTheWorld' );
-
-
-calls to MyApp::Action::Baz will succeed.
-calls to MyApp::Action::Foo will fail.
-calls to MyApp::Action::FooBar will pass.
-calls to MyApp::Action::TrueFoo will fail.
-calls to MyApp::Action::TrueFooBar will pass.
-calls to MyApp::Action::TrueFooBarDeleteTheWorld will fail.
-calls to MyApp::Action::FooBarDeleteTheWorld will fail.
-
-=cut
-
-sub restrict_actions {
-    my $self         = shift;
-    my $polarity     = shift;
-    my @restrictions = @_;
-
-    die "Polarity must be 'allow' or 'deny'"
-        unless $polarity eq "allow"
-        or $polarity     eq "deny";
-
-    $self->action_limits( [] ) unless $self->action_limits;
-
-    for my $restriction (@restrictions) {
-
-        # Fully qualify it if it's a string and not already so
-        my $base_path = Jifty->config->framework('ActionBasePath');
-        $restriction = $base_path . "::" . $restriction
-            unless ref $restriction
-            or $restriction =~ /^\Q$base_path\E/;
-
-        # Add to list of restrictions
-        push @{ $self->action_limits },
-            { $polarity => 1, restriction => $restriction };
-    }
-}
-
-=head3 is_allowed CLASS
-
-Returns false if the I<CLASS> name (which is fully qualified with the
-application's ActionBasePath if it is not already) is allowed to be
-executed.  See L</restrict_actions> above for the rules that the class
-name must pass.
-
-=cut
-
-sub is_allowed {
-    my $self  = shift;
-    my $class = shift;
-
-    my $base_path = Jifty->config->framework('ActionBasePath');
-    $class = $base_path . "::" . $class
-        unless $class =~ /^\Q$base_path\E/;
-
-    # Assume that it passes
-    my $allow = 1;
-
-    $self->action_limits( [] ) unless $self->action_limits;
-
-    for my $limit ( @{ $self->action_limits } ) {
-
-        # For each limit
-        if ( ( ref $limit->{restriction} and $class =~ $limit->{restriction} )
-            or ( $class eq $limit->{restriction} ) )
-        {
-
-            # If the restriction passes, set the current allow/deny
-            # bit according to if this was a positive or negative
-            # limit
-            $allow = $limit->{allow} ? 1 : 0;
-        }
-    }
-    return $allow;
-}
-
 =head3 form
 
 Returns the current L<Jifty::Web::Form> object, creating one if there
@@ -508,10 +366,7 @@
     $class = $1;    # 'untaint'
 
     # Prepend the base path (probably "App::Action") unless it's there already
-    my $base_path = Jifty->config->framework('ActionBasePath');
-    $class = $base_path . "::" . $class
-        unless $class =~ /^\Q$base_path\E::/
-        or $class     =~ /^Jifty::Action::/;
+    $class = Jifty->api->qualify($class);
 
     # The implementation class is provided by the client, so this
     # isn't a "shouldn't happen"
@@ -526,7 +381,7 @@
         return;
     }
 
-    $self->_add_action($action);
+    $self->{'actions'}{ $action->moniker } = $action;
 
     return $action;
 }

Modified: jifty/trunk/t/07-limit-actions.t
==============================================================================
--- jifty/trunk/t/07-limit-actions.t	(original)
+++ jifty/trunk/t/07-limit-actions.t	Tue Apr  4 20:14:42 2006
@@ -5,38 +5,52 @@
 
 =head1 DESCRIPTION
 
-Tests that Jifty->web->(allow|deny)_actions work; this is to
+Tests that Jifty->api->(allow|deny) work; this is to
 limit what users can do with temporary credentials (LetMes, etc)
 
 =cut
 
-use Jifty::Test tests => 12;
+use Jifty::Test tests => 22;
 
-use_ok('Jifty::Web');
-can_ok('Jifty::Web', 'setup_session');
-can_ok('Jifty::Web', 'session');
+use_ok('Jifty::API');
 
-my $web = Jifty::Web->new();
-$web->setup_session;
+my $api = Jifty::API->new();
 
-ok($web->is_allowed("Foo"), "Tasks default to positive limit");
+is(Jifty->config->framework("ActionBasePath"), "JiftyApp::Action", "Action base path is as expected");
 
-$web->allow_actions ( qr'.*' );
-ok($web->is_allowed("Foo"), "Positive limit doesn't cause negative limit");
+ok($api->is_allowed("Jifty::Action::Autocomplete"), "Some Jifty actions are allowed");
+ok(!$api->is_allowed("Jifty::Action::Record::Update"), "Most are not");
+ok($api->is_allowed("Foo"), "Unqualified tasks default to positive limit");
+ok($api->is_allowed("JiftyApp::Action::Foo"), "Qualified tasks default to positive limit");
 
-$web->deny_actions ( qr'.*' );
-ok(!$web->is_allowed("Foo"), "Later negative limit overrides");
- 
-$web->allow_actions ( qr'.*' );
-ok($web->is_allowed("Foo"), "Even later positive limit overrides again");
+eval { $api->allow ( qr'.*' ); };
+like($@, qr/security reasons/, "Can't allow all actions");
+
+$api->allow ( qr'Foo' );
+ok($api->is_allowed("Foo"), "Positive limit doesn't cause negative limit");
 
-$web->deny_actions  ( qr'Foo' );
-ok(!$web->is_allowed("Foo"), "Regex negative limit");
-ok(!$web->is_allowed("FooBar"), "Matches anywhere");
-ok(!$web->is_allowed("ILikeFood"), "Matches anywhere");
-ok($web->is_allowed("Bar"), "Doesn't impact other positive");
+$api->deny ( qr'Foo' );
+ok(!$api->is_allowed("Foo"), "Later negative limit overrides");
+ 
+$api->allow ( qr'Foo' );
+ok($api->is_allowed("Foo"), "Even later positive limit overrides again");
 
-$web->allow_actions  ( 'ILikeFood' );
-ok($web->is_allowed("ILikeFood"), "Positive string exact match");
+$api->deny  ( qr'Foo' );
+ok(!$api->is_allowed("Foo"), "Regex negative limit");
+ok(!$api->is_allowed("JiftyApp::Action::Foo"), "Regex negative limit, qualified");
+ok(!$api->is_allowed("FooBar"), "Matches anywhere");
+ok(!$api->is_allowed("ILikeFood"), "Matches anywhere");
+ok($api->is_allowed("Bar"), "Doesn't impact other positive");
+ok($api->is_allowed("JiftyApp::Action::Bar"), "Doesn't impact other positive, qualified");
+
+$api->allow  ( 'ILikeFood' );
+ok($api->is_allowed("ILikeFood"), "Positive string exact match, unqualified on unqualified");
+ok($api->is_allowed("JiftyApp::Action::ILikeFood"), "Positive string exact match, unqualified on qualified");
+ok(!$api->is_allowed("ILikeFood::More"), "Positive string subclass match, unqualified on unqualified");
+
+$api->allow  ( 'JiftyApp::Action::ILikeFood' );
+ok($api->is_allowed("ILikeFood"), "Positive string exact match, qualified on unqualified");
+ok($api->is_allowed("JiftyApp::Action::ILikeFood"), "Positive string exact match, qualified on qualified");
+ok(!$api->is_allowed("ILikeFood::More"), "Positive string subclass match, qualified on unqualified");
 
 1;


More information about the Jifty-commit mailing list