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

fix counts returned to non-staff users in apiv2 #4074

Merged
merged 2 commits into from
Mar 17, 2021

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented Mar 16, 2021

fix #4069

For non-staff users, we have queries where we filter objects based on the authorized_users property of the product or product type:

    def get_queryset(self):
        if not self.request.user.is_staff:
            return self.queryset.filter(
                Q(test__engagement__product__authorized_users__in=[self.request.user]) |
                Q(test__engagement__product__prod_type__authorized_users__in=[self.request.user])
            )
        else:
            return self.queryset

This works OK, but will join a couple of tables and could lead to objects being in the result multiple times. This PR adds .distinct() to resolve this.

There's also one place where we had 2 different queries for the list of findings and the count of that list.

@valentijnscholten
Copy link
Member Author

@StefanFl I didn't change the authorization v2 code, but there we might have a chance of duplicate items in the result as well.

@StefanFl
Copy link
Member

If it's save to use distinct() without other side effects then it would make sense to put it in there.

@valentijnscholten valentijnscholten changed the title fix counts returned in apiv2 fix counts returned to non-staff users in apiv2 Mar 16, 2021
@valentijnscholten
Copy link
Member Author

If it's save to use distinct() without other side effects then it would make sense to put it in there.

it's safe but needed probably because a join creates all possible combinations. side effect is that distinct() might make it slightly slower.

Copy link
Collaborator

@madchap madchap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Let's try that out.

@damiencarol
Copy link
Contributor

good catch!

@damiencarol damiencarol merged commit 3f728b5 into DefectDojo:dev Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants