Sentiaus commented on code in PR #5721: URL: https://github.com/apache/texera/pull/5721#discussion_r3487414380
########## amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResource.scala: ########## @@ -0,0 +1,133 @@ +/* + * 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.typesafe.scalalogging.LazyLogging +import org.apache.texera.auth.SessionUser +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 + + // state token → (uid, expiresAtMs) + private val pendingStates = new ConcurrentHashMap[String, (Int, 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")) + def getOAuth(@Auth sessionUser: SessionUser): Response = { + val stateToken = java.util.UUID.randomUUID().toString + pendingStates.put(stateToken, (sessionUser.getUid, System.currentTimeMillis() + 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) { Review Comment: Fixed. Went for a lazy expiration approach where when this function is called, it cleans up all stale tokens. One issue that could still occur is if a user themself, repeatedly triggers and interrupts the oauth flow, resulting in a build up of unmatched, non-stale state tokens, which could cause heavy resource usage for the in-memory cache. I don't think this is particularly an issue, as user's have to be authenticated to trigger the flow, but it's worth noting. ########## amber/src/test/scala/org/apache/texera/web/resource/auth/GoogleDriveAuthResourceSpec.scala: ########## @@ -0,0 +1,75 @@ +/* + * 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 org.apache.texera.auth.SessionUser +import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.flatspec.AnyFlatSpec + +class GoogleDriveAuthResourceSpec extends AnyFlatSpec { + + private def newSessionUser(): SessionUser = { + val user = new User + user.setUid(Integer.valueOf(1)) + user.setName("test") + user.setRole(UserRoleEnum.REGULAR) + user.setEmail("[email protected]") + new SessionUser(user) + } + + it should "return error HTML when code is missing" in { Review Comment: Fixed! -- 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]
