refactor(api): update authentication error handling to use AuthenticatedUserIsRequired

- Replaced instances of UserNotLoggedIn with AuthenticatedUserIsRequired across multiple API versions and utility classes.
- Updated error response handling in ResourceDocMiddleware and APIUtil to reflect the new authentication requirement.
- Ensured consistency in error messages and improved clarity in authentication checks throughout the codebase.
This commit is contained in:
hongwei 2026-01-20 15:21:57 +01:00
parent 8f2048c89e
commit de2ed5f61a
11 changed files with 59 additions and 27 deletions

View File

@ -310,7 +310,7 @@ Some general notes that apply to all end points that retrieve transactions:
// "first" : "first"
// }
//}"""),
// List(UserNotLoggedIn, UnknownError),
// List(AuthenticatedUserIsRequired, UnknownError),
//
// ApiTag("Banking") ::ApiTag("Accounts") :: apiTagMockedData :: Nil
// )
@ -319,7 +319,7 @@ Some general notes that apply to all end points that retrieve transactions:
// case "banking":: "accounts" :: Nil JsonGet _ => {
// cc =>
// for {
// (Full(u), callContext) <- authorizedAccess(cc, UserNotLoggedIn)
// (Full(u), callContext) <- authorizedAccess(cc, AuthenticatedUserIsRequired)
// } yield {
// (json.parse("""{
// "data" : {
@ -394,7 +394,7 @@ Some general notes that apply to all end points that retrieve transactions:
// "self" : "self"
// }
//}"""),
// List(UserNotLoggedIn, UnknownError),
// List(AuthenticatedUserIsRequired, UnknownError),
//
// ApiTag("Banking") ::ApiTag("Accounts") :: apiTagMockedData :: Nil
// )
@ -403,7 +403,7 @@ Some general notes that apply to all end points that retrieve transactions:
// case "banking":: "accounts" :: accountId:: "balance" :: Nil JsonGet _ => {
// cc =>
// for {
// (Full(u), callContext) <- authorizedAccess(cc, UserNotLoggedIn)
// (Full(u), callContext) <- authorizedAccess(cc, AuthenticatedUserIsRequired)
// } yield {
// (json.parse("""{
// "data" : {

View File

@ -652,7 +652,7 @@ object OpenAPI31JSONFactory extends MdcLoggable {
* Determines if an endpoint requires authentication
*/
private def requiresAuthentication(doc: ResourceDocJson): Boolean = {
doc.error_response_bodies.exists(_.contains("UserNotLoggedIn")) ||
doc.error_response_bodies.exists(_.contains("AuthenticatedUserIsRequired")) ||
doc.roles.nonEmpty ||
doc.description.toLowerCase.contains("authentication is required") ||
doc.description.toLowerCase.contains("user must be logged in")

View File

@ -1673,7 +1673,7 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{
errorResponseBodies ?+= AuthenticatedUserIsRequired
errorResponseBodies ?+= UserHasMissingRoles
}
// if authentication is required, add UserNotLoggedIn to errorResponseBodies
// if authentication is required, add AuthenticatedUserIsRequired to errorResponseBodies
if (description.contains(authenticationIsRequired)) {
errorResponseBodies ?+= AuthenticatedUserIsRequired
} else if (description.contains(authenticationIsOptional) && rolesIsEmpty) {
@ -1791,7 +1791,7 @@ object APIUtil extends MdcLoggable with CustomJsonFormats{
}.toMap
/**
* According errorResponseBodies whether contains UserNotLoggedIn and UserHasMissingRoles do validation.
* According errorResponseBodies whether contains AuthenticatedUserIsRequired and UserHasMissingRoles do validation.
* So can avoid duplicate code in endpoint body for expression do check.
* Note: maybe this will be misused, So currently just comment out.
*/

View File

@ -138,7 +138,7 @@ object ResourceDocMiddleware extends MdcLoggable{
IO.pure(Right((boxUser, cc)))
case Left(e) =>
// For anonymous access, we don't fail on auth errors - just continue with Empty user
// This allows endpoints without $UserNotLoggedIn to work without authentication
// This allows endpoints without $AuthenticatedUserIsRequired to work without authentication
logger.debug(s"[ResourceDocMiddleware] anonymousAccess threw exception (ignoring for anonymous): ${e.getClass.getName}: ${e.getMessage.take(100)}")
IO.pure(Right((Empty, cc)))
}

View File

@ -1395,7 +1395,7 @@ trait APIMethods200 {
// CreateMeetingJson("tokbox", "onboarding"),
// meetingJson,
// List(
// UserNotLoggedIn,
// AuthenticatedUserIsRequired,
// MeetingApiKeyNotConfigured,
// MeetingApiSecretNotConfigured,
// InvalidBankIdFormat,
@ -1415,7 +1415,7 @@ trait APIMethods200 {
// // TODO use these keys to get session and tokens from tokbox
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_key") ~> APIFailure(MeetingApiKeyNotConfigured, 403)
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_secret") ~> APIFailure(MeetingApiSecretNotConfigured, 403)
// u <- cc.user ?~! UserNotLoggedIn
// u <- cc.user ?~! AuthenticatedUserIsRequired
// _ <- tryo(assert(isValidID(bankId.value)))?~! InvalidBankIdFormat
// (bank, callContext) <- BankX(bankId, Some(cc)) ?~! BankNotFound
// postedData <- tryo {json.extract[CreateMeetingJson]} ?~! InvalidJsonFormat
@ -1455,7 +1455,7 @@ trait APIMethods200 {
// EmptyBody,
// meetingsJson,
// List(
// UserNotLoggedIn,
// AuthenticatedUserIsRequired,
// MeetingApiKeyNotConfigured,
// MeetingApiSecretNotConfigured,
// BankNotFound,
@ -1469,11 +1469,11 @@ trait APIMethods200 {
// cc =>
// if (APIUtil.getPropsAsBoolValue("meeting.tokbox_enabled", false)) {
// for {
// _ <- cc.user ?~! ErrorMessages.UserNotLoggedIn
// _ <- cc.user ?~! ErrorMessages.AuthenticatedUserIsRequired
// (bank, callContext ) <- BankX(bankId, Some(cc)) ?~! BankNotFound
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_key") ~> APIFailure(ErrorMessages.MeetingApiKeyNotConfigured, 403)
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_secret") ~> APIFailure(ErrorMessages.MeetingApiSecretNotConfigured, 403)
// u <- cc.user ?~! ErrorMessages.UserNotLoggedIn
// u <- cc.user ?~! ErrorMessages.AuthenticatedUserIsRequired
// (bank, callContext) <- BankX(bankId, Some(cc)) ?~! BankNotFound
// // now = Calendar.getInstance().getTime()
// meetings <- Meetings.meetingProvider.vend.getMeetings(bank.bankId, u)
@ -1510,7 +1510,7 @@ trait APIMethods200 {
// EmptyBody,
// meetingJson,
// List(
// UserNotLoggedIn,
// AuthenticatedUserIsRequired,
// BankNotFound,
// MeetingApiKeyNotConfigured,
// MeetingApiSecretNotConfigured,
@ -1526,7 +1526,7 @@ trait APIMethods200 {
// cc =>
// if (APIUtil.getPropsAsBoolValue("meeting.tokbox_enabled", false)) {
// for {
// u <- cc.user ?~! UserNotLoggedIn
// u <- cc.user ?~! AuthenticatedUserIsRequired
// (bank, callContext ) <- BankX(bankId, Some(cc)) ?~! BankNotFound
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_key") ~> APIFailure(ErrorMessages.MeetingApiKeyNotConfigured, 403)
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_secret") ~> APIFailure(ErrorMessages.MeetingApiSecretNotConfigured, 403)

View File

@ -1300,7 +1300,7 @@ trait APIMethods220 {
EmptyBody,
customerViewsJsonV220,
List(
UserNotLoggedIn,
AuthenticatedUserIsRequired,
BankNotFound,
AccountNotFound,
ViewNotFound
@ -1336,7 +1336,7 @@ trait APIMethods220 {
case "management" :: "connector" :: "metrics" :: Nil JsonGet _ => {
cc =>{
for {
u <- user ?~! ErrorMessages.UserNotLoggedIn
u <- user ?~! ErrorMessages.AuthenticatedUserIsRequired
_ <- booleanToBox(hasEntitlement("", u.userId, ApiRole.CanGetConnectorMetrics), s"$CanGetConnectorMetrics entitlement required")
} yield {

View File

@ -2989,7 +2989,7 @@ trait APIMethods310 {
// These following are only for `tokbox` stuff, for now, just ignore it.
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_key") ~> APIFailure(MeetingApiKeyNotConfigured, 403)
// _ <- APIUtil.getPropsValue("meeting.tokbox_api_secret") ~> APIFailure(MeetingApiSecretNotConfigured, 403)
// u <- cc.user ?~! UserNotLoggedIn
// u <- cc.user ?~! AuthenticatedUserIsRequired
// _ <- tryo(assert(isValidID(bankId.value)))?~! InvalidBankIdFormat
// (bank, callContext) <- Bank(bankId, Some(cc)) ?~! BankNotFound
// postedData <- tryo {json.extract[CreateMeetingJson]} ?~! InvalidJsonFormat

View File

@ -3766,7 +3766,7 @@ trait APIMethods600 {
// )
// ),
// List(
// UserNotLoggedIn,
// AuthenticatedUserIsRequired,
// UserHasMissingRoles,
// SystemViewNotFound,
// UnknownError
@ -4968,7 +4968,7 @@ trait APIMethods600 {
)
),
List(
UserNotLoggedIn,
AuthenticatedUserIsRequired,
UserHasMissingRoles,
UnknownError
),
@ -5446,7 +5446,7 @@ trait APIMethods600 {
)
),
List(
UserNotLoggedIn,
AuthenticatedUserIsRequired,
UserHasMissingRoles,
UnknownError
),
@ -5710,7 +5710,7 @@ trait APIMethods600 {
result = true
),
List(
UserNotLoggedIn,
AuthenticatedUserIsRequired,
UserHasMissingRoles,
InvalidJsonFormat,
UnknownError

View File

@ -41,7 +41,7 @@ object Http4s700 {
// Common prefix: /obp/v7.0.0
val prefixPath = Root / ApiPathZero.toString / implementedInApiVersion.toString
// ResourceDoc with $UserNotLoggedIn in errorResponseBodies indicates auth is required
// ResourceDoc with $AuthenticatedUserIsRequired 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(
@ -68,7 +68,7 @@ object Http4s700 {
)
// Route: GET /obp/v7.0.0/root
// Authentication is handled automatically by ResourceDocMiddleware based on $UserNotLoggedIn in ResourceDoc
// Authentication is handled automatically by ResourceDocMiddleware based on $AuthenticatedUserIsRequired 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" =>
@ -116,6 +116,38 @@ object Http4s700 {
Ok(response)
}
resourceDocs += ResourceDoc(
null,
implementedInApiVersion,
nameOf(getResourceDocsObpV700),
"GET",
"/resource-docs/API_VERSION/obp",
"Get Resource Docs",
s"""Get documentation about the RESTful resources on this server including example body payloads.
|
|* API_VERSION: The version of the API for which you want documentation
|
|Returns JSON containing information about the endpoints including:
|* Method (GET, POST, etc.)
|* URL path
|* Summary and description
|* Example request and response bodies
|* Required roles and permissions
|
|Optional query parameters:
|* tags - filter by API tags
|* functions - filter by function names
|* locale - specify language for descriptions
|* content - filter by content type""",
EmptyBody,
EmptyBody,
List(
UnknownError
),
List(apiTagDocumentation, apiTagApi),
http4sPartialFunction = Some(getResourceDocsObpV700)
)
val getResourceDocsObpV700: HttpRoutes[IO] = HttpRoutes.of[IO] {
case req @ GET -> `prefixPath` / "resource-docs" / requestedApiVersionString / "obp" =>
import com.openbankproject.commons.ExecutionContext.Implicits.global
@ -254,7 +286,7 @@ object Http4s700 {
}
// Routes with ResourceDocMiddleware - provides automatic validation based on ResourceDoc metadata
// Authentication is automatic based on $UserNotLoggedIn in ResourceDoc errorResponseBodies
// Authentication is automatic based on $AuthenticatedUserIsRequired in ResourceDoc errorResponseBodies
// This matches Lift's wrappedWithAuthCheck behavior
val wrappedRoutesV700Services: HttpRoutes[IO] = Implementations7_0_0.allRoutesWithMiddleware
}

View File

@ -73,7 +73,7 @@ class CardTest extends V500ServerSetupAsync with DefaultUsers {
val responseAnonymous = makePostRequest(requestAnonymous, write(properCardJson))
And(s"We should get 401 and get the authentication error")
responseAnonymous.code should equal(401)
responseAnonymous.body.toString contains(s"$UserNotLoggedIn") should be (true)
responseAnonymous.body.toString contains(s"$AuthenticatedUserIsRequired") should be (true)
Then(s"We call the authentication user, but without proper role: ${ApiRole.canCreateCardsForBank}")
val responseUserButNoRole = makePostRequest(requestWithAuthUser, write(properCardJson))

View File

@ -383,7 +383,7 @@ object OpenAPI31Exporter {
}
// Security
if (endpoint.roles.nonEmpty || !endpoint.errorCodes.exists(_.contains("UserNotLoggedIn"))) {
if (endpoint.roles.nonEmpty || !endpoint.errorCodes.exists(_.contains("AuthenticatedUserIsRequired"))) {
yaml.append(" security:\n")
yaml.append(" - DirectLogin: []\n")
yaml.append(" - GatewayLogin: []\n")