From 2dce192abe2d495c95816f05dd5a564306aa4b46 Mon Sep 17 00:00:00 2001
From: June Kelly <june@junek.xyz>
Date: Tue, 24 Aug 2021 09:54:22 +0100
Subject: [PATCH] Merge pull request #4672 from
 overleaf/sk-validate-currency-param

Subscription: validate currency in query param
GitOrigin-RevId: 0c9f841ba56b5ce85bbd2adeb3fb2d45d0ad753a
---
 .../Features/Subscription/SubscriptionController.js  | 10 +++++++---
 services/web/app/src/infrastructure/GeoIpLookup.js   | 10 ++++++++++
 .../src/Subscription/SubscriptionControllerTests.js  | 11 +++++++++++
 .../test/unit/src/infrastructure/GeoIpLookupTest.js  | 12 ++++++++++++
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js
index 19864fb2bb..18dcb23624 100644
--- a/services/web/app/src/Features/Subscription/SubscriptionController.js
+++ b/services/web/app/src/Features/Subscription/SubscriptionController.js
@@ -63,9 +63,13 @@ async function paymentPage(req, res) {
     if (!valid) {
       res.redirect('/user/subscription?hasSubscription=true')
     } else {
-      let currency = req.query.currency
-        ? req.query.currency.toUpperCase()
-        : undefined
+      let currency = null
+      if (req.query.currency) {
+        const queryCurrency = req.query.currency.toUpperCase()
+        if (GeoIpLookup.isValidCurrencyParam(queryCurrency)) {
+          currency = queryCurrency
+        }
+      }
       const {
         currencyCode: recommendedCurrency,
         countryCode,
diff --git a/services/web/app/src/infrastructure/GeoIpLookup.js b/services/web/app/src/infrastructure/GeoIpLookup.js
index 94bb5a0d66..4c9c56c3d8 100644
--- a/services/web/app/src/infrastructure/GeoIpLookup.js
+++ b/services/web/app/src/infrastructure/GeoIpLookup.js
@@ -17,6 +17,8 @@ const currencyMappings = {
   SE: 'SEK',
 }
 
+const validCurrencyParams = Object.values(currencyMappings).concat(['EUR'])
+
 // Countries which would likely prefer Euro's
 const EuroCountries = [
   'AT',
@@ -48,6 +50,13 @@ const EuroCountries = [
 
 _.each(EuroCountries, country => (currencyMappings[country] = 'EUR'))
 
+function isValidCurrencyParam(currency) {
+  if (!currency) {
+    return false
+  }
+  return validCurrencyParams.includes(currency)
+}
+
 function getDetails(ip, callback) {
   if (!ip) {
     return callback(new Error('no ip passed'))
@@ -89,6 +98,7 @@ function getCurrencyCode(ip, callback) {
 module.exports = {
   getDetails,
   getCurrencyCode,
+  isValidCurrencyParam,
   promises: {
     getDetails: promisify(getDetails),
     getCurrencyCode: promisifyMultiResult(getCurrencyCode, [
diff --git a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
index e334841f13..1f3e03388a 100644
--- a/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
+++ b/services/web/test/unit/src/Subscription/SubscriptionControllerTests.js
@@ -108,6 +108,7 @@ describe('SubscriptionController', function () {
       gaExperiments: {},
     }
     this.GeoIpLookup = {
+      isValidCurrencyParam: sinon.stub().returns(true),
       getCurrencyCode: sinon.stub(),
       promises: {
         getCurrencyCode: sinon.stub(),
@@ -263,6 +264,16 @@ describe('SubscriptionController', function () {
         }
         this.SubscriptionController.paymentPage(this.req, this.res)
       })
+
+      it('should use the geo ip currency if not valid', function (done) {
+        this.req.query.currency = 'WAT'
+        this.GeoIpLookup.isValidCurrencyParam.returns(false)
+        this.res.render = (page, opts) => {
+          opts.currency.should.equal(this.stubbedCurrencyCode)
+          done()
+        }
+        this.SubscriptionController.paymentPage(this.req, this.res)
+      })
     })
 
     describe('with a recurly subscription already', function () {
diff --git a/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js b/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js
index 45e0ba0fa3..473546e92f 100644
--- a/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js
+++ b/services/web/test/unit/src/infrastructure/GeoIpLookupTest.js
@@ -41,6 +41,18 @@ describe('GeoIpLookup', function () {
     }
   })
 
+  describe('isValidCurrencyParam', function () {
+    it('should reject invalid currency codes', function () {
+      expect(this.GeoIpLookup.isValidCurrencyParam('GBP')).to.equal(true)
+      expect(this.GeoIpLookup.isValidCurrencyParam('USD')).to.equal(true)
+      expect(this.GeoIpLookup.isValidCurrencyParam('AUD')).to.equal(true)
+      expect(this.GeoIpLookup.isValidCurrencyParam('EUR')).to.equal(true)
+      expect(this.GeoIpLookup.isValidCurrencyParam('WAT')).to.equal(false)
+      expect(this.GeoIpLookup.isValidCurrencyParam('NON')).to.equal(false)
+      expect(this.GeoIpLookup.isValidCurrencyParam('LOL')).to.equal(false)
+    })
+  })
+
   describe('getDetails', function () {
     beforeEach(function () {
       this.request.get.callsArgWith(1, null, null, this.stubbedResponse)