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

bug: APIS_DETAIL_VIEWS_ALLOWED = True does not make any difference #1400

Open
gythaogg opened this issue Nov 17, 2024 · 3 comments · May be fixed by #1399
Open

bug: APIS_DETAIL_VIEWS_ALLOWED = True does not make any difference #1400

gythaogg opened this issue Nov 17, 2024 · 3 comments · May be fixed by #1399
Assignees
Labels
bug Something isn't working (properly, as expected, at all) needs-attention This issue or pull request is in need of discussion, information, assessment by team members

Comments

@gythaogg
Copy link
Contributor

gythaogg commented Nov 17, 2024

Despite the setting APIS_DETAIL_VIEWS_ALLOWED = True the user is still forced to login to see the detail page.

And as far as I can tell this ensures that view permissions are always set for Detail views.

return [permission_fullname(self.permission_action_required, self.model)]

A fix is available in PR #1399

To Discuss:

  1. Do we need this setting at all - it only applies to the "view", create and update views require permissions anyway.
  2. By allowing ListView and disabling Detail views we are essentially only stopping the user from seeing the entity details and the relations in the same page.
  3. However the "details" of the objects can be seen from the ListView by selecting the columns and the relations of the objects can be seen in the relations list views.

I would like to suggest that we use a single setting to view entities that when set would allow the anonymous user read only access to both list and detail views. Does that make sense?

@gythaogg gythaogg added the bug Something isn't working (properly, as expected, at all) label Nov 17, 2024
gythaogg added a commit that referenced this issue Nov 17, 2024
Skip adding permissions to DetailView if APIS_DETAIL_VIEWS_ALLOWED is
set by using a DetailViewObjectMixin that overrides GenericModelMixin's
get_permission_required method.

fixes #1400
@gythaogg gythaogg assigned gythaogg and unassigned b1rger Nov 17, 2024
@gythaogg gythaogg added the needs-attention This issue or pull request is in need of discussion, information, assessment by team members label Nov 17, 2024
@b1rger
Copy link
Contributor

b1rger commented Nov 18, 2024

1. Do we need this setting at all - it only applies to the "view", create and update views require permissions anyway.

I think we don't need this setting anymore.

I would like to suggest that we use a single setting to view entities that when set would allow the anonymous user read only access to both list and detail views. Does that make sense?

At least in one project I worked on, the users did not want to make all entities public, but only some of them - which would not work with only one setting. Back then I introduced the APIS_VIEW_PASSES_TEST setting, which allows the project to write a simple function to implement custom permission checks

@gythaogg
Copy link
Contributor Author

gythaogg commented Nov 18, 2024

At least in one project I worked on, the users did not want to make all entities public, but only some of them - which would not work with only one setting.

Oh yes, that makes sense - but what I meant here was having two different settings for list views (APIS_LIST_VIEWS_ALLOWED) and detail views (APIS_DETAIL_VIEWS_ALLOWED) when list views give access to everything in the entity detail view (by allowing the user to add columns and via relation list views).

I am guessing APIS_VIEW_PASSES_TEST would also modify the queryset in the list views for anonymous users and not display the entities that shouldn't be made public.

In TibSchol I am implementing custom managers for different access tiers, but I'll check if I can use the APIS_VIEW_PASSES_TEST instead. Thanks.

@b1rger
Copy link
Contributor

b1rger commented Nov 18, 2024

Oh yes, that makes sense - but what I meant here was having two different settings for list views (APIS_LIST_VIEWS_ALLOWED) and detail views (APIS_DETAIL_VIEWS_ALLOWED) when list views give access to everything in the entity detail view (by allowing the user to add columns and via relation list views).

Ah, yes, you're totally right! The split is a remnant of old apis and both those settings are simple booleans, so there is no filtering possible with them.

I am guessing APIS_DETAIL_VIEWS_ALLOWED would also modify the queryset in the list views for anonymous users if they didn't want some entities to be made public.

In TibSchol I am custom managers for different access tiers, but I'll check if I can use the APIS_VIEW_PASSES_TEST instead. thanks.

Yes, please do! I'm still not sure if its a good approach, because the function can get very complex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working (properly, as expected, at all) needs-attention This issue or pull request is in need of discussion, information, assessment by team members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants