Copilot commented on code in PR #8073:
URL: https://github.com/apache/texera/pull/8073#discussion_r3931185590
##########
access-control-service/src/main/scala/org/apache/texera/service/resource/AccessControlResource.scala:
##########
@@ -77,6 +82,39 @@ object AccessControlResource extends LazyLogging {
}
}
+ /**
+ * Resolve which JupyterLab pod a request belongs to. This routes; it does
not authorize.
+ *
+ * Jupyter is loaded in an iframe and then issues its own requests for
assets, contents and
+ * kernel websockets. None of those can carry a Texera token, and there is
no session cookie
+ * to fall back on, so the caller cannot be authenticated per request. What
protects one
+ * user's notebooks from another is the per-user Jupyter token, which is
derived from a
+ * server-held secret and is unguessable; reaching the right pod without it
yields a 403 from
+ * Jupyter itself. Cross-pod traffic is blocked separately by a
NetworkPolicy.
+ */
+ private def routeToJupyter(uid: String): Response = {
+ val recordedUrl =
+ try {
+ val dao = new
UserJupyterDao(SqlServer.getInstance().createDSLContext().configuration())
+ Option(dao.fetchOneByUid(uid.toInt)).map(_.getInternalUrl)
+ } catch {
+ case e: Exception =>
+ logger.error(s"Failed to look up the Jupyter registered for user
$uid", e)
+ return Response.status(Response.Status.FORBIDDEN).build()
+ }
+
+ // Envoy routes on an authority, so the scheme and the base path are
stripped back off the
+ // address the provisioner recorded.
+ recordedUrl.map(url => new URI(url).getAuthority).filter(a => a != null &&
a.nonEmpty) match {
Review Comment:
`new URI(url)` can throw for a syntactically malformed stored value (for
example, a stray `%`), and this parsing happens outside the preceding catch.
ExtAuth then returns a 500 instead of the intended fail-closed 403. Parse
through `Try` before filtering the authority so every malformed registry row is
denied consistently.
##########
common/config/src/test/scala/org/apache/texera/common/config/KubernetesConfigSpec.scala:
##########
@@ -86,6 +86,10 @@ class KubernetesConfigSpec extends AnyFlatSpec with Matchers
{
)
// Empty by default: only a real deployment knows its own origin.
ifUnset("KUBERNETES_JUPYTER_TEXERA_ORIGIN")(KubernetesConfig.jupyterTexeraOrigin
shouldBe "")
+ // A prefix, not a full path: the provisioner appends the uid.
+ ifUnset("KUBERNETES_JUPYTER_BASE_URL")(KubernetesConfig.jupyterBaseUrl
shouldBe "/jupyter")
+ // Empty by default: only a real deployment knows its own origin.
+
ifUnset("KUBERNETES_JUPYTER_TEXERA_ORIGIN")(KubernetesConfig.jupyterTexeraOrigin
shouldBe "")
Review Comment:
This repeats the `KUBERNETES_JUPYTER_TEXERA_ORIGIN` assertion immediately
above and adds no coverage. Remove the duplicate so this block tests each
setting once.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
