401 on db conn fail, and move sort

README.md

@@ -224,6 +224,11 @@ Now you can disable root by setting `api.root.enabled` to `false` in `/etc/horiz
     - detect if a pattern is updated with service that has userInput w/o default values, and give warning
     - Consider changing all creates to POST, and update (via put/patch) return codes to 200
 
+## Changes in 2.10.0
+
+- Fixed another case for issue 264
+- Moved the sort of `/changes` data to exchange scala code (from the postgresql db), and simplified the query filters a little
+
 ## Changes in 2.9.0
 
 - Issue 284: Notification Framework no longer throws an error for empty db responses

src/main/resources/version.txt

@@ -1 +1 @@
-2.9.0
+2.10.0

src/main/scala/com/horizon/exchangeapi/AuthCache.scala

@@ -53,32 +53,34 @@ object AuthCache /* extends Control with ServletApiImplicits */ {
     def init(db: Database): Unit = { this.db = db } // we intentionally don't prime the cache. We let it build on every access so we can add the unhashed token
 
     // Try to authenticate the creds and return the type (user/node/agbot) it is, or None
-    def getValidType(creds: Creds, retry: Boolean = false): CacheIdType = {
+    def getValidType(creds: Creds, alreadyRetried: Boolean = false): Try[CacheIdType] = {
       //logger.debug("CacheId:getValidType(): attempting to authenticate to the exchange with " + creds)
       val cacheValue = getCacheValue(creds)
       logger.debug("cacheValue: " + cacheValue)
-      if (cacheValue.isFailure) return CacheIdType.None
-      // we got the hashed token from the cache or db, now verify the token passed in
-      val cacheVal = cacheValue.get
-      if (cacheVal.unhashedToken != "" && Password.check(creds.token, cacheVal.unhashedToken)) { // much faster than the bcrypt check below
-        //logger.debug("CacheId:getValidType(): successfully quick-validated " + creds.id + " and its pw using the cache/db")
-        return cacheVal.idType
-      } else if (Password.check(creds.token, cacheVal.hashedToken)) {
-        //logger.debug("CacheId:getValidType(): successfully validated " + creds.id + " and its pw using the cache/db")
-        return cacheVal.idType
-      } else {
-        // the creds were invalid
-        if (retry) {
-          // we already tried clearing the cache and retrying, so give up and return that they were bad creds
-          logger.debug("CacheId:getValidType(): user " + creds.id + " not authenticated in the exchange")
-          return CacheIdType.None
-        } else {
-          // If we only used a non-expired cache entry to get here, the cache entry could be stale (e.g. they recently changed their pw/token via a different instance of the exchange).
-          // So delete the cache entry from the db and try 1 more time
-          logger.debug("CacheId:getValidType(): user " + creds.id + " was not authenticated successfully, removing cache entry in case it was stale, and trying 1 more time")
-          removeOne(creds.id)
-          return getValidType(creds, retry = true)
-        }
+      cacheValue match {
+        case Failure(t) => return Failure(t)  // bubble up the specific failure
+        case Success(cacheVal) =>
+          // we got the hashed token from the cache or db, now verify the token passed in
+          if (cacheVal.unhashedToken != "" && Password.check(creds.token, cacheVal.unhashedToken)) { // much faster than the bcrypt check below
+            //logger.debug("CacheId:getValidType(): successfully quick-validated " + creds.id + " and its pw using the cache/db")
+            return Success(cacheVal.idType)
+          } else if (Password.check(creds.token, cacheVal.hashedToken)) {
+            //logger.debug("CacheId:getValidType(): successfully validated " + creds.id + " and its pw using the cache/db")
+            return Success(cacheVal.idType)
+          } else {
+            // the creds were invalid
+            if (alreadyRetried) {
+              // we already tried clearing the cache and retrying, so give up and return that they were bad creds
+              logger.debug("CacheId:getValidType(): user " + creds.id + " not authenticated in the exchange")
+              return Success(CacheIdType.None)  // this is distinguished from Failure, because we didn't hit an error trying to access the db, it's just that the creds weren't value
+            } else {
+              // If we only used a non-expired cache entry to get here, the cache entry could be stale (e.g. they recently changed their pw/token via a different instance of the exchange).
+              // So delete the cache entry from the db and try 1 more time
+              logger.debug("CacheId:getValidType(): user " + creds.id + " was not authenticated successfully, removing cache entry in case it was stale, and trying 1 more time")
+              removeOne(creds.id)
+              return getValidType(creds, alreadyRetried = true)
+            }
+          }
       }
     }
 

src/main/scala/com/horizon/exchangeapi/AuthorizationSupport.scala

@@ -268,10 +268,14 @@ trait AuthorizationSupport {
         //else throw new InvalidCredentialsException("invalid token")  <- hint==token means it *could* be a token, not that it *must* be
       }
       AuthCache.ids.getValidType(creds) match {
-        case CacheIdType.User => return toIUser
-        case CacheIdType.Node => return toINode
-        case CacheIdType.Agbot => return toIAgbot
-        case CacheIdType.None => throw new InvalidCredentialsException() // will be caught by AuthenticationSupport.authenticate()
+        case Success(cacheIdType) =>
+          cacheIdType match {
+            case CacheIdType.User => return toIUser
+            case CacheIdType.Node => return toINode
+            case CacheIdType.Agbot => return toIAgbot
+            case CacheIdType.None => throw new InvalidCredentialsException() // will be caught by AuthenticationSupport.authenticate()
+          }
+        case Failure(t) => throw t  // this is usually 1 of our exceptions - will be caught by AuthenticationSupport.authenticate()
       }
     }
 

src/main/scala/com/horizon/exchangeapi/OrgsRoutes.scala

@@ -505,7 +505,11 @@ trait OrgsRoutes extends JacksonSupport with AuthenticationSupport {
     } // end of exchAuth
   }
 
-  def buildResourceChangesResponse(inputList: scala.Seq[ResourceChangeRow], maxRecords : Int): ResourceChangesRespObject ={
+  def buildResourceChangesResponse(inputListUnsorted: scala.Seq[ResourceChangeRow], maxRecords : Int): ResourceChangesRespObject ={
+    // Sort the rows based on the changeId. Default order is ascending, which is what we want
+    logger.debug(s"POST /orgs/{orgid}/changes sorting ${inputListUnsorted.size} rows")
+    val inputList = inputListUnsorted.sortBy(_.changeId)  // Note: we are doing the sorting here instead of in the db via sql, because the latter seems to use a lot of db cpu
+
     // fill in some values we can before processing
     val exchangeVersion = ExchangeApi.adminVersion()
     val maxChangeIdOfQuery = inputList.last.changeId // this is the maximum changeId of the entire query from the db
@@ -565,16 +569,19 @@ trait OrgsRoutes extends JacksonSupport with AuthenticationSupport {
           // Variables to help with building the query
           val lastTime = reqBody.lastUpdated.getOrElse(ApiTime.beginningUTC)
           // filter by lastUpdated and changeId then filter by either it's in the org OR it's not in the same org but is public
-          var qFilter = ResourceChangesTQ.rows.filter(_.lastUpdated >= lastTime).filter(_.changeId >= reqBody.changeId).filter(u => (u.orgId === orgId) || (u.orgId =!= orgId && u.public === "true"))
-          ident match {
+          //var qFilter = ResourceChangesTQ.rows.filter(_.lastUpdated >= lastTime).filter(_.changeId >= reqBody.changeId).filter(u => (u.orgId === orgId) || (u.orgId =!= orgId && u.public === "true"))
+          val qFilter = ident match {
             case _: INode =>
               // if its a node calling then it doesn't want information about any other nodes
-              qFilter = qFilter.filter(u => (u.category === "node" && u.id === ident.getIdentity) || u.category =!= "node")
-            case _ => ;
+               ResourceChangesTQ.rows.filter(_.changeId >= reqBody.changeId).filter(_.lastUpdated >= lastTime).filter(u => (u.orgId === orgId) || (u.orgId =!= orgId && u.public === "true")).filter(u => (u.category === "node" && u.id === ident.getIdentity) || u.category =!= "node")
+            case _ =>
+              // Note: repeating some of the filters in both cases to make the final query less nested for the db
+              ResourceChangesTQ.rows.filter(_.changeId >= reqBody.changeId).filter(_.lastUpdated >= lastTime).filter(u => (u.orgId === orgId) || (u.orgId =!= orgId && u.public === "true"))
           }
-          val q = for { r <- qFilter.sortBy(_.changeId) } yield r //sort the response by changeId
+          //val q = for { r <- qFilter.sortBy(_.changeId) } yield r //sort the response by changeId
+          logger.debug(s"POST /orgs/$orgId/changes db query: ${qFilter.result.statements}")
           var qResp : scala.Seq[ResourceChangeRow] = null
-          db.run(q.result.asTry.flatMap({
+          db.run(qFilter.result.asTry.flatMap({
             case Success(qResult) =>
               //logger.debug("POST /orgs/" + orgId + "/changes changes : " + qOrgResult.toString())
               logger.debug("POST /orgs/" + orgId + "/changes changes : " + qResult.size)

src/test/bash/primedb.sh

@@ -78,6 +78,8 @@ agreementid1="${agreementbase}1"
 agreementid2="${agreementbase}2"
 agreementid3="${agreementbase}3"
 
+encodedPubKey="QUJDCg=="  # this is ABC base64 encoded
+
 #resname="res1"
 #resversion="7.8.9"
 #resid="${resname}_$resversion"
@@ -479,15 +481,15 @@ if [[ $rc != 200 ]]; then
       ]
     }
   ],
-  "publicKey": "ABC" }'
+  "publicKey": "'$encodedPubKey'" }'
 else
     echo "orgs/$orgid/nodes/$nodeid exists"
 fi
 
 rc=$(curlfind $userauth "orgs/$orgid/nodes/$nodeid2")
 checkrc "$rc" 200 404
 if [[ $rc != 200 ]]; then
-    curlcreate "PUT" $userauth "orgs/$orgid/nodes/$nodeid2" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "'$orgid'/'$patid'", "registeredServices": [{"url": "'$orgid'/'$svcurl'", "numAgreements": 1, "policy": "", "properties": []}], "publicKey": "ABC" }'
+    curlcreate "PUT" $userauth "orgs/$orgid/nodes/$nodeid2" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "'$orgid'/'$patid'", "registeredServices": [{"url": "'$orgid'/'$svcurl'", "numAgreements": 1, "policy": "", "properties": []}], "publicKey": "'$encodedPubKey'" }'
 else
     echo "orgs/$orgid/nodes/$nodeid2 exists"
 fi
@@ -496,7 +498,7 @@ if isPublicCloud; then
   rc=$(curlfind $userauthorg2 "orgs/$orgid2/nodes/$nodeid")
   checkrc "$rc" 200 404
   if [[ $rc != 200 ]]; then
-      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/nodes/$nodeid" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "'$orgid'/'$patid'", "registeredServices": [], "publicKey": "ABC" }'
+      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/nodes/$nodeid" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "'$orgid'/'$patid'", "registeredServices": [], "publicKey": "'$encodedPubKey'" }'
   else
       echo "orgs/$orgid2/nodes/$nodeid exists"
   fi
@@ -522,7 +524,7 @@ if isPublicCloud; then
   rc=$(curlfind $userauthorg2 "orgs/$orgid2/nodes/$nodeid2")
   checkrc "$rc" 200 404
   if [[ $rc != 200 ]]; then
-      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/nodes/$nodeid2" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "", "publicKey": "ABC" }'
+      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/nodes/$nodeid2" '{"token": "'$nodetoken'", "name": "rpi1", "pattern": "", "publicKey": "'$encodedPubKey'" }'
   else
       echo "orgs/$orgid2/nodes/$nodeid2 exists"
   fi
@@ -531,7 +533,7 @@ fi
 rc=$(curlfind $userauth "orgs/$orgid/agbots/$agbotid")
 checkrc "$rc" 200 404
 if [[ $rc != 200 ]]; then
-    curlcreate "PUT" $userauth "orgs/$orgid/agbots/$agbotid" '{"token": "'$agbottoken'", "name": "agbot", "publicKey": "ABC"}'
+    curlcreate "PUT" $userauth "orgs/$orgid/agbots/$agbotid" '{"token": "'$agbottoken'", "name": "agbot", "publicKey": "'$encodedPubKey'"}'
 else
     echo "orgs/$orgid/agbots/$agbotid exists"
 fi
@@ -540,7 +542,7 @@ if isPublicCloud; then
   rc=$(curlfind $userauthorg2 "orgs/$orgid2/agbots/$agbotid")
   checkrc "$rc" 200 404
   if [[ $rc != 200 ]]; then
-      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/agbots/$agbotid" '{"token": "'$agbottoken'", "name": "agbot", "publicKey": "ABC"}'
+      curlcreate "PUT" $userauthorg2 "orgs/$orgid2/agbots/$agbotid" '{"token": "'$agbottoken'", "name": "agbot", "publicKey": "'$encodedPubKey'"}'
   else
       echo "orgs/$orgid2/agbots/$agbotid exists"
   fi