This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7690-f8912a452b06f0f50c3f99c0318a7480470cc6f5 in repository https://gitbox.apache.org/repos/asf/texera.git
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 + } }
