bobbai00 commented on code in PR #3598: URL: https://github.com/apache/texera/pull/3598#discussion_r2294959278
########## core/access-control-service/project/plugins.sbt: ########## @@ -0,0 +1,19 @@ +// 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. + +addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.1") +addSbtPlugin("com.github.sbt" % "sbt-native-packager" % "1.9.16") Review Comment: Seems this plugin is redundant ########## core/config/src/main/scala/edu/uci/ics/texera/config/DefaultsConfig.scala: ########## @@ -19,6 +19,7 @@ package edu.uci.ics.texera.config import com.typesafe.config.{ConfigFactory, ConfigRenderOptions, ConfigValueType} + Review Comment: revert this change ########## core/amber/src/main/scala/edu/uci/ics/texera/web/ServletAwareConfigurator.scala: ########## @@ -29,46 +30,78 @@ import java.nio.charset.Charset import javax.websocket.HandshakeResponse import javax.websocket.server.{HandshakeRequest, ServerEndpointConfig} import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.CollectionConverters._ /** - * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, - * allow it to be accessed by Websocket connections. - * <pre> - * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- - * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> - * </pre> - */ + * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, + * allow it to be accessed by Websocket connections. + * <pre> + * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- + * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> + * </pre> + */ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with LazyLogging { override def modifyHandshake( - config: ServerEndpointConfig, - request: HandshakeRequest, - response: HandshakeResponse - ): Unit = { + config: ServerEndpointConfig, + request: HandshakeRequest, + response: HandshakeResponse + ): Unit = { try { - val params = - URLEncodedUtils.parse(new URI("?" + request.getQueryString), Charset.defaultCharset()) - params.asScala - .map(pair => pair.getName -> pair.getValue) - .toMap - .get("access-token") - .map(token => { - val claims = jwtConsumer.process(token).getJwtClaims - config.getUserProperties.put( - classOf[User].getName, - new User( - claims.getClaimValue("userId").asInstanceOf[Long].toInt, - claims.getSubject, - String.valueOf(claims.getClaimValue("email").asInstanceOf[String]), - null, - null, - null, - null, - null - ) + val headers = request.getHeaders.asScala.view.mapValues(_.asScala.headOption).toMap + if (headers.contains("x-user-cu-access")) { + // KUBERNETES MODE: Construct the User object from trusted headers + // coming from envoy and generated by access control service. + + val userId = headers.get("x-user-id").flatten.map(_.toInt).get + val userName = headers.get("x-user-name").flatten.get + val userEmail = headers.get("x-user-email").flatten.get + val cuAccess = headers.get("x-user-cu-access").flatten.getOrElse("") + config.getUserProperties.put("cuAccess", cuAccess) Review Comment: Rename to `user-computing-unit-access`, please avoid using `cu` in the string literals. ########## core/amber/src/main/scala/edu/uci/ics/texera/web/ServletAwareConfigurator.scala: ########## @@ -29,46 +30,78 @@ import java.nio.charset.Charset import javax.websocket.HandshakeResponse import javax.websocket.server.{HandshakeRequest, ServerEndpointConfig} import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.CollectionConverters._ /** - * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, - * allow it to be accessed by Websocket connections. - * <pre> - * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- - * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> - * </pre> - */ + * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, + * allow it to be accessed by Websocket connections. + * <pre> + * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- + * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> + * </pre> + */ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with LazyLogging { override def modifyHandshake( - config: ServerEndpointConfig, - request: HandshakeRequest, - response: HandshakeResponse - ): Unit = { + config: ServerEndpointConfig, + request: HandshakeRequest, + response: HandshakeResponse + ): Unit = { try { - val params = - URLEncodedUtils.parse(new URI("?" + request.getQueryString), Charset.defaultCharset()) - params.asScala - .map(pair => pair.getName -> pair.getValue) - .toMap - .get("access-token") - .map(token => { - val claims = jwtConsumer.process(token).getJwtClaims - config.getUserProperties.put( - classOf[User].getName, - new User( - claims.getClaimValue("userId").asInstanceOf[Long].toInt, - claims.getSubject, - String.valueOf(claims.getClaimValue("email").asInstanceOf[String]), - null, - null, - null, - null, - null - ) + val headers = request.getHeaders.asScala.view.mapValues(_.asScala.headOption).toMap + if (headers.contains("x-user-cu-access")) { + // KUBERNETES MODE: Construct the User object from trusted headers + // coming from envoy and generated by access control service. + + val userId = headers.get("x-user-id").flatten.map(_.toInt).get + val userName = headers.get("x-user-name").flatten.get + val userEmail = headers.get("x-user-email").flatten.get + val cuAccess = headers.get("x-user-cu-access").flatten.getOrElse("") Review Comment: 'cu' => 'computing-unit' ########## core/access-control-service/project/build.properties: ########## @@ -0,0 +1,18 @@ +# 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. + +sbt.version = 1.9.9 Review Comment: This file should not be pushed ########## core/access-control-service/src/main/scala/edu/uci/ics/texera/service/resource/AccessControlResource.scala: ########## @@ -0,0 +1,110 @@ +// 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 edu.uci.ics.texera.service.resource + +import com.typesafe.scalalogging.LazyLogging +import edu.uci.ics.texera.auth.JwtParser.parseToken +import edu.uci.ics.texera.auth.SessionUser +import edu.uci.ics.texera.auth.util.ComputingUnitAccess +import edu.uci.ics.texera.dao.SqlServer +import edu.uci.ics.texera.dao.jooq.generated.enums.PrivilegeEnum +import jakarta.ws.rs.{GET, POST, Path, PathParam, Produces} +import jakarta.ws.rs.core.{Context, HttpHeaders, MediaType, Response, UriInfo} + +import java.util.Optional +import scala.jdk.CollectionConverters.{CollectionHasAsScala, MapHasAsScala} + +@Produces(Array(MediaType.APPLICATION_JSON)) +@Path("/auth") +class AccessControlResource extends LazyLogging { + + private val computingUnitAccess: ComputingUnitAccess = new ComputingUnitAccess() + + private def performAuth( + uriInfo: UriInfo, + headers: HttpHeaders + ): Response = { + val queryParams: Map[String, String] = uriInfo + .getQueryParameters() + .asScala + .view + .mapValues(values => values.asScala.headOption.getOrElse("")) + .toMap + + logger.info(s"Request URI: ${uriInfo.getRequestUri} and headers: ${headers.getRequestHeaders.asScala} and queryParams: $queryParams") + + val token = queryParams.getOrElse( + "access-token", + headers + .getRequestHeader("Authorization") + .asScala + .headOption + .getOrElse("") + .replace("Bearer ", "") + ) + val cuid = queryParams.getOrElse("cuid", "") + val cuidInt = try { + cuid.toInt + } catch { + case _: NumberFormatException => + return Response.status(Response.Status.FORBIDDEN).build() + } + + var cuAccess: PrivilegeEnum = PrivilegeEnum.NONE + var userSession: Optional[SessionUser] = Optional.empty() + try { + userSession = parseToken(token) + if (userSession.isEmpty) + return Response.status(Response.Status.FORBIDDEN).build() + + val uid = userSession.get().getUid + cuAccess = computingUnitAccess.getComputingUnitAccess(cuidInt, uid) + if (cuAccess == PrivilegeEnum.NONE) + return Response.status(Response.Status.FORBIDDEN).build() + } catch { + case e: Exception => + return Response.status(Response.Status.FORBIDDEN).build() + } + + Response + .ok() + .header("x-user-cu-access", cuAccess.toString) + .header("x-user-id", userSession.get().getUid.toString) Review Comment: Put these as constants somewhere, as these literals are used in multiple places ########## core/amber/src/main/scala/edu/uci/ics/texera/web/ServletAwareConfigurator.scala: ########## @@ -29,46 +30,76 @@ import java.nio.charset.Charset import javax.websocket.HandshakeResponse import javax.websocket.server.{HandshakeRequest, ServerEndpointConfig} import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.CollectionConverters._ /** - * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, - * allow it to be accessed by Websocket connections. - * <pre> - * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- - * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> - * </pre> - */ + * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, + * allow it to be accessed by Websocket connections. + * <pre> + * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- + * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> + * </pre> + */ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with LazyLogging { override def modifyHandshake( - config: ServerEndpointConfig, - request: HandshakeRequest, - response: HandshakeResponse - ): Unit = { + config: ServerEndpointConfig, + request: HandshakeRequest, + response: HandshakeResponse + ): Unit = { try { - val params = - URLEncodedUtils.parse(new URI("?" + request.getQueryString), Charset.defaultCharset()) - params.asScala - .map(pair => pair.getName -> pair.getValue) - .toMap - .get("access-token") - .map(token => { - val claims = jwtConsumer.process(token).getJwtClaims - config.getUserProperties.put( - classOf[User].getName, - new User( - claims.getClaimValue("userId").asInstanceOf[Long].toInt, - claims.getSubject, - String.valueOf(claims.getClaimValue("email").asInstanceOf[String]), - null, - null, - null, - null, - null - ) + if (KubernetesConfig.kubernetesComputingUnitEnabled) { + // KUBERNETES MODE: Construct the User object from trusted headers + // coming from envoy and generated by permission service. + val headers = request.getHeaders.asScala.view.mapValues(_.asScala.headOption).toMap + + val userId = headers.get("x-user-id").flatten.map(_.toInt).get Review Comment: I don't see the change. Can you double check? ########## core/access-control-service/src/main/resources/logback.xml: ########## @@ -0,0 +1,55 @@ +<!-- + 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. +--> + +<configuration> + <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">--> + <!-- encoders are assigned the type ch.qos.logback.classic.encoder.PatternLayoutEncoder + by default --> + <encoder> + <pattern>[%date{ISO8601}] [%level] [%logger] [%thread] - %msg %n + </pattern> + </encoder> + </appender> + + + <appender name="FILE" class="ch.qos.logback.core.rolling.RollingFileAppender"> + <file>../log/access-control-worker.log</file> + <immediateFlush>true</immediateFlush> Review Comment: should be `access-control-service.log` ? ########## core/amber/src/main/scala/edu/uci/ics/texera/web/SessionState.scala: ########## @@ -53,6 +54,7 @@ class SessionState(session: Session) { private var currentWorkflowState: Option[WorkflowService] = None private var workflowSubscription = Disposable.empty() private var executionSubscription = Disposable.empty() + private var userCUAccess: PrivilegeEnum = PrivilegeEnum.NONE Review Comment: please use `userComputingUnitAccess` ########## core/amber/src/main/scala/edu/uci/ics/texera/web/ServletAwareConfigurator.scala: ########## @@ -29,46 +30,78 @@ import java.nio.charset.Charset import javax.websocket.HandshakeResponse import javax.websocket.server.{HandshakeRequest, ServerEndpointConfig} import scala.jdk.CollectionConverters.ListHasAsScala +import scala.jdk.CollectionConverters._ /** - * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, - * allow it to be accessed by Websocket connections. - * <pre> - * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- - * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> - * </pre> - */ + * This configurator extracts HTTPSession and associates it to ServerEndpointConfig, + * allow it to be accessed by Websocket connections. + * <pre> + * See <a href="https://stackoverflow.com/questions/17936440/accessing-httpsession- + * from-httpservletrequest-in-a-web-socket-serverendpoint"></a> + * </pre> + */ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with LazyLogging { override def modifyHandshake( - config: ServerEndpointConfig, - request: HandshakeRequest, - response: HandshakeResponse - ): Unit = { + config: ServerEndpointConfig, + request: HandshakeRequest, + response: HandshakeResponse + ): Unit = { try { - val params = - URLEncodedUtils.parse(new URI("?" + request.getQueryString), Charset.defaultCharset()) - params.asScala - .map(pair => pair.getName -> pair.getValue) - .toMap - .get("access-token") - .map(token => { - val claims = jwtConsumer.process(token).getJwtClaims - config.getUserProperties.put( - classOf[User].getName, - new User( - claims.getClaimValue("userId").asInstanceOf[Long].toInt, - claims.getSubject, - String.valueOf(claims.getClaimValue("email").asInstanceOf[String]), - null, - null, - null, - null, - null - ) + val headers = request.getHeaders.asScala.view.mapValues(_.asScala.headOption).toMap + if (headers.contains("x-user-cu-access")) { + // KUBERNETES MODE: Construct the User object from trusted headers + // coming from envoy and generated by access control service. + + val userId = headers.get("x-user-id").flatten.map(_.toInt).get + val userName = headers.get("x-user-name").flatten.get + val userEmail = headers.get("x-user-email").flatten.get + val cuAccess = headers.get("x-user-cu-access").flatten.getOrElse("") + config.getUserProperties.put("cuAccess", cuAccess) Review Comment: I also saw this literal is being used in multiple places, can you create a constant for it and use the constant? ########## core/access-control-service/src/main/scala/edu/uci/ics/texera/service/resource/AccessControlResource.scala: ########## @@ -0,0 +1,110 @@ +// 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 edu.uci.ics.texera.service.resource + +import com.typesafe.scalalogging.LazyLogging +import edu.uci.ics.texera.auth.JwtParser.parseToken +import edu.uci.ics.texera.auth.SessionUser +import edu.uci.ics.texera.auth.util.ComputingUnitAccess +import edu.uci.ics.texera.dao.SqlServer +import edu.uci.ics.texera.dao.jooq.generated.enums.PrivilegeEnum +import jakarta.ws.rs.{GET, POST, Path, PathParam, Produces} +import jakarta.ws.rs.core.{Context, HttpHeaders, MediaType, Response, UriInfo} + +import java.util.Optional +import scala.jdk.CollectionConverters.{CollectionHasAsScala, MapHasAsScala} + +@Produces(Array(MediaType.APPLICATION_JSON)) +@Path("/auth") +class AccessControlResource extends LazyLogging { + + private val computingUnitAccess: ComputingUnitAccess = new ComputingUnitAccess() + + private def performAuth( + uriInfo: UriInfo, + headers: HttpHeaders + ): Response = { + val queryParams: Map[String, String] = uriInfo + .getQueryParameters() + .asScala + .view + .mapValues(values => values.asScala.headOption.getOrElse("")) + .toMap + + logger.info(s"Request URI: ${uriInfo.getRequestUri} and headers: ${headers.getRequestHeaders.asScala} and queryParams: $queryParams") + + val token = queryParams.getOrElse( + "access-token", + headers + .getRequestHeader("Authorization") + .asScala + .headOption + .getOrElse("") + .replace("Bearer ", "") + ) + val cuid = queryParams.getOrElse("cuid", "") + val cuidInt = try { + cuid.toInt + } catch { + case _: NumberFormatException => + return Response.status(Response.Status.FORBIDDEN).build() + } + + var cuAccess: PrivilegeEnum = PrivilegeEnum.NONE + var userSession: Optional[SessionUser] = Optional.empty() + try { + userSession = parseToken(token) + if (userSession.isEmpty) + return Response.status(Response.Status.FORBIDDEN).build() + + val uid = userSession.get().getUid + cuAccess = computingUnitAccess.getComputingUnitAccess(cuidInt, uid) + if (cuAccess == PrivilegeEnum.NONE) + return Response.status(Response.Status.FORBIDDEN).build() + } catch { + case e: Exception => + return Response.status(Response.Status.FORBIDDEN).build() + } + + Response + .ok() + .header("x-user-cu-access", cuAccess.toString) + .header("x-user-id", userSession.get().getUid.toString) Review Comment: You can put it in `core/auth/src/main/scala/edu/uci/ics/texera/auth/util/` as a new file, say `HeaderField.scala` -- 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]
