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

[question] disable direct access to API when multitenancy is enabled? #1514

Closed
famarting opened this issue May 20, 2021 · 3 comments · Fixed by #1536
Closed

[question] disable direct access to API when multitenancy is enabled? #1514

famarting opened this issue May 20, 2021 · 3 comments · Fixed by #1536
Labels
Discussion Enhancement New feature or request

Comments

@famarting
Copy link
Contributor

Currently if multitenancy is enabled ,with or without authentication enabled, the default tenant can be used. It's possible to make requests straight to /apis/registry/v2 and it just works, while at the same time it's possible to do it providing the tenantId /t/{tenantId}/apis/registry/v2

I was considering if we should disallow the direct access to the API when multitenancy is enabled... in the case when authentication is enabled is not a big deal because the authentication should fail... but still I thought this idea is worth sharing.

I don't think this is high priority

@famarting famarting added Discussion Enhancement New feature or request labels May 20, 2021
@EricWittmann
Copy link
Member

I noticed this recently too and it was on my list to open an issue for it. So this is great.

I think we should disable direct access to everything under /apis/* when running in MT mode. Only access via /t/{tenantId} should be allowed. However, I think that should be controlled via application.properties so that we can continue to support a deployment architecture where the tenantId is provided some way other than /t/{tenantId} (e.g. provided as a Request header from some sort of router in front of the app).

@famarting
Copy link
Contributor Author

so then the ideal solution I think it would be to have a property to enable/disable such check, and to implement the check in RegistryApplicationServletFilter the check will reject the request if no tenant information is found. Currently the tenantId lookup is delegated to TenantIdResolver which already allows for passing the tenantId in a header.

@EricWittmann
Copy link
Member

Yes that's precisely what I was thinking for the impl. +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Enhancement New feature or request
Projects
None yet
2 participants