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

Support for tenant tokens #152

Open
archonic opened this issue Jun 8, 2022 · 4 comments
Open

Support for tenant tokens #152

archonic opened this issue Jun 8, 2022 · 4 comments
Labels
enhancement New feature or request needs more info This issue needs a minimal complete and verifiable example

Comments

@archonic
Copy link

archonic commented Jun 8, 2022

Description
v0.26.0 introduced tenant tokens but it looks like there aren't any opinions on tenant tokens in this gem.

Basic example
Specifics of a meilisearch tenant token integration are going to depend on how tenancy is handled but it would be nice to see examples that support acts_as_tenant and perhaps Apartment. It should be easy to figure out how to integrate with Apartment by looking at examples for acts_at_tenant.

I'm wondering how the client should be initiated when searching a tenanted resource. I have this index action for example:

def index
  @pagy, @documents = pagy_meilisearch(
    Document.includes(Document.search_includes).pagy_search(
      @search_terms,
      **index_search_options
    )
  )
end

In order to use a tenant token, would I have to initiate the client myself? Like this:

client = MeiliSearch::Client.new(ENV['MEILISEARCH_HOST'], current_account.ms_tenant_token)
Document.ms_search(...)

I'm also wondering about best practices for token refreshes. I currently generate a tenant token when the tenant is created and set the expiry to one year, and plan on creating a tenant key refresh job that runs once per month on keys that expire that month.

@brunoocasali
Copy link
Member

Hi @archonic, thanks for your issue!

Yes, currently there is no easy way to change the Meilisearch API key, used to query the resources internally. Because the data used to create the client comes from the configuration defined in the configuration side MeiliSearch::Rails.configuration = {}.

You are able to change this code at runtime but probably may have some problems with concurrency (not thread-safe).

I'm not sure if we will provide an example like you want (meilisearch-rails + apartment) here in the README, because we will have to be aware of apartment changes in order to make our tutorial always up to date.
But we definitely should add at least an easy way to change the apikey used to make the searches.

Do you have any idea about: "how the public API should look like"?

(disclaimer, I've never used the apartment gem before, the code below is just an idea for us to discuss)

Apartment::Tenant.switch('tenant_name') do
  MeiliSearch::Rails.change_api_key(tenant_data.current_user.meili_token) do
    # search code?...
  end
end

or

Apartment::Tenant.switch('tenant_name') do
  MeiliSearch::Rails.change_api_key(tenant_data.current_user.meili_token)
  
  # search code?...
end

About the expiry of your token I personally recommend you to have it as shorter as possible because, in case of a leak, the token will be sooner expired. I know that could be hard to implement but is definitely the best way (you can read more about it).

Anyway, let's discuss more this possible API, and then we can plan a way to make it real!

Thanks for this issue, we're really happy to see you around!

@brunoocasali brunoocasali added enhancement New feature or request needs more info This issue needs a minimal complete and verifiable example labels Jun 10, 2022
@archonic
Copy link
Author

archonic commented Jun 10, 2022

I realized that the attribute I wanted to filter by was not a user provided param. So as long as my app doesn't expose a Meilisearch JWT, I figure it's ok to scope by filter on the Rails side instead of using tenant tokens to scope. Still, I'm wondering how the gem can accommodate tenant tokens - I'd like to eventually have a front-end instant search feature. Especially if Actix can render the search results 🤘.

Apartment is no longer maintained so it may make more sense to have an example for acts_as_tenant or just pseudo code it.

In Apartment, as long and the request object is available and the "elevator" has populated the current_account, you don't need to explicitly switch tenants. Just being on one.example.com would switch into the one tenant for that request. If the request object isn't present then you'd need to explicitly switch into the tenant, such as in a migration, background job, setting up a test, or setting expectations for tenanted data.

Something like MeiliSearch::Rails.change_api_key(current_account.ms_tenant_token) { ... } works for me. I prefer the block to avoid bugs that result from forgetting to switch back. Apartment has Apartment::Tenant.switch which expects a block and Apartment::Tenant.switch! which will stay switched.

This may be an over complication though. Tenant tokens are specifically for scoped front-end searches, right? Perhaps the token should be provided to a search function directly like:

Document.ms_search_tenant(current_account.ms_tenant_token, @search_terms, { filter: "...", sort: [...] } )

That should prevent issues where you forget to switch back to an admin key before the app tries to create a new index, and it's more concise.

Happy to be here and thanks for making such an efficient and easy to use search service!

After thought: I wonder if I've misunderstood the purpose of a tenant token. Maybe this gem doesn't need tenant token integration if they're only designed for front-end implementations where Rails would not be answering requests?

@brunoocasali
Copy link
Member

Sorry if I let you alone, we are in the release cycle and all of our efforts are to make all the integrations ready when the v0.28 comes out!

I like the block approach as well, I've implemented something like that to switch databases and it was the perfect solution, I think it will be the same here.

I like the idea of having the ability to provide a different token at search-time, I think both can live together without any problem.

About instant-meilisearch, yes, we advise people to make the search in the front-end environment because it is where Meilisearch stands out! But if you do search in the backend you will still have a good performance and DX (it is not wrong).

But the tenant token is meant to just scope the data in the search time out of the box without having to apply other scripts in the data after the search.
You can read more if you want to about the feature here, directly in the specification or here at the docs.

So to answer this question Maybe this gem doesn't need tenant token integration if they're only designed for front-end implementations where Rails would not be answering requests?, I think this gem needs a rails-way to use the tenant-token feature!

@archonic
Copy link
Author

I think this gem needs a rails-way to use the tenant-token feature!

I agree. The gem can't assume that someone would have to use a front-end implementation to use that feature. I'm especially looking forward to building a front-end federated tenant search feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs more info This issue needs a minimal complete and verifiable example
Projects
None yet
Development

No branches or pull requests

3 participants