Skip to content

fix: Setting Parse Server option masterKeyIps: [] doesn't disable all IP addresses #8339

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

Open
wants to merge 5 commits into
base: alpha
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/ParseServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,9 +468,12 @@ function injectDefaults(options: ParseServerOptions) {
}
});

options.masterKeyIps = Array.from(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this really just be removed and the issue is fixed?

cc @dblythy

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe so. I will test and report back

new Set(options.masterKeyIps.concat(defaults.masterKeyIps, options.masterKeyIps))
);
options.masterKeyIps =
options.masterKeyIps?.length > 0
? Array.from(
new Set(options.masterKeyIps.concat(defaults.masterKeyIps, options.masterKeyIps))
)
: [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
options.masterKeyIps =
options.masterKeyIps?.length > 0
? Array.from(
new Set(options.masterKeyIps.concat(defaults.masterKeyIps, options.masterKeyIps))
)
: [];

I think these lines can be removed completely

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah looking closer, fully agreed 😄

I was actually confused initially and didn't want to make some shit, especially because I'm not being able to run tests locally due to this error. any ideas?

~/dev/parse-server fix/masterKeyIps
❯ npm test

> [email protected] pretest /Users/stewan/dev/parse-server
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=5.3.2} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} mongodb-runner start

  ◝ Starting a MongoDB deployment to test against...Error: Could not find download URL for version 5.3.2 {
  version: '5.3.2',
  arch: [ 'arm64', 'aarch64' ],
  platform: 'darwin',
  target: [
    { value: 'osx', priority: 1 },
    { value: 'osx-ssl', priority: 10 },
    { value: 'darwin', priority: 1 },
    { value: 'macos', priority: 1 }
  ],
  enterprise: false,
  cryptd: false
}
    at resolve (/Users/stewan/dev/parse-server/node_modules/mongodb-download-url/lib/index.js:177:15)
    at async getDownloadURL (/Users/stewan/dev/parse-server/node_modules/mongodb-download-url/lib/index.js:242:12)
npm ERR! Test failed.  See above for more details.

}

// Those can't be tested as it requires a subprocess
Expand Down
6 changes: 5 additions & 1 deletion src/middlewares.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,11 @@ export function handleParseHeaders(req, res, next) {
req.info = info;

let isMaster = info.masterKey === req.config.masterKey;
if (isMaster && !ipRangeCheck(clientIp, req.config.masterKeyIps || [])) {
if (
isMaster &&
req.config.masterKeyIps?.length &&
!ipRangeCheck(clientIp, req.config.masterKeyIps || [])
) {
isMaster = false;
}

Expand Down