You are viewing a plain text version of this content. The canonical link for it is here.

Posted to issues@hop.apache.org by "bamaer (via GitHub)" <gi...@apache.org> on 2026/09/15 18:03:32 UTC

[PR] Hop Web security follow-ups: mode NONE docs, threat model, EXTERNAL w… (hop)

bamaer opened a new pull request, #8402:
URL: https://github.com/apache/hop/pull/8402

…arning. fixes #8391

- hop-web.adoc: correct the mode NONE description (/ui open by design, /hop/* default-denied) and document allowUnauthenticatedServerApi / HOP_WEB_ALLOW_UNAUTHENTICATED_SERVER_API as the execution-server opt-in. Add an IMPORTANT note that EXTERNAL delegates rather than enforces.
- THREAT_MODEL.md: add Hop Web as a distinct deployment, scope the enable_auth assumption to hop-server, and describe the per-mode boundary.
- HopWebEntryPoint: log a warning when a session has no principal in a mode other than NONE, so EXTERNAL without a container security-constraint no longer fails open silently.

**Please** add a meaningful description for your change here

------------------------

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Run `mvn clean install apache-rat:check` to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
- [x] If you have a group of commits related to the same change, please squash your commits into one and force push your branch using `git rebase -i`.
- [x] Mention the appropriate issue in your description (for example: `addresses #123`), if applicable.

To make clear that you license your contribution under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.

- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
- [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).

--
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Hop Web security follow-ups: mode NONE docs, threat model, EXTERNAL w… (hop)

Posted by "mattcasters (via GitHub)" <gi...@apache.org>.

mattcasters commented on code in PR #8402:
URL: https://github.com/apache/hop/pull/8402#discussion_r4024060183


##########
THREAT_MODEL.md:
##########
@@ -254,6 +309,13 @@ where strict outbound TLS verification is required (see §9).
   pipeline execution = remote code execution.
 - Running Hop Server with `enable_auth=false` on an untrusted network →
   unauthenticated pipeline execution + metadata mutation over the JSON API.
+- Exposing **Hop Web in mode `NONE`** to an untrusted network → an open

Review Comment:
   **[suggestion]** The new §11 pattern says exposing Hop Web in mode `NONE` yields “an open authoring UI on `/ui`; and, if `HOP_WEB_ALLOW_UNAUTHENTICATED_SERVER_API` was enabled, unauthenticated pipeline execution over `/hop/*` as well.” That contrast implies execution is the flag’s problem. The GUI does not call `/hop/*` to run: `HopGuiPipelineGraph` uses `PipelineEngineFactory` / `LocalPipelineEngine` in-process. Mode `NONE` binds an unrestricted session (`RapSecurityContextProvider` → `HopSecurityContext.unrestricted()`), so anyone who can reach `/ui` can author **and** run pipelines even while `/hop/*` returns 403. §8 already says the session is unrestricted; §11’s “authoring UI” / “execution over `/hop/*` as well” split understates the default-install exposure.
   
   **Suggestion:** Word the misuse pattern as: exposed `/ui` in `NONE` is an unrestricted GUI that can run pipelines in-process; the flag additionally opens the remote `/hop/*` (and `/hop/api/v1/*`) execution API.



##########
THREAT_MODEL.md:
##########
@@ -54,7 +56,8 @@ the `hop-run` CLI, or submitted to a **Hop Server** for local/remote execution.
 | Execution engine | `engine/`, `engine-beam/` | Runs pipelines/workflows the operator authored — incl. transforms that touch files, DBs, network, and **scripting/exec** steps. |
 | Hop Server (servlet / HTTP) | `engine/` — package `org.apache.hop.www` + `HopServerMeta` in `org.apache.hop.server`; launched by the `hop-server` command (`org.apache.hop.www.HopServer`). *(The `org.apache.hop.server` connection helpers `HttpUtil`/`ServerConnectionManager` — see §9 — live in `core/`.)* | The **network trust boundary**: an embedded Jetty server whose servlets accept pipeline/workflow run requests over HTTP (`/hop/execPipeline`, `/hop/addPipeline`+`/hop/startExec`, `/hop/registerPackage`, …). Who may submit/run, and with what auth, lives here (§7, §8). |
 | Hop JSON API | `engine/src/main/java/org/apache/hop/www/api/` | **Part of Hop Server**, not a separate deployable. Mounted on `/hop/api/v1/` ([`WebServer.java`](engine/src/main/java/org/apache/hop/www/WebServer.java)) and exposes `execute/sync` (run a web service), `metadata` CRUD, `plugins` and `location` execution info. It sits inside the same `ConstraintSecurityHandler` as every servlet, so it inherits Hop Server's Basic/JAAS auth (§8). |
-| GUI (desktop / web) | `ui/` (`hop-ui`, SWT core), `rcp/` (desktop fragment), `rap/` (`hop-ui-rap`, RAP/RWT **web** GUI) | Authoring surface used by the trusted operator. `rap/` is the **web GUI**, not a server. |
+| GUI (desktop / web) | `ui/` (`hop-ui`, SWT core), `rcp/` (desktop fragment), `rap/` (`hop-ui-rap`, RAP/RWT **web** GUI) | Authoring surface used by the trusted operator. `rap/` is the **web GUI code**, not a server — but see the Hop Web row: the shipped web application deploys it together with Hop Server servlets. |
+| Hop Web (deployed web application) | `assemblies/web/` (WAR + `apache/hop-web` image), built from `rap/` + `engine/` | A **distinct network-facing deployment**, separate from both `rap/` and `hop-server`. The one WAR co-deploys the RAP UI on `/ui` and `/ui-dark`, `HopServerServlet` on `/hop/*`, and `HopApiApplication` on `/hop/api/v1/*` on the same origin ([`web.xml`](assemblies/web/src/main/resources/WEB-INF/web.xml)). It does **not** use Hop Server's `enable_auth` / `hop.pwd` Basic auth; it has its own auth modes (§8). Docker image binds `0.0.0.0:8080`. |

Review Comment:
   **[suggestion]** The new Hop Web row correctly calls the WAR a distinct network-facing deployment (RAP on `/ui`, `HopServerServlet` on `/hop/*`, `HopApiApplication` on `/hop/api/v1/*`, own auth modes). The sentences immediately after the table still say in-scope is “engine + Hop Server + plugins + connection layer + GUI glue” and “The **network-facing Hop Server** is where an untrusted actor can appear.” §7 still scopes the in-model adversary to “a remote actor against an exposed Hop Server.” §12 still omits Hop Web auth/deployables even though the header revision triggers were updated. The JSON API family row still says that API “inherits Hop Server's Basic/JAAS auth,” which is true for standalone `hop-server` but not for the co-deployed `/hop/api/v1/*` this row now documents. A triager can treat an exposed Hop Web `/ui` as out of model even though §8/§11 now describe it.
   
   **Suggestion:** Extend the in-scope / untrusted-actor sentences, §7, and §12 to name Hop Web as a second HTTP trust boundary. Qualify the JSON API row: standalone `hop-server` uses `ConstraintSecurityHandler`; Hop Web uses `HopBasicAuthFilter` / `HopOidcAuthFilter` / `HopServerAuthorizationFilter`.



##########
rap/src/main/java/org/apache/hop/ui/hopgui/HopWebEntryPoint.java:
##########
@@ -159,8 +160,23 @@ protected void createContents(Composite parent) {
           "Hop Web security: user ''{0}'' roles={1}",
           securityContext.getUsername(), securityContext.getRoleIds());
     } else {
-      LogChannel.UI.logDebug(
-          "Hop Web security: no authenticated principal (mode NONE or unrestricted)");
+      HopSecurityConfig.AuthMode mode = resolveAuthMode();
+      if (mode == HopSecurityConfig.AuthMode.NONE) {
+        LogChannel.UI.logDebug("Hop Web security: no authenticated principal (mode NONE)");
+      } else {
+        // Authentication is configured but the request carries no principal. In EXTERNAL mode

Review Comment:
   **[nit]** The six-line comment restates the log message and embeds EXTERNAL / `web.xml` / Security-tab design rationale. The log text already explains what to do.
   
   **Suggestion:** Drop the block comment (or keep one line: principal-less non-`NONE` session is unexpected; log at error so EXTERNAL-without-constraint is visible).



##########
docs/hop-user-manual/modules/ROOT/pages/hop-gui/hop-web.adoc:
##########
@@ -249,6 +262,12 @@ To keep audit data across container restarts, mount a volume and set the same pa
 
 The default Hop Web docker image picks up `tomcat-users.xml` and `web.xml` files and moves them to the correct location before Hop Web starts.
 
+IMPORTANT: Mode `EXTERNAL` is *delegation, not enforcement*.
+Hop itself does not challenge anyone in this mode: it only reads the `Principal` the servlet container (or authenticating reverse proxy) put on the request.
+The `web.xml` that ships with Hop Web contains no `<security-constraint>`, so selecting `EXTERNAL` without supplying your own constraint over `/*` leaves `/ui` **completely open** while the Security tab reports authentication as enabled.

Review Comment:
   **[suggestion]** Both the user manual and the threat model say that `EXTERNAL` without a constraint leaves `/ui` open “while the Security tab reports authentication as enabled.” The mode combo is `EXTERNAL`, but the status line for a principal-less session is `ConfigSecurityTab.Status.Unrestricted`: “Session is unrestricted (no AuthN). Config mode: {0}.” Issue #8391 described this more accurately as the tab *showing the contradiction*. The current wording overstates what the UI claims.
   
   **Suggestion:** Say the General tab can be set to `EXTERNAL` (an authenticated mode) while the status line still reads unrestricted / no AuthN — that is the operator-visible contradiction, not a green “authentication enabled” indicator.



-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Hop Web security follow-ups: mode NONE docs, threat model, EXTERNAL w… (hop)

Posted by "bamaer (via GitHub)" <gi...@apache.org>.

bamaer merged PR #8402:
URL: https://github.com/apache/hop/pull/8402


-- 
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: issues-unsubscribe@hop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org