Copilot commented on code in PR #5252: URL: https://github.com/apache/texera/pull/5252#discussion_r3448059837
########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,202 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Injectable, NgZone } from "@angular/core"; +import { HttpClient } from "@angular/common/http"; +import { Observable, Subject, from, firstValueFrom } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +export interface DriveTokenResponse { + status: string; + accessToken?: string; +} + +export interface DriveFolder { + id: string; + name: string; +} + +// gapi is loaded via the script tag in index.html +declare var gapi: any; +declare var google: any; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly BASE = `${AppSettings.getApiEndpoint()}/auth/google/drive`; + private readonly CONFIG_URL = `${AppSettings.getApiEndpoint()}/auth/google/config`; + + private connected$ = new Subject<void>(); + private pickerLoaded = false; + + constructor( + private http: HttpClient, + private ngZone: NgZone + ) {} + + connect(reauth = false): void { + this.http.get(`${this.BASE}/connect?reauth=${reauth}`, { responseType: "text" }).subscribe(url => { + const popup = window.open(url, "gdrive-connect", "width=500,height=600"); + + const onMessage = (event: MessageEvent) => { + if (event.data === "gdrive-connected") { + window.removeEventListener("message", onMessage); + popup?.close(); + this.ngZone.run(() => this.connected$.next()); + } + }; + + window.addEventListener("message", onMessage); + }); + } Review Comment: `DriveService.connect()` accepts any `postMessage` with data "gdrive-connected". Without validating `event.origin` (and ideally `event.source`), a malicious page can spoof the connection event and force UI state changes / close the popup. Also, if the popup is blocked or never posts back, the message listener is never removed. ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,202 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Injectable, NgZone } from "@angular/core"; +import { HttpClient } from "@angular/common/http"; +import { Observable, Subject, from, firstValueFrom } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +export interface DriveTokenResponse { + status: string; + accessToken?: string; +} + +export interface DriveFolder { + id: string; + name: string; +} + +// gapi is loaded via the script tag in index.html +declare var gapi: any; +declare var google: any; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly BASE = `${AppSettings.getApiEndpoint()}/auth/google/drive`; + private readonly CONFIG_URL = `${AppSettings.getApiEndpoint()}/auth/google/config`; + + private connected$ = new Subject<void>(); + private pickerLoaded = false; + + constructor( + private http: HttpClient, + private ngZone: NgZone + ) {} + + connect(reauth = false): void { + this.http.get(`${this.BASE}/connect?reauth=${reauth}`, { responseType: "text" }).subscribe(url => { + const popup = window.open(url, "gdrive-connect", "width=500,height=600"); + + const onMessage = (event: MessageEvent) => { + if (event.data === "gdrive-connected") { + window.removeEventListener("message", onMessage); + popup?.close(); + this.ngZone.run(() => this.connected$.next()); + } + }; + + window.addEventListener("message", onMessage); + }); + } + + onConnected(): Observable<void> { + return this.connected$.asObservable(); + } + + getToken(): Observable<DriveTokenResponse> { + return this.http.get<DriveTokenResponse>(`${this.BASE}/token`); + } + + exportToDrive(blob: Blob, fileName: string): Observable<void> { + const result$ = new Subject<void>(); + + Promise.all([this.loadPicker(), this.getAccessToken()]).then(([, accessToken]) => { + if (!accessToken) { + result$.error(new Error("Not connected to Google Drive")); + return; + } + + this.http.get<{ clientId: string; apiKey: string }>(this.CONFIG_URL).subscribe(config => { + const folderView = new google.picker.DocsView(google.picker.ViewId.FOLDERS) + .setIncludeFolders(true) + .setSelectFolderEnabled(true) + .setMimeTypes("application/vnd.google-apps.folder"); + + const picker = new google.picker.PickerBuilder() + .addView(folderView) + .setOAuthToken(accessToken) + .setDeveloperKey(config.apiKey) + .setTitle("Choose a folder to export to") + .setCallback((data: any) => { + if (data.action === google.picker.Action.PICKED) { + const folderId = data.docs[0].id; + this.ngZone.run(() => { + this.uploadToDrive(blob, fileName, folderId, accessToken).subscribe({ + next: () => { + result$.next(); + result$.complete(); + }, + error: (err: unknown) => result$.error(err), + }); + }); + } else if (data.action === google.picker.Action.CANCEL) { + this.ngZone.run(() => result$.complete()); + } + }) + .build(); + + picker.setVisible(true); + }); + }); + + return result$.asObservable(); + } + + openFolderPicker(): Observable<DriveFolder> { + const result$ = new Subject<DriveFolder>(); + + Promise.all([this.loadPicker(), this.getAccessToken()]).then(([, accessToken]) => { + if (!accessToken) { + result$.error(new Error("Not connected to Google Drive")); + return; + } + + this.http.get<{ clientId: string; apiKey: string }>(this.CONFIG_URL).subscribe(config => { + const folderView = new google.picker.DocsView(google.picker.ViewId.FOLDERS) + .setIncludeFolders(true) + .setSelectFolderEnabled(true) + .setMimeTypes("application/vnd.google-apps.folder"); + + const picker = new google.picker.PickerBuilder() + .addView(folderView) + .setOAuthToken(accessToken) + .setDeveloperKey(config.apiKey) + .setTitle("Choose a folder to export to") + .setCallback((data: any) => { + if (data.action === google.picker.Action.PICKED) { + const doc = data.docs[0]; + this.ngZone.run(() => { + result$.next({ id: doc.id, name: doc.name }); + result$.complete(); + }); + } else if (data.action === google.picker.Action.CANCEL) { + this.ngZone.run(() => result$.complete()); + } + }) + .build(); + + picker.setVisible(true); + }); + }); Review Comment: Same as `exportToDrive()`: if the config request fails or the initial Promise rejects, `openFolderPicker()` never completes/errors its observable, leaving subscribers hanging. ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.texera.web.resource.auth + +import io.dropwizard.auth.Auth +import com.fasterxml.jackson.databind.ObjectMapper +import com.typesafe.scalalogging.LazyLogging +import org.apache.texera.auth.{SessionUser, TokenEncryptionService} +import org.apache.texera.web.model.http.response.DriveTokenIssueResponse +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.dao.jooq.generated.tables.daos.UserOauthTokenDao +import org.apache.texera.dao.jooq.generated.tables.pojos.UserOauthToken +import org.apache.texera.dao.SqlServer +import org.apache.texera.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest, + GoogleRefreshTokenRequest, + GoogleTokenResponse +} +import com.google.api.client.auth.oauth2.TokenResponseException +import com.google.api.client.http.javanet.NetHttpTransport +import com.google.api.client.json.gson.GsonFactory + +import java.util.concurrent.ConcurrentHashMap +import javax.annotation.security.RolesAllowed +import javax.ws.rs._ +import javax.ws.rs.core.MediaType +import javax.ws.rs.core.Response + +object GoogleDriveAuthResource { + private val STATUS_OK = "ok" + private val STATUS_NO_REFRESH_TOKEN = "no_refresh_token" + private val STATUS_INVALID_GRANT = "invalid_grant" + private val PROVIDER_GOOGLE_DRIVE = "google_drive" + + private val STATE_TTL_MS = 10 * 60 * 1000L + + private val mapper = new ObjectMapper() + + // state token → (uid, expiresAtMs) + private val pendingStates = new ConcurrentHashMap[String, (Int, Long)]() + + private def oauthTokenDao = + new UserOauthTokenDao( + SqlServer + .getInstance() + .createDSLContext() + .configuration + ) +} + +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + final private lazy val clientId = UserSystemConfig.googleClientId + final private lazy val clientSecret = UserSystemConfig.googleClientSecret + final private lazy val redirectUri = UserSystemConfig.appDomain + .map(domain => s"https://$domain/api/auth/google/drive/callback";) + .getOrElse("http://localhost:4200/api/auth/google/drive/callback";) + + @GET + @Path("/token") + @RolesAllowed(Array("REGULAR", "ADMIN")) + def getDriveAccessToken(@Auth sessionUser: SessionUser): Response = { + val uid = sessionUser.getUid + val record = oauthTokenDao.fetchByUid(uid).stream() + .filter(r => r.getProvider == PROVIDER_GOOGLE_DRIVE) + .findFirst() + .orElse(null) + + if (record == null) { + return Response.ok(DriveTokenIssueResponse(STATUS_NO_REFRESH_TOKEN, None)).build() + } + + try { + val blob = mapper.readTree(TokenEncryptionService.decrypt(record.getAuthBlob)) + val refreshToken = blob.get("refreshToken").asText() + + val tokenResponse = new GoogleRefreshTokenRequest( + new NetHttpTransport(), + GsonFactory.getDefaultInstance, + refreshToken, + clientId, + clientSecret + ).execute() + + Response.ok(DriveTokenIssueResponse(STATUS_OK, Some(tokenResponse.getAccessToken))).build() + } catch { + case e: TokenResponseException => + if (e.getDetails != null && e.getDetails.getError == STATUS_INVALID_GRANT) { + Response.ok(DriveTokenIssueResponse(STATUS_INVALID_GRANT, None)).build() + } else { + logger.error("Failed to refresh access token", e) + Response.status(Response.Status.INTERNAL_SERVER_ERROR).build() + } + case e: Exception => + logger.error("Unexpected error refreshing access token", e) + Response.status(Response.Status.INTERNAL_SERVER_ERROR).build() + } + } + + @GET + @Path("/callback") + @Produces(Array(MediaType.TEXT_HTML, MediaType.APPLICATION_JSON)) + def getCallback( + @QueryParam("code") @DefaultValue("") code: String, + @QueryParam("state") @DefaultValue("") state: String + ): Response = { + if (code.isEmpty || state.isEmpty) { + return Response.status(Response.Status.BAD_REQUEST).build() + } + try { + val entry = pendingStates.remove(state) + if (entry == null || System.currentTimeMillis() > entry._2) { + return Response + .status(Response.Status.UNAUTHORIZED) + .entity("OAuth state token is invalid or expired") + .build() + } + + val uid = entry._1 + + val tokenResponse: GoogleTokenResponse = new GoogleAuthorizationCodeTokenRequest( + new NetHttpTransport(), + GsonFactory.getDefaultInstance, + clientId, + clientSecret, + code, + redirectUri + ).execute() + + val blobMap = new java.util.HashMap[String, String]() + blobMap.put("refreshToken", tokenResponse.getRefreshToken) + blobMap.put("scopes", tokenResponse.getScope) + val blobJson = mapper.writeValueAsString(blobMap) + val encryptedBlob = TokenEncryptionService.encrypt(blobJson) Review Comment: `tokenResponse.getRefreshToken` can be null (Google only returns a refresh token on first consent or when `prompt=consent`). Storing a null refresh token will overwrite any existing valid token and break future refreshes. Guard against a missing refresh token and return a clear error instructing the UI to retry with `reauth=true` (or preserve the existing token). ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,202 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Injectable, NgZone } from "@angular/core"; +import { HttpClient } from "@angular/common/http"; +import { Observable, Subject, from, firstValueFrom } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +export interface DriveTokenResponse { + status: string; + accessToken?: string; +} + +export interface DriveFolder { + id: string; + name: string; +} + +// gapi is loaded via the script tag in index.html +declare var gapi: any; +declare var google: any; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly BASE = `${AppSettings.getApiEndpoint()}/auth/google/drive`; + private readonly CONFIG_URL = `${AppSettings.getApiEndpoint()}/auth/google/config`; + + private connected$ = new Subject<void>(); + private pickerLoaded = false; + + constructor( + private http: HttpClient, + private ngZone: NgZone + ) {} + + connect(reauth = false): void { + this.http.get(`${this.BASE}/connect?reauth=${reauth}`, { responseType: "text" }).subscribe(url => { + const popup = window.open(url, "gdrive-connect", "width=500,height=600"); + + const onMessage = (event: MessageEvent) => { + if (event.data === "gdrive-connected") { + window.removeEventListener("message", onMessage); + popup?.close(); + this.ngZone.run(() => this.connected$.next()); + } + }; + + window.addEventListener("message", onMessage); + }); + } + + onConnected(): Observable<void> { + return this.connected$.asObservable(); + } + + getToken(): Observable<DriveTokenResponse> { + return this.http.get<DriveTokenResponse>(`${this.BASE}/token`); + } + + exportToDrive(blob: Blob, fileName: string): Observable<void> { + const result$ = new Subject<void>(); + + Promise.all([this.loadPicker(), this.getAccessToken()]).then(([, accessToken]) => { + if (!accessToken) { + result$.error(new Error("Not connected to Google Drive")); + return; + } + + this.http.get<{ clientId: string; apiKey: string }>(this.CONFIG_URL).subscribe(config => { + const folderView = new google.picker.DocsView(google.picker.ViewId.FOLDERS) + .setIncludeFolders(true) + .setSelectFolderEnabled(true) + .setMimeTypes("application/vnd.google-apps.folder"); + + const picker = new google.picker.PickerBuilder() + .addView(folderView) + .setOAuthToken(accessToken) + .setDeveloperKey(config.apiKey) + .setTitle("Choose a folder to export to") + .setCallback((data: any) => { + if (data.action === google.picker.Action.PICKED) { + const folderId = data.docs[0].id; + this.ngZone.run(() => { + this.uploadToDrive(blob, fileName, folderId, accessToken).subscribe({ + next: () => { + result$.next(); + result$.complete(); + }, + error: (err: unknown) => result$.error(err), + }); + }); + } else if (data.action === google.picker.Action.CANCEL) { + this.ngZone.run(() => result$.complete()); + } + }) + .build(); + + picker.setVisible(true); + }); + }); Review Comment: If the config request fails or `loadPicker()` / `getAccessToken()` rejects, `exportToDrive()` leaves `result$` neither completed nor errored, which can hang callers waiting for completion. Add an HTTP error callback and a Promise `.catch(...)` to propagate errors into the observable. ########## amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala: ########## @@ -160,6 +164,7 @@ class TexeraWebApplication environment.jersey.register(classOf[UserQuotaResource]) environment.jersey.register(classOf[AdminSettingsResource]) environment.jersey.register(classOf[AIAssistantResource]) + environment.jersey.register(classOf[GoogleDriveAuthResource]) Review Comment: `GoogleDriveAuthResource` is defined as a sub-resource (it has no class-level `@Path`). Registering it as a root Jersey resource is at best a no-op and at worst can cause Jersey resource validation issues. Since it’s already exposed via `GoogleAuthResource` at `/auth/google/drive`, this extra registration should be removed. ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.spec.ts: ########## @@ -0,0 +1,161 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { NgZone } from "@angular/core"; +import { DriveService } from "./drive.service"; +import { AppSettings } from "../../../../common/app-setting"; +import { firstValueFrom } from "rxjs"; +import { commonTestProviders } from "../../../../common/testing/test-utils"; + +describe("DriveService", () => { + let service: DriveService; + let httpMock: HttpTestingController; + let ngZone: NgZone; + + const BASE = `${AppSettings.getApiEndpoint()}/auth/google/drive`; + + beforeEach(() => { + // Stub gapi so loadPicker() resolves without the real script + vi.stubGlobal("gapi", { + load: (_feature: string, cb: () => void) => cb(), + }); + + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [DriveService, ...commonTestProviders], + }); + + service = TestBed.inject(DriveService); + httpMock = TestBed.inject(HttpTestingController); + ngZone = TestBed.inject(NgZone); + }); + + afterEach(() => { + httpMock.verify(); + vi.unstubAllGlobals(); + }); + + describe("getToken", () => { + it("calls the token endpoint and returns the response", async () => { + const promise = firstValueFrom(service.getToken()); + + const req = httpMock.expectOne(`${BASE}/token`); + expect(req.request.method).toBe("GET"); + req.flush({ status: "ok", accessToken: "abc123" }); + + const result = await promise; + expect(result.status).toBe("ok"); + expect(result.accessToken).toBe("abc123"); + }); + + it("returns no_refresh_token status when user has not connected Drive", async () => { + const promise = firstValueFrom(service.getToken()); + + httpMock.expectOne(`${BASE}/token`).flush({ status: "no_refresh_token" }); + + const result = await promise; + expect(result.status).toBe("no_refresh_token"); + expect(result.accessToken).toBeUndefined(); + }); + }); + + describe("connect", () => { + it("fetches the connect URL and opens a popup", () => { + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); + + service.connect(); + + const req = httpMock.expectOne(`${BASE}/connect?reauth=false`); + expect(req.request.method).toBe("GET"); + req.flush("https://accounts.google.com/o/oauth2/auth?...";); + + expect(openSpy).toHaveBeenCalledWith( + "https://accounts.google.com/o/oauth2/auth?...";, + "gdrive-connect", + "width=500,height=600" + ); + }); + + it("passes reauth=true when reauth flag is set", () => { + vi.spyOn(window, "open").mockReturnValue(null); + + service.connect(true); + + httpMock.expectOne(`${BASE}/connect?reauth=true`).flush("https://accounts.google.com/...";); + }); + + it("emits on onConnected() when the popup posts gdrive-connected", fakeAsync(() => { + const mockPopup = { close: vi.fn() } as unknown as Window; + vi.spyOn(window, "open").mockReturnValue(mockPopup); + + let connected = false; + service.onConnected().subscribe(() => { + connected = true; + }); + + service.connect(); + httpMock.expectOne(`${BASE}/connect?reauth=false`).flush("https://accounts.google.com/...";); + + ngZone.run(() => { + window.dispatchEvent(new MessageEvent("message", { data: "gdrive-connected" })); + }); + tick(); + + expect(connected).toBe(true); + expect(mockPopup.close).toHaveBeenCalled(); + })); + + it("does not emit on onConnected() for unrelated messages", fakeAsync(() => { + vi.spyOn(window, "open").mockReturnValue(null); + + let connected = false; + service.onConnected().subscribe(() => { + connected = true; + }); + + service.connect(); + httpMock.expectOne(`${BASE}/connect?reauth=false`).flush("https://accounts.google.com/...";); + + window.dispatchEvent(new MessageEvent("message", { data: "some-other-message" })); + tick(); + + expect(connected).toBe(false); + })); + }); + + describe("exportToDrive", () => { + it("errors the observable when no access token is available", fakeAsync(async () => { + // Token endpoint returns no_refresh_token so getAccessToken returns null + const result$ = service.exportToDrive(new Blob(["test"]), "test.json"); + + // getToken is called inside getAccessToken Review Comment: This test subscribes to `result$` only after calling `tick()`. Because `exportToDrive()` uses a `Subject`, the error can be emitted before the subscription is attached (and then be lost), making the test flaky/incorrect. Subscribe before flushing microtasks/timers. ########## frontend/src/app/app-routing.module.ts: ########## @@ -143,6 +144,10 @@ routes.push({ path: "discussion", component: FlarumComponent, }, + { + path: "google-drive", + component: GoogleDriveConnectComponent, + }, Review Comment: This PR adds a new authenticated route `/dashboard/user/google-drive` that renders `GoogleDriveConnectComponent`, but the PR description focuses on integrating Drive export into existing download surfaces. If this page is only for development/testing, consider removing it (and its import) to avoid shipping an unlinked debug UI; if it’s intended as a user-facing settings page, it likely needs UX integration (navigation entry, styling, copy) and should be mentioned in the PR description. ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.spec.ts: ########## @@ -0,0 +1,161 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { TestBed, fakeAsync, tick } from "@angular/core/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { NgZone } from "@angular/core"; +import { DriveService } from "./drive.service"; +import { AppSettings } from "../../../../common/app-setting"; +import { firstValueFrom } from "rxjs"; +import { commonTestProviders } from "../../../../common/testing/test-utils"; + +describe("DriveService", () => { + let service: DriveService; + let httpMock: HttpTestingController; + let ngZone: NgZone; + + const BASE = `${AppSettings.getApiEndpoint()}/auth/google/drive`; + + beforeEach(() => { + // Stub gapi so loadPicker() resolves without the real script + vi.stubGlobal("gapi", { + load: (_feature: string, cb: () => void) => cb(), + }); + + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [DriveService, ...commonTestProviders], + }); + + service = TestBed.inject(DriveService); + httpMock = TestBed.inject(HttpTestingController); + ngZone = TestBed.inject(NgZone); + }); + + afterEach(() => { + httpMock.verify(); + vi.unstubAllGlobals(); + }); + + describe("getToken", () => { + it("calls the token endpoint and returns the response", async () => { + const promise = firstValueFrom(service.getToken()); + + const req = httpMock.expectOne(`${BASE}/token`); + expect(req.request.method).toBe("GET"); + req.flush({ status: "ok", accessToken: "abc123" }); + + const result = await promise; + expect(result.status).toBe("ok"); + expect(result.accessToken).toBe("abc123"); + }); + + it("returns no_refresh_token status when user has not connected Drive", async () => { + const promise = firstValueFrom(service.getToken()); + + httpMock.expectOne(`${BASE}/token`).flush({ status: "no_refresh_token" }); + + const result = await promise; + expect(result.status).toBe("no_refresh_token"); + expect(result.accessToken).toBeUndefined(); + }); + }); + + describe("connect", () => { + it("fetches the connect URL and opens a popup", () => { + const openSpy = vi.spyOn(window, "open").mockReturnValue(null); + + service.connect(); + + const req = httpMock.expectOne(`${BASE}/connect?reauth=false`); + expect(req.request.method).toBe("GET"); + req.flush("https://accounts.google.com/o/oauth2/auth?...";); + + expect(openSpy).toHaveBeenCalledWith( + "https://accounts.google.com/o/oauth2/auth?...";, + "gdrive-connect", + "width=500,height=600" + ); + }); + + it("passes reauth=true when reauth flag is set", () => { + vi.spyOn(window, "open").mockReturnValue(null); + + service.connect(true); + + httpMock.expectOne(`${BASE}/connect?reauth=true`).flush("https://accounts.google.com/...";); + }); + + it("emits on onConnected() when the popup posts gdrive-connected", fakeAsync(() => { + const mockPopup = { close: vi.fn() } as unknown as Window; + vi.spyOn(window, "open").mockReturnValue(mockPopup); + + let connected = false; + service.onConnected().subscribe(() => { + connected = true; + }); + + service.connect(); + httpMock.expectOne(`${BASE}/connect?reauth=false`).flush("https://accounts.google.com/...";); + + ngZone.run(() => { + window.dispatchEvent(new MessageEvent("message", { data: "gdrive-connected" })); + }); Review Comment: With the proposed `DriveService.connect()` origin/source validation, this test must include `origin: window.location.origin` (and the popup `source`) in the dispatched `MessageEvent`; otherwise the handler will ignore it and the test will fail. ########## frontend/src/app/app-routing.constant.ts: ########## @@ -46,3 +46,5 @@ export const DASHBOARD_ADMIN_EXECUTION = `${DASHBOARD_ADMIN}/execution`; export const DASHBOARD_ADMIN_SETTINGS = `${DASHBOARD_ADMIN}/settings`; export const DASHBOARD_SEARCH = `${DASHBOARD}/search`; + +export const DASHBOARD_USER_GOOGLE_DRIVE = `${DASHBOARD_USER}/google-drive`; Review Comment: `DASHBOARD_USER_GOOGLE_DRIVE` is added but not referenced anywhere in the frontend (only its declaration exists). If this route constant isn't needed, removing it avoids dead exports and keeps the routing constants list tidy. -- 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]
