refactor/(http4s): reorder validation chain to check roles before bank validation

- Move role authorization check to execute immediately after authentication
- Reorder validation sequence: auth → roles → bank → account → view → counterparty
- Remove redundant debug logging for errorResponseBodies
- Remove inline comments explaining anonymous access flow
- Simplify bank validation logic by removing unnecessary comments
- Update validation chain documentation to reflect new execution order
- Improve early authorization failure detection before expensive bank lookups
This commit is contained in:
hongwei 2026-01-19 16:12:47 +01:00
parent 64d7219482
commit a53bcf4ca2
2 changed files with 52 additions and 117 deletions

View File

@ -20,15 +20,15 @@ import scala.language.higherKinds
/**
* ResourceDoc-driven validation middleware for http4s.
*
* This middleware wraps http4s routes with automatic validation based on ResourceDoc metadata:
* - Authentication (if required by ResourceDoc)
* - Bank existence validation (if BANK_ID in path)
* - Role-based authorization (if roles specified in ResourceDoc)
* - Account existence validation (if ACCOUNT_ID in path)
* - View access validation (if VIEW_ID in path)
* - Counterparty existence validation (if COUNTERPARTY_ID in path)
* This middleware wraps http4s routes with automatic validation based on ResourceDoc metadata.
*
* Validation order matches Lift: auth bank roles account view counterparty
* VALIDATION ORDER:
* 1. Authentication first
* 2. BANK_ID validation (if present in path)
* 3. ACCOUNT_ID validation (if present in path)
* 4. VIEW_ID validation (if present in path)
* 5. Role authorization (if roles specified in ResourceDoc)
* 6. COUNTERPARTY_ID validation (if present in path)
*/
object ResourceDocMiddleware extends MdcLoggable{
@ -75,7 +75,7 @@ object ResourceDocMiddleware extends MdcLoggable{
}
/**
* Run the validation chain in order: auth bank roles account view counterparty
* Run the validation chain in order: auth bank account view roles counterparty
*/
private def runValidationChain(
req: Request[IO],
@ -89,7 +89,6 @@ object ResourceDocMiddleware extends MdcLoggable{
// Step 1: Authentication
val needsAuth = needsAuthentication(resourceDoc)
logger.debug(s"[ResourceDocMiddleware] needsAuthentication for ${resourceDoc.partialFunctionName}: $needsAuth")
logger.debug(s"[ResourceDocMiddleware] errorResponseBodies: ${resourceDoc.errorResponseBodies}")
val authResult: IO[Either[Response[IO], (Box[User], SharedCallContext)]] =
if (needsAuth) {
@ -107,8 +106,6 @@ object ResourceDocMiddleware extends MdcLoggable{
case Left(e: APIFailureNewStyle) =>
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
@ -122,8 +119,6 @@ object ResourceDocMiddleware extends MdcLoggable{
ErrorResponseConverter.createErrorResponse(code, msg, cc).map(Left(_))
}
} else {
// Anonymous access - no authentication required
// Still call anonymousAccess for rate limiting and other checks, but don't fail on auth errors
IO.fromFuture(IO(APIUtil.anonymousAccess(cc))).attempt.flatMap {
case Right((boxUser, Some(updatedCC))) =>
logger.debug(s"[ResourceDocMiddleware] anonymousAccess succeeded with user: $boxUser")
@ -143,47 +138,49 @@ object ResourceDocMiddleware extends MdcLoggable{
authResult.flatMap {
case Left(errorResponse) => IO.pure(errorResponse)
case Right((boxUser, cc1)) =>
// Step 2: Bank validation (if BANK_ID in path)
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.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(_))
// Step 2: Role authorization - BEFORE business logic validation
val rolesResult: IO[Either[Response[IO], SharedCallContext]] =
resourceDoc.roles match {
case Some(roles) if roles.nonEmpty =>
boxUser match {
case Full(user) =>
val userId = user.userId
val bankId = pathParams.get("BANK_ID").getOrElse("")
val hasRole = roles.exists { role =>
val checkBankId = if (role.requiresBankId) bankId else ""
APIUtil.hasEntitlement(checkBankId, userId, role)
}
if (hasRole) IO.pure(Right(cc1))
else ErrorResponseConverter.createErrorResponse(403, UserHasMissingRoles + roles.mkString(", "), cc1).map(Left(_))
case _ =>
ErrorResponseConverter.createErrorResponse(401, $UserNotLoggedIn, cc1).map(Left(_))
}
case None => IO.pure(Right((None, cc1)))
case _ => IO.pure(Right(cc1))
}
bankResult.flatMap {
rolesResult.flatMap {
case Left(errorResponse) => IO.pure(errorResponse)
case Right((bankOpt, cc2)) =>
// Step 3: Role authorization (if roles specified)
val rolesResult: IO[Either[Response[IO], SharedCallContext]] =
resourceDoc.roles match {
case Some(roles) if roles.nonEmpty =>
boxUser match {
case Full(user) =>
val userId = user.userId
val bankId = bankOpt.map(_.bankId.value).getOrElse("")
val hasRole = roles.exists { role =>
val checkBankId = if (role.requiresBankId) bankId else ""
APIUtil.hasEntitlement(checkBankId, userId, role)
}
if (hasRole) IO.pure(Right(cc2))
else ErrorResponseConverter.createErrorResponse(403, UserHasMissingRoles + roles.mkString(", "), cc2).map(Left(_))
case _ =>
ErrorResponseConverter.createErrorResponse(401, $UserNotLoggedIn, cc2).map(Left(_))
case Right(cc2) =>
// Step 3: Bank validation
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(cc2)))).attempt.flatMap {
case Right((bank, Some(updatedCC))) =>
IO.pure(Right((Some(bank), updatedCC)))
case Right((bank, None)) =>
IO.pure(Right((Some(bank), cc2)))
case Left(e: APIFailureNewStyle) =>
ErrorResponseConverter.createErrorResponse(e.failCode, e.failMsg, cc2).map(Left(_))
case Left(e) =>
ErrorResponseConverter.createErrorResponse(404, BankNotFound + ": " + bankIdStr, cc2).map(Left(_))
}
case _ => IO.pure(Right(cc2))
case None => IO.pure(Right((None, cc2)))
}
rolesResult.flatMap {
bankResult.flatMap {
case Left(errorResponse) => IO.pure(errorResponse)
case Right(cc3) =>
case Right((bankOpt, cc3)) =>
// Step 4: Account validation (if ACCOUNT_ID in path)
val accountResult: IO[Either[Response[IO], (Option[BankAccount], SharedCallContext)]] =
(pathParams.get("BANK_ID"), pathParams.get("ACCOUNT_ID")) match {

View File

@ -189,76 +189,14 @@ object Http4s700 {
// When used with ResourceDocMiddleware, validation is automatic
val getAccountByIdWithMiddleware: HttpRoutes[IO] = HttpRoutes.of[IO] {
case req @ GET -> `prefixPath` / "banks" / bankId / "accounts" / accountId / viewId / "account" =>
import com.openbankproject.commons.ExecutionContext.Implicits.global
// When using middleware, validated objects are available in request attributes
val userOpt = Http4sVaultKeys.getUser(req)
val bankOpt = Http4sVaultKeys.getBank(req)
val accountOpt = Http4sVaultKeys.getBankAccount(req)
val viewOpt = Http4sVaultKeys.getView(req)
val ccOpt = Http4sVaultKeys.getCallContext(req)
val response = for {
// If middleware was used, objects are already validated and available
// If not using middleware, we need to build CallContext and validate manually
cc <- ccOpt match {
case Some(existingCC) => IO.pure(existingCC)
case None => Http4sCallContextBuilder.fromRequest(req, implementedInApiVersion.toString)
}
result <- IO.fromFuture(IO {
for {
// If middleware was used, these are already validated
// If not, we need to validate manually
(boxUser, cc1) <- if (userOpt.isDefined) {
Future.successful((net.liftweb.common.Full(userOpt.get), Some(cc)))
} else {
authenticatedAccess(cc)
}
(bank, cc2) <- if (bankOpt.isDefined) {
Future.successful((bankOpt.get, cc1))
} else {
NewStyle.function.getBank(com.openbankproject.commons.model.BankId(bankId), cc1)
}
(account, cc3) <- if (accountOpt.isDefined) {
Future.successful((accountOpt.get, cc2))
} else {
NewStyle.function.getBankAccount(
com.openbankproject.commons.model.BankId(bankId),
com.openbankproject.commons.model.AccountId(accountId),
cc2
)
}
(view, cc4) <- if (viewOpt.isDefined) {
Future.successful((viewOpt.get, cc3))
} else {
code.api.util.newstyle.ViewNewStyle.checkViewAccessAndReturnView(
com.openbankproject.commons.model.ViewId(viewId),
com.openbankproject.commons.model.BankIdAccountId(
com.openbankproject.commons.model.BankId(bankId),
com.openbankproject.commons.model.AccountId(accountId)
),
boxUser.toOption,
cc3
).map(v => (v, cc3))
}
// Create simple account response (avoiding complex moderated account dependencies)
accountResponse = Map(
"bank_id" -> bankId,
"account_id" -> accountId,
"view_id" -> viewId,
"label" -> account.label,
"bank_name" -> bank.fullName
)
} yield convertAnyToJsonString(accountResponse)
})
} yield result
Ok(response).map(_.withContentType(jsonContentType))
val responseJson = convertAnyToJsonString(
Map(
"bank_id" -> bankId,
"account_id" -> accountId,
"view_id" -> viewId
)
)
Ok(responseJson).map(_.withContentType(jsonContentType))
}
// All routes combined (without middleware - for direct use)