Merge pull request #2345 from hongwei1/develop

refactor/remove the basicUrlValidation, use basicUriAndQueryStringVal

obp-api/src/main/scala/code/api/util/APIUtil.scala

@@ -777,20 +777,6 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{
     }
   }
 
-  def basicUrlValidation(urlString: String): Boolean = {
-    //in scala test - org.scalatest.FeatureSpecLike.scenario: 
-    // redirectUrl = http%3A%2F%2Flocalhost%3A8016%3Foauth_token%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461
-    // URLDecoder.decode(urlString,"UTF-8")-->http://localhost:8016?oauth_token=EBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK&oauth_verifier=63461
-    val regex =
-      """((([A-Za-z]{3,9}:(?:\/\/)?)(?:[-;:&=\+\$,\w]+@)?[A-Za-z0-9.-]+(:[0-9]+)?|(?:www.|[-;:&=\+\$,\w]+@)[A-Za-z0-9.-]+)((?:\/[\+~%\/.\w-_]*)?\??(?:[-\+=&;%@.\w_\/]*)#?(?:[\w]*))?)""".r
-    val decodeUrlValue = URLDecoder.decode(urlString, "UTF-8").trim()
-    decodeUrlValue match {
-      case regex(_*) if (decodeUrlValue.length <= 2048) => true
-      case _ => false
-    }
-  }
-
-
   /** only  A-Z, a-z, 0-9,-,_,. =, & and max length <= 2048  */
   def basicUriAndQueryStringValidation(urlString: String): Boolean = {
     val regex =

obp-api/src/main/scala/code/snippet/OAuthWorkedThanks.scala

@@ -48,8 +48,10 @@ class OAuthWorkedThanks extends MdcLoggable {
     val redirectUrl = ObpS.param("redirectUrl").map(urlDecode(_))
     logger.debug(s"OAuthWorkedThanks.thanks.redirectUrl $redirectUrl")
     //extract the clean(omit the parameters) redirect url from request url
-    val requestedRedirectURL = Helper.extractCleanRedirectURL(redirectUrl.openOr("invalidRequestedRedirectURL")) openOr("invalidRequestedRedirectURL")
-    logger.debug(s"OAuthWorkedThanks.thanks.requestedRedirectURL $requestedRedirectURL")
+    val staticPortionOfRedirectUrl = Helper.getStaticPortionOfRedirectURL(redirectUrl.openOr("invalidRequestedRedirectURL")) openOr("invalidRequestedRedirectURL")
+    val hostOnlyOfRedirectUrlLegacy = Helper.getHostOnlyOfRedirectURL(staticPortionOfRedirectUrl) openOr("invalidRequestedRedirectURL")
+    logger.debug(s"OAuthWorkedThanks.thanks.staticPortionOfRedirectUrl $staticPortionOfRedirectUrl")
+    logger.debug(s"OAuthWorkedThanks.thanks.hostOnlyOfRedirectUrlLegacy $hostOnlyOfRedirectUrlLegacy")
 
     val requestedOauthToken = Helper.extractOauthToken(redirectUrl.openOr("No Oauth Token here")) openOr("No Oauth Token here")
     logger.debug(s"OAuthWorkedThanks.thanks.requestedOauthToken $requestedOauthToken")
@@ -62,13 +64,10 @@ class OAuthWorkedThanks extends MdcLoggable {
 
     redirectUrl match {
       case Full(url) =>
-        //this redirect url is checked by following, no open redirect issue.
-        // TODO maybe handle case of extra trailing / on the url ?
+        val incorrectRedirectUrlMessage =  s"The validRedirectURL is $validRedirectURL but the staticPortionOfRedirectUrl was $staticPortionOfRedirectUrl" 
 
-        val incorrectRedirectUrlMessage =  s"The validRedirectURL is $validRedirectURL but the requestedRedirectURL was $requestedRedirectURL"
-
-
-        if(validRedirectURL.equals(requestedRedirectURL)) {
+        //hostOnlyOfRedirectUrlLegacy is deprecated now, we use the staticPortionOfRedirectUrl stead,
+        if(validRedirectURL.equals(staticPortionOfRedirectUrl)|| validRedirectURL.equals(hostOnlyOfRedirectUrlLegacy)) {
           "#redirect-link [href]" #> url &
           ".app-name"#> appName //there may be several places to be modified in html, so here use the class, not the id.
         }else{

obp-api/src/main/scala/code/util/Helper.scala

@@ -1,9 +1,8 @@
 package code.util
 
-import java.net.{Socket, SocketException}
+import java.net.{Socket, SocketException, URL}
 import java.util.UUID.randomUUID
 import java.util.{Date, GregorianCalendar}
-
 import code.api.util.{APIUtil, CallContext, CallContextLight, CustomJsonFormats}
 import code.api.{APIFailureNewStyle, Constant}
 import code.api.util.APIUtil.fullBoxOrException
@@ -20,7 +19,9 @@ import com.openbankproject.commons.util.{ReflectUtils, RequiredFieldValidation,
 import com.tesobe.CacheKeyFromArguments
 import net.liftweb.http.S
 import net.liftweb.util.Helpers
+import net.liftweb.util.Helpers.tryo
 import net.sf.cglib.proxy.{Enhancer, MethodInterceptor, MethodProxy}
+
 import java.lang.reflect.Method
 import scala.concurrent.Future
 import scala.util.Random
@@ -167,8 +168,30 @@ object Helper extends Loggable {
     prettyRender(decompose(input))
   }
 
-  def extractCleanRedirectURL(input: String): Box[String] = {
-    Full(input.split("\\?oauth_token=")(0))
+
+  /**
+   * 
+   * @param redirectUrl eg: http://localhost:8082/oauthcallback?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018
+   * @return http://localhost:8082/oauthcallback
+   */
+  def getStaticPortionOfRedirectURL(redirectUrl: String): Box[String] = {
+    tryo(redirectUrl.split("\\?")(0)) //return everything before the "?"
+  }
+
+  /**
+   * extract clean redirect url from input value, because input may have some parameters, such as the following examples  <br/> 
+   * eg1: http://localhost:8082/oauthcallback?....--> http://localhost:8082 <br/> 
+   * eg2: http://localhost:8016?oautallback?=3NLMGV ...--> http://localhost:8016
+   *
+   * @param redirectUrl -> http://localhost:8082/oauthcallback?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018
+   * @return hostOnlyOfRedirectURL-> http://localhost:8082
+   */
+  @deprecated("We can not only use hostname as the redirectUrl, now add new method `getStaticPortionOfRedirectURL` ","05.12.2023")  
+  def getHostOnlyOfRedirectURL(redirectUrl: String): Box[String] = {
+    val url = new URL(redirectUrl) //eg: http://localhost:8082/oauthcallback?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018
+    val protocol = url.getProtocol() // http
+    val authority = url.getAuthority()// localhost:8082, this will contain the port.
+    tryo(s"$protocol://$authority") // http://localhost:8082 
   }
 
   /**
@@ -461,7 +484,7 @@ object Helper extends Loggable {
           }else if((args.length>0) && args.apply(0).toString.equalsIgnoreCase("consumer_key")){
             result.asInstanceOf[Box[String]].filter(APIUtil.basicConsumerKeyValidation(_)==SILENCE_IS_GOLDEN)
           }else if((args.length>0) && args.apply(0).toString.equalsIgnoreCase("redirectUrl")){
-            result.asInstanceOf[Box[String]].filter(APIUtil.basicUrlValidation(_))
+            result.asInstanceOf[Box[String]].filter(APIUtil.basicUriAndQueryStringValidation(_))
           } else{
             result.asInstanceOf[Box[String]].filter(APIUtil.checkMediumString(_)==SILENCE_IS_GOLDEN)
           }

obp-api/src/test/scala/code/util/APIUtilTest.scala

@@ -698,12 +698,18 @@ class APIUtilTest extends FeatureSpec with Matchers with GivenWhenThen with Prop
     APIUtil.getObpFormatOperationId("xxx") should be ("xxx")
   }
 
-  feature("test APIUtil.basicUrlValidation method") {
+  feature("test APIUtil.basicUriAndQueryStringValidation method") {
     val testString1 = "https%3A%2F%2Fapisandbox.openbankproject.com%2Foauth%2Fauthorize%3Fnext%3D%2Fen%2Fusers%2Fmyuser%26oauth_token%3DWTOBT2YRCTMI1BCCF4XAIKRXPLLZDZPFAIL5K03Z%26oauth_verifier%3D45381"
     val testString2 = "http%3A%2F%2Flocalhost%3A8016%3Foauth_token%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461"
-
-    APIUtil.basicUrlValidation(testString1) should be (true)
-    APIUtil.basicUrlValidation(testString2) should be (true)
+    val testString3 = "myapp://callback?oauth_token=%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461"
+    val testString4 = "fb00000000:://callback?oauth_token=%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461"
+    val testString5 = "http://127.0.0.1:8000/oauth/authorize?next=/en/metrics/api/&oauth_token=TN0124OCPRCL4KUJRF5LNLVMRNHTVZPJDBS2PNWU&oauth_verifier=10470"
+
+    APIUtil.basicUriAndQueryStringValidation(testString1) should be (true)
+    APIUtil.basicUriAndQueryStringValidation(testString2) should be (true)
+    APIUtil.basicUriAndQueryStringValidation(testString3) should be (true)
+    APIUtil.basicUriAndQueryStringValidation(testString4) should be (true)
+    APIUtil.basicUriAndQueryStringValidation(testString5) should be (true)
 
   }
 

obp-api/src/test/scala/code/util/HelperTest.scala

@@ -35,13 +35,32 @@ import org.scalatest.{FeatureSpec, GivenWhenThen, Matchers}
 
 class HelperTest extends FeatureSpec with Matchers with GivenWhenThen with PropsReset {
 
-  feature("test APIUtil.basicUrlValidation method") {
+  feature("test APIUtil.getStaticPortionOfRedirectURL method") {
+    // The redirectURl is `http://localhost:8082/oauthcallback`
     val testString1 = "http://localhost:8082/oauthcallback?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018"
     val testString2 = "http://localhost:8082?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018"
+    val testString3 = "myapp://callback?oauth_token=%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461"
+    val testString4 = "fb00000000:://callback?oauth_token=%3DEBRZBMOPDXEUGGJP421FPFGK01IY2DGM5O3TLVSK%26oauth_verifier%3D63461"
+    val testString5 = "http://127.0.0.1:8000/oauth/authorize?next=/en/metrics/api/&oauth_token=TN0124OCPRCL4KUJRF5LNLVMRNHTVZPJDBS2PNWU&oauth_verifier=10470"
 
-    Helper.extractCleanRedirectURL(testString1).head should be("http://localhost:8082/oauthcallback")
-    Helper.extractCleanRedirectURL(testString2).head should be("http://localhost:8082")
+    Helper.getStaticPortionOfRedirectURL(testString1).head should be("http://localhost:8082/oauthcallback")
+    Helper.getStaticPortionOfRedirectURL(testString2).head should be("http://localhost:8082")
+    Helper.getStaticPortionOfRedirectURL(testString3).head should be("myapp://callback")
+    Helper.getStaticPortionOfRedirectURL(testString4).head should be("fb00000000:://callback")
+    Helper.getStaticPortionOfRedirectURL(testString5).head should be("http://127.0.0.1:8000/oauth/authorize")
+  }
+
+  feature("test APIUtil.getHostOnlyOfRedirectURL method") {
+    // The redirectURl is `http://localhost:8082/oauthcallback`
+    val testString1 = "http://localhost:8082/oauthcallback?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018"
+    val testString2 = "http://localhost:8082/oauthcallback"
+    val testString3 = "http://localhost:8082?oauth_token=G5AEA2U1WG404EGHTIGBHKRR4YJZAPPHWKOMNEEV&oauth_verifier=53018"
+    val testString4 = "http://localhost:8082"
 
+    Helper.getHostOnlyOfRedirectURL(testString1).head should be("http://localhost:8082")
+    Helper.getHostOnlyOfRedirectURL(testString2).head should be("http://localhost:8082")
+    Helper.getHostOnlyOfRedirectURL(testString3).head should be("http://localhost:8082")
+    Helper.getHostOnlyOfRedirectURL(testString4).head should be("http://localhost:8082")
   }
 
 }