Skip to content

Commit

Permalink
fix(errors): back button works on 403
Browse files Browse the repository at this point in the history
This commit refactors the error pages to use the same folders and test
structure.  It moves some of the CSS to a central place and makes sure
that authentication works appropriately.

The most important change is that ALL states are sent to the client
along with their authorization status.  This is so that the app can tell
the difference between a 404 and a 403 effectively.

Closes #846.
  • Loading branch information
Jonathan Niles authored and sfount committed Nov 19, 2016
1 parent 4b7e3a4 commit 97afc22
Show file tree
Hide file tree
Showing 12 changed files with 210 additions and 172 deletions.
8 changes: 8 additions & 0 deletions client/src/css/structure.css
Original file line number Diff line number Diff line change
Expand Up @@ -802,3 +802,11 @@ growl-notification.fading.ng-leave.ng-leave-active {
.modal-tall {
max-height: 80vh;
}

/* 403 and 404 error pages */
.content-error {
margin-top : calc(50vh - 190px);
max-width: 400px;
margin-right: auto;
margin-left: auto
}
75 changes: 45 additions & 30 deletions client/src/js/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var bhima = angular.module('bhima', [
'growlNotifications', 'ngAnimate', 'ngSanitize', 'ui.select'
]);

function bhimaConfig($stateProvider, $urlRouterProvider, $urlMatcherFactoryProvider) {
function bhimaConfig($stateProvider, $urlMatcherFactoryProvider) {

// allow trailing slashes in routes
$urlMatcherFactoryProvider.strictMode(false);
Expand Down Expand Up @@ -221,16 +221,15 @@ function bhimaConfig($stateProvider, $urlRouterProvider, $urlMatcherFactoryProvi
templateUrl: 'partials/admin/transaction_type/transaction_type.html'
})

.state('error403', {
url : '/error403',
templateUrl : 'partials/error403/error403.html'
.state('403', {
templateUrl : 'partials/errors/403.html'
})
.state('error404', {
url : '/error404',
templateUrl : 'partials/error404/error404.html'
});

$urlRouterProvider.otherwise('error404');
// this is a catch-all state. It matches all URLs and preserves the URL in the top bar.
.state('404', {
url: '{path:.*}',
templateUrl : 'partials/errors/404.html'
});
}

function translateConfig($translateProvider) {
Expand Down Expand Up @@ -261,10 +260,6 @@ function startupConfig($rootScope, $state, $uibModalStack, SessionService, amMom
var isLoggedIn = !!SessionService.user;
var isLoginState = next.indexOf('#/login') !== -1;

if (next.indexOf('/error403') !== -1) {
$state.go('/error403');
}

// if the user is logged in and trying to access the login state, deny the
// attempt with a message "Cannot return to login. Please log out from the
// Settings Page."
Expand All @@ -289,35 +284,55 @@ function startupConfig($rootScope, $state, $uibModalStack, SessionService, amMom
// trigger a $state.go() to the login state, it will not be stopped - the
// $locationChangeStart event will only prevent the URL from changing ... not
// the actual state transition! So, we need this to stop $stateChange events.

// var paths recovered all the path that the user is allowed to enter
// Tests if the path has elements and other common paths are not called
// if the test is positive, the current path is verified in the path list
// if the current path does not exist in the path list in this case the user will rédirrigé to error403 page

$rootScope.$on('$stateChangeStart', function (event, next) {
var isLoggedIn = !!SessionService.user;
var isLoginState = next.name.indexOf('login') !== -1;

if (isLoggedIn && isLoginState) {
event.preventDefault();
Notify.warn('AUTH.CANNOT_RETURN_TO_LOGIN');
return;
}

// check if we are going to an error state;
var isErrorState = (
next.name.indexOf('404') !== -1 ||
next.name.indexOf('403') !== -1
);

var currentPath = $location.$$path;
var paths = SessionService.path;
// pass through
if (isErrorState) {
console.log('Going to Error State');
return;
}

var publicRoutes = ['/', '/settings', '/login', '/error404', '/error403'];
console.log('checking state...', next);

if (paths && publicRoutes.indexOf(currentPath) === -1) {
var authorized = paths.some(function (data) {
return currentPath.indexOf(data.path) === 0;
});
// verify that the user is authorized to go to the next state

if (!authorized) {
$location.path('/error403');
}
// get the current hash

// strip the question mark
var path = $location.path();

console.log('path', path);

var paths = SessionService.paths;
var publicRoutes = ['/', '/settings', '/login'];

var isPublicPath = publicRoutes.indexOf(path) > -1;

// pass through
if (!paths || isPublicPath) { return; }

// check authorization
var authorized = paths.some(function (data) {
return path.indexOf(data.path) === 0 && data.authorized;
});

if (!authorized) {
event.preventDefault();
$state.go('403', {}, { location : 'replace' });
}
});

Expand Down Expand Up @@ -443,7 +458,7 @@ function uiSelectConfig(uiSelectConfig) {
bhima.constant('bhConstants', constantConfig());

// configure services, providers, factories
bhima.config(['$stateProvider', '$urlRouterProvider', '$urlMatcherFactoryProvider', bhimaConfig]);
bhima.config(['$stateProvider', '$urlMatcherFactoryProvider', bhimaConfig]);
bhima.config(['$translateProvider', translateConfig]);
bhima.config(['uiSelectConfig', uiSelectConfig]);
bhima.config(['tmhDynamicLocaleProvider', localeConfig]);
Expand Down
11 changes: 6 additions & 5 deletions client/src/js/services/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@ function SessionService($sessionStorage, $http, $location, util, $rootScope) {

// set the user, enterprise, and project for the session
// this should happen right after login
function create(user, enterprise, project, path) {
function create(user, enterprise, project, paths) {
$storage.user = user;
$storage.enterprise = enterprise;
$storage.project = project;
$storage.path = path;
$storage.paths = paths;

// update bindings
load();
Expand All @@ -55,7 +55,7 @@ function SessionService($sessionStorage, $http, $location, util, $rootScope) {
delete $storage.user;
delete $storage.enterprise;
delete $storage.project;
delete $storage.path;
delete $storage.paths;

// update bindings
load();
Expand All @@ -75,8 +75,9 @@ function SessionService($sessionStorage, $http, $location, util, $rootScope) {
return $http.post('/login', credentials)
.then(util.unwrapHttpResponse)
.then(function (session) {

// create the user session in the $storage
create(session.user, session.enterprise, session.project, session.path);
create(session.user, session.enterprise, session.project, session.paths);

// navigate to the main page
$location.url('/');
Expand Down Expand Up @@ -113,7 +114,7 @@ function SessionService($sessionStorage, $http, $location, util, $rootScope) {
service.user = $storage.user;
service.enterprise = $storage.enterprise;
service.project = $storage.project;
service.path = $storage.path;
service.paths = $storage.paths;
}

// if the $rootScope emits 'session.destroy', destroy the session
Expand Down
19 changes: 0 additions & 19 deletions client/src/partials/error403/error403.html

This file was deleted.

19 changes: 0 additions & 19 deletions client/src/partials/error404/error404.html

This file was deleted.

24 changes: 24 additions & 0 deletions client/src/partials/errors/403.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<div class="flex-content">
<div class="container-fluid">
<div class="row">
<div class="col-md-4 col-md-offset-4">

<div class="panel panel-danger content-error" data-error="403">
<div class="panel-heading clearfix text-center text-danger">
<strong>
<i class="fa fa-danger"></i>
{{ "FORM.LABELS.ERROR_403" | translate }}
</strong>
</div>

<div class="panel-body text-center">
<strong> {{"FORM.WARNINGS.NOT_AUTHORISED" | translate}}</strong>
<div style="background-image:radial-gradient(#D487A1, #FFFFFF); font-size:500%; color:white; text-shadow: 0px 0px 9px #777;">
403
</div>
</div>
</div>
</div>
</div>
</div>
</div>
22 changes: 22 additions & 0 deletions client/src/partials/errors/404.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<div class="flex-content">
<div class="container-fluid" style="height: 100%;">
<div class="row">
<div class="col-md-4 col-md-offset-4">

<div class="panel panel-default content-error" data-error="404">
<div class="panel-heading clearfix text-center">
<strong>
<i class="fa fa-danger"></i>
<span class="text-danger">{{ "FORM.LABELS.ERROR_404" | translate }} </span>
</strong>
</div>

<div class="panel-body text-center">
<strong> {{"FORM.WARNINGS.PAGE_NOT_FOUND" | translate}}</strong>
<div style="background-image:radial-gradient(#CBCBCB, #FFFFFF); font-size:500%; color:white; text-shadow: 0px 0px 9px #777;"> 404 </div>
</div>
</div>
</div>
</div>
</div>
</div>
31 changes: 16 additions & 15 deletions server/controllers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,32 @@ function login(req, res, next) {
session.user = rows[0];

// next make sure this user has permissions
sql =
`SELECT p.user_id, p.unit_id, u.path
FROM permission AS p
JOIN unit AS u ON u.id = p.unit_id
WHERE p.user_id = ? `;
sql = `
SELECT IF(permission.user_id = ?, 1, 0) authorized, unit.path
FROM unit LEFT JOIN permission
ON unit.id = permission.unit_id;
`;

return db.exec(sql, [session.user.id]);
})
.then(function (rows) {
.then(modules => {

let unauthorized = modules.every(mod => !mod.authorized);

// if no permissions, notify the user that way
if (rows.length === 0) {
if (unauthorized) {
throw new Unauthorized('This user does not have any permissions.');
}

session.path = rows;
session.paths = modules;

// update the database for when the user logged in
sql =
'UPDATE user SET user.active = 1, user.last_login = ? WHERE user.id = ?;';

return db.exec(sql, [new Date(), session.user.id]);
})
.then(function () {
.then(() => {

// we need to construct the session on the client side, including:
// the current enterprise
Expand All @@ -105,9 +107,9 @@ function login(req, res, next) {

return db.exec(sql, [session.user.enterprise_id]);
})
.then(function (rows) {
.then(rows => {

if (rows.length === 0) {
if (!rows.length) {
throw new InternalServerError('There are no enterprises registered in the database!');
}

Expand All @@ -120,19 +122,18 @@ function login(req, res, next) {

return db.exec(sql, [session.user.project_id]);
})
.then(function (rows) {
if (rows.length === 0) {
.then(rows => {
if (!rows.length) {
throw new Unauthorized('No project matching the provided id.');
}

session.project = rows[0];


// bind the session variables
req.session.project = session.project;
req.session.user = session.user;
req.session.enterprise = session.enterprise;
req.session.path = session.path;
req.session.paths = session.paths;

// broadcast LOGIN event
Topic.publish(Topic.channels.APP, {
Expand Down
Loading

0 comments on commit 97afc22

Please sign in to comment.