Yicong-Huang commented on code in PR #5721: URL: https://github.com/apache/texera/pull/5721#discussion_r3541625265
########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin + + 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("/connect") + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Produces(Array(MediaType.TEXT_PLAIN)) + def getOAuth(): Response = { + val now = System.currentTimeMillis() + pendingStates.entrySet().removeIf(e => now > e.getValue) + + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, now + STATE_TTL_MS) + + val url = new GoogleAuthorizationCodeRequestUrl( + clientId, + redirectUri, + java.util.Arrays.asList("https://www.googleapis.com/auth/drive.file";) + ) + .setState(stateToken) + .build() + + Response.ok(url).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.ok(errorHtml("Connection failed: invalid request. Please try again.")).build() Review Comment: this should return a 400 bad request ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin + + 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("/connect") + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Produces(Array(MediaType.TEXT_PLAIN)) + def getOAuth(): Response = { + val now = System.currentTimeMillis() + pendingStates.entrySet().removeIf(e => now > e.getValue) + + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, now + STATE_TTL_MS) + + val url = new GoogleAuthorizationCodeRequestUrl( + clientId, + redirectUri, + java.util.Arrays.asList("https://www.googleapis.com/auth/drive.file";) + ) + .setState(stateToken) + .build() + + Response.ok(url).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.ok(errorHtml("Connection failed: invalid request. Please try again.")).build() + } + try { + val expiresAt = pendingStates.remove(state) + if (expiresAt == null || System.currentTimeMillis() > expiresAt) { + return Response + .ok(errorHtml("Connection failed: the authorisation request expired. Please try again.")) + .build() + } + + new GoogleAuthorizationCodeTokenRequest( + new NetHttpTransport(), + GsonFactory.getDefaultInstance, + clientId, + clientSecret, + code, + redirectUri + ).execute() + + val html = + """<html><body><script> + |window.opener.postMessage('gdrive-connected', window.location.origin); + |window.close(); + |</script></body></html>""".stripMargin + Response.ok(html).build() + } catch { + case e: TokenResponseException => + logger.error("Google token exchange failed in callback", e) + Response + .ok( + errorHtml( + "Connection failed: could not complete sign-in with Google. Please try again." + ) + ) + .build() + case e: Exception => + logger.error("Unexpected error in OAuth callback", e) + Response.ok(errorHtml("An unexpected error occurred. Please try again.")).build() Review Comment: don't return `Response.ok` (200) when an error happens. and don't construct the error html in an OK's body... This should fail with a 502 status because the upstream service (OAuth) failed ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin Review Comment: why manually write an Html from the backend? backend just return a data/signal, frontend should decide how to render the data. ########## frontend/src/app/dashboard/component/user/user-dataset/user-dataset-explorer/dataset-detail.component.ts: ########## @@ -67,6 +67,9 @@ import { FilesUploaderComponent } from "../../files-uploader/files-uploader.comp import { NzProgressComponent } from "ng-zorro-antd/progress"; import { UserDatasetStagedObjectsListComponent } from "./user-dataset-staged-objects-list/user-dataset-staged-objects-list.component"; import { NzInputDirective } from "ng-zorro-antd/input"; +import { NzDropdownDirective, NzDropdownMenuComponent } from "ng-zorro-antd/dropdown"; +import { NzMenuDirective, NzMenuItemComponent } from "ng-zorro-antd/menu"; +import { DriveService } from "../../../../service/user/google-drive/drive.service"; Review Comment: can we name it Google Drive service? drive service can be confusing. ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,82 @@ +/** + * 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 } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly CONNECT_URL = `${AppSettings.getApiEndpoint()}/auth/google/drive/connect`; Review Comment: please do not put the endpoint under `auth`, it is confusing. `auth/` is for our application authentication. ########## amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala: ########## @@ -143,6 +147,7 @@ class TexeraWebApplication environment.jersey.register(classOf[AuthResource]) environment.jersey.register(classOf[GoogleAuthResource]) + environment.jersey.register(classOf[GoogleDriveAuthResource]) Review Comment: can we move this out of texera web? we want to make each of the services self contained. Also, this feature is about exporting results to Google Drive, it is not about auth. Although I know in order to send data to Google Drive you need to authenticate (login) to Google Drive. but the naming can be confusing. the endpoint is better to be `GoogleDriveExportResource` and moved to result service. ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin + + 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("/connect") + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Produces(Array(MediaType.TEXT_PLAIN)) + def getOAuth(): Response = { + val now = System.currentTimeMillis() + pendingStates.entrySet().removeIf(e => now > e.getValue) + + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, now + STATE_TTL_MS) + + val url = new GoogleAuthorizationCodeRequestUrl( + clientId, + redirectUri, + java.util.Arrays.asList("https://www.googleapis.com/auth/drive.file";) + ) + .setState(stateToken) + .build() + + Response.ok(url).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.ok(errorHtml("Connection failed: invalid request. Please try again.")).build() + } + try { + val expiresAt = pendingStates.remove(state) + if (expiresAt == null || System.currentTimeMillis() > expiresAt) { + return Response + .ok(errorHtml("Connection failed: the authorisation request expired. Please try again.")) + .build() + } + + new GoogleAuthorizationCodeTokenRequest( + new NetHttpTransport(), + GsonFactory.getDefaultInstance, + clientId, + clientSecret, + code, + redirectUri + ).execute() + + val html = + """<html><body><script> + |window.opener.postMessage('gdrive-connected', window.location.origin); + |window.close(); + |</script></body></html>""".stripMargin + Response.ok(html).build() Review Comment: just return OK (200) with an empty body. write html on the frontend ########## common/config/src/main/resources/user-system.conf: ########## @@ -27,6 +27,9 @@ user-sys { clientId = "" clientId = ${?USER_SYS_GOOGLE_CLIENT_ID} + clientSecret = "" + clientSecret = ${?USER_SYS_GOOGLE_CLIENT_SECRET} Review Comment: can we reuse google auth's secret? do we need another one? ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,82 @@ +/** + * 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 } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly CONNECT_URL = `${AppSettings.getApiEndpoint()}/auth/google/drive/connect`; + + constructor( + private http: HttpClient, + private ngZone: NgZone + ) {} + + connect(): Observable<void> { + return new Observable(observer => { + let popupCleanup: (() => void) | undefined; + + const subscription = this.http.get(this.CONNECT_URL, { responseType: "text" }).subscribe({ + next: url => { + const popup = window.open(url, "gdrive-connect", "width=500,height=600"); + + if (!popup) { + observer.error(new Error("Popup blocked. Please allow popups for this site.")); + return; + } + + const onMessage = (event: MessageEvent) => { + if (event.origin !== window.location.origin) return; + if (event.source !== popup) return; + if (event.data === "gdrive-connected") { + window.removeEventListener("message", onMessage); + popup.close(); + this.ngZone.run(() => { + observer.next(); + observer.complete(); + }); + } else if (event.data === "gdrive-error") { + window.removeEventListener("message", onMessage); + this.ngZone.run(() => { + observer.error(new Error("Google Drive connection failed")); + }); + } Review Comment: I failed to understand this piece of code. can you do the following: 1. use .html to write html code change. 2. use angular way to update the components, instead of changing `window.xxx` 3. leave only js/ts logic here. and add comments to explain? ########## frontend/src/app/dashboard/component/user/list-item/list-item.component.ts: ########## @@ -257,6 +262,16 @@ export class ListItemComponent implements OnChanges { } }; + public onClickExportToDrive(): void { + this.driveService + .connect() + .pipe(untilDestroyed(this)) + .subscribe({ + next: () => this.notificationService.success("Connected to Google Drive"), + error: () => this.notificationService.error("Failed to connect to Google Drive"), Review Comment: you need backend to return non-200 status code to trigger this code path ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin + + 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("/connect") + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Produces(Array(MediaType.TEXT_PLAIN)) + def getOAuth(): Response = { + val now = System.currentTimeMillis() + pendingStates.entrySet().removeIf(e => now > e.getValue) + + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, now + STATE_TTL_MS) + + val url = new GoogleAuthorizationCodeRequestUrl( + clientId, + redirectUri, + java.util.Arrays.asList("https://www.googleapis.com/auth/drive.file";) + ) + .setState(stateToken) + .build() + + Response.ok(url).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.ok(errorHtml("Connection failed: invalid request. Please try again.")).build() + } + try { + val expiresAt = pendingStates.remove(state) + if (expiresAt == null || System.currentTimeMillis() > expiresAt) { + return Response + .ok(errorHtml("Connection failed: the authorisation request expired. Please try again.")) + .build() + } + + new GoogleAuthorizationCodeTokenRequest( + new NetHttpTransport(), + GsonFactory.getDefaultInstance, + clientId, + clientSecret, + code, + redirectUri + ).execute() + + val html = + """<html><body><script> + |window.opener.postMessage('gdrive-connected', window.location.origin); + |window.close(); + |</script></body></html>""".stripMargin + Response.ok(html).build() + } catch { + case e: TokenResponseException => + logger.error("Google token exchange failed in callback", e) + Response + .ok( + errorHtml( + "Connection failed: could not complete sign-in with Google. Please try again." + ) + ) Review Comment: also should be 502 ########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,136 @@ +/* + * 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 com.typesafe.scalalogging.LazyLogging +import org.apache.texera.web.resource.auth.GoogleDriveAuthResource._ +import org.apache.texera.common.config.UserSystemConfig +import com.google.api.client.googleapis.auth.oauth2.{ + GoogleAuthorizationCodeRequestUrl, + GoogleAuthorizationCodeTokenRequest +} +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 STATE_TTL_MS = 10 * 60 * 1000L + + // Maps state token → expiresAtMs. + // Expired entries are swept out on each getOAuth call to prevent unbounded growth from abandoned flows. + private val pendingStates = new ConcurrentHashMap[String, Long]() +} + +@Path("/auth/google/drive") +@Consumes(Array(MediaType.APPLICATION_JSON)) +@Produces(Array(MediaType.APPLICATION_JSON)) +class GoogleDriveAuthResource extends LazyLogging { + + private def errorHtml(message: String): String = + s"""<html><body> + |<p style="font-family:sans-serif;padding:20px">$message</p> + |<script> + |window.opener?.postMessage('gdrive-error', window.location.origin); + |setTimeout(function(){ window.close(); }, 10000); + |</script> + |</body></html>""".stripMargin + + 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("/connect") + @RolesAllowed(Array("REGULAR", "ADMIN")) + @Produces(Array(MediaType.TEXT_PLAIN)) + def getOAuth(): Response = { + val now = System.currentTimeMillis() + pendingStates.entrySet().removeIf(e => now > e.getValue) + + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, now + STATE_TTL_MS) + + val url = new GoogleAuthorizationCodeRequestUrl( + clientId, + redirectUri, + java.util.Arrays.asList("https://www.googleapis.com/auth/drive.file";) + ) + .setState(stateToken) + .build() + + Response.ok(url).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.ok(errorHtml("Connection failed: invalid request. Please try again.")).build() + } + try { + val expiresAt = pendingStates.remove(state) + if (expiresAt == null || System.currentTimeMillis() > expiresAt) { + return Response + .ok(errorHtml("Connection failed: the authorisation request expired. Please try again.")) + .build() + } Review Comment: authorisation -> authorization? how come a request can expire, why do we have to check for the time? can you just return 502? ########## frontend/src/app/dashboard/service/user/google-drive/drive.service.ts: ########## @@ -0,0 +1,82 @@ +/** + * 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 } from "rxjs"; +import { AppSettings } from "../../../../common/app-setting"; + +@Injectable({ + providedIn: "root", +}) +export class DriveService { + private readonly CONNECT_URL = `${AppSettings.getApiEndpoint()}/auth/google/drive/connect`; + + constructor( + private http: HttpClient, + private ngZone: NgZone + ) {} + + connect(): Observable<void> { Review Comment: you need a token from Google Drive right? and you can just connect/login once and cache the token? -- 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]
