AngularJS shopping cart using local storage

Two weeks ago I started writing some Javascript and AngularJS code for a customer. As a proud newby I must also be aware of the dangers on the big bad Internet. In particular vulnerabilities such as XSS attacks.

I’ve already performed a vulnerability scan and this showed I didn’t restrict session cookies to httponly. However nothing about the Javascript or AngularJS scripting.

My familiarity with JavaScript and AngularJS is somewhat limited.

var kvv = angular.module('kvv', ['ngStorage']);

kvv.controller('CartController', ["$scope", "$localStorage", "$sessionStorage", "$timeout", function($scope, $localStorage, $sessionStorage, $timeout) {

if ($localStorage.items === undefined) {
    $localStorage.items = [];
};

$scope.localStorage = $localStorage

$scope.remove = function(index) {
  $localStorage.items.splice(index, 1);
};

$scope.checked = false;

$scope.addToCart = function(index, title, desc, price, timeout) {

  $scope.checked = true;
  var found = false;
  angular.forEach($localStorage.items, function(items) {
      if (items.id  === index) {
        $timeout(function() {
          (items.quantity++);
          $scope.checked = false;
        }, 750);
        found = true;
    }
  });
  if (!found) {
    $timeout(function() {
        $localStorage.items.push(angular.extend({
        id: index,
        title: title,
        quantity: 1,
        price: price}, index))
        $scope.checked = false;
        },750);
    }
};

$scope.itemWithoutBtw = function(index) {
    var itemWithoutBtw = 0;
    angular.forEach($localStorage.items, function(items) {
        itemWithoutBtw += items.price / 106 * 100;
    })
    return itemWithoutBtw;
};

$scope.total = function(index) {
        var total = 0;
        angular.forEach($localStorage.items, function(items) {
            total += items.quantity * items.price;
        })
        return total;
};

$scope.totalBtw = function(index) {
        var totalBtw = 0;
        var total = $scope.total();
        angular.forEach($localStorage.items, function(items) {
            totalBtw = total / 106 * 6;
        })
        return totalBtw;
};

$scope.totalWithBtw = function(index) {
        var totalWithBtw = 0;
        angular.forEach($localStorage.items, function(items) {
            totalWithBtw += (items.quantity * items.price) + (items.quantity * items.price  / 100 * 6);
        })
        return totalWithBtw;
};
$scope.totalWithoutBtw = function(index) {
        var total = $scope.total();
        var totalBtw = $scope.totalBtw();
        var totalWithoutBtw = 0;
        angular.forEach($localStorage.items, function(items) {
            totalWithoutBtw = total - totalBtw;
        })
        return totalWithoutBtw;
};

$scope.orderTotal = function(index) {
        var orderTotal = 0;
        angular.forEach($localStorage.items, function(items) {
            orderTotal += items.quantity;
        })
        return orderTotal;
};

$scope.orderDelete = function(index) {
    delete $localStorage.items;
    $localStorage.items = [];
    $('html, body').animate({scrollTop: 0}, 2500);
};

}]);

This is interesting,

you have quite a bit business logic in your code there..
If I were try to break this, I would definitely change the price of the items I buy in the local storage to 1 cent and see if the server side would accept that price. (It should not, and recalculate any pricing from item master data). Next I would place a breakpoint in some of your btw related routines and make it return 1 cent and see if the server side could be broken that way.

Other than that, I see no security flaws in there. Let me know if you want me to do a review of other aspects of your code.

Couple of more points –

1. Why have you given time delay of 750 to your timeout function. It might cause multiple duplicate items (same items might show up as different individual row instead of incremented count within one row) on UI if user keeps clicking on “add item” (which triggers addToCart scope function) frequently.
2. Why are you keeping reference of localStorage at scope level? Are you using it anywhere in scope ? (referring to this – $scope.localStorage = $localStorage)
3. You can improve time complexities of following scope methods (itemWithoutBtw, total, totalBtw etc) and avoid O(n) iteration if you modify your add / remove code diligently. If you still want to iterate on entire cart from code maintainability aspect, you can introduce one variable to read cart state, and cache these values to avoid unnecessary iteration on entire cart until cart state changes.

$scope.itemWithoutBtw = 0;
$scope.addToCart = function(index, title, desc, price, timeout) {
  // Same code as of above, removing this to save space here, Below I wrote for single quantity case. You can write similar logic if item already exists.
  if (!found) {
    $timeout(function() {
        $localStorage.items.push(angular.extend({
        id: index,
        title: title,
        quantity: 1,
        price: price}, index))
        // Below line is added here additionally..
        $scope.itemWithoutBtw += price / 106 * 100;
        $scope.checked = false;
        },750);
    }
};

// You dont need this function and can use scope var directly.
$scope.itemWithoutBtw = function(index) {
    return $scope.itemWithoutBtw;
};

4. Supporting @konijn point – if you want to store price in localStorage, ensure that you have server side logic to override price and other sensitive cart item details to avoid hacks there. Try showing up overridden price to user as well ( by syncning with server ) when you user is about to pay. That ways you will lead to a better user experience and less dissatisfaction among users.