search

discolabs / django-shopify-auth

A package for adding Shopify authentication to a Django app.
MIT License
145 stars 54 forks source link

Added Shopify user profile to avoid customizing user profile #66

Closed aprams closed 3 years ago

aprams commented 3 years ago

The main issue I encountered was the lacking support for logging into the admin interface, which I tried solving by creating a separate ShopifyUserProfile, which is used for authentication instead of having to create a custom user model.

Instead, now you can just create a one-to-one-field on whatever user model you like.

This is a WIP (see ToDos below), but before I go into the cleanup, I would like to check with @gavinballard @stlk whether you agree on the general direction and gather your input.

Please share your thoughts and feedback.

Open ToDos right now are:

My black formatter ran over the files as well, which caused some formatting adjustments as well, hopefully for the better and for more uniformness.

stlk commented 3 years ago

Thank you for the PR! This is a very good idea, however I'm hesitant to just change the default. We might add this approach in separate Django app in the same package. I'm doing the same with cookieless auth.

Also please don't introduce formatting changes and functionality in the same PR. It makes it very hard to review the code.

aprams commented 3 years ago

Sure, sorry for the formatting, I can undo that.

So you'd suggest creating another app, sounds good! Will the cookieless auth become more or less the way to go and should I wait for that to include it in the changes? (=> Cookieless + user profile)

stlk commented 3 years ago

No worries :)

I take both session token(formely cookieless. Trying to move to new terminology and it's not easy ;)) auth and different model structure as experiments. And I would prefer if we keep them separate for a while. Session token app doesn't have it's own model and has only 3 assumptions about the user model. myshopify_domain field and install, uninstall methods. I hope we'll be able to make session token auth app work with both model structures.

aprams commented 3 years ago

Ok sounds good - thanks for the headsup. I'm currently waiting for the PR on token based auth and I'll then see whether it fits my use case or not. It looks more lightweight so far though, and I agree that it should be doable with both model structures. If not, I might approach you with a new PR / issue on that. If you want to discuss this direction / need support here, let me know. :)