This is an automated email from the ASF dual-hosted git repository.
jongyoul pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new f988eef9fe [ZEPPELIN-6521] Preserve withCredentials on HTTP requests
in production builds
f988eef9fe is described below
commit f988eef9fe9a2853988193c7572c3b01d554441c
Author: dae won <[email protected]>
AuthorDate: Fri Aug 7 12:43:40 2026 +0900
[ZEPPELIN-6521] Preserve withCredentials on HTTP requests in production
builds
### What is this PR for?
In production builds the interceptor re-clones from the original
`httpRequest` when it adds `X-Requested-With`, discarding the `withCredentials:
true` clone made one line earlier. `clone()` inherits from whatever it was
cloned from (`update.withCredentials ?? this.withCredentials`), so the
production request goes out with the default `false`.
```diff
let httpRequestUpdated = httpRequest.clone({ withCredentials: true
});
if (environment.production) {
- httpRequestUpdated = httpRequest.clone({ setHeaders: {
'X-Requested-With': 'XMLHttpRequest' } });
+ httpRequestUpdated = httpRequestUpdated.clone({ setHeaders: {
'X-Requested-With': 'XMLHttpRequest' } });
}
```
Two things the ticket does not cover. First, `BaseUrlService` builds the
REST base from `location`, so every call to Zeppelin's own API is same-origin
and the browser attaches cookies whether or not the flag is set; the flag has
no reachable effect on those calls today. The classic UI's equivalent service
remaps the port when the UI is served from the grunt dev server
(`zeppelin-web/src/components/base-url/base-url.service.js:26-29`), which is
what made `withCredentials` meaningful th [...]
### Scope and related issues
**Third-party URL fetches go through this interceptor too, and this change
narrows what they can reach.** `NoteImportComponent` passes a user-supplied URL
straight to `HttpClient` (`note-import.component.ts:49`), so the "import note
from URL" request carries the same `withCredentials` and `X-Requested-With` as
a call to Zeppelin's own API. A browser rejects a credentialed cross-origin
response whose `Access-Control-Allow-Origin` is `*`, and that wildcard is what
public file hosts serve.
Measured in Chromium against local servers reproducing each CORS
configuration, with the `raw.githubusercontent.com` preflight response checked
directly:
| target host | dev build | production today | production after this PR |
|---|---|---|---|
| rejects `X-Requested-With` at preflight — `raw.githubusercontent.com`
answers the preflight with 403 | blocked | blocked | blocked |
| allows `X-Requested-With`, serves `Access-Control-Allow-Origin: *` |
blocked | works | blocked |
Only the second row changes, and it changes production to match what
development already does, which is what this ticket asks for.
The underlying problem is that the interceptor does not distinguish
Zeppelin's own API from an arbitrary URL. The classic UI handles it by
overriding `withCredentials: false` for exactly this request
(`zeppelin-web/src/components/note-import/note-import.controller.js:95-97`);
the new UI has no equivalent. That is a separate defect which predates this
ticket and already breaks the feature in development builds today.
Scoping the interceptor so that `withCredentials` and `X-Requested-With`
are applied only to Zeppelin's own API would resolve it, and would not change
anything this PR does for API calls. I have not filed a ticket for it yet — I
would rather hear whether that scoping belongs in this PR or in a follow-up.
### What type of PR is it?
Bug Fix
### Todos
* [x] - Derive the production clone from the already-credentialed request
* [x] - Confirm `X-Requested-With` is still added in production builds
* [x] - Confirm development builds are unaffected
### What is the Jira issue?
* [ZEPPELIN-6521](https://issues.apache.org/jira/browse/ZEPPELIN-6521)
### How should this be tested?
There is no unit-test runner for this app to add a spec to: the `zeppelin`
project in `angular.json` declares only `build`, `serve`, `extract-i18n` and
`lint`, and `zeppelin-web-angular/src` contains no `.spec.ts`. A Playwright
test is also a poor fit here, because the local suite runs against `ng serve`
(`playwright.config.js:16`), where `environment.production` is `false` and the
affected branch never executes — such a test would pass with or without this
change unless run in CI mod [...]
**1. Lint**
```
cd zeppelin-web-angular && npm run lint
```
Exit 0. 15 pre-existing `member-ordering` warnings in
`projects/zeppelin-visualization`, none in the changed file; `lint:react`
clean; `prettier --check` reports all files formatted.
**2. `HttpRequest.clone()` inheritance, using the real class**
Running both the old and the new form of the production branch through
Angular's real `HttpRequest` class:
```
development branch withCredentials=true X-Requested-With=(none)
production, before withCredentials=false X-Requested-With=XMLHttpRequest
production, after withCredentials=true X-Requested-With=XMLHttpRequest
```
**3. Production bundle in a real browser**
`ng build --configuration production` twice — once with this commit and
once with it reverted, changing nothing else — serving `dist/zeppelin`
statically and instrumenting the `XMLHttpRequest.prototype.withCredentials`
setter. `BaseUrlService` derives the REST base from `location`, so the app
issues its normal bootstrap calls against the static server; they 404, but
Angular assigns `withCredentials` before `send()`, which is what is being
observed.
```
API requests withCredentials assignments
X-Requested-With
before this commit 4 []
XMLHttpRequest
after this commit 4 [true, true, true, true]
XMLHttpRequest
```
`X-Requested-With` is present in both runs, which confirms the production
branch really executed and that the change does not drop the header.
### Screenshots (if appropriate)
N/A
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No — see "Scope and related
issues" for the one behaviour that changes for third-party URL fetches
* Does this needs documentation? No
Closes #5377 from big-cir/ZEPPELIN-6521.
Signed-off-by: Jongyoul Lee <[email protected]>
---
.../share/note-import/note-import-modal.spec.ts | 42 ++++++++++++++++++++++
.../src/app/app-http.interceptor.ts | 2 +-
.../app/share/note-import/note-import.component.ts | 8 +++--
3 files changed, 48 insertions(+), 4 deletions(-)
diff --git
a/zeppelin-web-angular/e2e/tests/share/note-import/note-import-modal.spec.ts
b/zeppelin-web-angular/e2e/tests/share/note-import/note-import-modal.spec.ts
index 229967d719..6361759afd 100644
--- a/zeppelin-web-angular/e2e/tests/share/note-import/note-import-modal.spec.ts
+++ b/zeppelin-web-angular/e2e/tests/share/note-import/note-import-modal.spec.ts
@@ -10,6 +10,8 @@
* limitations under the License.
*/
+import { createServer } from 'node:http';
+
import { test, expect } from '@playwright/test';
import { HomePage } from '../../../models/home-page';
import { NoteImportModal } from '../../../models/note-import-modal';
@@ -69,6 +71,46 @@ test.describe('Note Import Modal', () => {
await expect(noteImportModal.importNoteButton).toBeEnabled();
});
+ test('Given URL tab is selected, When importing from wildcard CORS origin,
Then response should be readable', async () => {
+ const corsServer = createServer((request, response) => {
+ response.setHeader('Access-Control-Allow-Headers', 'X-Requested-With');
+ response.setHeader('Access-Control-Allow-Methods', 'GET, OPTIONS');
+ response.setHeader('Access-Control-Allow-Origin', '*');
+ response.setHeader('Connection', 'close');
+
+ if (request.method === 'OPTIONS') {
+ response.writeHead(204);
+ response.end();
+ return;
+ }
+
+ response.setHeader('Content-Type', 'application/json');
+ response.end(JSON.stringify({ name: 'Missing paragraphs' }));
+ });
+
+ await new Promise<void>((resolve, reject) => {
+ corsServer.once('error', reject);
+ corsServer.listen(0, '127.0.0.1', resolve);
+ });
+
+ try {
+ const address = corsServer.address();
+ if (!address || typeof address === 'string') {
+ throw new Error('Failed to bind CORS test server');
+ }
+
+ await noteImportModal.switchToUrlTab();
+ await
noteImportModal.setImportUrl(`http://127.0.0.1:${address.port}/note.json`);
+ await noteImportModal.clickImportNote();
+
+ await expect(noteImportModal.errorAlert).toHaveText('Invalid JSON');
+ } finally {
+ await new Promise<void>((resolve, reject) => {
+ corsServer.close(error => (error ? reject(error) : resolve()));
+ });
+ }
+ });
+
test('Given Import Note modal is open, When entering import name, Then name
should be set', async () => {
const importName = `Imported Note ${Date.now()}`;
await noteImportModal.setImportAsName(importName);
diff --git a/zeppelin-web-angular/src/app/app-http.interceptor.ts
b/zeppelin-web-angular/src/app/app-http.interceptor.ts
index b1e287622a..6a6a4a1853 100644
--- a/zeppelin-web-angular/src/app/app-http.interceptor.ts
+++ b/zeppelin-web-angular/src/app/app-http.interceptor.ts
@@ -28,7 +28,7 @@ export class AppHttpInterceptor implements HttpInterceptor {
intercept(httpRequest: HttpRequest<any>, next: HttpHandler):
Observable<HttpEvent<any>> {
let httpRequestUpdated = httpRequest.clone({ withCredentials: true });
if (environment.production) {
- httpRequestUpdated = httpRequest.clone({ setHeaders: {
'X-Requested-With': 'XMLHttpRequest' } });
+ httpRequestUpdated = httpRequestUpdated.clone({ setHeaders: {
'X-Requested-With': 'XMLHttpRequest' } });
}
return next.handle(httpRequestUpdated).pipe(
map(event => {
diff --git
a/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
b/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
index a73627b9ba..ef5fdb7f40 100644
--- a/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
+++ b/zeppelin-web-angular/src/app/share/note-import/note-import.component.ts
@@ -10,7 +10,7 @@
* limitations under the License.
*/
-import { HttpClient } from '@angular/common/http';
+import { HttpBackend, HttpClient } from '@angular/common/http';
import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit } from
'@angular/core';
import { ConfigurationService, MessageService, TicketService } from
'@zeppelin/services';
@@ -37,6 +37,7 @@ export class NoteImportComponent extends
MessageListenersManager implements OnIn
errorText?: string;
importLoading = false;
wsMaxLimit?: number;
+ private readonly externalHttpClient: HttpClient;
@MessageListener(OP.IMPORT_NOTE)
noteImported(_: MessageReceiveDataTypeMap[OP.IMPORT_NOTE]) {
@@ -46,7 +47,7 @@ export class NoteImportComponent extends
MessageListenersManager implements OnIn
importNote() {
this.errorText = '';
this.importLoading = true;
- this.httpClient.get(this.importUrl ?? '').subscribe(
+ this.externalHttpClient.get(this.importUrl ?? '').subscribe(
data => {
this.importLoading = false;
this.processImportJson(data);
@@ -106,9 +107,10 @@ export class NoteImportComponent extends
MessageListenersManager implements OnIn
private configurationService: ConfigurationService,
private cdr: ChangeDetectorRef,
private nzModalRef: NzModalRef,
- private httpClient: HttpClient
+ httpBackend: HttpBackend
) {
super(messageService);
+ this.externalHttpClient = new HttpClient(httpBackend);
}
async ngOnInit() {
