Merge lp:~xaav/wikkid/auth-provider into lp:wikkid

[xaav]

Implements authentication support.

[xaav (xaav)]

This is sort-of done, but a bit messy.

[Tim Penhey (thumper)]

I think the idea of an authorization provider is a good idea. But not convinced on determining authorisation based on view name. I feel that it could end up with a lot of different authorisation checks needed.

How do you feel this would be used?

What about some general roles? Like View and Edit? I was considering the possibility of having "locked" pages, using the first line of of the wiki page, a little like he way moin does it.

[xaav (xaav)]

>
> How do you feel this would be used?

Basically wikkid would just verify whether an action was allowed or not, and
if not display an access denied page.

What about some general roles? Like View and Edit? I was considering the
> possibility of having "locked" pages, using the first line of of the wiki
> page, a little like he way moin does it.

The way it is now is that it is up to the authorization provider to manage
roles, allowing the most flexibility.

I sort of need to clean this up, the way it is now it's sort of hackish.

On Tue, Jun 14, 2011 at 5:59 PM, Tim Penhey <email address hidden>wrote:

> I think the idea of an authorization provider is a good idea. But not
> convinced on determining authorisation based on view name. I feel that it
> could end up with a lot of different authorisation checks needed.
>
> How do you feel this would be used?
>
> What about some general roles? Like View and Edit? I was considering the
> possibility of having "locked" pages, using the first line of of the wiki
> page, a little like he way moin does it.
> --
> https://code.launchpad.net/~xaav/wikkid/auth-provider/+merge/63755
> You are the owner of lp:~xaav/wikkid/auth-provider.
>

[Tim Penhey (thumper)]

On Wed, 15 Jun 2011 13:31:04 you wrote:
> > How do you feel this would be used?
>
> Basically wikkid would just verify whether an action was allowed or not,
> and if not display an access denied page.
>
> What about some general roles? Like View and Edit? I was considering the
>
> > possibility of having "locked" pages, using the first line of of the wiki
> > page, a little like he way moin does it.
>
> The way it is now is that it is up to the authorization provider to manage
> roles, allowing the most flexibility.

Well... we probably don't want that entirely.

If you think about what we want people to do, there isn't really much.

Pretty much just View and Edit.

By passing through the View name, we are expecting all providers to handle all
views, and by adding a view this would require a change to all providers.

We don't want that.

[xaav (xaav)]

>
> By passing through the View name, we are expecting all providers to handle
> all
> views, and by adding a view this would require a change to all providers.

That's the "hackiness" that I was referring to.

On Tue, Jun 14, 2011 at 8:55 PM, Tim Penhey <email address hidden>wrote:

> On Wed, 15 Jun 2011 13:31:04 you wrote:
> > > How do you feel this would be used?
> >
> > Basically wikkid would just verify whether an action was allowed or not,
> > and if not display an access denied page.
> >
> > What about some general roles? Like View and Edit? I was considering
> the
> >
> > > possibility of having "locked" pages, using the first line of of the
> wiki
> > > page, a little like he way moin does it.
> >
> > The way it is now is that it is up to the authorization provider to
> manage
> > roles, allowing the most flexibility.
>
> Well... we probably don't want that entirely.
>
> If you think about what we want people to do, there isn't really much.
>
> Pretty much just View and Edit.
>
> By passing through the View name, we are expecting all providers to handle
> all
> views, and by adding a view this would require a change to all providers.
>
> We don't want that.
>
> --
> https://code.launchpad.net/~xaav/wikkid/auth-provider/+merge/63755
> You are the owner of lp:~xaav/wikkid/auth-provider.
>

[Tim Penhey (thumper)]

On Wed, 15 Jun 2011 14:13:27 you wrote:
> > By passing through the View name, we are expecting all providers to
> > handle all
> > views, and by adding a view this would require a change to all providers.
>
> That's the "hackiness" that I was referring to.

OK, how about a slight change then.

What about an expected "permission" attribute to the base view class.
Initialised to something boring like None

The view registration code then makes sure that the permission is set.

We use 'view' and 'edit' for now, and add the appropriate permission to all
the views.

[lp:~xaav/wikkid/auth-provider]

71. By xaav on 2011-06-15

Revised authorization.

[xaav (xaav)]

I changed it so that the authorization was removed from the base view and added to each individual view.

Unmerged revisions

71. By xaav on 2011-06-15

Revised authorization.

70. By xaav on 2011-06-07

Cleaned up authorization.

69. By xaav on 2011-06-07

Completed auth provider.

68. By xaav on 2011-06-07

Completed auth provider.

67. By xaav on 2011-06-07

Changed to Authorization Provider.

66. By xaav on 2011-06-07

Added OpenProvider.

65. By xaav on 2011-06-07

Added auth provider.

64. By xaav <email address hidden> on 2011-06-07

Linked bug

63. By xaav on 2011-06-07

Added provider interface.

=== modified file 'wikkid/app.py'
--- wikkid/app.py	2010-11-22 00:00:31 +0000
+++ wikkid/app.py	2011-06-15 02:41:34 +0000
@@ -17,6 +17,7 @@
 from webob.exc import HTTPException, HTTPNotFound
 
 from wikkid.context import ExecutionContext
+from wikkid.provider.openprovider import OpenProvider
 from wikkid.dispatcher import get_view
 from wikkid.fileutils import FileIterable
 from wikkid.model.factory import ResourceFactory
@@ -47,10 +48,14 @@
 class WikkidApp(object):
     """The main wikkid application."""
 
-    def __init__(self, filestore, skin_name=None, execution_context=None):
+    def __init__(self, filestore, skin_name=None,
+                 authorization_provider=None, execution_context=None):
+        if authorization_provider is None:
+            authorization_provider = OpenProvider()
         if execution_context is None:
             execution_context = ExecutionContext()
         self.execution_context = execution_context
+        self.authorization_provider = authorization_provider
         self.filestore = filestore
         self.resource_factory = ResourceFactory(self.filestore)
         # Need to load the initial templates for the skin.
@@ -85,7 +90,8 @@
             resource_path, action = parse_url(path)
             model = self.resource_factory.get_resource_at_path(resource_path)
             try:
-                view = get_view(model, action, request, self.execution_context)
+                view = get_view(model, action, request, self.execution_context,
+                                self.authorization_provider)
                 response = view.render(self.skin)
             except HTTPException, e:
                 response = e
 
=== modified file 'wikkid/dispatcher.py'
--- wikkid/dispatcher.py	2010-11-12 09:00:42 +0000
+++ wikkid/dispatcher.py	2011-06-15 02:41:34 +0000
@@ -18,12 +18,13 @@
 from zope.interface import providedBy
 
 from wikkid.context import ExecutionContext
+from wikkid.provider.openprovider import OpenProvider
 
 # The view registry needs to map an Interface and a name to a class.
 _VIEW_REGISTRY = {}
 
 
-def get_view(obj, view_name, request, ec=ExecutionContext()):
+def get_view(obj, view_name, request, ec=ExecutionContext(), ap=OpenProvider()):
     """Get the most relevant view for the object for the specified name.
 
     Iterate through the provided interfaces of the object and look in the view
@@ -33,7 +34,7 @@
     for interface in interfaces:
         try:
             klass = _VIEW_REGISTRY[(interface, view_name)]
-            instance = klass(obj, request, ec)
+            instance = klass(obj, request, ec, ap)
             instance.initialize()
             return instance
         except KeyError:
 
=== added file 'wikkid/interface/provider.py'
--- wikkid/interface/provider.py	1970-01-01 00:00:00 +0000
+++ wikkid/interface/provider.py	2011-06-15 02:41:34 +0000
@@ -0,0 +1,15 @@
+#
+# Copyright (C) 2010 Wikkid Developers.
+#
+# This software is licensed under the GNU Affero General Public License
+# version 3 (see the file LICENSE).
+
+"""Interface for the authorization provider."""
+
+from zope.interface import Interface
+
+class IAuthorizationProvider(Interface):
+    """Verifies if a user is allowed to do an action."""
+    
+    def action_allowed(action):
+        """Verifies whether an action is allowed""" 
 \ No newline at end of file
 
=== added directory 'wikkid/provider'
=== added file 'wikkid/provider/__init__.py'
--- wikkid/provider/__init__.py	1970-01-01 00:00:00 +0000
+++ wikkid/provider/__init__.py	2011-06-15 02:41:34 +0000
@@ -0,0 +1,7 @@
+#
+# Copyright (C) 2010 Wikkid Developers.
+#
+# This software is licensed under the GNU Affero General Public License
+# version 3 (see the file LICENSE).
+
+"""Contains authentication providers."""
 \ No newline at end of file
 
=== added file 'wikkid/provider/openprovider.py'
--- wikkid/provider/openprovider.py	1970-01-01 00:00:00 +0000
+++ wikkid/provider/openprovider.py	2011-06-15 02:41:34 +0000
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2010 Wikkid Developers.
+#
+# This software is licensed under the GNU Affero General Public License
+# version 3 (see the file LICENSE).
+
+"""Allows all to edit the wiki."""
+
+from zope.interface import implements
+from wikkid.interface.provider import IAuthorizationProvider
+
+
+class OpenProvider(object):
+    """Allow all to perform all actions."""
+    
+    implements(IAuthorizationProvider)
+    
+    def action_allowed(self, action):
+        return True
 \ No newline at end of file
 
=== modified file 'wikkid/view/base.py'
--- wikkid/view/base.py	2010-11-22 09:23:40 +0000
+++ wikkid/view/base.py	2011-06-15 02:41:34 +0000
@@ -45,8 +45,10 @@
 
     __metaclass__ = BaseViewMetaClass
 
-    def __init__(self, context, request, execution_context):
+    def __init__(self, context, request, execution_context,
+                 authorization_provider):
         self.execution_context = execution_context
+        self.authorization_provider = authorization_provider
         self.context = context
         self.request = request
         if request is not None:
@@ -120,13 +122,15 @@
         :return: A `Response` object.
         """
         return Response(body)
-
+        
     def render(self, skin):
         """Render the page.
 
         Return a tuple of content type and content.
         """
+
         self.before_render()
+
         return self._render(skin)
 
 
 
=== modified file 'wikkid/view/edit.py'
--- wikkid/view/edit.py	2010-06-17 10:45:52 +0000
+++ wikkid/view/edit.py	2011-06-15 02:41:34 +0000
@@ -9,6 +9,7 @@
 from wikkid.view.base import BaseView
 from wikkid.view.urls import canonical_url
 from wikkid.view.utils import expand_wiki_name
+from webob.exc import HTTPUnauthorized
 
 
 class BaseEditView(BaseView):
@@ -30,3 +31,7 @@
     def cancel_url(self):
         """The link for the cancel button."""
         return self.context.preferred_path
+    
+    def before_render(self):
+        if(not self.authorization_provider.action_allowed('edit')):
+            raise HTTPUnauthorized()
 
=== modified file 'wikkid/view/wiki.py'
--- wikkid/view/wiki.py	2011-04-23 08:49:27 +0000
+++ wikkid/view/wiki.py	2011-06-15 02:41:34 +0000
@@ -32,6 +32,10 @@
     name = 'view'
     is_default = True
     template = 'view_page'
+    
+    def before_render(self):
+        if(not self.authorization_provider.action_allowed('view')):
+            raise HTTPUnauthorized()
 
     @property
     def content(self):
@@ -44,6 +48,9 @@
 
         For example, /FrontPage.txt will redirect to /FrontPage
         """
+        
+        self.before_render()
+        
         preferred = self.context.preferred_path
         if self.context.path != preferred:
             raise HTTPTemporaryRedirect(location=preferred)