This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 0de241723e test(access-control): pin token extraction and the
privilege guard (#7690)
0de241723e is described below
commit 0de241723eb80ac23cc765d2b22c20bb7321bb9b
Author: Xinyuan Lin <[email protected]>
AuthorDate: Sat Aug 15 21:03:14 2026 +0000
test(access-control): pin token extraction and the privilege guard (#7690)
### What changes were proposed in this PR?
`AccessControlResource` is the ext-authz endpoint the gateway calls on
every request. It was at **74.2% of 159 lines** behind a 13-test spec,
and the residue was concentrated in `extractTokenFromBody` -- which the
entire suite invoked exactly once, with a body carrying no token, so
only the "everything returns None" skeleton ran.
Tests **13 -> 32**. Measured across the whole module (the file is also
touched by sibling specs, so a single-spec figure would not be
comparable to the reported one):
| | Before | After |
|---|---|---|
| lines | 74.2% | **152/159 (95.6%)** |
| branches | -- | 74/112 (66.1%) |
Now covered: all three body parsers (JSON, form-urlencoded, multipart,
including both boundary shapes), the `access-token` query parameter, the
full token-precedence chain, both missing arms of the route whitelist,
cuid resolution from path vs query, and the privilege and identity
lookups.
### Verification, and what it found
Review proposed 20 mutations. **All 20 survived the 22-test suite** --
every finding was real, none was a false alarm. 19 are now killed, each
verified red on its intended test by name from the JUnit XML; the
twentieth is an equivalent mutant, explained below. 24 distinct
mutations were run over two passes, one at a time, with `git diff
--quiet` on the production file asserted after every single run.
**The most serious finding: the authorization guard itself was
unpinned.** Deleting
```scala
if (cuAccess == PrivilegeEnum.NONE) return FORBIDDEN
```
left all 22 tests green. The suite's only negative case used a cuid
absent from the fixture DB, so its 403 came from the routing check --
the privilege check was never the reason for any rejection anywhere.
Fixed with a fixture computing unit that exists and is routable but on
which the test user holds no privilege.
Three other axes were constants across the whole suite, so a lookup
could be replaced by a literal and nothing noticed:
| Surviving mutation | Why nothing noticed |
|---|---|
| `getComputingUnitAccess(cuidInt, uid)` -> `(cuidInt, 1)` | every
request authenticated as uid 1 |
| the `x-user-id` header hard-coded to `"1"` | same |
| `qToken.orElse(hToken).orElse(bToken)` reordered | only
query-vs-header was ever discriminated, never header-vs-body |
Also fixed: the multipart regexes were pinned only by fixtures too
simple to discriminate them (`(.*?)` vs `(.*)`, the `[^\r\n]*` attribute
tail, `\s*=\s*` spacing, and `\r?\n` vs `\r\n`), and three `.trim` /
`.map(_.trim)` calls that no fixture ever padded.
### One mutation is deliberately left alive
At `:193`, `val key = if (idx >= 0) p.substring(0, idx) else p` -> `else
""` survives, and that is correct rather than a gap. The `idx < 0` leg
is the only one reaching that `else`, and in the same leg `raw` is
hard-coded to `""` at `:195`, so `v` is always empty and `found` is
never assigned. No input can distinguish the two -- killing it would
require mutating `:195` simultaneously. The line is executed; it is
simply not behaviourally observable.
### Deliberately not included
The `catch` at `:136-137` is unreachable: `JwtParser.parseToken`
swallows every exception internally and returns `Optional.empty`, and
the only other throw source is a Scala `object` with no injection seam
whose failure mode would corrupt the shared MockTexeraDB pool.
Two observations are reported rather than pinned, so neither is
cemented: `extractTokenFromBody` never consults `Content-Type` and
accepts the first `token` it finds in any of the three formats (bounded,
since the value must still be a validly signed JWT), and one
privilege-comparison question I have raised with a maintainer directly
rather than here.
No production file is touched.
### Any related issues, documentation, discussions?
Closes #7689
### How was this PR tested?
```
sbt "AccessControlService/testOnly
org.apache.texera.AccessControlResourceSpec"
```
```
[info] Total number of tests run: 32
[info] Tests: succeeded 32, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```
The full module is green too (`AccessControlService/jacoco`, 56 tests),
which matters here because `LiteLLMProxyAuthSpec` shares the JVM and
resets the logback context. `Test/scalafmtCheck` and `Test/scalafix
--check` both pass.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
---
.../apache/texera/AccessControlResourceSpec.scala | 401 ++++++++++++++++++++-
1 file changed, 387 insertions(+), 14 deletions(-)
diff --git
a/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
b/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
index b4f1b89617..10ff44db7f 100644
---
a/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
+++
b/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
@@ -42,7 +42,8 @@ import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers
import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
-import java.net.URI
+import java.net.{URI, URLEncoder}
+import java.nio.charset.StandardCharsets
import java.util
class AccessControlResourceSpec
@@ -60,6 +61,18 @@ class AccessControlResourceSpec
private val testRecordedUri: String =
"computing-unit-2.compute-unit-svc.default.svc.cluster.local:8888"
+ // A second, deliberately different recorded URI. Routing tests that target
the
+ // read-only unit assert on this value so a Host header that ignored the
looked-up
+ // row (or hard-coded the other unit's host) would fail.
+ private val testReadOnlyRecordedUri: String =
+ "computing-unit-5.compute-unit-svc.default.svc.cluster.local:9999"
+
+ // Recorded URI of the routable unit testUser1 has no privilege on. It
exists so a
+ // request refused by the privilege check cannot be confused with one
refused for
+ // being unroutable.
+ private val testNoAccessRecordedUri: String =
+ "computing-unit-6.compute-unit-svc.default.svc.cluster.local:7777"
+
private val testUser1: User = {
val user = new User()
user.setUid(1)
@@ -112,8 +125,39 @@ class AccessControlResourceSpec
cu
}
+ // A routable computing unit on which testUser1 holds READ (not WRITE).
Every other
+ // fixture grants WRITE, so this is the only unit whose privilege header can
tell a
+ // real lookup apart from a hard-coded WRITE.
+ private val testCUReadOnly: WorkflowComputingUnit = {
+ val cu = new WorkflowComputingUnit()
+ cu.setUid(2)
+ cu.setType(WorkflowComputingUnitTypeEnum.kubernetes)
+ cu.setCuid(5)
+ cu.setName("test-cu-read-only")
+ cu.setUri(testReadOnlyRecordedUri)
+ cu
+ }
+
+ // A perfectly routable computing unit owned by testUser2 on which testUser1
is
+ // granted nothing at all. It is the only fixture whose refusal can come
from the
+ // privilege check itself: every other refusal in this suite is produced
either by
+ // the unit not existing or by it having no recorded URI.
+ private val testCUNoAccess: WorkflowComputingUnit = {
+ val cu = new WorkflowComputingUnit()
+ cu.setUid(testUser2.getUid)
+ cu.setType(WorkflowComputingUnitTypeEnum.kubernetes)
+ cu.setCuid(6)
+ cu.setName("test-cu-no-access")
+ cu.setUri(testNoAccessRecordedUri)
+ cu
+ }
+
private var token: String = _
+ // A token for the *other* user. Tests that must tell "the authenticated
user" apart
+ // from "user 1" need a second identity to compare against.
+ private var token2: String = _
+
override protected def beforeAll(): Unit = {
initializeDBAndReplaceDSLContext()
val userDao = new UserDao(getDSLContext.configuration())
@@ -126,6 +170,9 @@ class AccessControlResourceSpec
computingUnitDao.insert(testCU)
computingUnitDao.insert(testCUNoUri)
computingUnitDao.insert(testCUBlankUri)
+ computingUnitDao.insert(testCUReadOnly)
+ // Deliberately inserted with NO computing_unit_user_access row for
testUser1.
+ computingUnitDao.insert(testCUNoAccess)
// Grant testUser1 WRITE access to every test computing unit so the routing
// logic (not the access check) is what each routing test exercises.
@@ -137,8 +184,14 @@ class AccessControlResourceSpec
computingUnitOfUserDao.insert(cuAccess)
}
- val claims = JwtAuth.jwtClaims(testUser1)
- token = JwtAuth.jwtToken(claims)
+ val readOnlyAccess = new ComputingUnitUserAccess()
+ readOnlyAccess.setUid(testUser1.getUid)
+ readOnlyAccess.setCuid(testCUReadOnly.getCuid)
+ readOnlyAccess.setPrivilege(PrivilegeEnum.READ)
+ computingUnitOfUserDao.insert(readOnlyAccess)
+
+ token = JwtAuth.jwtToken(JwtAuth.jwtClaims(testUser1))
+ token2 = JwtAuth.jwtToken(JwtAuth.jwtClaims(testUser2))
}
override protected def afterAll(): Unit = {
@@ -204,14 +257,16 @@ class AccessControlResourceSpec
response.getStatus shouldBe Response.Status.FORBIDDEN.getStatusCode
}
- "AccessControlResource" should "return FORBIDDEN when user does not have
access to the computing unit" in {
+ "AccessControlResource" should "return FORBIDDEN for an unknown computing
unit" in {
// Mock the request context
val mockUriInfo = mock(classOf[UriInfo])
val mockHttpHeaders = mock(classOf[HttpHeaders])
- // Prepare query parameters with a computing unit ID (cuid)
+ // cuid 1 is not in the fixture DB at all, so this exercises the "no such
unit"
+ // path. The privilege check itself is exercised by the no-privilege test
below,
+ // which uses a unit that exists and is routable.
val queryParams = new MultivaluedHashMap[String, String]()
- queryParams.add("cuid", "1") // Assuming user 1 does not have access to
cuid 1
+ queryParams.add("cuid", "1")
// Prepare request headers with the generated JWT
val requestHeaders = new MultivaluedHashMap[String, String]()
@@ -271,6 +326,16 @@ class AccessControlResourceSpec
response.getHeaderString("Host") shouldBe testRecordedUri
}
+ it should "return FORBIDDEN when the user has no privilege on a routable
computing unit" in {
+ // testCUNoAccess exists, is owned by testUser2, and has a recorded URI —
so the
+ // routing check would happily let this through. The only thing that can
refuse it
+ // is the PrivilegeEnum.NONE guard.
+ val (uri, headers) = mockRequest("/pve/system",
Some(testCUNoAccess.getCuid.toString))
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.FORBIDDEN.getStatusCode
+ }
+
it should "refuse the connection when no URI is recorded for the computing
unit" in {
val (uri, headers) = mockRequest(testPath,
Some(testCUNoUri.getCuid.toString))
val response = new AccessControlResource().authorizeGet(uri, headers)
@@ -285,25 +350,47 @@ class AccessControlResourceSpec
response.getStatus shouldBe Response.Status.FORBIDDEN.getStatusCode
}
+ /**
+ * Builds a mocked request context.
+ *
+ * @param authorizationHeader `None` models a client that sent no
Authorization
+ * header at all — JAX-RS returns `null` from
+ * `getRequestHeader` in that case, which is
what the
+ * body-token tests need so the body is the only
token
+ * source left.
+ * @param accessTokenQueryParam value for the `access-token` query
parameter, when
+ * the token travels in the URL instead.
+ */
private def mockRequest(
path: String,
- cuidQueryParam: Option[String]
+ cuidQueryParam: Option[String],
+ authorizationHeader: Option[String] = Some("Bearer " + token),
+ accessTokenQueryParam: Option[String] = None
): (UriInfo, HttpHeaders) = {
val mockUriInfo = mock(classOf[UriInfo])
val mockHttpHeaders = mock(classOf[HttpHeaders])
val queryParams = new MultivaluedHashMap[String, String]()
cuidQueryParam.foreach(queryParams.add("cuid", _))
+ accessTokenQueryParam.foreach(queryParams.add("access-token", _))
val requestHeaders = new MultivaluedHashMap[String, String]()
- requestHeaders.add("Authorization", "Bearer " + token)
when(mockUriInfo.getQueryParameters).thenReturn(queryParams)
when(mockUriInfo.getRequestUri).thenReturn(new URI(testURI))
when(mockUriInfo.getPath).thenReturn(path)
+
+ authorizationHeader match {
+ case Some(header) =>
+ requestHeaders.add("Authorization", header)
+ when(mockHttpHeaders.getRequestHeader("Authorization"))
+ .thenReturn(util.Arrays.asList(header))
+ case None =>
+ // JAX-RS hands back null for a header the client never sent.
+ when(mockHttpHeaders.getRequestHeader("Authorization"))
+ .thenReturn(null.asInstanceOf[util.List[String]])
+ }
when(mockHttpHeaders.getRequestHeaders).thenReturn(requestHeaders)
- when(mockHttpHeaders.getRequestHeader("Authorization"))
- .thenReturn(util.Arrays.asList("Bearer " + token))
(mockUriInfo, mockHttpHeaders)
}
@@ -315,18 +402,34 @@ class AccessControlResourceSpec
response.getStatus shouldBe Response.Status.OK.getStatusCode
}
- it should "return OK for /pve/pves/{cuid} (cuid extracted from path)" in {
- val (uri, headers) = mockRequest(s"/pve/pves/${testCU.getCuid}", None)
+ // The three path shapes below use the form Jersey actually hands to the
resource:
+ // UriInfo.getPath returns the path relative to the base URI, so a check for
+ // /auth/api/pve/... arrives as "auth/api/pve/..." — no leading slash, and
with the
+ // "auth/" and "api/" segments still attached. That is exactly what the
optional
+ // `^/?(?:auth/)?(?:api/|wsapi/)?` prefix groups on each pve regex exist to
absorb.
+ it should "return OK for the gateway-relative form of a pve route" in {
+ val (uri, headers) = mockRequest("auth/api/pve/system",
Some(testCU.getCuid.toString))
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "return OK for pve/pves/{cuid} (cuid extracted from path)" in {
+ val (uri, headers) = mockRequest(s"auth/api/pve/pves/${testCU.getCuid}",
None)
val response = new AccessControlResource().authorizeDelete(uri, headers)
response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
}
- it should "return OK for /pve/{cuid}/{pveName}/packages/{packageName} (cuid
extracted from path)" in {
- val (uri, headers) =
mockRequest(s"/pve/${testCU.getCuid}/myenv/packages/numpy", None)
+ it should "return OK for pve/{cuid}/{pveName}/packages/{packageName} (cuid
extracted from path)" in {
+ val (uri, headers) =
+ mockRequest(s"auth/api/pve/${testCU.getCuid}/myenv/packages/numpy", None)
val response = new AccessControlResource().authorizeDelete(uri, headers)
response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
}
it should "return FORBIDDEN for a PVE path with no cuid in query or path" in
{
@@ -349,4 +452,274 @@ class AccessControlResourceSpec
response.getStatus shouldBe Response.Status.OK.getStatusCode
}
+
+ it should "report the privilege and the recorded URI of the requested
computing unit" in {
+ val (uri, headers) = mockRequest("/pve/system",
Some(testCUReadOnly.getCuid.toString))
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ // testUser1 holds READ (not WRITE) on this unit, and this unit's recorded
URI
+ // differs from every other fixture's, so both headers are answered from
the
+ // row actually looked up rather than from a constant.
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString(
+ HeaderField.UserComputingUnitAccess
+ ) shouldBe PrivilegeEnum.READ.toString
+ response.getHeaderString("Host") shouldBe testReadOnlyRecordedUri
+ }
+
+ it should "resolve the privilege and identity of the authenticated user, not
a fixed one" in {
+ // testUser2 owns testCUReadOnly, so the owner branch of the lookup gives
it WRITE,
+ // while testUser1 — the identity every other test authenticates as — only
holds
+ // READ on that same unit. Answering for the wrong user is therefore
visible in
+ // both the privilege header and the forwarded identity headers.
+ val (uri, headers) = mockRequest(
+ "/pve/system",
+ Some(testCUReadOnly.getCuid.toString),
+ authorizationHeader = Some("Bearer " + token2)
+ )
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString(
+ HeaderField.UserComputingUnitAccess
+ ) shouldBe PrivilegeEnum.WRITE.toString
+ response.getHeaderString(HeaderField.UserId) shouldBe
testUser2.getUid.toString
+ response.getHeaderString(HeaderField.UserName) shouldBe testUser2.getName
+ response.getHeaderString(HeaderField.UserEmail) shouldBe testUser2.getEmail
+ }
+
+ it should "fall back to the cuid in the path when the cuid query parameter
is empty" in {
+ // Envoy forwards `?cuid=` verbatim when the client sends the parameter
with no
+ // value; an empty parameter must not shadow the cuid embedded in the path.
+ val (uri, headers) = mockRequest(s"/pve/pves/${testCUReadOnly.getCuid}",
Some(""))
+ val response = new AccessControlResource().authorizeDelete(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testReadOnlyRecordedUri
+ }
+
+ it should "prefer the cuid query parameter over the cuid embedded in the
path" in {
+ // Path says unit 5 (READ, its own host), query says unit 2 (WRITE,
another host):
+ // both headers show which one the resolution actually used.
+ val (uri, headers) =
+ mockRequest(s"/pve/pves/${testCUReadOnly.getCuid}",
Some(testCU.getCuid.toString))
+ val response = new AccessControlResource().authorizeDelete(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString(
+ HeaderField.UserComputingUnitAccess
+ ) shouldBe PrivilegeEnum.WRITE.toString
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "authorize the /wsapi/workflow-websocket route using the
access-token query parameter" in {
+ // The Authorization header carries a token that cannot be parsed, so a
200 is
+ // only reachable if the access-token query parameter is read AND
preferred.
+ val (uri, headers) = mockRequest(
+ "/wsapi/workflow-websocket",
+ Some(testCU.getCuid.toString),
+ authorizationHeader = Some("Bearer not-a-real-jwt"),
+ accessTokenQueryParam = Some(token)
+ )
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "authorize the /api/executions/result/export route" in {
+ val (uri, headers) =
+ mockRequest("/api/executions/result/export",
Some(testCU.getCuid.toString))
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ }
+
+ it should "ignore an empty access-token query parameter and fall back to the
Authorization header" in {
+ val (uri, headers) = mockRequest(
+ "/pve/system",
+ Some(testCU.getCuid.toString),
+ accessTokenQueryParam = Some("")
+ )
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ }
+
+ it should "accept a lower-case bearer scheme" in {
+ // RFC 7235 makes the auth scheme case-insensitive, so a client sending
"bearer"
+ // must not be turned away.
+ val (uri, headers) = mockRequest(
+ "/pve/system",
+ Some(testCU.getCuid.toString),
+ authorizationHeader = Some("bearer " + token)
+ )
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "accept an Authorization value that is a padded bare token" in {
+ // The scheme is stripped only if present, so a client that sends the raw
token is
+ // still understood — and the surrounding whitespace has to be removed
before the
+ // JWT parser sees it. (Padding after a "Bearer " scheme would prove
nothing: the
+ // strip regex already eats the leading whitespace, and jose4j ignores
trailing
+ // whitespace because it lands in the base64url signature segment. Only a
+ // scheme-less value can carry *leading* whitespace this far, and leading
+ // whitespace corrupts the signing input.)
+ val (uri, headers) = mockRequest(
+ "/pve/system",
+ Some(testCU.getCuid.toString),
+ authorizationHeader = Some(" " + token + " ")
+ )
+ val response = new AccessControlResource().authorizeGet(uri, headers)
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ //
---------------------------------------------------------------------------
+ // Token carried in the request body.
+ //
+ // Each of these sends no Authorization header and no access-token query
+ // parameter, so the body is the only token source left: the 200 can only
come
+ // from the body-parsing branch under test.
+ //
---------------------------------------------------------------------------
+
+ private val multipartBoundary: String = "----TexeraTestBoundary"
+ private val crlf: String = "\r\n"
+
+ /**
+ * A realistic two-part CRLF multipart document whose first part is named
"token".
+ *
+ * Everything about its shape is load-bearing: the Content-Disposition line
carries
+ * a trailing attribute after `name="token"`, the value is padded with
spaces, and a
+ * second part follows before the closing boundary — so the capture has to
stop at
+ * the *next* boundary rather than running to the last one.
+ */
+ private def multipartTwoParts(value: String): String =
+ s"--$multipartBoundary$crlf" +
+ s"""Content-Disposition: form-data; name="token";
filename="t.txt"$crlf""" +
+ crlf +
+ s" $value $crlf" +
+ s"--$multipartBoundary$crlf" +
+ s"""Content-Disposition: form-data; name="other"$crlf""" +
+ crlf +
+ s"hello$crlf" +
+ s"--$multipartBoundary--"
+
+ /**
+ * A single-part document that stops at the value, so only the run-to-end
fallback
+ * can recover the token. Deliberately built unlike [[multipartTwoParts]]:
LF-only
+ * line breaks (a client that does not send CRLF), whitespace around the
`=` in the
+ * Content-Disposition, a trailing attribute, and a padded value.
+ */
+ private def multipartRunToEnd(value: String): String =
+ s"--$multipartBoundary\n" +
+ "Content-Disposition: form-data; name = \"token\"; filename=\"t.txt\"\n"
+
+ "\n" +
+ s" $value "
+
+ private def bodyOnlyRequest(cuid: Int): (UriInfo, HttpHeaders) =
+ mockRequest("/pve/system", Some(cuid.toString), authorizationHeader = None)
+
+ it should "read the token out of an x-www-form-urlencoded body" in {
+ // A single `token=<value>` pair — the shape a form encoder produces for a
one-field
+ // form, and the one body with no '&' in it at all.
+ // Encoded the way a form encoder would: the surrounding spaces become
'+', so
+ // a reader that skipped URL-decoding would see "+<jwt>+" and reject it.
+ val encoded = URLEncoder.encode(s" $token ", StandardCharsets.UTF_8.name())
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ val response = new AccessControlResource().authorizePost(uri, headers,
s"token=$encoded")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "keep scanning an urlencoded body past a valueless token pair" in {
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ // The leading "token" pair has no '=' and therefore no value; it must not
be
+ // accepted as the token, and the scan must continue to the real pair.
+ val response =
+ new AccessControlResource().authorizePost(uri, headers,
s"token&other=1&token=$token")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ }
+
+ it should "take the first token pair when an urlencoded body carries
duplicates" in {
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ // Duplicate keys are not specified by the form-encoding spec, so the
resolution
+ // rule is a deliberate choice: the scan stops at the first usable value.
Both
+ // tokens here are valid and testUser2 owns this unit, so both orderings
return
+ // 200 — only the forwarded identity says which credential was selected.
+ val response =
+ new AccessControlResource().authorizePost(uri, headers,
s"token=$token&token=$token2")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString(HeaderField.UserId) shouldBe
testUser1.getUid.toString
+ }
+
+ it should "read the token out of a JSON body" in {
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ // Jackson preserves the padding, so the value has to be trimmed before it
reaches
+ // the JWT parser.
+ val response =
+ new AccessControlResource().authorizePost(uri, headers, s"""{"token":"
$token "}""")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "read the token out of a multipart body terminated by a boundary"
in {
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ val response =
+ new AccessControlResource().authorizePost(uri, headers,
multipartTwoParts(token))
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "read the token out of a multipart body that ends with the token"
in {
+ val (uri, headers) = bodyOnlyRequest(testCU.getCuid)
+
+ // No closing boundary follows the value, so only the run-to-end fallback
can
+ // recover the token.
+ val response =
+ new AccessControlResource().authorizePost(uri, headers,
multipartRunToEnd(token))
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
+
+ it should "prefer the Authorization header over a token in the request body"
in {
+ // Header and body carry *different* valid identities. testUser2 owns this
unit, so
+ // either choice returns 200; only the forwarded identity reveals which
token was
+ // used to authenticate.
+ val (uri, headers) = mockRequest("/pve/system",
Some(testCU.getCuid.toString))
+ val response =
+ new AccessControlResource().authorizePost(uri, headers, s"token=$token2")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString(HeaderField.UserId) shouldBe
testUser1.getUid.toString
+ }
+
+ it should "fall back to the request body when the Authorization header is a
bare bearer scheme" in {
+ // "Bearer " with nothing after it leaves an empty token; it must be
discarded so
+ // the next source in the chain still gets a chance.
+ val (uri, headers) = mockRequest(
+ "/pve/system",
+ Some(testCU.getCuid.toString),
+ authorizationHeader = Some("Bearer ")
+ )
+ val response = new AccessControlResource().authorizePost(uri, headers,
s"token=$token")
+
+ response.getStatus shouldBe Response.Status.OK.getStatusCode
+ response.getHeaderString("Host") shouldBe testRecordedUri
+ }
}
