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

DjangoModelPermissions shows API root for unauthenticated users #8425

Closed
mschoettle opened this issue Mar 24, 2022 · 7 comments
Closed

DjangoModelPermissions shows API root for unauthenticated users #8425

mschoettle opened this issue Mar 24, 2022 · 7 comments
Labels
Milestone

Comments

@mschoettle
Copy link

I noticed that when using permissions.IsAuthenticated, the APIRootView returns a 403. However, when using DjangoModelPermissions this is not the case. It does show the root with all available endpoints.

DjangoModelPermissions.has_permission(...) does have a check to ensure the user is authenticated in the code (introduced in #5376) but it happens after the special case handling is done for APIRootView (introduced in #2905).

def has_permission(self, request, view):
# Workaround to ensure DjangoModelPermissions are not applied
# to the root view when using DefaultRouter.
if getattr(view, '_ignore_model_permissions', False):
return True
if not request.user or (
not request.user.is_authenticated and self.authenticated_users_only):
return False

The authentication check should come first followed by the special case for APIRootView to be consistent with other permission classes.

I would be happy to provide a PR to address this.

@TTycho
Copy link

TTycho commented Apr 7, 2022

I agree, I had the same problem. I solved this by changing the order of checking for _ignore_model_permissions and checking the authentication status. So:

    def has_permission(self, request, view):
        if not request.user or (
           not request.user.is_authenticated and self.authenticated_users_only):
            return False

        # Workaround to ensure DjangoModelPermissions are not applied
        # to the root view when using DefaultRouter.
        if getattr(view, '_ignore_model_permissions', False):
            return True

I think this is the correct order because _ignore_model_permissions does not imply that it authenticated_users_only should be ignored.

Can anyone argue why it should not be in this order?

@stale
Copy link

stale bot commented Jun 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@auvipy
Copy link
Member

auvipy commented Nov 28, 2022

c0d95cb

@auvipy
Copy link
Member

auvipy commented Nov 28, 2022

c0d95cb

does this fix the issue?

@TTycho
Copy link

TTycho commented Nov 28, 2022

Yes! :-) This issue can be closed as fixed!

@auvipy auvipy closed this as completed Nov 29, 2022
@auvipy auvipy added this to the 3.15 milestone Nov 29, 2022
@TTycho
Copy link

TTycho commented Dec 6, 2022

For others stumbling upon this ticket, this fix only solves the root view being visible to unauthenticated users. An that endpoints a user has no permissions to is still be visible to all authenticated users.

@mschoettle
Copy link
Author

For others stumbling upon this ticket, this fix only solves the root view being visible to unauthenticated users.

That's what this ticket is about :)

An that endpoints a user has no permissions to is still be visible to all authenticated users.

Do you mean that the root view shows endpoints that the authenticated user even though the user has no permission to "use" them? If that is something you are looking for I suggest to open a new issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants