bennylope / django-organizations

:couple: Multi-user accounts for Django projects
http://django-organizations.readthedocs.org/
BSD 2-Clause "Simplified" License
1.3k stars 212 forks source link

Page not found 404 #228

Closed obvionaoe closed 1 year ago

obvionaoe commented 2 years ago

Hi, I tried following the Getting Started guide, but now when I try to access the path accounts/ or invitations/ I get a 404.

When accessing the accounts/ path it also auto redirects to accounts/login which returns a 404.

I also had to change the url snippet in the guide to use the path function instead of the url one because I was getting

AttributeError: 'tuple' object has no attribute 'split_contents'

on both urls.

I'm using Django 4.0.1, DRF 3.13.1, DRF-simplejwt 5.0.0, django-extensions 3.1.5 and django-organizations 2.0.1.

I also tried disabling DRF-simplejwt in case authentication was the issue but it led to the same errors.

Thanks

bennylope commented 2 years ago

Does this look like a problem with the documentation, or the code?

obvionaoe commented 2 years ago

url was deprecated in favour of path, so I believe the documentation is outdated in that regard.

The rest I'm not sure what the problem might be

obvionaoe commented 2 years ago

So, I really want to use this for my project, I created a repo where you can reproduce my problem! obvionaoe/django_org_test If you could take a look, I'd really appreciate it!

I still think it might have to do with authentication, as it looks like it tries to redirect to a login, but I don't know how to debug it

bennylope commented 2 years ago

Thank you @obvionaoe I'll take a look!

bennylope commented 2 years ago

There are several issues in the sample project you shared regarding project architecture and layout.

The 404 error

Regarding the 404, if I access the given URL, e.g http://localhost:8000/accounts/ that URL maps to the OrganizationList view in the organization.urls module, which applies login_required. So when I see the 404 it's actually on this URL, http://localhost:8000/accounts/login/?next=/accounts/ which doesn't exist in your URL configuration.

But why are you being redirected to /accounts/login/?

Because that's the default value for settings.LOGIN_URL which is what Django's authentication system redirects to when a non-authenticated user tries to access a view protected by a login requirement. What you're missing is a user-facing authentication page. You can create this yourself or use something like django-allauth.

Migration issues

It was a little bit of a struggle to get this up and running and I was confused by why the helper script had commands to delete migrations.

The problem is that you have defined both your system user model and a django-organizations dependency in the polls app (via the ForeignKey in TestModel.owner), which creates a circular dependency. I got everything working after I cleared out the migrations, commented out TestModel.owner, and rebuilt the migrations.

Circular dependency between polls and organizations

In the above image, the organizations app and the polls app have mutual dependence. This is a recipe for pain and confusion. You want an architecture more like the following:

No more circular dependency

Remove your custom User model to a separate app which has zero dependencies on django-organizations and preferably no other apps in your project, first or third party. All it should "know about" is users as individuals. You can create your own app or find a third party app to suit this purpose. I published an app to satisfy this need for myself called django-email-users which may be of use as an example; the code itself is quite old however!

URLs

I don't think this actually caused any problems, but from my own experience is a best practice: always include "pass through" URLs at the end of your URLs configuration.

In your root URLs config the urlpatterns looks like so:

urlpatterns = [
    path('', include('polls.urls')),
    path('', include('djoser.urls')),
    path('', include('djoser.urls.jwt')),
    path('accounts/', include('organizations.urls')),
    path('invitations/', include(invitation_backend().get_urls())),
]

The potential problem here is that your explicitly configured URL paths, which here are accounts/ and invitations/ are potentially subject to being intercepted by URLs in the preceding included paths.

Defining explicit paths first guarantees that those URLs will be checked first, making debugging URL issues much simpler.

In the following example I've added django-allauth for user account management and modified the URL structure. This guarantees now that user management (registration, authentication) routes are checked first, then organizational account routes, and then everything else without a unique path prefix.

urlpatterns = [
    path('accounts/', include('allauth.urls')),
    path('accounts/', include('organizations.urls')),
    path('invitations/', include(invitation_backend().get_urls())),
    path('', include('polls.urls')),
    path('', include('djoser.urls')),
    path('', include('djoser.urls.jwt')),
]
obvionaoe commented 2 years ago

Is there a way to configure Django Organizations to use a different permission class, meaning instead of the default Django LoginRequired class use DRF's IsAuthenticated class? As that one is the one that works with DRF, Djoser and SimpleJWT.

In regards to the circular dependency error, that's a good tip, thank you! I'll check out your email users app :)

obvionaoe commented 2 years ago

It's weird that login_required doesn't recognize DRF's Authentications classes

satyam4484 commented 1 year ago

I did'nt get much what the problem is but related to permission classes we can define the custom permission in django .