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

Ignore private fields input on user register #6047

Merged

Conversation

gfpacheco
Copy link
Contributor

Fixes #5834

  • Ignore private fields before passing input to resolver

PS: I'm only using GraphQL so I don't know if the same bug could be used to exploit the REST API.

@gfpacheco gfpacheco force-pushed the bug/ignore-private-fields-on-register branch from fcf542d to 18a180a Compare May 4, 2020 13:11
@codecov
Copy link

codecov bot commented May 4, 2020

Codecov Report

Merging #6047 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #6047   +/-   ##
=======================================
  Coverage   19.34%   19.34%           
=======================================
  Files         863      863           
  Lines       12060    12060           
  Branches     1935     1935           
=======================================
  Hits         2333     2333           
  Misses       8153     8153           
  Partials     1574     1574           
Flag Coverage Δ
#front 14.57% <ø> (ø)
#unit 39.96% <ø> (ø)

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 c9925f3...677b250. Read the comment docs.

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

Hey, thanks for your help ! You can however create a new input type for the register mutation instead of filtering the fields by end ;) You can add the new input type in the declaration field and update the query :)

@gfpacheco gfpacheco force-pushed the bug/ignore-private-fields-on-register branch 2 times, most recently from ace6bca to 4612063 Compare May 4, 2020 15:39
@gfpacheco
Copy link
Contributor Author

@alexandrebodin

Oh yes, I'm much happier with this solution. I wasn't sure that was something you would want, I didn't want to jump in changing input types, but that's definitely how things should be done.

@alexandrebodin alexandrebodin added issue: bug Issue reporting a bug source: plugin:graphql Source is plugin/graphql package labels May 4, 2020
@alexandrebodin alexandrebodin added this to the 3.0.0-beta.20.2 milestone May 4, 2020
alexandrebodin
alexandrebodin previously approved these changes May 4, 2020
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM, Thank you for this PR :D Will merge when test pass ;)

@alexandrebodin
Copy link
Member

Hey @gfpacheco Looks like this is breaking the register mutation test :D

@gfpacheco
Copy link
Contributor Author

Sorry, I just assumed the input name didn't matter, forgot you must declare it when using in a variable. Fixed

@alexandrebodin alexandrebodin added the flag: 💥 Breaking change This PR contains breaking changes and should not be merged label May 4, 2020
@alexandrebodin
Copy link
Member

Hey @gfpacheco can you just verify the DCO check so we can merge your PR ? :D

@gfpacheco gfpacheco force-pushed the bug/ignore-private-fields-on-register branch from 16afbfc to 6256a7b Compare May 5, 2020 17:14
Signed-off-by: Guilherme Pacheco <guilherme.f.pacheco@gmail.com>
Signed-off-by: Guilherme Pacheco <guilherme.f.pacheco@gmail.com>
@gfpacheco gfpacheco force-pushed the bug/ignore-private-fields-on-register branch from 6256a7b to 677b250 Compare May 6, 2020 13:42
@gfpacheco
Copy link
Contributor Author

@alexandrebodin All the checks have passed and I also rebased the branch onto master

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thank you for this PR

@alexandrebodin alexandrebodin merged commit 2b3d94d into strapi:master May 7, 2020
@alexandrebodin alexandrebodin modified the milestones: 3.0.0-rc.0, 3.0.0-rc.1 May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag: 💥 Breaking change This PR contains breaking changes and should not be merged issue: bug Issue reporting a bug source: plugin:graphql Source is plugin/graphql package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GraphQL plugin and the register mutation input customization
3 participants