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

feat: add dashboard option allowAnonymousUser #2066

Open
wants to merge 31 commits into
base: alpha
Choose a base branch
from

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Mar 23, 2022

New Pull Request Checklist

Issue Description

Parse Dashboard assumes security if it detects a remote server, which is unreliable. This adds the option allowAnonymousUser which defaults to false.

Related issue: #2065

Approach

  • Adds a Parse Dashboard option allowAnonymousUser that defaults to false. Setting this to true allows Dashboard to run with no users.
  • Adds ability to specify options via command line or via config file
  • Adds console.error to frontend so solutions to errors can be identified by developers

This also add additional security for when dashboard is run on localhost with no options. Previously Dashboard was effectively assuming the dev parameter if localhost. This PR will only run Dashboard in dev mode if it's implicitly set. To get previous localhost behaviour, the dev parameter is required to be set.

TODOs before merging

  • Add changes to documentation (guides, repository pages, in-code descriptions)
  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Mar 23, 2022

Thanks for opening this pull request!

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

@dblythy dblythy requested a review from a team March 23, 2022 03:26
Parse-Dashboard/app.js Outdated Show resolved Hide resolved
Parse-Dashboard/app.js Outdated Show resolved Hide resolved
@mtrezza
Copy link
Member

mtrezza commented Mar 26, 2022

Still some checks failing. Please request another review once this is ready.

@dblythy dblythy requested a review from mtrezza March 28, 2022 01:17
@mtrezza
Copy link
Member

mtrezza commented Mar 28, 2022

Is this a breaking change? Does an existing deployment need a reconfiguration to maintain the same security?

@dblythy
Copy link
Member Author

dblythy commented Mar 28, 2022

I would say so, an existing localhost configuration would need to be changed but remote servers should function the same as before

@mtrezza
Copy link
Member

mtrezza commented Mar 28, 2022

I guess our deprecation policy should be (or is in practice already) extending to Parse Dashboard. So breaking changes should probably happen accumulated once a year only as well. Otherwise we may have a breaking change every other month.

Is there a way to make this non-breaking? For example, if the new option is not set, fall back to the old behavior.

@dblythy
Copy link
Member Author

dblythy commented Mar 31, 2022

The only way I think is if we set dev to true by default, but that brings security implications

@mtrezza
Copy link
Member

mtrezza commented Apr 4, 2022

Yes, I think we should avoid that

@mtrezza
Copy link
Member

mtrezza commented Apr 4, 2022

Could you rebase this on alpha?

semantic-release-bot and others added 16 commits April 4, 2022 00:15
## [4.1.1-alpha.1](parse-community/parse-dashboard@4.1.0...4.1.1-alpha.1) (2022-04-04)

### Bug Fixes

* security upgrade js-beautify from 1.14.0 to 1.14.1 ([parse-community#2077](parse-community#2077)) ([e4ea787](parse-community@e4ea787))
* security vulnerability bump minimist from 1.2.5 to 1.2.6 ([parse-community#2070](parse-community#2070)) ([3d0407e](parse-community@3d0407e))
* adding internal class (e.g. `_User`) fails due to prefixed underscore ([parse-community#2036](parse-community#2036)) ([f80bd07](parse-community@f80bd07))
Snyk has created this PR to upgrade express from 4.17.2 to 4.17.3.

See this package in npm:
https://www.npmjs.com/package/express

See this project in Snyk:
https://app.snyk.io/org/acinader/project/3e039b91-2450-4b56-8420-baf56cab388e?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com>
@dblythy dblythy force-pushed the allowAnonymousUser branch from 6921339 to b9cc888 Compare April 4, 2022 00:52
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

It seems the discussion got stuck at how to avoid making this a breaking change. Once this has been figured out please request another review.

@mtrezza mtrezza removed the request for review from a team April 14, 2022 09:40
@mtrezza mtrezza force-pushed the alpha branch 2 times, most recently from d32a043 to 0f42a34 Compare May 1, 2022 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants