feat: selectively enable / disable default authentication adapters

[dblythy]

New Pull Request Checklist

• I am not disclosing a vulnerability.
• I am creating this PR in reference to an issue.

Issue Description

I would say 100% of configurations use none, or a couple of the internal auth adaptors. However, by default, any auth type is enabled. This can lead to

Related issue: #7952

Approach

Adds enabled option to auth options. If enabled === false, the auth won't be able to be used.

TODOs before merging

• Add tests
• Add changes to documentation (guides, repository pages, in-code descriptions)
• Add security check
• Add new Parse Error codes to Parse JS SDK
• A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

[parse-github-assistant]

Thanks for opening this pull request!

• 🎉 We are excited about your hands-on contribution!

[codecov]

Codecov Report

Merging #7953 (3aadb58) into alpha (88b4d9d) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            alpha    #7953      +/-   ##
==========================================
- Coverage   94.13%   94.13%   -0.01%     
==========================================
  Files         182      182              
  Lines       13630    13635       +5     
==========================================
+ Hits        12831    12835       +4     
- Misses        799      800       +1     

Impacted Files	Coverage Δ
src/Options/index.js	100.00% <ø> (ø)
src/Options/Definitions.js	100.00% <100.00%> (ø)
src/RestWrite.js	94.56% <100.00%> (+0.18%)	⬆️
src/batch.js	92.98% <0.00%> (-1.76%)	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	95.46% <0.00%> (-0.16%)	⬇️
src/ParseServerRESTController.js	98.48% <0.00%> (+1.51%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88b4d9d...3aadb58. Read the comment docs.

[Moumouls]

LGTM

[Moumouls]

LGTM

[Moumouls]

LGTM (this is the good one ahha)

[mtrezza]

Should we already add a deprecation warning to this that adapters will be disabled by default in the future?

[mtrezza]

@dblythy Could you resolve the conflicts so this can be reviewed?

[mtrezza]

Look good! Anything you want to add before merge?

[mtrezza]

This is missing the parameter definition. We don't have any adapter definitions strangely, so you can only do it like this:

Add to src/Options/index.js:

auth: ?(AuthAdapter[]);
// ...
export interface AuthAdapter {
  /* Is `true` if the auth adapter is enabled, `false` otherwise.
  :DEFAULT: true */
  enabled: ?boolean;
}

[dblythy]

Thanks for that @mtrezza 😊

[mtrezza]

Looks good! So we have the new option documented in the API reference at least. If you add a short note to the auth adapter docs in the Parse Server guide when you have some time, that would be great. In the meantime I think we can go ahead and merge.

[parseplatformorg]

🎉 This change has been released in version 5.3.0-alpha.13

[parseplatformorg]

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

[parseplatformorg]

🎉 This change has been released in version 5.3.0

[FransGH]

Hi @dblythy, hope you can help. I'm using a custom auth adapter and just stumbled on this (breaking?) change after updating to the latest 5.x (5.1.1).

On my dev-system, where a config json is passed via API, all works well with a custom auth adaptor (as described in http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication).

On staging (and production), when passing the same config via CLI in a docker, the log just shows "Error: [object Object] should be a comma separated string or an array". Took some time but finally figured out that the auth parameter was the cause:

auth: {
   env: 'PARSE_SERVER_AUTH_PROVIDERS',
   help: 'Configuration for your authentication providers, as stringified JSON. See http://docs.parseplatform.org/parse-server/guide/#oauth-and-3rd-party-authentication',
   action: parsers.arrayParser
 },

I'm probably missing something but what is currently (as of 5.5.x and 6.x) the recommend way to install/configure a custom auth adapter?

[dblythy]

Can you share your auth config?

[FransGH]

Sure, here's a simple config that reproduces the issue with a standard auth-config (same happens with a custom auth-adapter as it basically just adds a custom module path):

{
  "appName": "test",
  "appId": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
  "masterKey": "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy",
  "logLevel": "error",
  "auth": {
    "facebook": {
      "appIds": "FACEBOOK APP ID"
    }
  }
}

I get the following error with parse server 5.5.1 CLI:

Error: [object Object] should be a comma separated string or an array

Passing the exact same config.json via API works.

const express = require('express');
const ParseServer = require('parse-server').ParseServer;
const fs = require('fs');

let api = new ParseServer(JSON.parse(fs.readFileSync('config.json')));

let app = express();
app.use('/parse', api);
app.listen(1337, () => console.log('parse-server running on port 1337.'));

Looking at the code it appears that the main issue is that 'auth' parameter definition was changed from an "parsers.objectParser" to "parsers.arrayParser". The auth parameter is however an object, not an object array. The test confirms this as it also passes in an object:

await reconfigureServer({
       auth: {
         myoauth: {
           enabled: false,
           module: path.resolve(__dirname, 'support/myoauth'), // relative path as it's run from src
         },
       },
     });

@mtrezza adding you to the discussion as it seems you where involved in changing the parameter definition. Any idea how this can be best fixed?

I'm meanwhile looking into using Parse.User._registerAuthenticationProvider to dynamically load a custom auth-adapter, bypassing the need to configure via the auth option.

[mtrezza]

@FransGH Just to reiterate on what has already been said at some other place of discussion; if you encounter an issue, please open a new issue to discuss. This makes it easier to follow for others and increases the chances that an issue will be fixed, as the discussion is not fragmented over multiple issues / PRs. If you comment on closed PRs or issue, chances are it won't be followed up or tracked properly.

[FransGH]

Sure, no problem. Creating an issue was the next thing I would have done if there was no further followup. But I'm thankful for how quickly this was picked up and look forward testing it on 5.5.1.

[mtrezza]

Great, thanks for reporting the issue and testing the fix!