diff mbox

Make it possible, via a config setting, to use OpenID for authentication

Message ID 20110415171819.19037.93126.stgit@localhost6.localdomain6
State Superseded
Headers show

Commit Message

Guilherme Salgado April 15, 2011, 5:27 p.m. UTC 
The default still is to authenticate against the local user database, though.

Signed-off-by: Guilherme Salgado <guilherme.salgado@linaro.org>
---

This basically just adds instructions for people who may want to have their
patchwork instance as an OpenID relying party.  The only code changes are the
addition of the login_url() context processor, so that we always use the
LOGIN_URL defined in the settings, and the moving of the link to register from
the base template to the /accounts/login page.  The rationale for the latter
change is that the register link doesn't make sense on instances that use
OpenID, but I think that change is reasonable as registering on a given
instance is not something people will do often, so there's no reason to show a
link to that on every page, when the user is not logged in.

Notice that none of the changes above require either the openid-auth app or the
python-openid library, so people deploying new patchwork instances or updating
existing ones won't have any unpleasant surprises if they don't have them
installed.

 apps/patchwork/context_processors.py |    6 +++---
 apps/settings.py                     |   16 +++++++++++++++-
 apps/urls.py                         |    2 ++
 docs/INSTALL                         |   21 +++++++++++++++++++++
 htdocs/css/style.css                 |    4 ++++
 lib/sql/grant-openid.mysql.sql       |    7 +++++++
 lib/sql/grant-openid.postgres.sql    |   15 +++++++++++++++
 templates/base.html                  |    4 ++--
 templates/registration/login.html    |    6 ++++++
 9 files changed, 75 insertions(+), 6 deletions(-)
 create mode 100644 lib/sql/grant-openid.mysql.sql
 create mode 100644 lib/sql/grant-openid.postgres.sql

Comments

Jeremy Kerr April 19, 2011, 6:10 a.m. UTC | #1 
Hi Guilherme,

> This basically just adds instructions for people who may want to have their
> patchwork instance as an OpenID relying party.  The only code changes are the
> addition of the login_url() context processor, so that we always use the
> LOGIN_URL defined in the settings,

Looks good, just one thing:

>  and the moving of the link to register from
> the base template to the /accounts/login page.  The rationale for the latter
> change is that the register link doesn't make sense on instances that use
> OpenID, but I think that change is reasonable as registering on a given
> instance is not something people will do often, so there's no reason to show a
> link to that on every page, when the user is not logged in.

I'd prefer to show the register link along with the login link always -
it doesn't make sense to click on a 'login' link if you know you don't
have an account (or don't know that you don't need one, in the case of
OpenID).

For OpenID-enabled patchwork instances, the registration link should
still be there, but could potentially direct to a login form, explaining
the use of OpenID.

Cheers,


Jeremy
Guilherme Salgado April 19, 2011, 12:36 p.m. UTC | #2 
On Tue, 2011-04-19 at 14:10 +0800, Jeremy Kerr wrote:
> Hi Guilherme,
> 
> > This basically just adds instructions for people who may want to have their
> > patchwork instance as an OpenID relying party.  The only code changes are the
> > addition of the login_url() context processor, so that we always use the
> > LOGIN_URL defined in the settings,
> 
> Looks good, just one thing:
> 
> >  and the moving of the link to register from
> > the base template to the /accounts/login page.  The rationale for the latter
> > change is that the register link doesn't make sense on instances that use
> > OpenID, but I think that change is reasonable as registering on a given
> > instance is not something people will do often, so there's no reason to show a
> > link to that on every page, when the user is not logged in.
> 
> I'd prefer to show the register link along with the login link always -
> it doesn't make sense to click on a 'login' link if you know you don't
> have an account (or don't know that you don't need one, in the case of
> OpenID).

Fair enough.

> 
> For OpenID-enabled patchwork instances, the registration link should
> still be there, but could potentially direct to a login form, explaining
> the use of OpenID.

The only caveat is that if patchwork is configured to use a fixed OpenID
provider, there will be no login form as clicking the 'login' link will
take the user straight to the fixed OpenID provider, so I've created a
new page to take users to explain that to users and tell them to just
login. I'll submit a second version of this patch shortly.

Cheers,
diff mbox

Patch

diff --git a/apps/patchwork/context_processors.py b/apps/patchwork/context_processors.py
index f4ab5a9..b458eef 100644
--- a/apps/patchwork/context_processors.py
+++ b/apps/patchwork/context_processors.py
@@ -17,9 +17,9 @@ 
 # along with Patchwork; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
+from django.conf import settings
 
 from patchwork.models import Bundle
-from patchwork.utils import order_map, get_order
 
 def bundle(request):
     user = request.user
@@ -28,5 +28,5 @@  def bundle(request):
     return {'bundles': Bundle.objects.filter(owner = user)}
 
 
-def patchlists(request):
-
+def login_url(request):
+    return dict(login_url=settings.LOGIN_URL)
diff --git a/apps/settings.py b/apps/settings.py
index fd234af..5986cef 100644
--- a/apps/settings.py
+++ b/apps/settings.py
@@ -67,6 +67,18 @@  ROOT_URLCONF = 'apps.urls'
 LOGIN_URL = '/accounts/login'
 LOGIN_REDIRECT_URL = '/user/'
 
+# To make your Patchwork instance an OpenID relying party, you need to
+# uncomment the lines below in your local_settings.py, and
+#  - Add 'django_openid_auth' to INSTALLED_APPS;
+#  - Add 'django_openid_auth.auth.OpenIDBackend' to AUTHENTICATION_BACKENDS;
+#  - Uncomment the '^openid/' url pattern in apps/urls.py
+# OPENID_CREATE_USERS = True
+# OPENID_UPDATE_DETAILS_FROM_SREG = True
+# LOGIN_URL = '/openid/login/'
+# The line below is optional and will cause the given URL to be always used as
+# the OpenID provider, so users won't have to enter their identity URL.
+# OPENID_SSO_SERVER_URL = 'https://login.launchpad.net/'
+
 # If you change the ROOT_DIR setting in your local_settings.py, you'll need to
 # re-define the variables that use this (MEDIA_ROOT and TEMPLATE_DIRS) too.
 ROOT_DIR = '/srv/patchwork'
@@ -85,7 +97,9 @@  TEMPLATE_CONTEXT_PROCESSORS = (
     "django.core.context_processors.auth",
     "django.core.context_processors.debug",
     "django.core.context_processors.i18n",
-    "django.core.context_processors.media")
+    "django.core.context_processors.media",
+    "patchwork.context_processors.login_url",
+    )
 
 AUTH_PROFILE_MODULE = "patchwork.userprofile"
 
diff --git a/apps/urls.py b/apps/urls.py
index 3894708..619ebdf 100644
--- a/apps/urls.py
+++ b/apps/urls.py
@@ -40,6 +40,8 @@  urlpatterns = patterns('',
         name='registration_register'),
 
     (r'^accounts/', include('registration.urls')),
+    # Uncomment the line below to use OpenID for authentication.
+    # (r'^openid/', include('django_openid_auth.urls')),
 
     # Uncomment this for admin:
      (r'^admin/', include(admin.site.urls)),
diff --git a/docs/INSTALL b/docs/INSTALL
index ee87e4d..d482a3c 100644
--- a/docs/INSTALL
+++ b/docs/INSTALL
@@ -92,6 +92,21 @@  in brackets):
          cd ../python
          ln -s ../packages/django-registration/registration ./registration
 
+        Two other libraries we may use, in case you use OpenID for
+        authentication, are django-openid-auth and the Python OpenID library.
+        The former is named python-django-openid-auth in Debian/Ubuntu and the
+        latter python-openid, but if they're not available in your
+        distribution, you can follow the steps below to get them:
+
+         cd lib/packages
+         wget http://launchpad.net/django-openid-auth/trunk/0.3/+download/django-openid-auth-0.3.tar.gz
+         wget --no-check-certificate https://github.com/openid/python-openid/tarball/2.2.5 -O python-openid-2.2.5.tgz
+         tar zxvf django-openid-auth-0.3.tar.gz
+         tar zxvf python-openid-2.2.5.tgz
+         cd ../python
+         ln -s ../packages/django-openid-auth-0.3/django_openid_auth ./django_openid_auth
+         ln -s ../packages/openid-python-openid-b666238/openid ./openid
+
         We also use some Javascript libraries:
 
          cd lib/packages
@@ -144,9 +159,15 @@  in brackets):
 
         Postgresql:
           psql -f lib/sql/grant-all.postgres.sql patchwork
+          # If your instance uses OpenID for authentication, you'll also need
+          # the following line
+          psql -f lib/sql/grant-openid.postgres.sql patchwork
 
         MySQL:
           mysql patchwork < lib/sql/grant-all.mysql.sql
+          # If your instance uses OpenID for authentication, you'll also need
+          # the following line
+          mysql patchwork < lib/sql/grant-openid.mysql.sql
 
 
 3. Apache setup
diff --git a/htdocs/css/style.css b/htdocs/css/style.css
index 2b9f770..cbb2670 100644
--- a/htdocs/css/style.css
+++ b/htdocs/css/style.css
@@ -469,6 +469,10 @@  div.box table.vertical {
 	margin-right: auto;
 }
 
+.bigger-font {
+	font-size: 120%
+}
+
 /* columns */
 .leftcol {
 	float: left;
diff --git a/lib/sql/grant-openid.mysql.sql b/lib/sql/grant-openid.mysql.sql
new file mode 100644
index 0000000..9a7edbf
--- /dev/null
+++ b/lib/sql/grant-openid.mysql.sql
@@ -0,0 +1,7 @@ 
+BEGIN;
+GRANT SELECT, UPDATE, INSERT, DELETE ON django_openid_auth_nonce TO 'www-data'@localhost;
+GRANT SELECT, UPDATE, INSERT, DELETE ON django_openid_auth_useropenid TO 'www-data'@localhost;
+GRANT SELECT, UPDATE, INSERT, DELETE ON django_openid_auth_association TO 'www-data'@localhost;
+
+COMMIT;
+
diff --git a/lib/sql/grant-openid.postgres.sql b/lib/sql/grant-openid.postgres.sql
new file mode 100644
index 0000000..e854f17
--- /dev/null
+++ b/lib/sql/grant-openid.postgres.sql
@@ -0,0 +1,15 @@ 
+BEGIN;
+-- give necessary permissions to the web server. Becuase the admin is all
+-- web-based, these need to be quite permissive
+GRANT SELECT, UPDATE, INSERT, DELETE ON
+	django_openid_auth_nonce,
+	django_openid_auth_useropenid,
+	django_openid_auth_association,
+TO "www-data";
+GRANT SELECT, UPDATE ON
+	django_openid_auth_association_id_seq,
+	django_openid_auth_nonce_id_seq,
+	django_openid_auth_useropenid_id_seq,
+TO "www-data";
+
+COMMIT;
diff --git a/templates/base.html b/templates/base.html
index e14470e..780c103 100644
--- a/templates/base.html
+++ b/templates/base.html
@@ -28,9 +28,9 @@ 
      <a href="{% url patchwork.views.user.profile %}">profile</a> ::
      <a href="{% url auth_logout %}">logout</a>
 {% else %}
-     <a href="{% url auth_login %}">login</a>
+     <a href="{{ login_url }}">login</a>
      <br/>
-     <a href="{% url registration_register %}">register</a>
+     <span>&nbsp;</span>
 {% endif %}
    </div>
    <div style="clear: both;"></div>
diff --git a/templates/registration/login.html b/templates/registration/login.html
index 2dfc2a7..0d5a465 100644
--- a/templates/registration/login.html
+++ b/templates/registration/login.html
@@ -22,6 +22,12 @@ 
    <input type="submit" value="Login"/>
   </td>
  </tr>
+ <tr>
+  <td colspan="2" class="bigger-font">
+   Don't have an account yet?
+   <a href="{% url registration_register %}">Create one now</a>
+  </td>
+ </tr>
 </table>
 </form>
 {% endblock %}