aicam commented on code in PR #6866: URL: https://github.com/apache/texera/pull/6866#discussion_r3779409150
########## bin/mounter/mounter.py: ########## Review Comment: I agree, the problem is that mounter is a light weight microservice which only perform mounting. It is notable that the security leak is only in `unmount` endpoint, although mounting endpoint does not check token and CU access but it just mount it and user can not use it because S3 Servlet proxy in `file-service` check user access in every request, so we have two issues: 1- cuid=B can unmount a LakeFS repo from user A 2- cuid=B can mount a LakeFS repo from user A but can not access it To address them, I changed the design slightly: 1- Removed unmount endpoint. It is not needed because mounter service receives termination signal from K8s so no need to have an endpoint. It is now removed in this PR. 2- We put `access-control-service` in between CU and mounter. The diagram is updated but this PR does not include that. Later in #6896 we will add new endpoint to access control service and also add network policy to Envoy Gateway to make sure only access control service can talk to mounter and we do access control in the service. This way we keep mounter a simple microservice that only performs mounting. ########## bin/mounter/mounter.py: ########## Review Comment: I agree, the problem is that mounter is a light weight microservice which only perform mounting. It is notable that the security leak is only in `unmount` endpoint, although mounting endpoint does not check token and CU access but it just mount it and user can not use it because S3 Servlet proxy in `file-service` check user access in every request, so we have two issues: 1- cuid=B can unmount a LakeFS repo from user A 2- cuid=B can mount a LakeFS repo from user A but can not access it To address them, I changed the design slightly: 1- Removed unmount endpoint. It is not needed because mounter service receives termination signal from K8s so no need to have an endpoint. It is now removed in this PR. 2- We put `access-control-service` in between CU and mounter. The diagram is updated but this PR does not include that. Later in #6896 we will add new endpoint to access control service and also add network policy to Envoy Gateway to make sure only access control service can talk to mounter and we do access control in the service. This way we keep mounter a simple microservice that only performs mounting. -- 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]
