Skip to content

Commit b093fc8

Browse files
wenyang-caowenyang-cao
committed
fix: handle non-UUID path segment in GET/DELETE /apikeys
Signed-off-by: wenyang-cao <wenyang.cao@ibm.com>
1 parent 4632c2a commit b093fc8

3 files changed

Lines changed: 91 additions & 76 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
44

55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
66

7+
## [2.133.0](https://github.com/open-horizon/exchange-api/pull/786) - 2025-07-07
8+
- Issue: #785: Fixed incorrect routing for POST /apikeys/{keyid} with invalid UUID: now correctly rejects extra path segments. Improved GET/DELETE /apikeys/{keyid} to return 400 Bad Request for invalid UUID format instead of falling back to 405.
9+
710
## [2.132.0](https://github.com/open-horizon/exchange-api/pull/792) - 2025-07-03
811
- Enables OAuth authentication without creating an existing or new identity for the route .../v1/myorgs.
912

‎src/main/resources/version.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.132.0
1+
2.133.0

‎src/main/scala/org/openhorizon/exchangeapi/route/apikey/UserApiKeys.scala‎

Lines changed: 87 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import java.util.UUID
2222

2323
import scala.concurrent.{ExecutionContext, Future}
2424
import scala.concurrent.duration._
25-
import scala.util.{Failure, Success}
25+
import scala.util.{Failure, Success, Try}
2626

2727
import io.swagger.v3.oas.annotations.parameters.RequestBody
2828
import scalacache.modes.scalaFuture._
@@ -181,46 +181,50 @@ trait UserApiKeys extends JacksonSupport with AuthenticationSupport {
181181
def deleteUserApiKey(@Parameter(hidden = true) identity: Identity2,
182182
@Parameter(hidden = true) organization: String,
183183
@Parameter(hidden = true) username: String,
184-
@Parameter(hidden = true) keyid: UUID,
184+
@Parameter(hidden = true) keyidStr: String,
185185
@Parameter(hidden = true) resourceUuidOpt: Option[UUID]): Route = complete {
186-
187-
resourceUuidOpt match {
188-
case Some(userUuid) =>
189-
val deleteQuery = Compiled {
190-
ApiKeysTQ.filter(k => k.id === keyid && k.user === userUuid && k.orgid === organization)
191-
.filterIf(identity.isStandardUser)(k => k.user === identity.identifier.get)
192-
.filterIf(identity.isOrgAdmin)(k => k.orgid === identity.organization)
193-
.filterIf(identity.isHubAdmin)(k => UsersTQ.filter(u => u.user === k.user && ((u.isHubAdmin || u.isOrgAdmin) && !(u.organization === "root" && u.username === "root"))).exists)
194-
}
195-
196-
db.run((for {
197-
deleted <- deleteQuery.delete
198-
// _ <- if (deleted > 0) {
199-
// ResourceChangesTQ += ResourceChange(
200-
// 0L,
201-
// organization,
202-
// keyid.toString,
203-
// ResChangeCategory.APIKEY,
204-
// public = false,
205-
// ResChangeResource.APIKEY,
206-
// ResChangeOperation.DELETED
207-
// ).toResourceChangeRow
208-
// } else {
209-
// DBIO.successful(0)
210-
// }
211-
} yield deleted).transactionally).map {
212-
case 0 =>
213-
(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, ExchMsg.translate("apikey.not.found")))
214-
case _ =>
215-
(StatusCodes.NoContent, ApiResponse(ApiRespType.OK, ExchMsg.translate("apikey.deleted")))
216-
}.recover {
217-
case ex =>
218-
logger.error(s"Error deleting API key $keyid for $organization/$username", ex)
219-
(StatusCodes.InternalServerError, ApiResponse(ApiRespType.INTERNAL_ERROR, ExchMsg.translate("apikey.deletion.failed")))
186+
Try(UUID.fromString(keyidStr)) match {
187+
case Failure(_) =>
188+
Future.successful((StatusCodes.BadRequest, ApiResponse(ApiRespType.BAD_INPUT, "Invalid UUID format for API key ID")))
189+
case Success(keyid) =>
190+
resourceUuidOpt match {
191+
case Some(userUuid) =>
192+
val deleteQuery = Compiled {
193+
ApiKeysTQ.filter(k => k.id === keyid && k.user === userUuid && k.orgid === organization)
194+
.filterIf(identity.isStandardUser)(k => k.user === identity.identifier.get)
195+
.filterIf(identity.isOrgAdmin)(k => k.orgid === identity.organization)
196+
.filterIf(identity.isHubAdmin)(k => UsersTQ.filter(u => u.user === k.user && ((u.isHubAdmin || u.isOrgAdmin) && !(u.organization === "root" && u.username === "root"))).exists)
197+
}
198+
199+
db.run((for {
200+
deleted <- deleteQuery.delete
201+
// _ <- if (deleted > 0) {
202+
// ResourceChangesTQ += ResourceChange(
203+
// 0L,
204+
// organization,
205+
// keyid.toString,
206+
// ResChangeCategory.APIKEY,
207+
// public = false,
208+
// ResChangeResource.APIKEY,
209+
// ResChangeOperation.DELETED
210+
// ).toResourceChangeRow
211+
// } else {
212+
// DBIO.successful(0)
213+
// }
214+
} yield deleted).transactionally).map {
215+
case 0 =>
216+
(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, ExchMsg.translate("apikey.not.found")))
217+
case _ =>
218+
(StatusCodes.NoContent, ApiResponse(ApiRespType.OK, ExchMsg.translate("apikey.deleted")))
219+
}.recover {
220+
case ex =>
221+
logger.error(s"Error deleting API key $keyid for $organization/$username", ex)
222+
(StatusCodes.InternalServerError, ApiResponse(ApiRespType.INTERNAL_ERROR, ExchMsg.translate("apikey.deletion.failed")))
223+
}
224+
225+
case None =>
226+
Future.successful((StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, "User not found")))
220227
}
221-
222-
case None =>
223-
complete(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, "User not found"))
224228
}
225229
}
226230

@@ -259,41 +263,47 @@ trait UserApiKeys extends JacksonSupport with AuthenticationSupport {
259263
new responses.ApiResponse(responseCode = "404", description = "not found")
260264
)
261265
)
262-
def getUserApiKeyById(@Parameter(hidden = true) identity: Identity2,
263-
@Parameter(hidden = true) organization: String,
264-
@Parameter(hidden = true) username: String,
265-
@Parameter(hidden = true) keyid: UUID,
266-
@Parameter(hidden = true) resourceUuidOpt: Option[UUID]): Route = complete {
267-
resourceUuidOpt match {
268-
case Some(userUuid) =>
269-
val keyQuery = Compiled {
270-
ApiKeysTQ.filter(k => k.id === keyid && k.user === userUuid && k.orgid === organization)
271-
.filterIf(identity.isStandardUser)(k => k.user === identity.identifier.get)
272-
.filterIf(identity.isOrgAdmin)(k => k.orgid === identity.organization)
273-
.filterIf(identity.isHubAdmin)(k => UsersTQ.filter(u => u.user === k.user && ((u.isHubAdmin || u.isOrgAdmin) && !(u.organization === "root" && u.username === "root"))).exists)
274-
.take(1)
275-
}
276-
277-
db.run(keyQuery.result.headOption).map {
278-
case Some(keyRow) =>
279-
val ownerStr = s"$organization/$username"
280-
val metadata = new ApiKeyMetadata(keyRow, ownerStr)
281-
(StatusCodes.OK, metadata)
282-
266+
def getUserApiKeyById(@Parameter(hidden = true) identity: Identity2,
267+
@Parameter(hidden = true) organization: String,
268+
@Parameter(hidden = true) username: String,
269+
@Parameter(hidden = true) keyidStr: String,
270+
@Parameter(hidden = true) resourceUuidOpt: Option[UUID]): Route = complete {
271+
Try(UUID.fromString(keyidStr)) match {
272+
case Failure(_) =>
273+
Future.successful((StatusCodes.BadRequest, ApiResponse(ApiRespType.BAD_INPUT, "Invalid UUID format for API key ID")))
274+
275+
case Success(keyid) =>
276+
resourceUuidOpt match {
277+
case Some(userUuid) =>
278+
val keyQuery = Compiled {
279+
ApiKeysTQ.filter(k => k.id === keyid && k.user === userUuid && k.orgid === organization)
280+
.filterIf(identity.isStandardUser)(k => k.user === identity.identifier.get)
281+
.filterIf(identity.isOrgAdmin)(k => k.orgid === identity.organization)
282+
.filterIf(identity.isHubAdmin)(k => UsersTQ.filter(u => u.user === k.user && ((u.isHubAdmin || u.isOrgAdmin) && !(u.organization === "root" && u.username === "root"))).exists)
283+
.take(1)
284+
}
285+
286+
db.run(keyQuery.result.headOption).map {
287+
case Some(keyRow) =>
288+
val ownerStr = s"$organization/$username"
289+
val metadata = new ApiKeyMetadata(keyRow, ownerStr)
290+
(StatusCodes.OK, metadata)
291+
292+
case None =>
293+
(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, ExchMsg.translate("apikey.not.found")))
294+
}.recover {
295+
case ex =>
296+
logger.error(s"Failed to get API key $keyid for $organization/$username", ex)
297+
(StatusCodes.InternalServerError, ApiResponse(ApiRespType.INTERNAL_ERROR, "Failed to retrieve API key"))
298+
}
299+
283300
case None =>
284-
(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, ExchMsg.translate("apikey.not.found")))
285-
}.recover {
286-
case ex =>
287-
logger.error(s"Failed to get API key $keyid for $organization/$username", ex)
288-
(StatusCodes.InternalServerError, ApiResponse(ApiRespType.INTERNAL_ERROR, "Failed to retrieve API key"))
301+
Future.successful((StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, "User not found")))
289302
}
290-
291-
case None =>
292-
complete(StatusCodes.NotFound, ApiResponse(ApiRespType.NOT_FOUND, "User not found"))
293303
}
294304
}
295305

296-
def userApiKeys(identity: Identity2): Route = {
306+
def userApiKeys(identity: Identity2): Route = {
297307
pathPrefix("orgs" / Segment / "users" / Segment / "apikeys") { (organization, username) =>
298308
val resourceType = "user"
299309
val resource = OrgAndId(organization, username).toString
@@ -307,20 +317,22 @@ def userApiKeys(identity: Identity2): Route = {
307317
}
308318

309319
def routeMethods(resourceIdentity: Option[UUID]): Route = {
310-
post {
311-
exchAuth(TUser(resource, resourceIdentity), Access.WRITE, validIdentity = identity) { _ =>
312-
postUserApiKey(identity, organization, username, resourceIdentity)
320+
pathEnd {
321+
post {
322+
exchAuth(TUser(resource, resourceIdentity), Access.WRITE, validIdentity = identity) { _ =>
323+
postUserApiKey(identity, organization, username, resourceIdentity)
324+
}
313325
}
314326
} ~
315-
path(JavaUUID) { keyid =>
327+
path(Segment) { keyidStr =>
316328
get {
317329
exchAuth(TUser(resource, resourceIdentity), Access.READ, validIdentity = identity) { _ =>
318-
getUserApiKeyById(identity, organization, username, keyid, resourceIdentity)
330+
getUserApiKeyById(identity, organization, username, keyidStr, resourceIdentity)
319331
}
320332
} ~
321333
delete {
322334
exchAuth(TUser(resource, resourceIdentity), Access.WRITE, validIdentity = identity) { _ =>
323-
deleteUserApiKey(identity, organization, username, keyid, resourceIdentity)
335+
deleteUserApiKey(identity, organization, username, keyidStr, resourceIdentity)
324336
}
325337
}
326338
}

0 commit comments

Comments
 (0)