diff --git a/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala b/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala index 9025a727c..88ee49776 100644 --- a/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala +++ b/src/main/scala/code/api/1_2_1/OBPAPI1.2.1.scala @@ -65,8 +65,6 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { implicit def successToJson(success: SuccessMessage): JValue = Extraction.decompose(success) - val defaultAuthProvider = Props.get("hostname","") - val dateFormat = ModeratedTransaction.dateFormat val apiPrefix = "obp" / "v1.2.1" oPrefix _ @@ -261,12 +259,12 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { oauthServe(apiPrefix { //get access for specific user - case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: userId :: Nil JsonGet json => { + case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: providerId :: userId :: Nil JsonGet json => { user => for { u <- user ?~ "user not found" account <- BankAccount(bankId, accountId) - permission <- account permission(u, defaultAuthProvider, userId) + permission <- account permission(u, providerId, userId) } yield { val views = JSONFactory.createViewsJSON(permission.views) successJsonResponse(Extraction.decompose(views)) @@ -276,13 +274,13 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { oauthServe(apiPrefix{ //add access for specific user to a list of views - case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: userId :: "views" :: Nil JsonPost json -> _ => { + case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: providerId :: userId :: "views" :: Nil JsonPost json -> _ => { user => for { u <- user ?~ "user not found" account <- BankAccount(bankId, accountId) viewIds <- tryo{json.extract[ViewIdsJson]} ?~ "wrong format JSON" - addedViews <- account addPermissions(u, viewIds.views, defaultAuthProvider, userId) + addedViews <- account addPermissions(u, viewIds.views, providerId, userId) } yield { val viewJson = JSONFactory.createViewsJSON(addedViews) successJsonResponse(Extraction.decompose(viewJson), 201) @@ -292,13 +290,13 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { oauthServe(apiPrefix{ //add access for specific user to a specific view - case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: userId :: "views" :: viewId :: Nil JsonPost json -> _ => { + case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: providerId :: userId :: "views" :: viewId :: Nil JsonPost json -> _ => { user => for { u <- user ?~ "user not found" account <- BankAccount(bankId, accountId) view <- View.fromUrl(viewId, account) - isAdded <- account addPermission(u, viewId, defaultAuthProvider, userId) + isAdded <- account addPermission(u, viewId, providerId, userId) if(isAdded) } yield { val viewJson = JSONFactory.createViewJSON(view) @@ -309,12 +307,12 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { oauthServe(apiPrefix{ //delete access for specific user to one view - case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: userId :: "views" :: viewId :: Nil JsonDelete json => { + case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: providerId :: userId :: "views" :: viewId :: Nil JsonDelete json => { user => for { u <- user ?~ "user not found" account <- BankAccount(bankId, accountId) - isRevoked <- account revokePermission(u, viewId, defaultAuthProvider, userId) + isRevoked <- account revokePermission(u, viewId, providerId, userId) if(isRevoked) } yield noContentJsonResponse } @@ -322,12 +320,12 @@ object OBPAPI1_2_1 extends OBPRestHelper with Loggable { oauthServe(apiPrefix{ //delete access for specific user to all the views - case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: userId :: "views" :: Nil JsonDelete json => { + case "banks" :: bankId :: "accounts" :: accountId :: "permissions" :: providerId :: userId :: "views" :: Nil JsonDelete json => { user => for { u <- user ?~ "user not found" account <- BankAccount(bankId, accountId) - isRevoked <- account revokeAllPermissions(u, defaultAuthProvider, userId) + isRevoked <- account revokeAllPermissions(u, providerId, userId) if(isRevoked) } yield noContentJsonResponse } diff --git a/src/main/scala/code/api/OBPRestHelper.scala b/src/main/scala/code/api/OBPRestHelper.scala index f93ffd8b9..230969164 100644 --- a/src/main/scala/code/api/OBPRestHelper.scala +++ b/src/main/scala/code/api/OBPRestHelper.scala @@ -41,11 +41,36 @@ import code.util.APIUtil._ import code.model.User import code.api.OAuthHandshake._ +trait APIFailure{ + val responseCode : Int + val msg : String +} + +//if you change this, think about backwards compatibility! All existing +//versions of the API return this failure message, so if you change it, make sure +//that all stable versions retain the same behavior +case class UserNotFound(providerId : String, userId: String) extends APIFailure { + val responseCode = 400 //TODO: better as 404? -> would break some backwards compatibility (or at least the tests!) + + //to reiterate the comment about preserving backwards compatibility: + //consider the case that an app may be parsing this string to decide what message to show their users + //e.g. when granting view permissions, an app may not give their users a choice of provider and only + //allow them to grant permissions to users from a certain hardcoded provider. In this case, showing this error + //message is undesired and confusing. So in fact that app may be doing some regex stuff to try to match the string below + //so that they can provide a useful message to their users. Obviously in the future this should be redesigned in a better + //way, perhaps by using error codes. + val msg = s"user $userId not found at provider $providerId" +} + class OBPRestHelper extends RestHelper with Loggable { implicit def jsonResponseBoxToJsonReponse(box: Box[JsonResponse]): JsonResponse = { box match { case Full(r) => r + case ParamFailure(_, _, _, apiFailure : APIFailure) => { + logger.info("API Failure: " + apiFailure.msg + " ($apiFailure.responseCode)") + errorJsonResponse(apiFailure.msg, apiFailure.responseCode) + } case Failure(msg, _, _) => { logger.info("API Failure: " + msg) errorJsonResponse(msg) diff --git a/src/main/scala/code/model/User.scala b/src/main/scala/code/model/User.scala index 16f4a8924..db63fe92c 100644 --- a/src/main/scala/code/model/User.scala +++ b/src/main/scala/code/model/User.scala @@ -36,6 +36,7 @@ import net.liftweb.json.JsonDSL._ import net.liftweb.json.JsonAST.JObject import code.model.dataAccess.LocalStorage import net.liftweb.common.Box +import code.api.UserNotFound trait User { @@ -73,5 +74,8 @@ object User { LocalStorage.getUserByApiId(id) def findByProviderId(provider : String, idGivenByProvider : String) = - LocalStorage.getUserByProviderId(provider, idGivenByProvider) + //if you change this, think about backwards compatibility! All existing + //versions of the API return this failure message, so if you change it, make sure + //that all stable versions retain the same behavior + LocalStorage.getUserByProviderId(provider, idGivenByProvider) ~> UserNotFound(provider, idGivenByProvider) } \ No newline at end of file diff --git a/src/test/scala/code/api/API121Test.scala b/src/test/scala/code/api/API121Test.scala index a4900ea71..713e51e8f 100644 --- a/src/test/scala/code/api/API121Test.scala +++ b/src/test/scala/code/api/API121Test.scala @@ -42,7 +42,6 @@ import _root_.net.liftweb.json.Extraction import _root_.net.liftweb.json.Serialization import _root_.net.liftweb.json.Serialization.write import _root_.net.liftweb.json.JsonAST.{JValue, JObject} -import org.mortbay.jetty.nio.SelectChannelConnector import net.liftweb.json.NoTypeHints import net.liftweb.json.JsonDSL._ import scala.util.Random._ @@ -97,12 +96,14 @@ class API1_2_1Test extends ServerSetup{ secret(randomString(40).toLowerCase). saveMe + val defaultProvider = Props.get("hostname","") + lazy val consumer = new Consumer (testConsumer.key,testConsumer.secret) // create the access token lazy val tokenDuration = weeks(4) - lazy val obpuser1 = - APIUser.create. + lazy val obpuser1 = + APIUser.create.provider_(defaultProvider). saveMe override def specificSetup() ={ @@ -133,7 +134,7 @@ class API1_2_1Test extends ServerSetup{ // create a user for test purposes lazy val obpuser2 = - APIUser.create. + APIUser.create.provider_(defaultProvider). saveMe //we create an access token for the other user @@ -153,7 +154,7 @@ class API1_2_1Test extends ServerSetup{ // create a user for test purposes lazy val obpuser3 = - APIUser.create. + APIUser.create.provider_(defaultProvider). saveMe //we create an access token for the other user @@ -391,28 +392,28 @@ class API1_2_1Test extends ServerSetup{ } def getUserAccountPermission(bankId : String, accountId : String, userId : String, consumerAndToken: Option[(Consumer, Token)]) : APIResponse = { - val request = v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ userId <@(consumerAndToken) + val request = v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions" / defaultProvider / userId <@(consumerAndToken) makeGetRequest(request) } def grantUserAccessToView(bankId : String, accountId : String, userId : String, viewId : String, consumerAndToken: Option[(Consumer, Token)]) : APIResponse= { - val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ userId / "views" / viewId).POST <@(consumerAndToken) + val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ defaultProvider / userId / "views" / viewId).POST <@(consumerAndToken) makePostRequest(request) } def grantUserAccessToViews(bankId : String, accountId : String, userId : String, viewIds : List[String], consumerAndToken: Option[(Consumer, Token)]) : APIResponse= { - val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ userId / "views").POST <@(consumerAndToken) + val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ defaultProvider / userId / "views").POST <@(consumerAndToken) val viewsJson = ViewIdsJson(viewIds) makePostRequest(request, write(viewsJson)) } def revokeUserAccessToView(bankId : String, accountId : String, userId : String, viewId : String, consumerAndToken: Option[(Consumer, Token)]) : APIResponse= { - val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ userId / "views" / viewId).DELETE <@(consumerAndToken) + val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ defaultProvider / userId / "views" / viewId).DELETE <@(consumerAndToken) makeDeleteRequest(request) } def revokeUserAccessToAllViews(bankId : String, accountId : String, userId : String, consumerAndToken: Option[(Consumer, Token)]) : APIResponse= { - val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ userId / "views").DELETE <@(consumerAndToken) + val request = (v1_2Request / "banks" / bankId / "accounts" / accountId / "permissions"/ defaultProvider / userId / "views").DELETE <@(consumerAndToken) makeDeleteRequest(request) } diff --git a/src/test/scala/code/api/API12Test.scala b/src/test/scala/code/api/API12Test.scala index 33bb5d3e8..3a3e98cd2 100644 --- a/src/test/scala/code/api/API12Test.scala +++ b/src/test/scala/code/api/API12Test.scala @@ -42,7 +42,6 @@ import _root_.net.liftweb.json.Extraction import _root_.net.liftweb.json.Serialization import _root_.net.liftweb.json.Serialization.write import _root_.net.liftweb.json.JsonAST.{JValue, JObject} -import org.mortbay.jetty.nio.SelectChannelConnector import net.liftweb.json.NoTypeHints import net.liftweb.json.JsonDSL._ import scala.util.Random._