[Spaces] - Accessing non-existent Spaces

[legrego]

Opened to track #21408 (comment) (cc @ycombinator)

When accessing any Kibana endpoint with Spaces enabled, the Spaces plugin should ensure that the space exists before allowing the request to continue. If the space does not exist, then a 404 should be thrown instead.

[legrego]

Awaiting #21995 for its SpacesClient

[legrego]

@kobelb Do you have any opinions on how we should proceed with this?

I was originally thinking that the Spaces SOC could check the current space before executing the get/create/etc request. The easiest way to do this is via the SpacesClient since that already has the appropriate logic. If we take this approach, we have a couple of options:

1. If the space is not found (or if user is not authorized), then return the generic "not found" error.
2. If the space is not found (or if user is not authorized), then return whatever error is thrown by the SpacesClient.

Option 1 has the benefit of being a little more user-friendly but ends up changing some of the expected error codes/messages (where we may have previously thrown a 403, we will now throw a 404)

Option 2 has the benefit of being more consistent internally, but also ends up leaking error messages that don't line up with the user's operation. For example, a user could get an error message saying they're not authorized to get a space, when in reality they just tried to create an index pattern.

A couple of other options:

3. Have the Spaces SOC get an instance of the internalRepository, and use that to check only for the existence of the active space, and perform any auth checks as a side effect. This feels like side-stepping the "middleware" approach we've taken with the SOC wrappers, but I guess it's not a whole lot different than using the SpacesClient within the Spaces SOC.

4. Have the spaces_request_interceptor query for the active space before the request ever makes it to the SOC. This would be a more holistic approach, but I'm concerned about doing this check on every single request that comes through to Kibana. It feels unnecessarily expensive to me. However...this option gives us the opportunity to issue a redirect to users who try to access an invalid space, which is a nice ux bonus.

My preference as of right now is Option 1, but wanted to get your thoughts.

[legrego]

After talking with @kobelb, we decided on Option 1 for now, and we will open a followup issue to more gracefully handle the scenario where a user types in a URL that points to an invalid space

[kobelb]

Doing this pre-check sent us down a few rabbit holes, so I recommended that we not address this deficiency at this point in time. We're generally pretty lax about checking "referential integrity" for most things in Kibana and Elasticsearch's APIs, so while this would be nice to have, I don't think it's a must-have at this point in time. This becomes easier to implement once we are able to remove the legacy fallback in 7.0, so I'd recommend waiting until then to do so.

[ycombinator]

This issue was filed based on a comment I made originally in another issue: #21408 (comment). There were actually two issues I noticed:

1. That we allowed requests to go through even if a space was non-existent, instead of returning a 404. That's what this issue here is about.
2. That the requests that do go through for non-existent spaces, get applied to the default space. So, for example, if a user had a typo in their space ID, things would appear to work without the user knowing they were actually working against the default space. This one seems a bit more trappy than the first one, so I want to make sure we at least address this one (or already have) for the initial release of Spaces.

[kobelb]

Thanks @ycombinator, that makes sense. We definitely don't want this happening:

That the requests that do go through for non-existent spaces, get applied to the default space. So, for example, if a user had a typo in their space ID, things would appear to work without the user knowing they were actually working against the default space. This one seems a bit more trappy than the first one, so I want to make sure we at least address this one (or already have) for the initial release of Spaces.

I just verified against spaces-phase-1 that this is no longer occurring, this was likely fixed by the changes we made to use the url-context as the actual ID of the document.