Merge pull request #286 from bmpotter/401-on-db-conn-fail

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)
+            }
+          }
       }
     }
 
@@ -198,7 +200,7 @@ object AuthCache /* extends Control with ServletApiImplicits */ {
           val isValue = respVector.head
           logger.debug("CacheBoolean:getId(): " + id + " was not in the cache but found in the db, adding it with value " + isValue + " to the cache")
           Success(isValue)
-        } else Failure(new IdNotFoundException)
+        } else Failure(new IdNotFoundForAuthorizationException)
       } catch {
         // Handle db problems
         case timeout: java.util.concurrent.TimeoutException =>
@@ -281,7 +283,7 @@ object AuthCache /* extends Control with ServletApiImplicits */ {
           val owner = respVector.head
           logger.debug("CacheOwner:getId(): " + id + " found in the db, adding it with value " + owner + " to the cache")
           Success(owner)
-        } else Failure(new IdNotFoundException)
+        } else Failure(new IdNotFoundForAuthorizationException)
       } catch {
         // Handle db problems
         case timeout: java.util.concurrent.TimeoutException =>

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/main/scala/com/horizon/exchangeapi/auth/Exceptions.scala

@@ -57,8 +57,11 @@ class IamApiTimeoutException(msg: String) extends AuthException(HttpCode.GW_TIME
 // An error occurred while building the SSLSocketFactory with the self-signed cert
 class SelfSignedCertException(msg: String) extends AuthException(HttpCode.INTERNAL_ERROR, ApiRespType.INTERNAL_ERROR, msg)
 
-// Only used internally: The local exchange id was not found in the db
-class IdNotFoundException extends AuthException(HttpCode.INTERNAL_ERROR, ApiRespType.INTERNAL_ERROR, "id not found")
+// The creds id was not found in the db
+class IdNotFoundException(msg: String = ExchMsg.translate("invalid.credentials")) extends AuthException(HttpCode.BADCREDS, ApiRespType.BADCREDS, msg)
+
+// The id was not found in the db when looking for owner or isPublic
+class IdNotFoundForAuthorizationException(msg: String = ExchMsg.translate("access.denied")) extends AuthException(HttpCode.ACCESS_DENIED, ApiRespType.ACCESS_DENIED, msg)
 
 class AuthInternalErrorException(msg: String) extends AuthException(HttpCode.INTERNAL_ERROR, ApiRespType.INTERNAL_ERROR, msg)
 

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

src/test/scala/exchangeapi/UsersSuite.scala

@@ -44,7 +44,8 @@ class UsersSuite extends FunSuite {
   val USERAUTH = ("Authorization", "Basic " + ApiUtils.encode(creds))
   val ORG2USERAUTH = ("Authorization", "Basic " + ApiUtils.encode(org2user + ":" + pw))
   //val encodedCreds = Base64.getEncoder.encodeToString(creds.getBytes("utf-8"))
-  val ENCODEDAUTH = ("Authorization", "Basic " + ApiUtils.encode(creds))
+  val BADUSERAUTH = ("Authorization", "Basic " + ApiUtils.encode(s"bad$orguser:$pw"))
+  val USERAUTHBAD = ("Authorization", "Basic " + ApiUtils.encode(s"$orguser:${pw}bad"))
   val user2 = "u2" // this is NOT an admin user
   val orguser2 = orgid + "/" + user2
   val pw2 = user2 + " pw" // intentionally adding a space in the pw
@@ -376,10 +377,9 @@ class UsersSuite extends FunSuite {
     assert(u.email === user + "@gmail.com")
   }
 
-  /** Update the normal user with encoded creds */
-  test("PUT /orgs/" + orgid + "/users/" + user + " - update normal with encoded creds") {
+  test("PUT /orgs/" + orgid + "/users/" + user + " - update normal with creds") {
     val input = PostPutUsersRequest(pw, admin = true, user + "-updated@gmail.com")
-    val response = Http(URL + "/users/" + user).postData(write(input)).method("put").headers(CONTENT).headers(ACCEPT).headers(ENCODEDAUTH).asString
+    val response = Http(URL + "/users/" + user).postData(write(input)).method("put").headers(CONTENT).headers(ACCEPT).headers(USERAUTH).asString
     info("code: " + response.code + ", response.body: " + response.body)
     assert(response.code === HttpCode.PUT_OK.intValue)
   }
@@ -408,9 +408,8 @@ class UsersSuite extends FunSuite {
     assert(response.code === HttpCode.ACCESS_DENIED.intValue)
   }
 
-  /** Verify the encoded update worked */
-  test("GET /orgs/" + orgid + "/users/" + user + " - encoded creds") {
-    val response: HttpResponse[String] = Http(URL + "/users/" + user).headers(ACCEPT).headers(ENCODEDAUTH).asString
+  test("GET /orgs/" + orgid + "/users/" + user + " - with creds") {
+    val response: HttpResponse[String] = Http(URL + "/users/" + user).headers(ACCEPT).headers(USERAUTH).asString
     info("code: " + response.code)
     // info("code: "+response.code+", response.body: "+response.body)
     assert(response.code === HttpCode.OK.intValue)
@@ -420,7 +419,6 @@ class UsersSuite extends FunSuite {
     assert(u.email === user + "-updated@gmail.com")
   }
 
-  /** Confirm user/pw for user */
   test("POST /orgs/" + orgid + "/users/" + user + "/confirm") {
     val response = Http(URL + "/users/" + user + "/confirm").method("post").headers(ACCEPT).headers(USERAUTH).asString
     info("code: " + response.code + ", response.body: " + response.body)
@@ -429,19 +427,16 @@ class UsersSuite extends FunSuite {
     assert(postConfirmResp.code === ApiRespType.OK)
   }
 
-  /** Confirm encoded user/pw for user */
-  test("POST /orgs/" + orgid + "/users/" + user + "/confirm - encoded") {
-    //info("encodedCreds=" + encodedCreds + ".")
-    val response = Http(URL + "/users/" + user + "/confirm").method("post").headers(ACCEPT).headers(ENCODEDAUTH).asString
+  test("POST /orgs/" + orgid + "/users/" + user + "/confirm - bad user") {
+    val response = Http(URL + "/users/" + user + "/confirm").method("post").headers(ACCEPT).headers(BADUSERAUTH).asString
     info("code: " + response.code + ", response.body: " + response.body)
-    assert(response.code === HttpCode.POST_OK.intValue)
+    assert(response.code === HttpCode.BADCREDS.intValue)
     val postConfirmResp = parse(response.body).extract[ApiResponse]
-    assert(postConfirmResp.code === ApiRespType.OK)
+    assert(postConfirmResp.code === ApiRespType.BADCREDS)
   }
 
-  /** Confirm user/pw for bad pw */
   test("POST /orgs/" + orgid + "/users/" + user + "/confirm - bad pw") {
-    val response = Http(URL + "/users/" + user + "/confirm").method("post").headers(ACCEPT).headers(("Authorization", "Basic " + user + ":badpw")).asString
+    val response = Http(URL + "/users/" + user + "/confirm").method("post").headers(ACCEPT).headers(USERAUTHBAD).asString
     info("code: " + response.code + ", response.body: " + response.body)
     assert(response.code === HttpCode.BADCREDS.intValue)
     val postConfirmResp = parse(response.body).extract[ApiResponse]