Merge lp:~dooferlad/linaro-license-protection/logout-link-on-openid-denied into lp:~linaro-automation/linaro-license-protection/trunk

Proposed by James Tunnicliffe
Status: Merged
Approved by: James Tunnicliffe
Approved revision: 113
Merged at revision: 114
Proposed branch: lp:~dooferlad/linaro-license-protection/logout-link-on-openid-denied
Merge into: lp:~linaro-automation/linaro-license-protection/trunk
Diff against target: 91 lines (+57/-5)
3 files modified
license_protected_downloads/openid_auth.py (+23/-4)
license_protected_downloads/tests/test_openid_auth.py (+17/-1)
templates/openid_forbidden_template.html (+17/-0)
To merge this branch: bzr merge lp:~dooferlad/linaro-license-protection/logout-link-on-openid-denied
Reviewer Review Type Date Requested Status
Stevan Radaković Approve
Review via email: mp+120995@code.launchpad.net

Description of the change

Add a custom HTTP response forbidden page when we fail to be part of the correct Launchpad team to allow access to a file. The page lists the teams that you need to be a member of and allows you to log in again to give you the opportunity to make sure that you share the right team membership to give you file access (if you have it).

To post a comment you must log in.
Revision history for this message
Stevan Radaković (stevanr) wrote :

Since you're using template, I'd rather you just pass the openid_teams as an array to the template and construct the teams string over there... Does this sound viable?

review: Needs Fixing
Revision history for this message
Stevan Radaković (stevanr) wrote :

Approving, as agreed in the infra hangout.

review: Approve
Revision history for this message
James Tunnicliffe (dooferlad) wrote :

As discussed a moment ago, it may be possible, but in terms of what I can do quickly and us wanting to release soon, I think it is reasonable to mark that as possible future work.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'license_protected_downloads/openid_auth.py'
--- license_protected_downloads/openid_auth.py 2012-07-11 13:13:56 +0000
+++ license_protected_downloads/openid_auth.py 2012-08-23 13:40:54 +0000
@@ -1,8 +1,7 @@
1from django.conf import settings1from django.conf import settings
2from django.shortcuts import redirect2from django.shortcuts import redirect, render_to_response
3
4from django.http import HttpResponseForbidden
5from django.contrib.auth.models import Group3from django.contrib.auth.models import Group
4import bzr_version
65
76
8class OpenIDAuth:7class OpenIDAuth:
@@ -24,4 +23,24 @@
24 if group.name in openid_teams:23 if group.name in openid_teams:
25 return None24 return None
2625
27 return HttpResponseForbidden("Not Authorized")26 # Construct a nice string of openid teams that will allow access to
27 # the requested file
28 if len(openid_teams) > 1:
29 teams_string = "one of the " + openid_teams.pop(0) + " "
30 if len(openid_teams) > 1:
31 teams_string += ", ".join(openid_teams[0:-1])
32
33 teams_string += " or " + openid_teams[-1] + " teams"
34 else:
35 teams_string = "the " + openid_teams[0] + " team"
36
37 response = render_to_response(
38 'openid_forbidden_template.html',
39 {'login': settings.LOGIN_URL + "?next=" + request.path,
40 'authenticated': request.user.is_authenticated(),
41 'openid_teams': teams_string,
42 'revno': bzr_version.get_my_bzr_revno(),
43 })
44
45 response.status_code = 403
46 return response
2847
=== modified file 'license_protected_downloads/tests/test_openid_auth.py'
--- license_protected_downloads/tests/test_openid_auth.py 2012-07-11 13:13:56 +0000
+++ license_protected_downloads/tests/test_openid_auth.py 2012-08-23 13:40:54 +0000
@@ -57,7 +57,23 @@
57 response = OpenIDAuth.process_openid_auth(mock_request, openid_teams)57 response = OpenIDAuth.process_openid_auth(mock_request, openid_teams)
58 self.assertIsNotNone(response)58 self.assertIsNotNone(response)
59 self.assertTrue(isinstance(response, HttpResponse))59 self.assertTrue(isinstance(response, HttpResponse))
60 self.assertEquals(403, response.status_code)60 self.assertContains(response,
61 "You need to be the member of the linaro team in Launchpad.",
62 status_code=403)
63
64 def test_check_no_team_membership_authed_multi_teams(self):
65 mock_request = self.make_mock_request()
66 mock_request.user.is_authenticated.return_value = True
67 mock_request.user.groups.all.return_value = [
68 self.make_mock_group("another-group")]
69 openid_teams = ["linaro", "batman", "catwoman", "joker"]
70 response = OpenIDAuth.process_openid_auth(mock_request, openid_teams)
71 self.assertIsNotNone(response)
72 self.assertTrue(isinstance(response, HttpResponse))
73 self.assertContains(response,
74 "You need to be the member of one of the linaro batman, catwoman "
75 "or joker teams in Launchpad.",
76 status_code=403)
6177
62 @patch("django.contrib.auth.models.Group.objects.get_or_create")78 @patch("django.contrib.auth.models.Group.objects.get_or_create")
63 def test_auto_adding_groups(self, get_or_create_mock):79 def test_auto_adding_groups(self, get_or_create_mock):
6480
=== added file 'templates/openid_forbidden_template.html'
--- templates/openid_forbidden_template.html 1970-01-01 00:00:00 +0000
+++ templates/openid_forbidden_template.html 2012-08-23 13:40:54 +0000
@@ -0,0 +1,17 @@
1{% extends "header.html" %}
2
3{% block content %}
4 <h1>Forbidden</h1>
5 <p>
6 You do not have the correct group membership to access this file.
7
8 You need to be the member of {{ openid_teams }} in Launchpad.
9
10 If you think you should have access, you can try logging in again and check that
11 one of the required group memberships are selected.
12 </p>
13 <p>
14 <a href="{{ login }}">Login again</a>
15 </p>
16</table>
17{% endblock %}

Subscribers

People subscribed via source and target branches