Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow anyone able to view a service instance to be able to see which spaces are shared with #3930

Closed
Benjamintf1 opened this issue Aug 14, 2024 · 11 comments

Comments

@Benjamintf1
Copy link
Member

Issue

Right now, only the service owners(people with access to the origin space) are able to get the service instances 'shared_spaces'. To be able to determine which apps are able to be bound to a service, a user would have to do 1 get for every space they have access to be able to determine which spaces the service is in. The cli also isn't able to view shared spaces when calling cf service <service>

Context

A user who has access to even all of the shared spaces is unable to get shared_spaces. they can use fields to get the name of the space and org it comes from, but they can't get shared_spaces.

Steps to Reproduce

Create multiple spaces. Share service from donor space to multiple shared space. Create a user who only has access to shared spaces. Try and figure out how to determine which spaces the service is shared with.

Expected result

The user should be able to do so just by using the service's shared_spaces endpoint.

Current result

Only a user who has access to the origin space is able to access those details (even if they have access to none of the shared spaces, they can still see them).

Possible Fix

Allow users to get access to shared_spaces if they can access the service.

@Gerg
Copy link
Member

Gerg commented Aug 14, 2024

I believe @blgm and @FelisiaM originally worked on this

@Benjamintf1
Copy link
Member Author

#3931

@blgm
Copy link
Member

blgm commented Aug 15, 2024

Yes, @FelisiaM and I were on the team who worked on the V3 API for a this. I think it was based on an earlier implementation that was partly V2 and partly V3. I don’t think we had strong reasons for not allowing this. We were aiming to match existing behaviour, and leave the door open to improvements in the future. This request sounds very reasonable, and as long as it doesn’t inadvertently create security holes then it makes sense.

@philippthun
Copy link
Member

I think exposing the guids of shared spaces is fine, but we should prohibit the exposure of space names to users that are not allowed to read from those spaces. So I guess the corresponding field decorator needs to be adjusted as well.

@Benjamintf1
Copy link
Member Author

Right now there isn't any filtering at all of allowing or prohibiting space or org names. If you have access to the endpoint, you can ready all the names fields can expose. I don't think this would be a change, but something to consider going forward if we wanted to at some point restrict it more.

@Benjamintf1
Copy link
Member Author

Benjamintf1 commented Sep 4, 2024

Just want to clarify what I wrote yesturday. Right now, on existing published versions of capi, users can use fields to access the name of orgs and spaces they don't have access to on the shared_spaces endpoint. Allowing users of a instance in a shared space to access those names would not fundamentally change this fact.

@Benjamintf1
Copy link
Member Author

any further thoughts anyone?

@sethboyles
Copy link
Member

users can use fields to access the name of orgs and spaces they don't have access to on the shared_spaces endpoint. Allowing users of a instance in a shared space to access those names would not fundamentally change this fact.

That would still expose more space names to more users, right? Since right now this only leaks when you have permission to view the originating space?

@Benjamintf1
Copy link
Member Author

Benjamintf1 commented Sep 5, 2024

It would allow more users to see more space names. I'm less concerned because I actually don't necessarily see a "correct hirarchy" between spaces shared. The originating space could be a "more secure" space, or a "less secure space". Giving the shared spaces users ability to read feels more like "level setting" then "exposing to less priviledged users" to me to be honest. (and everyone always had access to the name of the space and org of the originating space)

@philippthun
Copy link
Member

As I changed another decorator a while ago due to a permission issue (see #3692), I created a new PR #3962 (based on #3931, thus needs a rebase later on) that adapts the field decorators and filters out space and organization names based on permissions. This is the relevant commit: ddc380

@philippthun
Copy link
Member

PR #3931 has been merged, so this issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants