-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
API V3: Don't allow leaking teams through expandable fields #11471
Conversation
readthedocs/api/v3/views.py
Outdated
ListModelMixin, | ||
GenericViewSet, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was allowing /organizations/projects/{project}
which we don't want, since all project endpoints are under /projects/{project}
.
if self.has_admin_permission(user, organization): | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the owner didn't belong to any admin teams, we were not granting permissions :_
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just chiming in here on the frontend/docs pieces, these look good. I'm leaving review on the API changes for @humitos, he has most context on the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable to me 👍
Does it need any updates from the permissions changes (#11485)
teams
is removed from organizations and projectsteams
). There is no need for this anymore, so they were merged.Ref https://github.com/readthedocs/readthedocs-corporate/issues/1736
📚 Documentation previews 📚
docs
): https://docs--11471.org.readthedocs.build/en/11471/dev
): https://dev--11471.org.readthedocs.build/en/11471/