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

discussion: very long secured api-keys cause problems while searching #1035

Open
CSNWEB opened this issue Feb 25, 2020 · 24 comments · May be fixed by #1297
Open

discussion: very long secured api-keys cause problems while searching #1035

CSNWEB opened this issue Feb 25, 2020 · 24 comments · May be fixed by #1297

Comments

@CSNWEB
Copy link

CSNWEB commented Feb 25, 2020

We use very long secured API keys, which works fine with the 3.x version of the package but version 4.x breaks this, as the key seems to be appended to the URL regardless how long it is. It basically reintroduces this: #319. I think version 4 should also sent long api keys via the POST body.

@nunomaduro
Copy link

We have added this section into to the docs:

Keep in mind that the more limitations you set, the longer the key. You might face network limitations with a key longer than 500 characters, so consider this when adding restrictions.

At the moment, we don't have plans of having this feature on the 4.x. What's your use case?

@CSNWEB
Copy link
Author

CSNWEB commented Feb 25, 2020

As the data a user can access in our app is highly individualized, we need to apply a long set of filters in order to get only results that are suitable for the user. As the other issue shows other people face this issue as well. It would be great if you could reconsider this. At least you should add it to the upgrade guide that this breaks when upgrading from 3.x to 4.x

@nunomaduro nunomaduro changed the title Request uri too large is back with version 4.x discussion: very long secured api-keys cause problems while searching Feb 25, 2020
@nunomaduro
Copy link

I see. For the moment, if you heavily rely on this feature, I propose you keep using the 3.x. If we receive enough demand, we will reconsider this. Let's keep the issue open for a while.

@nunomaduro
Copy link

nunomaduro commented Feb 27, 2020

Meanwhile, a non-official workaround that is just here for investigation proposes. 🍛

import algoliasearch from 'algoliasearch/lite';

const appId = 'YourApplicationID';
const apiKey = 'YourSecuredApiKey';
const index = algoliasearch(appId, apiKey).initIndex('index_name');

// Workaround start
delete index.transporter.queryParameters['x-algolia-api-key'];
const read = index.transporter.read;
index.transporter.read = (request, requestOptions) => {
  return read(request, { apiKey, ...requestOptions });
};
// Workaround end

const results = await index.search('');

@nunomaduro
Copy link

cc @Haroenv @maxiloc

@nunomaduro
Copy link

@jackmccloy
Copy link

I see. For the moment, if you heavily rely on this feature, I propose you keep using the 3.x. If we receive enough demand, we will reconsider this. Let's keep the issue open for a while.

hey @nunomaduro, i'm bumping into this issue too and just saw your note that it'll be a "wontfix" unless it receives more demand. our situation seems to be identical to @CSNWEB's - we apply a long set of filters, generating very long secured API keys. I imagine this is also true for many use cases where search needs to be highly personalized.

it would be great if this limitation was removed so we can upgrade from v3 to v4. thanks!

@Haroenv
Copy link
Contributor

Haroenv commented Aug 14, 2020

Hi @jackmccloy, so far we indeed have seen this issue only very sporadically, which is why it's not been fixed yet. From my research, I can see that URLs more have a 2000± character limit (not 500) (see https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers). Could you explain the exact use case and where it gets blocked? Thanks!

@Haroenv Haroenv reopened this Aug 14, 2020
@jackmccloy
Copy link

sorry for the delay @Haroenv. our use case is very similar to @CSNWEB's - we apply a long set of filters because the permissions around what any particular user can search for and access are very granular

@Haroenv
Copy link
Contributor

Haroenv commented Sep 2, 2020

How long is the URL, and where does it cause problems @jackmccloy?

@jpreynat
Copy link

jpreynat commented Aug 2, 2021

We also encounter the same error when switching to the client v4.
On our end, we encounter the Header too large ApiError and have to use the proposed workaround by explicitly removing the api key from the headers:

delete index.transporter.headers['x-algolia-api-key'];

@Haroenv
Copy link
Contributor

Haroenv commented Aug 2, 2021

hey @jpreynat, how long was your URL and at which point did you get the error, was that from the Algolia backend?

@jpreynat
Copy link

jpreynat commented Aug 2, 2021

Hi @Haroenv, it wasn't an issue with the URL but the size of the header.

I got this issue as soon as I switched from v3 to v4 when calling client.search().

@jpreynat
Copy link

jpreynat commented Aug 2, 2021

Actually, the workaround is working from our Node backend, but from the frontend, no results are returned anymore, so this is definitely problematic for us.
I've tried passing the api key in the query parameters, but this time I end with the request uri is too large and status 414 error. The full URI with the api key is 8660 characters long.

@Haroenv
Copy link
Contributor

Haroenv commented Aug 2, 2021

What throws this error?

@jpreynat
Copy link

jpreynat commented Aug 4, 2021

Just fetching the URL with everything in the query parameters returns this error.

@Haroenv
Copy link
Contributor

Haroenv commented Aug 5, 2021

I've looked it up, and the error indeed comes from nginx, but sending it via the url or via headers doesn't make a difference I think, the default in nginx (as far as I can tell we use the default for this) https://nginx.org/en/docs/http/ngx_http_core_module.html#large_client_header_buffers is 8k, so an api key that's so long will still error, even if it's sent by headers @jpreynat.

I'd have to check, but you can make the client force sending auth via the headers https://codesandbox.io/s/upbeat-tu-4juop?file=/src/app.js. I think that still would throw the nginx error

import algoliasearch from 'algoliasearch';
import { AuthMode } from '@algolia/client-common';

const client = algoliasearch('latency', '6be0576ff61c053d5f9a3225e2a90f76', {
  authMode: AuthMode.WithinHeaders, // authMode: 1
});
const index = client.initIndex('instant_search');

docs for authMode: https://www.algolia.com/doc/api-client/advanced/configure-the-client/javascript/?client=javascript#auth-mode

@jpreynat
Copy link

jpreynat commented Aug 5, 2021

Thanks for the details @Haroenv.

Do you have any idea why this works with the v3 client though? We’re using the exact same keys in both cases.

If this can’t be modified on your end, what would you recommend us to do then?

Thanks again for your help 🙏

@maxiloc
Copy link

maxiloc commented Aug 5, 2021

v3 had the ability to put the key in the body instead of the header in case the api key was too long.

if (
this.apiKey.length > MAX_API_KEY_LENGTH &&
initialOpts.body !== undefined &&
(initialOpts.body.params !== undefined || // index.search()
initialOpts.body.requests !== undefined) // client.search()
) {
initialOpts.body.apiKey = this.apiKey;
headers = this._computeRequestHeaders({
additionalUA: additionalUA,
withApiKey: false,
headers: initialOpts.headers
});
} else {

@Haroenv
Copy link
Contributor

Haroenv commented Aug 5, 2021

Thanks for chiming in @maxiloc, in v3 this indeed happened automatically, while in v4 there's a specific option for it (authMode). Did you try it with the withinHeaders method @jpreynat? I might have misread the error message in the nginx spec and it allows longer headers than uri itself. The documentation isn't very elaborate on that part.

@Haroenv
Copy link
Contributor

Haroenv commented Aug 5, 2021

Oh, I read it right now, I forgot it could go in the body of a request as well, and that likely has a different limit, the workaround given by @nunomaduro in #1035 (comment) should still work in that case I think

@jpreynat
Copy link

jpreynat commented Aug 5, 2021

Thanks for clarifying this @maxiloc 👍

@Haroenv I didn't try the withinHeaders method yet. But if the headers in nginx are limited to 8k characters, I'm scared we might hit the limit sometimes since my URL with the querystring was larger than this. I'll give it a try thought.

When trying the workaround, it worked when using Node, but when using the bundled JS library in the browser, I received only empty results with the same key, so there might be some other changes to do in this case.

Haroenv added a commit that referenced this issue Aug 6, 2021
fixes #1035

Implementation is done by:
- adding a new auth mode
- adding data as the return type for createAuth
- expose data from transporter
- serialize transporter data
- map api key back to query parameter if GET
@Haroenv Haroenv linked a pull request Aug 6, 2021 that will close this issue
2 tasks
@nunomaduro nunomaduro removed their assignment Nov 8, 2021
@BohdanYavorskyi
Copy link

@Haroenv @nunomaduro one more request to add withBody authMode. What is the reason why it's still the issue and not merged?

@BohdanYavorskyi
Copy link

@jpreynat how have you fixed that issue on your end?

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

Successfully merging a pull request may close this issue.

7 participants