diff --git a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala index 33040e83d4..a19cfa823b 100644 --- a/obp-api/src/main/scala/code/api/util/ErrorMessages.scala +++ b/obp-api/src/main/scala/code/api/util/ErrorMessages.scala @@ -148,6 +148,7 @@ object ErrorMessages { val InvalidSignalChannelName = "OBP-10057: Invalid Signal Channel name. " + "Signal Channel names must use only alphanumeric characters, dots, hyphens, and underscores, " + "and be between 1 and 128 characters long." + val SignalChannelNotFound = "OBP-10058: Signal Channel not found. " diff --git a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala index 8221f69018..b99aa8f11c 100644 --- a/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala +++ b/obp-api/src/main/scala/code/api/v4_0_0/Http4s400.scala @@ -3057,8 +3057,7 @@ object Http4s400 { // View object — so a soft fallback is fine here. Views.views.vend.systemView(ViewId(viewIdStr)) .or(Views.views.vend.customView(ViewId(viewIdStr), BankIdAccountId(account.bankId, account.accountId))) - .openOrThrowException(s"$ViewNotFound Current view_id($viewIdStr)") - } + } map (unboxFullOrFail(_, Some(cc), s"$ViewNotFound Current view_id($viewIdStr)", 400)) // SS.init populates Lift thread-globals (used by `SS.user` inside the // connector). The connector's first line `SS.user` resolves synchronously // inside this block, capturing the user; subsequent flatMap stages run on @@ -6392,6 +6391,9 @@ object Http4s400 { case req @ GET -> `prefixPath` / "banks" / _ / "user-invitations" / secretLink => EndpointHelpers.withUserAndBank(req) { (_, bank, cc) => for { + _ <- code.util.Helper.booleanToFuture(InvalidNumber, cc = Some(cc)) { + scala.util.Try(secretLink.toLong).isSuccess + } (invitation, _) <- NewStyle.function.getUserInvitation(bank.bankId, secretLink.toLong, Some(cc)) } yield JSONFactory400.createUserInvitationJson(invitation) } @@ -7095,7 +7097,7 @@ object Http4s400 { "Get My Api Collection Endpoint", s"""Get Api Collection Endpoint By API_COLLECTION_NAME and OPERATION_ID. | - |${userAuthenticationMessage(false)} + |${userAuthenticationMessage(true)} |""".stripMargin, EmptyBody, apiCollectionEndpointJson400, @@ -7113,7 +7115,7 @@ object Http4s400 { "Get Api Collection Endpoints", s"""Get Api Collection Endpoints By API_COLLECTION_ID. | - |${userAuthenticationMessage(false)} + |${userAuthenticationMessage(true)} |""".stripMargin, EmptyBody, apiCollectionEndpointsJson400, diff --git a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala index 96e1d695d0..010020be91 100644 --- a/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala +++ b/obp-api/src/main/scala/code/api/v5_1_0/Http4s510.scala @@ -3050,6 +3050,9 @@ object Http4s510 { com.openbankproject.commons.util.JsonAliases.parse(cc.httpBody.getOrElse("")).extract[ConsumerJwtPostJsonV510] } pem = APIUtil.`getPSD2-CERT`(cc.requestHeaders) + _ <- Helper.booleanToFuture(X509GeneralError, 400, Some(cc)) { + pem.exists(_.trim.nonEmpty) + } _ <- Helper.booleanToFuture(PostJsonIsNotSigned, 400, Some(cc)) { JwtUtil.verifyJwt(postedJwt.jwt, pem.getOrElse("")) } diff --git a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala index 01b6bae3a7..2e2f62e9eb 100644 --- a/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala +++ b/obp-api/src/main/scala/code/api/v6_0_0/Http4s600.scala @@ -2621,10 +2621,10 @@ object Http4s600 { code.api.cache.RedisMessaging.validateChannelName(channelName) } info <- Future(code.api.cache.RedisMessaging.channelInfo(channelName)) - (count, ttl) <- info match { - case Some((c, t)) => Future.successful((c, t)) - case None => Future.failed(new RuntimeException(s"Channel '$channelName' not found")) + _ <- Helper.booleanToFuture(s"$SignalChannelNotFound Channel '$channelName' not found", 404, cc = Some(cc)) { + info.isDefined } + (count, ttl) = info.get } yield SignalChannelInfoJsonV600(channelName, count, ttl) } } @@ -8779,6 +8779,7 @@ object Http4s600 { EmptyBody, connectorTracesJsonV600, List( + $AuthenticatedUserIsRequired, InvalidDateFormat, UnknownError ), @@ -9868,10 +9869,14 @@ object Http4s600 { | |Optional query parameter `tag` — filter to products that have the given tag (e.g. `?tag=featured`). Tag matching is case-insensitive. | - |${userAuthenticationMessage(!getApiProductsIsPublic)}""".stripMargin, + |${userAuthenticationMessage(true)}""".stripMargin, EmptyBody, apiProductsJsonV600, - List(UnknownError), + // Intentional drift from Lift's APIMethods600.scala source-of-truth: Lift gated this on + // the getApiProductsIsPublic prop (public by default); the http4s handler always calls + // withUser, same simplification already documented for the rest of the api-products + // bucket above. AuthenticatedUserIsRequired reflects what the handler actually enforces. + List($AuthenticatedUserIsRequired, UnknownError), apiTagApi :: apiTagApiProduct :: Nil, None, http4sPartialFunction = Some(getAllApiProductsV600) @@ -9886,10 +9891,13 @@ object Http4s600 { | |Optional query parameter `tag` — filter to products that carry the given tag (e.g. `?tag=featured`). Tag matching is case-insensitive. Repeat `tag=` to require multiple tags. | - |${userAuthenticationMessage(!getProductsIsPublic)}""".stripMargin, + |${userAuthenticationMessage(true)}""".stripMargin, EmptyBody, productsJsonV600, - List(UnknownError), + // Intentional drift from Lift's APIMethods600.scala source-of-truth: Lift gated this on + // the getProductsIsPublic prop (public by default); the http4s handler always calls + // withUser. AuthenticatedUserIsRequired reflects what the handler actually enforces. + List($AuthenticatedUserIsRequired, UnknownError), apiTagProduct :: Nil, None, http4sPartialFunction = Some(getAllProductsV600) @@ -13347,6 +13355,7 @@ object Http4s600 { EmptyBody, configPropsJsonV600, List( + $AuthenticatedUserIsRequired, UnknownError ), apiTagApi :: Nil, diff --git a/obp-api/src/main/scala/code/bankconnectors/package.scala b/obp-api/src/main/scala/code/bankconnectors/package.scala index 7d835d1097..5fc8a750ee 100644 --- a/obp-api/src/main/scala/code/bankconnectors/package.scala +++ b/obp-api/src/main/scala/code/bankconnectors/package.scala @@ -12,6 +12,7 @@ import code.methodrouting.{MethodRouting, MethodRoutingT} import code.metrics.{ConnectorTraceProvider, ConnectorMetricsProvider, ConnectorCountsRedis} import code.util.Helper import code.util.Helper.MdcLoggable +import com.github.dwickern.macros.NameOf.nameOf import com.openbankproject.commons.model.{AccountId, BankId} import com.openbankproject.commons.util.ReflectUtils.{findMethodByArgs, getConstructorArgs} import com.openbankproject.commons.ExecutionContext.Implicits.global @@ -48,7 +49,13 @@ package object bankconnectors extends MdcLoggable { val intercept: InvocationHandler = new InvocationHandler { override def invoke(proxy: AnyRef, method: Method, args: Array[AnyRef]): AnyRef = { - if (method.getReturnType.getName == "scala.concurrent.Future" && !canOpenFuture(method.getName)) { + if (method.getName == nameOf(StubConnector.callableMethods)) { + // callableMethods is declared directly on Connector with a real default body, so + // isInheritedMember (declaring class == Connector) does not catch it and it would + // otherwise be routed as a connector call - NPE on the way, since args is null for + // this no-arg method. Answer it from the empty stub, same as InternalConnector does. + StubConnector.callableMethods + } else if (method.getReturnType.getName == "scala.concurrent.Future" && !canOpenFuture(method.getName)) { throw new RuntimeException(ServiceIsTooBusy + s"Current Service(${method.getName})") } else { if (method.getName.contains("$default$") || ConnectorProxy.isInheritedMember(method)) { diff --git a/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala b/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala index 285cac71a9..942ec7cc7f 100644 --- a/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala +++ b/obp-api/src/test/scala/code/api/sweep/AuthSweepTest.scala @@ -3,7 +3,7 @@ package code.api.sweep import cats.effect.IO import cats.effect.unsafe.IORuntime import code.api.util.APIUtil.ResourceDoc -import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, UserHasMissingRoles} +import code.api.util.ErrorMessages.{ApplicationNotIdentified, AuthenticatedUserIsRequired, UserHasMissingRoles} import code.api.util.http4s.Http4sApp import code.setup.{DefaultUsers, ServerSetupWithTestData} import fs2.Stream @@ -102,35 +102,75 @@ class AuthSweepTest extends ServerSetupWithTestData with DefaultUsers { private def describe(doc: ResourceDoc): String = s"${doc.operationId} ${doc.requestVerb} ${EndpointCatalog.concretePath(doc)}" + /** + * Deviations that are deliberate, with the reason each one is not a defect. + * + * A signed-off list rather than a hard zero: the two entries here are both behaviour somebody + * chose and wrote down at the endpoint itself. Anything NOT listed still fails. + */ + private val expectedAuthDeviation: Map[String, String] = Map( + "OBPv4.0.0-verifyRequestSignResponse" -> + ("Refuses with OBP-20311 'The Request is not signed' -- JWS request signing, a third " + + "authentication mechanism alongside user and application. ResourceDoc has no way to " + + "declare it: authMode covers user/application only, so neither the doc nor this sweep " + + "can express the requirement. The 401 is correct; only the message differs."), + "OBPv4.0.0-createTransactionRequestFreeForm" -> + ("Answers 400 InsufficientAuthorisationToCreateTransactionRequest rather than 403. The " + + "endpoint deliberately does no upfront view/role check and delegates the decision to " + + "checkAuthorisationToCreateTransactionRequest inside the connector -- its own comment " + + "says so. Whether an authorisation failure ought to be 400 at all is a product " + + "question, not something to change from inside a sweep.") + ) + // ── the three checks, each returning a failure line or None ────────────────── private def checkAnonymousIs401(doc: ResourceDoc): Option[String] = { val (code, json) = call(doc.requestVerb, EndpointCatalog.concretePath(doc), Map.empty) if (code != 401) Some(s"${describe(doc)} -- expected 401 for an anonymous call, got $code") - else if (messageOf(json) != AuthenticatedUserIsRequired) - Some(s"${describe(doc)} -- 401 but message was '${messageOf(json)}', expected '$AuthenticatedUserIsRequired'") - else None + else if (messageOf(json) == AuthenticatedUserIsRequired) + None + else expectedAuthDeviation.get(doc.operationId) match { + case Some(_) => None + case None => + Some(s"${describe(doc)} -- 401 but message was '${messageOf(json)}', expected '$AuthenticatedUserIsRequired'") + } } + /** + * A doc that asks for no USER may still ask for an APPLICATION, and that is not a defect. + * + * OBP has more than one way to refuse an anonymous caller: OBP-20001 "User not logged in" is + * user authentication, OBP-20200 "The application cannot be identified" is consumer/application + * authentication. `EndpointCatalog.needsAuthentication` reproduces the middleware's predicate, + * which reads only errorResponseBodies and roles -- both about the user -- so an endpoint that + * requires a consumer (e.g. createConsentRequest, which authenticates the calling TPP via + * Client Credentials, not a logged-in user) is classified "public" here and would otherwise + * fail this assertion for doing exactly what its doc says. + */ private def checkPublicIsNot401(doc: ResourceDoc): Option[String] = { - val (code, _) = call(doc.requestVerb, EndpointCatalog.concretePath(doc), Map.empty) - if (code == 401) - Some(s"${describe(doc)} -- declares no authentication requirement yet answered 401 anonymously") - else None + val (code, json) = call(doc.requestVerb, EndpointCatalog.concretePath(doc), Map.empty) + val msg = messageOf(json) + if (code != 401) None + else if (msg.startsWith(ApplicationNotIdentified.take(9))) None + else Some(s"${describe(doc)} -- declares no authentication requirement yet answered 401 " + + s"anonymously with '$msg'") } private def checkNoRoleIs403(doc: ResourceDoc): Option[String] = { val path = EndpointCatalog.concretePath(doc, realEntities) val (code, json) = call(doc.requestVerb, path, noRoleHeaders) val roles = doc.roles.getOrElse(Nil).map(_.toString).mkString(",") - if (code != 403) - Some(s"${doc.operationId} ${doc.requestVerb} $path -- roles $roles: " + - s"expected 403 for a user holding no entitlements, got $code") - else if (!messageOf(json).startsWith(UserHasMissingRoles)) - Some(s"${doc.operationId} ${doc.requestVerb} $path -- 403 but message was " + - s"'${messageOf(json)}', expected it to start with '$UserHasMissingRoles'") - else None + if (code == 403 && messageOf(json).startsWith(UserHasMissingRoles)) None + else expectedAuthDeviation.get(doc.operationId) match { + case Some(_) => None + case None if code != 403 => + Some(s"${doc.operationId} ${doc.requestVerb} $path -- roles $roles: " + + s"expected 403 for a user holding no entitlements, got $code") + case None => + Some(s"${doc.operationId} ${doc.requestVerb} $path -- 403 but message was " + + s"'${messageOf(json)}', expected it to start with '$UserHasMissingRoles'") + } } // ── the sweep, one scenario per version ───────────────────────────────────── diff --git a/obp-api/src/test/scala/code/api/sweep/SuccessSweepTest.scala b/obp-api/src/test/scala/code/api/sweep/SuccessSweepTest.scala index 69fc079403..38a455cdc3 100644 --- a/obp-api/src/test/scala/code/api/sweep/SuccessSweepTest.scala +++ b/obp-api/src/test/scala/code/api/sweep/SuccessSweepTest.scala @@ -94,7 +94,11 @@ class SuccessSweepTest extends ServerSetupWithTestData with DefaultUsers { "OBPv4.0.0-getMyApiCollectionByName" -> "400 OBP-30079: no ApiCollection named API_COLLECTION_NAME", "OBPv4.0.0-getMyApiCollectionEndpoints" -> "400 OBP-30079: no ApiCollection named API_COLLECTION_NAME", "OBPv6.0.0-getWebUiProp" -> "400 OBP-08003: no WebUi prop named WEBUI_PROP_NAME", - "OBPv7.0.0-getRoutingScheme" -> "404 OBP-30514: no routing scheme named SCHEME" + "OBPv7.0.0-getRoutingScheme" -> "404 OBP-30514: no routing scheme named SCHEME", + // CHANNEL_NAME is a placeholder to EndpointCatalog (isPlaceholder matches the _NAME suffix) + // but not to hasNoPathVariable's narrower regex below, so this endpoint lands in the + // no-setup-required bucket even though its path names an entity nothing creates. + "OBPv6.0.0-getSignalChannelInfo" -> "404 OBP-10058: no Signal Channel named CHANNEL_NAME" ) private def realBankId: Option[String] =