Change permissions calls in v1.2.1 to require a user's auth provider to be specified

This commit is contained in:
Everett Sochowski 2014-02-28 17:33:50 +01:00
parent 104fd1bb75
commit 3fded9b66c
5 changed files with 51 additions and 24 deletions

View File

@ -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
}

View File

@ -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)

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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._