Skip to content

Default masterKeyIps does not work on localhost #8316

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
4 tasks done
dblythy opened this issue Nov 15, 2022 · 19 comments · Fixed by #8322
Closed
4 tasks done

Default masterKeyIps does not work on localhost #8316

dblythy opened this issue Nov 15, 2022 · 19 comments · Fixed by #8322
Labels
block:major Needs to be resolved before next major release; remove label afterwards state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:bug Impaired feature or lacking behavior that is likely assumed

Comments

@dblythy
Copy link
Member

dblythy commented Nov 15, 2022

New Issue Checklist

Issue Description

Introduced in #8281, the new default does not allow local IPs.

This is due to the fact that getClientIp returns an ipv6 IP (::1), whereas the masterKeyIps default is 127.0.0.1

Steps to reproduce

Run a local instance of Parse Server alpha with Dashboard

Actual Outcome

Dashboard should load

Expected Outcome

"MasterKey is required"

Environment

Server

  • Parse Server version: alpha
  • Operating system: macos
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): localhost

Database

  • System (MongoDB or Postgres): mongo
  • Database version: 5.0
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): atlas

Client

  • SDK (iOS, Android, JavaScript, PHP, Unity, etc): js
  • SDK version: 3.4.4

Logs

@parse-github-assistant
Copy link

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

@dblythy
Copy link
Member Author

dblythy commented Nov 15, 2022

Solution is to change the default to ::1, or resolve getClientIp to an ipv4 address

@mtrezza
Copy link
Member

mtrezza commented Nov 15, 2022

Solution is to change the default to ::1

How would that work on a machine where IPv6 is not enabled? Can't we just add both values as default? ['::1','127.0.0.1']

@mtrezza mtrezza added type:bug Impaired feature or lacking behavior that is likely assumed block:major Needs to be resolved before next major release; remove label afterwards labels Nov 15, 2022
@dblythy
Copy link
Member Author

dblythy commented Nov 15, 2022

Yes, that would fix it also. Should masterKeyIps always allow localhost? Otherwise developer will have to configure masterKeyIps: ["::1", "127.0.0.1", "ip", "ip"],

I'm thinking internally it should be allowedIps = ["::1", "127.0.0.1", ...config.masterKeyIps],, masterKeyIps can default to [] (localhost only)

@mtrezza
Copy link
Member

mtrezza commented Nov 15, 2022

Should masterKeyIps always allow localhost?

Why?

@dblythy
Copy link
Member Author

dblythy commented Nov 15, 2022

I think it would be expected that masterKeyIps is external ips, and that cloud code, code run on the same instance as Parse Server, should always be allowed.

@mtrezza
Copy link
Member

mtrezza commented Nov 15, 2022

How can the use of masterKey be completely disabled?

@dblythy
Copy link
Member Author

dblythy commented Nov 15, 2022

Maybe by not setting a masterKey?

@mtrezza
Copy link
Member

mtrezza commented Nov 15, 2022

Does Parse Server start without masterkey?

@dblythy
Copy link
Member Author

dblythy commented Nov 16, 2022

Not at the moment

@mtrezza
Copy link
Member

mtrezza commented Nov 16, 2022

Then setting masterKeyIps to an empty array is the only way at the moment. So I'd say we should not set localhost as a non-removable allow-entry.

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-alpha.11

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Nov 25, 2022
@stewones
Copy link
Contributor

Then setting masterKeyIps to an empty array is the only way at the moment. So I'd say we should not set localhost as a non-removable allow-entry.

it's not working currently @mtrezza

sent a PR to fix #8339

@dblythy
Copy link
Member Author

dblythy commented Nov 28, 2022

So I'd say we should not set localhost as a non-removable allow-entry.

I think we missed that still though, as the default options are always concatenated to any additional set masterKeyIps

https://github.com/parse-community/parse-server/blob/alpha/src/ParseServer.js#L471-L473

@mtrezza
Copy link
Member

mtrezza commented Nov 28, 2022

Oh, did #8339 aim to fix that and remove the concatenation? I reopened.

@dblythy
Copy link
Member Author

dblythy commented Nov 28, 2022

I don't think so, I think #8339 was designed to make masterKeyIps: [] === access from anywhere, which is incorrect. We can open a new PR fixing the concatenation

@stewones
Copy link
Contributor

I don't think so, I think #8339 was designed to make masterKeyIps: [] === access from anywhere, which is incorrect. We can open a new PR fixing the concatenation

sort of, I misunderstood the idea initially but I think it's ok now. please take a 2nd look at the PR

@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jan 31, 2023
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 6.0.0

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
block:major Needs to be resolved before next major release; remove label afterwards state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version type:bug Impaired feature or lacking behavior that is likely assumed
Projects
None yet
4 participants