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

Get ParseConfig parameters with Master Key #5954

Merged
merged 4 commits into from
Aug 21, 2019

Conversation

mtrezza
Copy link
Member

@mtrezza mtrezza commented Aug 20, 2019

Adds a property to Parse Config parameter to make it only retrievable with master key.

@mtrezza
Copy link
Member Author

mtrezza commented Aug 20, 2019

This feature requires parse-community/parse-dashboard#1233.

@mtrezza mtrezza mentioned this pull request Aug 20, 2019
@codecov
Copy link

codecov bot commented Aug 20, 2019

Codecov Report

Merging #5954 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5954      +/-   ##
==========================================
- Coverage   93.68%   93.66%   -0.03%     
==========================================
  Files         156      156              
  Lines       10850    10857       +7     
==========================================
+ Hits        10165    10169       +4     
- Misses        685      688       +3
Impacted Files Coverage Δ
src/Routers/GlobalConfigRouter.js 100% <100%> (ø) ⬆️
src/RestWrite.js 93.72% <0%> (-0.17%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.52% <0%> (-0.17%) ⬇️

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 fddd9c2...0108bb0. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Aug 20, 2019

@mtrezza
Copy link
Member Author

mtrezza commented Aug 20, 2019

The feature you are referring to restricts PUTs to the master key. The new feature allows for individual config parameters to be GETtable only with master key.

See #5930 for feature description.

@mtrezza
Copy link
Member Author

mtrezza commented Aug 20, 2019

@dplewis Any idea why the test is passing for mongodb but fails for postgres?

@dplewis
Copy link
Member

dplewis commented Aug 20, 2019

I have an idea why, let me pull this down and see.

@dplewis
Copy link
Member

dplewis commented Aug 21, 2019

You have to add the masterKeyOnly field to the SchemaController otherwise you get an undefined column error

@mtrezza
Copy link
Member Author

mtrezza commented Aug 21, 2019

@dplewis Thanks, what are the implications when I add that field to the schema controller? That field is not always present, for example when upgrading from an existing parse server installation - could that cause any issues?

@dplewis
Copy link
Member

dplewis commented Aug 21, 2019

Its automatically gets updated when the server starts. schemaUpgrade

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM!

@dplewis dplewis changed the title Add masterkey parameters Get ParseConfig parameters with Master Key Aug 21, 2019
Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

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

It looks good to me. LGTM!

@davimacedo davimacedo merged commit 89e8868 into parse-community:master Aug 21, 2019
@mtrezza mtrezza deleted the add_masterkey_parameters branch September 3, 2019 01:28
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* added saving, retrieving

* added tests

* fixed typo

* added masterKeyOnly to schema controller
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.

3 participants