PedroBern / django-graphql-auth

Django registration and authentication with GraphQL.

https://django-graphql-auth.readthedocs.io/en/latest/

MIT License

329 stars 106 forks source link

Drop Exclude Fields support and use Fields #27

Closed panosangelopoulos closed 4 years ago

panosangelopoulos commented 4 years ago

Currently, the library only supports exclude fields, but this seems extremely dangerous because whenever a new field is added to the user model, we have to remember to exclude it in the settings, it is more than likely that we forget this and the field is exposed to public by accident.

To prevent this, with the current PR we will support fields on the Meta class of the UserNode, which is read from the new setting USER_NODE_FIELDS.

The whitelist approach is much safer as using only exclude fields.

codecov-io commented 4 years ago

Codecov Report

Merging #27 into master will not change coverage by %. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #27   +/-   ##
=======================================
  Coverage   97.23%   97.23%           
=======================================
  Files          12       12           
  Lines         615      615           
=======================================
  Hits          598      598           
  Misses         17       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 74771f4...c3dd88a. Read the comment docs.

PedroBern commented 4 years ago

You are totally right! But I think we should drop completely the exclude fields. I didn't understand why you revert it on the second commit. Maybe would be better to do this o v0.4, since it would drop a settings. I plan to discuss some v0.4 features on #23, I'm just not into it yet because my time is very short right now, but I'm really looking forward to it.

panosangelopoulos commented 4 years ago

@PedroBern thanks for approving it! I can completely drop the exclude fields but first I would like to discuss it with you. Of course, if it's ok then I can add a new commit where I will remove it.

PedroBern commented 4 years ago

I thank you for the PR! You know what you are doing, you can do all the necessary changes to make it work. I've just created the v0.4 branch, please make the PR for it. Also, would be great if you can update the documentation about the new changes :)

panosangelopoulos commented 4 years ago

Sure thing, @PedroBern! Changed the base branch and update the docs accordingly.