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

Proposed by xaav on 2011-06-07
Status: Needs review
Proposed branch: lp:~xaav/wikkid/auth-provider
Merge into: lp:wikkid
Diff against target: 204 lines (+71/-6)
8 files modified
wikkid/app.py (+8/-2)
wikkid/dispatcher.py (+3/-2)
wikkid/interface/provider.py (+15/-0)
wikkid/provider/__init__.py (+7/-0)
wikkid/provider/openprovider.py (+20/-0)
wikkid/view/base.py (+6/-2)
wikkid/view/edit.py (+5/-0)
wikkid/view/wiki.py (+7/-0)
To merge this branch: bzr merge lp:~xaav/wikkid/auth-provider
Reviewer Review Type Date Requested Status
Wikkid Hackers 2011-06-07 Pending
Review via email: mp+63755@code.launchpad.net

Description of the change

Implements authentication support.

To post a comment you must log in.
xaav (xaav) wrote :

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

Tim Penhey (thumper) 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.

xaav (xaav) 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.

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) 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.

xaav (xaav) 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.

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) wrote :

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 updated on 2011-06-15
71. By xaav on 2011-06-15

Revised authorization.

xaav (xaav) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'wikkid/app.py'
2--- wikkid/app.py 2010-11-22 00:00:31 +0000
3+++ wikkid/app.py 2011-06-15 02:41:34 +0000
4@@ -17,6 +17,7 @@
5 from webob.exc import HTTPException, HTTPNotFound
6
7 from wikkid.context import ExecutionContext
8+from wikkid.provider.openprovider import OpenProvider
9 from wikkid.dispatcher import get_view
10 from wikkid.fileutils import FileIterable
11 from wikkid.model.factory import ResourceFactory
12@@ -47,10 +48,14 @@
13 class WikkidApp(object):
14 """The main wikkid application."""
15
16- def __init__(self, filestore, skin_name=None, execution_context=None):
17+ def __init__(self, filestore, skin_name=None,
18+ authorization_provider=None, execution_context=None):
19+ if authorization_provider is None:
20+ authorization_provider = OpenProvider()
21 if execution_context is None:
22 execution_context = ExecutionContext()
23 self.execution_context = execution_context
24+ self.authorization_provider = authorization_provider
25 self.filestore = filestore
26 self.resource_factory = ResourceFactory(self.filestore)
27 # Need to load the initial templates for the skin.
28@@ -85,7 +90,8 @@
29 resource_path, action = parse_url(path)
30 model = self.resource_factory.get_resource_at_path(resource_path)
31 try:
32- view = get_view(model, action, request, self.execution_context)
33+ view = get_view(model, action, request, self.execution_context,
34+ self.authorization_provider)
35 response = view.render(self.skin)
36 except HTTPException, e:
37 response = e
38
39=== modified file 'wikkid/dispatcher.py'
40--- wikkid/dispatcher.py 2010-11-12 09:00:42 +0000
41+++ wikkid/dispatcher.py 2011-06-15 02:41:34 +0000
42@@ -18,12 +18,13 @@
43 from zope.interface import providedBy
44
45 from wikkid.context import ExecutionContext
46+from wikkid.provider.openprovider import OpenProvider
47
48 # The view registry needs to map an Interface and a name to a class.
49 _VIEW_REGISTRY = {}
50
51
52-def get_view(obj, view_name, request, ec=ExecutionContext()):
53+def get_view(obj, view_name, request, ec=ExecutionContext(), ap=OpenProvider()):
54 """Get the most relevant view for the object for the specified name.
55
56 Iterate through the provided interfaces of the object and look in the view
57@@ -33,7 +34,7 @@
58 for interface in interfaces:
59 try:
60 klass = _VIEW_REGISTRY[(interface, view_name)]
61- instance = klass(obj, request, ec)
62+ instance = klass(obj, request, ec, ap)
63 instance.initialize()
64 return instance
65 except KeyError:
66
67=== added file 'wikkid/interface/provider.py'
68--- wikkid/interface/provider.py 1970-01-01 00:00:00 +0000
69+++ wikkid/interface/provider.py 2011-06-15 02:41:34 +0000
70@@ -0,0 +1,15 @@
71+#
72+# Copyright (C) 2010 Wikkid Developers.
73+#
74+# This software is licensed under the GNU Affero General Public License
75+# version 3 (see the file LICENSE).
76+
77+"""Interface for the authorization provider."""
78+
79+from zope.interface import Interface
80+
81+class IAuthorizationProvider(Interface):
82+ """Verifies if a user is allowed to do an action."""
83+
84+ def action_allowed(action):
85+ """Verifies whether an action is allowed"""
86\ No newline at end of file
87
88=== added directory 'wikkid/provider'
89=== added file 'wikkid/provider/__init__.py'
90--- wikkid/provider/__init__.py 1970-01-01 00:00:00 +0000
91+++ wikkid/provider/__init__.py 2011-06-15 02:41:34 +0000
92@@ -0,0 +1,7 @@
93+#
94+# Copyright (C) 2010 Wikkid Developers.
95+#
96+# This software is licensed under the GNU Affero General Public License
97+# version 3 (see the file LICENSE).
98+
99+"""Contains authentication providers."""
100\ No newline at end of file
101
102=== added file 'wikkid/provider/openprovider.py'
103--- wikkid/provider/openprovider.py 1970-01-01 00:00:00 +0000
104+++ wikkid/provider/openprovider.py 2011-06-15 02:41:34 +0000
105@@ -0,0 +1,20 @@
106+# -*- coding: utf-8 -*-
107+#
108+# Copyright (C) 2010 Wikkid Developers.
109+#
110+# This software is licensed under the GNU Affero General Public License
111+# version 3 (see the file LICENSE).
112+
113+"""Allows all to edit the wiki."""
114+
115+from zope.interface import implements
116+from wikkid.interface.provider import IAuthorizationProvider
117+
118+
119+class OpenProvider(object):
120+ """Allow all to perform all actions."""
121+
122+ implements(IAuthorizationProvider)
123+
124+ def action_allowed(self, action):
125+ return True
126\ No newline at end of file
127
128=== modified file 'wikkid/view/base.py'
129--- wikkid/view/base.py 2010-11-22 09:23:40 +0000
130+++ wikkid/view/base.py 2011-06-15 02:41:34 +0000
131@@ -45,8 +45,10 @@
132
133 __metaclass__ = BaseViewMetaClass
134
135- def __init__(self, context, request, execution_context):
136+ def __init__(self, context, request, execution_context,
137+ authorization_provider):
138 self.execution_context = execution_context
139+ self.authorization_provider = authorization_provider
140 self.context = context
141 self.request = request
142 if request is not None:
143@@ -120,13 +122,15 @@
144 :return: A `Response` object.
145 """
146 return Response(body)
147-
148+
149 def render(self, skin):
150 """Render the page.
151
152 Return a tuple of content type and content.
153 """
154+
155 self.before_render()
156+
157 return self._render(skin)
158
159
160
161=== modified file 'wikkid/view/edit.py'
162--- wikkid/view/edit.py 2010-06-17 10:45:52 +0000
163+++ wikkid/view/edit.py 2011-06-15 02:41:34 +0000
164@@ -9,6 +9,7 @@
165 from wikkid.view.base import BaseView
166 from wikkid.view.urls import canonical_url
167 from wikkid.view.utils import expand_wiki_name
168+from webob.exc import HTTPUnauthorized
169
170
171 class BaseEditView(BaseView):
172@@ -30,3 +31,7 @@
173 def cancel_url(self):
174 """The link for the cancel button."""
175 return self.context.preferred_path
176+
177+ def before_render(self):
178+ if(not self.authorization_provider.action_allowed('edit')):
179+ raise HTTPUnauthorized()
180
181=== modified file 'wikkid/view/wiki.py'
182--- wikkid/view/wiki.py 2011-04-23 08:49:27 +0000
183+++ wikkid/view/wiki.py 2011-06-15 02:41:34 +0000
184@@ -32,6 +32,10 @@
185 name = 'view'
186 is_default = True
187 template = 'view_page'
188+
189+ def before_render(self):
190+ if(not self.authorization_provider.action_allowed('view')):
191+ raise HTTPUnauthorized()
192
193 @property
194 def content(self):
195@@ -44,6 +48,9 @@
196
197 For example, /FrontPage.txt will redirect to /FrontPage
198 """
199+
200+ self.before_render()
201+
202 preferred = self.context.preferred_path
203 if self.context.path != preferred:
204 raise HTTPTemporaryRedirect(location=preferred)

Subscribers

People subscribed via source and target branches