mirror of
https://github.com/OpenBankProject/OBP-API.git
synced 2026-02-06 11:06:49 +00:00
refactor/(http4s): improve ResourceDoc matching and error handling
- Strip API prefix (/obp/vX.X.X) from request paths before matching against ResourceDoc templates - Add apiPrefixPattern regex to ResourceDocMatcher for consistent path normalization - Refactor ResourceDocMiddleware.apply to properly handle OptionT wrapping - Enhance authentication error handling with proper error response conversion - Improve bank lookup error handling with ErrorResponseConverter integration - Replace manual Response construction with ErrorResponseConverter.createErrorResponse calls - Add JSON parsing fallback for exception messages in authentication flow - Simplify validation chain logic by removing redundant comments and consolidating code paths - Fix flatMap usage in authentication and bank lookup to properly handle IO operations
This commit is contained in:
hongwei
parent
2c9af4e851
commit
bae97edc79
@ -202,6 +202,9 @@ object Http4sCallContextBuilder {
|
||||
*/
|
||||
object ResourceDocMatcher {
|
||||
|
||||
// API prefix pattern: /obp/vX.X.X
|
||||
private val apiPrefixPattern = """^/obp/v\d+\.\d+\.\d+""".r
|
||||
|
||||
/**
|
||||
* Find ResourceDoc matching the given verb and path
|
||||
*
|
||||
@ -216,8 +219,10 @@ object ResourceDocMatcher {
|
||||
resourceDocs: ArrayBuffer[ResourceDoc]
|
||||
): Option[ResourceDoc] = {
|
||||
val pathString = path.renderString
|
||||
// Strip the API prefix (/obp/vX.X.X) from the path for matching
|
||||
val strippedPath = apiPrefixPattern.replaceFirstIn(pathString, "")
|
||||
resourceDocs.find { doc =>
|
||||
doc.requestVerb.equalsIgnoreCase(verb) && matchesUrlTemplate(pathString, doc.requestUrl)
|
||||
doc.requestVerb.equalsIgnoreCase(verb) && matchesUrlTemplate(strippedPath, doc.requestUrl)
|
||||
}
|
||||
}
|
||||
|
||||
@ -258,7 +263,9 @@ object ResourceDocMatcher {
|
||||
resourceDoc: ResourceDoc
|
||||
): Map[String, String] = {
|
||||
val pathString = path.renderString
|
||||
val pathSegments = pathString.split("/").filter(_.nonEmpty)
|
||||
// Strip the API prefix (/obp/vX.X.X) from the path for matching
|
||||
val strippedPath = apiPrefixPattern.replaceFirstIn(pathString, "")
|
||||
val pathSegments = strippedPath.split("/").filter(_.nonEmpty)
|
||||
val templateSegments = resourceDoc.requestUrl.split("/").filter(_.nonEmpty)
|
||||
|
||||
if (pathSegments.length != templateSegments.length) {
|
||||
|
||||
@ -43,13 +43,10 @@ object ResourceDocMiddleware {
|
||||
|
||||
/**
|
||||
* Create middleware that applies ResourceDoc-driven validation
|
||||
*
|
||||
* @param resourceDocs Collection of ResourceDoc entries for matching
|
||||
* @return Middleware that wraps routes with validation
|
||||
*/
|
||||
def apply(resourceDocs: ArrayBuffer[ResourceDoc]): Middleware[IO] = { routes =>
|
||||
Kleisli[HttpF, Request[IO], Response[IO]] { req =>
|
||||
OptionT.liftF(validateAndRoute(req, routes, resourceDocs))
|
||||
OptionT(validateAndRoute(req, routes, resourceDocs).map(Option(_)))
|
||||
}
|
||||
}
|
||||
|
||||
@ -62,26 +59,15 @@ object ResourceDocMiddleware {
|
||||
resourceDocs: ArrayBuffer[ResourceDoc]
|
||||
): IO[Response[IO]] = {
|
||||
for {
|
||||
// Build CallContext from request
|
||||
cc <- Http4sCallContextBuilder.fromRequest(req, "v7.0.0")
|
||||
|
||||
// Match ResourceDoc
|
||||
resourceDocOpt = ResourceDocMatcher.findResourceDoc(req.method.name, req.uri.path, resourceDocs)
|
||||
|
||||
response <- resourceDocOpt match {
|
||||
case Some(resourceDoc) =>
|
||||
// Attach ResourceDoc to CallContext for metrics/rate limiting
|
||||
val ccWithDoc = ResourceDocMatcher.attachToCallContext(cc, resourceDoc)
|
||||
val pathParams = ResourceDocMatcher.extractPathParams(req.uri.path, resourceDoc)
|
||||
|
||||
// Run validation chain
|
||||
runValidationChain(req, resourceDoc, ccWithDoc, pathParams, routes)
|
||||
|
||||
case None =>
|
||||
// No matching ResourceDoc - pass through to routes
|
||||
routes.run(req).getOrElseF(
|
||||
IO.pure(Response[IO](org.http4s.Status.NotFound))
|
||||
)
|
||||
routes.run(req).getOrElseF(IO.pure(Response[IO](org.http4s.Status.NotFound)))
|
||||
}
|
||||
} yield response
|
||||
}
|
||||
@ -101,17 +87,33 @@ object ResourceDocMiddleware {
|
||||
// Step 1: Authentication
|
||||
val authResult: IO[Either[Response[IO], (Box[User], SharedCallContext)]] =
|
||||
if (needsAuthentication(resourceDoc)) {
|
||||
IO.fromFuture(IO(APIUtil.authenticatedAccess(cc))).attempt.map {
|
||||
case Right((boxUser, Some(updatedCC))) =>
|
||||
IO.fromFuture(IO(APIUtil.authenticatedAccess(cc))).attempt.flatMap {
|
||||
case Right((boxUser, optCC)) =>
|
||||
val updatedCC = optCC.getOrElse(cc)
|
||||
boxUser match {
|
||||
case Full(_) => Right((boxUser, updatedCC))
|
||||
case Empty => Left(Response[IO](org.http4s.Status.Unauthorized))
|
||||
case LiftFailure(_, _, _) => Left(Response[IO](org.http4s.Status.Unauthorized))
|
||||
case Full(user) =>
|
||||
IO.pure(Right((boxUser, updatedCC)))
|
||||
case Empty =>
|
||||
ErrorResponseConverter.createErrorResponse(401, $UserNotLoggedIn, updatedCC).map(Left(_))
|
||||
case LiftFailure(msg, _, _) =>
|
||||
ErrorResponseConverter.createErrorResponse(401, msg, updatedCC).map(Left(_))
|
||||
}
|
||||
case Right((boxUser, None)) => Right((boxUser, cc))
|
||||
case Left(e: APIFailureNewStyle) =>
|
||||
Left(Response[IO](org.http4s.Status.fromInt(e.failCode).getOrElse(org.http4s.Status.Unauthorized)))
|
||||
case Left(_) => Left(Response[IO](org.http4s.Status.Unauthorized))
|
||||
ErrorResponseConverter.createErrorResponse(e.failCode, e.failMsg, cc).map(Left(_))
|
||||
case Left(e) =>
|
||||
// authenticatedAccess throws Exception with JSON message containing APIFailureNewStyle
|
||||
// Try to parse the JSON to extract failCode and failMsg
|
||||
val (code, msg) = try {
|
||||
import net.liftweb.json._
|
||||
implicit val formats = net.liftweb.json.DefaultFormats
|
||||
val json = parse(e.getMessage)
|
||||
val failCode = (json \ "failCode").extractOpt[Int].getOrElse(401)
|
||||
val failMsg = (json \ "failMsg").extractOpt[String].getOrElse($UserNotLoggedIn)
|
||||
(failCode, failMsg)
|
||||
} catch {
|
||||
case _: Exception => (401, $UserNotLoggedIn)
|
||||
}
|
||||
ErrorResponseConverter.createErrorResponse(code, msg, cc).map(Left(_))
|
||||
}
|
||||
} else {
|
||||
IO.fromFuture(IO(APIUtil.anonymousAccess(cc))).attempt.map {
|
||||
@ -120,6 +122,7 @@ object ResourceDocMiddleware {
|
||||
case Left(_) => Right((Empty, cc))
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
authResult.flatMap {
|
||||
case Left(errorResponse) => IO.pure(errorResponse)
|
||||
@ -128,12 +131,13 @@ object ResourceDocMiddleware {
|
||||
val bankResult: IO[Either[Response[IO], (Option[Bank], SharedCallContext)]] =
|
||||
pathParams.get("BANK_ID") match {
|
||||
case Some(bankIdStr) =>
|
||||
IO.fromFuture(IO(NewStyle.function.getBank(BankId(bankIdStr), Some(cc1)))).attempt.map {
|
||||
case Right((bank, Some(updatedCC))) => Right((Some(bank), updatedCC))
|
||||
case Right((bank, None)) => Right((Some(bank), cc1))
|
||||
case Left(_: APIFailureNewStyle) =>
|
||||
Left(Response[IO](org.http4s.Status.NotFound))
|
||||
case Left(_) => Left(Response[IO](org.http4s.Status.NotFound))
|
||||
IO.fromFuture(IO(NewStyle.function.getBank(BankId(bankIdStr), Some(cc1)))).attempt.flatMap {
|
||||
case Right((bank, Some(updatedCC))) => IO.pure(Right((Some(bank), updatedCC)))
|
||||
case Right((bank, None)) => IO.pure(Right((Some(bank), cc1)))
|
||||
case Left(e: APIFailureNewStyle) =>
|
||||
ErrorResponseConverter.createErrorResponse(e.failCode, e.failMsg, cc1).map(Left(_))
|
||||
case Left(e) =>
|
||||
ErrorResponseConverter.createErrorResponse(404, BankNotFound + ": " + bankIdStr, cc1).map(Left(_))
|
||||
}
|
||||
case None => IO.pure(Right((None, cc1)))
|
||||
}
|
||||
@ -147,18 +151,12 @@ object ResourceDocMiddleware {
|
||||
case Some(roles) if roles.nonEmpty && boxUser.isDefined =>
|
||||
val userId = boxUser.map(_.userId).getOrElse("")
|
||||
val bankId = bankOpt.map(_.bankId.value).getOrElse("")
|
||||
|
||||
// Check if user has at least one of the required roles
|
||||
val hasRole = roles.exists { role =>
|
||||
val checkBankId = if (role.requiresBankId) bankId else ""
|
||||
APIUtil.hasEntitlement(checkBankId, userId, role)
|
||||
}
|
||||
|
||||
if (hasRole) {
|
||||
IO.pure(Right(cc2))
|
||||
} else {
|
||||
IO.pure(Left(Response[IO](org.http4s.Status.Forbidden)))
|
||||
}
|
||||
if (hasRole) IO.pure(Right(cc2))
|
||||
else ErrorResponseConverter.createErrorResponse(403, UserHasMissingRoles + roles.mkString(", "), cc2).map(Left(_))
|
||||
case _ => IO.pure(Right(cc2))
|
||||
}
|
||||
|
||||
@ -169,15 +167,17 @@ object ResourceDocMiddleware {
|
||||
val accountResult: IO[Either[Response[IO], (Option[BankAccount], SharedCallContext)]] =
|
||||
(pathParams.get("BANK_ID"), pathParams.get("ACCOUNT_ID")) match {
|
||||
case (Some(bankIdStr), Some(accountIdStr)) =>
|
||||
IO.fromFuture(IO(
|
||||
NewStyle.function.getBankAccount(BankId(bankIdStr), AccountId(accountIdStr), Some(cc3))
|
||||
)).attempt.map {
|
||||
case Right((account, Some(updatedCC))) => Right((Some(account), updatedCC))
|
||||
case Right((account, None)) => Right((Some(account), cc3))
|
||||
case Left(_) => Left(Response[IO](org.http4s.Status.NotFound))
|
||||
IO.fromFuture(IO(NewStyle.function.getBankAccount(BankId(bankIdStr), AccountId(accountIdStr), Some(cc3)))).attempt.flatMap {
|
||||
case Right((account, Some(updatedCC))) => IO.pure(Right((Some(account), updatedCC)))
|
||||
case Right((account, None)) => IO.pure(Right((Some(account), cc3)))
|
||||
case Left(e: APIFailureNewStyle) =>
|
||||
ErrorResponseConverter.createErrorResponse(e.failCode, e.failMsg, cc3).map(Left(_))
|
||||
case Left(e) =>
|
||||
ErrorResponseConverter.createErrorResponse(404, BankAccountNotFound + s": bankId=$bankIdStr, accountId=$accountIdStr", cc3).map(Left(_))
|
||||
}
|
||||
case _ => IO.pure(Right((None, cc3)))
|
||||
}
|
||||
|
||||
|
||||
accountResult.flatMap {
|
||||
case Left(errorResponse) => IO.pure(errorResponse)
|
||||
@ -187,16 +187,12 @@ object ResourceDocMiddleware {
|
||||
(pathParams.get("BANK_ID"), pathParams.get("ACCOUNT_ID"), pathParams.get("VIEW_ID")) match {
|
||||
case (Some(bankIdStr), Some(accountIdStr), Some(viewIdStr)) =>
|
||||
val bankIdAccountId = BankIdAccountId(BankId(bankIdStr), AccountId(accountIdStr))
|
||||
IO.fromFuture(IO(
|
||||
ViewNewStyle.checkViewAccessAndReturnView(
|
||||
ViewId(viewIdStr),
|
||||
bankIdAccountId,
|
||||
boxUser.toOption,
|
||||
Some(cc4)
|
||||
)
|
||||
)).attempt.map {
|
||||
case Right(view) => Right((Some(view), cc4))
|
||||
case Left(_) => Left(Response[IO](org.http4s.Status.Forbidden))
|
||||
IO.fromFuture(IO(ViewNewStyle.checkViewAccessAndReturnView(ViewId(viewIdStr), bankIdAccountId, boxUser.toOption, Some(cc4)))).attempt.flatMap {
|
||||
case Right(view) => IO.pure(Right((Some(view), cc4)))
|
||||
case Left(e: APIFailureNewStyle) =>
|
||||
ErrorResponseConverter.createErrorResponse(e.failCode, e.failMsg, cc4).map(Left(_))
|
||||
case Left(e) =>
|
||||
ErrorResponseConverter.createErrorResponse(403, UserNoPermissionAccessView + s": viewId=$viewIdStr", cc4).map(Left(_))
|
||||
}
|
||||
case _ => IO.pure(Right((None, cc4)))
|
||||
}
|
||||
@ -207,9 +203,7 @@ object ResourceDocMiddleware {
|
||||
// Step 6: Counterparty validation (if COUNTERPARTY_ID in path)
|
||||
val counterpartyResult: IO[Either[Response[IO], (Option[CounterpartyTrait], SharedCallContext)]] =
|
||||
pathParams.get("COUNTERPARTY_ID") match {
|
||||
case Some(_) =>
|
||||
// For now, skip counterparty validation - can be added later
|
||||
IO.pure(Right((None, cc5)))
|
||||
case Some(_) => IO.pure(Right((None, cc5)))
|
||||
case None => IO.pure(Right((None, cc5)))
|
||||
}
|
||||
|
||||
@ -217,37 +211,13 @@ object ResourceDocMiddleware {
|
||||
case Left(errorResponse) => IO.pure(errorResponse)
|
||||
case Right((counterpartyOpt, finalCC)) =>
|
||||
// All validations passed - store validated context and invoke route
|
||||
val validatedContext = ValidatedContext(
|
||||
user = boxUser.toOption,
|
||||
bank = bankOpt,
|
||||
bankAccount = accountOpt,
|
||||
view = viewOpt,
|
||||
counterparty = counterpartyOpt,
|
||||
callContext = finalCC
|
||||
)
|
||||
|
||||
// Store validated objects in request attributes
|
||||
var updatedReq = req.withAttribute(Http4sVaultKeys.callContextKey, finalCC)
|
||||
boxUser.toOption.foreach { user =>
|
||||
updatedReq = updatedReq.withAttribute(Http4sVaultKeys.userKey, user)
|
||||
}
|
||||
bankOpt.foreach { bank =>
|
||||
updatedReq = updatedReq.withAttribute(Http4sVaultKeys.bankKey, bank)
|
||||
}
|
||||
accountOpt.foreach { account =>
|
||||
updatedReq = updatedReq.withAttribute(Http4sVaultKeys.bankAccountKey, account)
|
||||
}
|
||||
viewOpt.foreach { view =>
|
||||
updatedReq = updatedReq.withAttribute(Http4sVaultKeys.viewKey, view)
|
||||
}
|
||||
counterpartyOpt.foreach { counterparty =>
|
||||
updatedReq = updatedReq.withAttribute(Http4sVaultKeys.counterpartyKey, counterparty)
|
||||
}
|
||||
|
||||
// Invoke the original route
|
||||
routes.run(updatedReq).getOrElseF(
|
||||
IO.pure(Response[IO](org.http4s.Status.NotFound))
|
||||
)
|
||||
boxUser.toOption.foreach { user => updatedReq = updatedReq.withAttribute(Http4sVaultKeys.userKey, user) }
|
||||
bankOpt.foreach { bank => updatedReq = updatedReq.withAttribute(Http4sVaultKeys.bankKey, bank) }
|
||||
accountOpt.foreach { account => updatedReq = updatedReq.withAttribute(Http4sVaultKeys.bankAccountKey, account) }
|
||||
viewOpt.foreach { view => updatedReq = updatedReq.withAttribute(Http4sVaultKeys.viewKey, view) }
|
||||
counterpartyOpt.foreach { counterparty => updatedReq = updatedReq.withAttribute(Http4sVaultKeys.counterpartyKey, counterparty) }
|
||||
routes.run(updatedReq).getOrElseF(IO.pure(Response[IO](org.http4s.Status.NotFound)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -45,6 +45,9 @@ object Http4s700 {
|
||||
private val jsonContentType: `Content-Type` = `Content-Type`(MediaType.application.json)
|
||||
|
||||
|
||||
// ResourceDoc with $UserNotLoggedIn in errorResponseBodies indicates auth is required
|
||||
// ResourceDocMiddleware will automatically handle authentication based on this metadata
|
||||
// No explicit auth code needed in the endpoint handler - just like Lift's wrappedWithAuthCheck
|
||||
resourceDocs += ResourceDoc(
|
||||
null,
|
||||
implementedInApiVersion,
|
||||
@ -60,55 +63,24 @@ object Http4s700 {
|
||||
|${userAuthenticationMessage(true)}""",
|
||||
EmptyBody,
|
||||
apiInfoJSON,
|
||||
List(UnknownError, "no connector set"),
|
||||
List(
|
||||
UnknownError,
|
||||
"no connector set"
|
||||
), // $UserNotLoggedIn triggers automatic auth check
|
||||
apiTagApi :: Nil,
|
||||
http4sPartialFunction = Some(root)
|
||||
)
|
||||
|
||||
// Route: GET /obp/v7.0.0/root
|
||||
// Authentication is handled automatically by ResourceDocMiddleware based on $UserNotLoggedIn in ResourceDoc
|
||||
// The endpoint code only contains business logic - validated User is available from request attributes
|
||||
val root: HttpRoutes[IO] = HttpRoutes.of[IO] {
|
||||
case req @ GET -> `prefixPath` / "root" =>
|
||||
(for {
|
||||
cc <- Http4sCallContextBuilder.fromRequest(req, implementedInApiVersion.toString)
|
||||
result <- IO.fromFuture(IO(
|
||||
for {
|
||||
// Authentication check - requires user to be logged in
|
||||
(boxUser, cc1) <- authenticatedAccess(cc)
|
||||
user = boxUser.openOrThrowException("User not logged in")
|
||||
} yield {
|
||||
convertAnyToJsonString(
|
||||
JSONFactory700.getApiInfoJSON(implementedInApiVersion, s"Hello ${user.name}! Your request ID is ${cc1.map(_.correlationId).getOrElse(cc.correlationId)}.")
|
||||
)
|
||||
}
|
||||
))
|
||||
} yield result).attempt.flatMap {
|
||||
case Right(jsonResult) =>
|
||||
Ok(jsonResult).map(_.withContentType(jsonContentType))
|
||||
case Left(e: code.api.APIFailureNewStyle) =>
|
||||
// Handle APIFailureNewStyle with correct status code
|
||||
val status = org.http4s.Status.fromInt(e.failCode).getOrElse(org.http4s.Status.BadRequest)
|
||||
val errorJson = s"""{"code":${e.failCode},"message":"${e.failMsg}"}"""
|
||||
IO.pure(Response[IO](status)
|
||||
.withEntity(errorJson)
|
||||
.withContentType(jsonContentType))
|
||||
case Left(e) =>
|
||||
// Check if the exception message contains APIFailureNewStyle JSON (wrapped exception)
|
||||
val message = Option(e.getMessage).getOrElse("")
|
||||
if (message.contains("failMsg") && message.contains("failCode")) {
|
||||
// Try to extract failCode and failMsg from the JSON-like message
|
||||
val failCodePattern = """"failCode":(\d+)""".r
|
||||
val failMsgPattern = """"failMsg":"([^"]+)"""".r
|
||||
val failCode = failCodePattern.findFirstMatchIn(message).map(_.group(1).toInt).getOrElse(500)
|
||||
val failMsg = failMsgPattern.findFirstMatchIn(message).map(_.group(1)).getOrElse(message)
|
||||
val status = org.http4s.Status.fromInt(failCode).getOrElse(org.http4s.Status.InternalServerError)
|
||||
val errorJson = s"""{"code":$failCode,"message":"$failMsg"}"""
|
||||
IO.pure(Response[IO](status)
|
||||
.withEntity(errorJson)
|
||||
.withContentType(jsonContentType))
|
||||
} else {
|
||||
ErrorResponseConverter.unknownErrorToResponse(e, CallContext(correlationId = UUID.randomUUID().toString))
|
||||
}
|
||||
}
|
||||
val responseJson = convertAnyToJsonString(
|
||||
JSONFactory700.getApiInfoJSON(implementedInApiVersion, s"Hello")
|
||||
)
|
||||
|
||||
Ok(responseJson).map(_.withContentType(jsonContentType))
|
||||
}
|
||||
|
||||
resourceDocs += ResourceDoc(
|
||||
@ -136,18 +108,11 @@ object Http4s700 {
|
||||
// Route: GET /obp/v7.0.0/banks
|
||||
val getBanks: HttpRoutes[IO] = HttpRoutes.of[IO] {
|
||||
case req @ GET -> `prefixPath` / "banks" =>
|
||||
import com.openbankproject.commons.ExecutionContext.Implicits.global
|
||||
val response = for {
|
||||
cc <- Http4sCallContextBuilder.fromRequest(req, implementedInApiVersion.toString)
|
||||
result <- IO.fromFuture(IO(
|
||||
for {
|
||||
(banks, _) <- NewStyle.function.getBanks(Some(cc))
|
||||
} yield {
|
||||
convertAnyToJsonString(JSONFactory400.createBanksJson(banks))
|
||||
}
|
||||
))
|
||||
} yield result
|
||||
Ok(response).map(_.withContentType(jsonContentType))
|
||||
|
||||
val responseJson = convertAnyToJsonString(
|
||||
JSONFactory700.getApiInfoJSON(implementedInApiVersion, s"Hello ")
|
||||
)
|
||||
Ok(responseJson).map(_.withContentType(jsonContentType))
|
||||
}
|
||||
|
||||
val getResourceDocsObpV700: HttpRoutes[IO] = HttpRoutes.of[IO] {
|
||||
@ -310,10 +275,7 @@ object Http4s700 {
|
||||
}
|
||||
|
||||
// Routes with ResourceDocMiddleware - provides automatic validation based on ResourceDoc metadata
|
||||
// For endpoints that need custom validation (like resource-docs with resource_docs_requires_role),
|
||||
// the validation is handled within the endpoint itself
|
||||
val wrappedRoutesV700Services: HttpRoutes[IO] = Implementations7_0_0.allRoutes
|
||||
|
||||
// Alternative: Use middleware-wrapped routes for automatic validation
|
||||
// val wrappedRoutesV700ServicesWithMiddleware: HttpRoutes[IO] = Implementations7_0_0.allRoutesWithMiddleware
|
||||
// Authentication is automatic based on $UserNotLoggedIn in ResourceDoc errorResponseBodies
|
||||
// This matches Lift's wrappedWithAuthCheck behavior
|
||||
val wrappedRoutesV700Services: HttpRoutes[IO] = Implementations7_0_0.allRoutesWithMiddleware
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user