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

New DisableAPIAuth option should ignore all passed API tokens #5747

Closed
pbennett opened this issue Sep 18, 2023 · 3 comments · Fixed by #6067
Closed

New DisableAPIAuth option should ignore all passed API tokens #5747

pbennett opened this issue Sep 18, 2023 · 3 comments · Fixed by #6067
Labels
bug Something isn't working

Comments

@pbennett
Copy link

The new DisableAPIAuth config option added in 3.18 (#5625) is quite nice, but unfortunately for any code that already passes tokens, the server considers it an invalid API token.

Shouldn't it simply ignore any token passed in? If no token is acceptable, shouldn't 'any' token just be ignored ?
This seems the easiest path to allowing a server to migrate to not requiring tokens. Existing callers (with pre-arranged token) continue to work, but new callers don't have to pass one in.

  • Software version: 3.18.0

Steps to reproduce

  1. Set DisableAPIAuth: true in config.json of server. Call API setting X-Algo-API-Token to any possible API token value, ie: -H X-Algo-Api-Token:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
  2. The call will fail with 'Invalid API token' even though api auth is disabled.
@pbennett pbennett added the bug Something isn't working label Sep 18, 2023
@pbennett
Copy link
Author

In fact, it appears that passing the X-Algo-API-Token even with a blank token is considered invalid.

@joe-p
Copy link
Contributor

joe-p commented Oct 20, 2023

I think the solution here is to simply not add the MakeAuth middleware when node.Config().DisableAPIAuth is true.

middlewares.MakeAuth(TokenHeader, []string{adminAPIToken, apiToken}),

@gmalouf
Copy link
Contributor

gmalouf commented Jul 12, 2024

I'm looking at this now (apologies for the delay), it appears we did not correctly disable the authentication check. Preparing a follow-up PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants