Uploaded image for project: 'Magnolia Shop (closed)'
  1. Magnolia Shop (closed)
  2. MSHOP-136

Store real customer IP address in shopping carts

XMLWordPrintable

    • Icon: Improvement Improvement
    • Resolution: Won't Do
    • Icon: Neutral Neutral
    • None
    • 1.1.4
    • None
    • Yes

      In the current Worldsteel Shop Module the stored shopping carts do not include the real customer IP address for our (and most) real-world production environments. See screenshot.

      The reason is that the the module uses HttpServletRequest#remoteAddr() to get the IP address and in any real-world hosting environment this is typically not the customer's external IP address but rather an internal server's IP address. In our case it is always 127.0.0.1.

      The real customer IP address (or actually: a comma-separated list of IP addresses) is commonly stored by infrastructure components such as load balancers in the X-Forwarded-For HTTP request header. In order to use this header, if available, I suggest to replace the following code in SaveAndConfirmFormProcessor#internalProcess:

      cart.setUserIP(request.getRemoteAddr() + ":" + request.getRemotePort());
      

      by:

            String userIPAddress;
              // note: in most hosting environments HttpServletRequest#remoteAddr() gives the internal IP address of the request
              // (e.g. 127.0.0.1) and not the external IP address of the user.
              // The user's external IP address is commonly placed in the X-Forwarded-For HTTP request header so check for that.
              String xForwardForHeader = request.getHeader("X-Forwarded-For");
              if (null != xForwardForHeader) {
                userIPAddress = xForwardForHeader;
              } else {
                userIPAddress = request.getRemoteAddr();
              }
              cart.setUserIP(userIPAddress + ":" + request.getRemotePort());
      

      Do you agree? And if so, could you add it to the code base?

      In our project we could override the SaveAndConfirmFormProcessor#internalProcess and implement this ourselves but I would rather not because it is a complex method. Besides, this addition would benefit all users I think.

      Do note that this is a little hard to test because for a real test you need an environment where the X-Forwarded-For header is actually set.

        Acceptance criteria

              Unassigned Unassigned
              edgar Edgar Vonk
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Created:
                Updated:
                Resolved:

                  Task DoD