From 09b5679ee79c59a71888e4f403c5ad0f94a891e1 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
Date: Mon, 14 Oct 2013 18:01:04 +0200
Subject: [PATCH] Clean up the authorization code a bit

---
 src/lib/Hydra/Controller/Project.pm   |  6 ++---
 src/lib/Hydra/Controller/User.pm      |  6 ++---
 src/lib/Hydra/Helper/CatalystUtils.pm | 39 +++++++++++++++++----------
 3 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/src/lib/Hydra/Controller/Project.pm b/src/lib/Hydra/Controller/Project.pm
index f2cede92..04c9771a 100644
--- a/src/lib/Hydra/Controller/Project.pm
+++ b/src/lib/Hydra/Controller/Project.pm
@@ -114,10 +114,8 @@ sub edit : Chained('projectChain') PathPart Args(0) {
 
 sub requireMayCreateProjects {
     my ($c) = @_;
-
-    requireLogin($c) if !$c->user_exists;
-
-    error($c, "Only administrators or authorised users can perform this operation.")
+    requireUser($c);
+    accessDenied($c, "Only administrators or authorised users can perform this operation.")
         unless $c->check_user_roles('admin') || $c->check_user_roles('create-projects');
 }
 
diff --git a/src/lib/Hydra/Controller/User.pm b/src/lib/Hydra/Controller/User.pm
index e4f0ecc4..c43f7d16 100644
--- a/src/lib/Hydra/Controller/User.pm
+++ b/src/lib/Hydra/Controller/User.pm
@@ -150,7 +150,7 @@ sub currentUser :Path('/current-user') :ActionClass('REST') { }
 sub currentUser_GET {
     my ($self, $c) = @_;
 
-    requireLogin($c) if !$c->user_exists;
+    requireUser($c);
 
     $self->status_ok(
         $c,
@@ -166,9 +166,9 @@ sub currentUser_GET {
 sub user :Chained('/') PathPart('user') CaptureArgs(1) {
     my ($self, $c, $userName) = @_;
 
-    requireLogin($c) if !$c->user_exists;
+    requireUser($c);
 
-    error($c, "You do not have permission to edit other users.")
+    accessDenied($c, "You do not have permission to edit other users.")
         if $userName ne $c->user->username && !isAdmin($c);
 
     $c->stash->{user} = $c->model('DB::Users')->find($userName)
diff --git a/src/lib/Hydra/Helper/CatalystUtils.pm b/src/lib/Hydra/Helper/CatalystUtils.pm
index 9f257418..b58c988e 100644
--- a/src/lib/Hydra/Helper/CatalystUtils.pm
+++ b/src/lib/Hydra/Helper/CatalystUtils.pm
@@ -15,8 +15,8 @@ use feature qw/switch/;
 our @ISA = qw(Exporter);
 our @EXPORT = qw(
     getBuild getPreviousBuild getNextBuild getPreviousSuccessfulBuild
-    error notFound
-    requireLogin requireProjectOwner requireAdmin requirePost isAdmin isProjectOwner
+    error notFound accessDenied
+    forceLogin requireUser requireProjectOwner requireAdmin requirePost isAdmin isProjectOwner
     trim
     getLatestFinishedEval
     sendEmail
@@ -102,6 +102,13 @@ sub notFound {
 }
 
 
+sub accessDenied {
+    my ($c, $msg) = @_;
+    $c->response->status(403);
+    error($c, $msg);
+}
+
+
 sub backToReferer {
     my ($c) = @_;
     $c->response->redirect($c->session->{referer} || $c->uri_for('/'));
@@ -110,7 +117,7 @@ sub backToReferer {
 }
 
 
-sub requireLogin {
+sub forceLogin {
     my ($c) = @_;
     $c->session->{referer} = $c->request->uri;
     $c->response->redirect($c->uri_for('/login'));
@@ -118,36 +125,40 @@ sub requireLogin {
 }
 
 
+sub requireUser {
+    my ($c) = @_;
+    forceLogin($c) if !$c->user_exists;
+}
+
+
 sub isProjectOwner {
     my ($c, $project) = @_;
-
-    return $c->user_exists && ($c->check_user_roles('admin') || $c->user->username eq $project->owner->username || defined $c->model('DB::ProjectMembers')->find({ project => $project, userName => $c->user->username }));
+    return
+        $c->user_exists &&
+        (isAdmin($c) ||
+         $c->user->username eq $project->owner->username ||
+         defined $c->model('DB::ProjectMembers')->find({ project => $project, userName => $c->user->username }));
 }
 
 
 sub requireProjectOwner {
     my ($c, $project) = @_;
-
-    requireLogin($c) if !$c->user_exists;
-
-    error($c, "Only the project members or administrators can perform this operation.")
+    requireUser($c);
+    accessDenied($c, "Only the project members or administrators can perform this operation.")
         unless isProjectOwner($c, $project);
 }
 
 
 sub isAdmin {
     my ($c) = @_;
-
     return $c->user_exists && $c->check_user_roles('admin');
 }
 
 
 sub requireAdmin {
     my ($c) = @_;
-
-    requireLogin($c) if !$c->user_exists;
-
-    error($c, "Only administrators can perform this operation.")
+    requireUser($c);
+    accessDenied($c, "Only administrators can perform this operation.")
         unless isAdmin($c);
 }