Organizations: Test access from anonymous users and users outside the organization #8603

stsewd · 2021-10-19T22:55:41Z

Currently, I'm checking the global option
of ALLOW_PRIVATE_REPOS to give access to public views
to users outside the org.
I'm using the same patter we already have with projects about having a public queryset.

We could introduce a privacy level for the organization if we want.

… organization Currently I'm checking the global option of `ALLOW_PRIVATE_REPOS` to give access to public views to users outside the org. We could introduce a privacy level for the organization if we want.

readthedocs/organizations/urls/public.py

stale · 2022-03-02T10:34:20Z

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.

stale · 2022-05-02T08:23:28Z

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.

ericholscher · 2022-05-02T18:05:25Z

@stsewd is this still a valid PR? Seems useful, so we should review it if so.

stsewd · 2022-05-24T18:46:25Z

@stsewd is this still a valid PR? Seems useful, so we should review it if so.

It's, it should be ready now

ericholscher

This looks like a solid change. Also asking @humitos for a review to ✅ it, since I'm not 100% the implications here.

We could introduce a privacy level for the organization if we want.

Privacy levels are already quite confusing. We definitely don't want to add more of them unless we really need to.

ericholscher · 2022-05-31T21:19:34Z

readthedocs/organizations/managers.py

+        Return all teams the user has access to.
+
+        If ``ALLOW_PRIVATE_REPOS`` is `False`, all teams are public by default.
+        Otherwise, we return only the teams where the user is a member.


Huh.. do we want teams to always be public? I guess the example here is Orgs & Teams on .org once we enable them. I guess I'm fine defaulting them public, since everything is default public now.

I had the same thought as Eric here. Where, or in which situations, are we going to show these teams to anonymous users?

How do other platforms work here? In GitHub for example, I don't think you have access to see all the teams inside an organization. For example, in ours https://github.com/orgs/readthedocs/people, I don't think we have a way to list backend, frontend and advocate teams there. However, on PRs I can see those teams publicly: #929. When logged in, I can access to its members as well https://github.com/orgs/readthedocs/teams/backend

Another thing that GitHub has related to permissions on teams/organizations is "I want to show publicly that I'm member of this organization". Which adds another layer of complexity here.

I'm just describing how another platform works, I'm not saying we need to implement any of this, but just to keep in mind where are we going with this and how do we want it to work.

ericholscher · 2022-05-31T21:20:37Z

readthedocs/organizations/querysets.py

        return self.filter(
            Q(owners__in=[user]) | Q(teams__members__in=[user]),
        ).distinct()

    def for_admin_user(self, user):
+        """List all organizations where the user is an owner."""
+        if not user.is_authenticated:
+            return self.none()


I assume these are just optimizations?

humitos

I took a look at this and the changes look fine. However, I'm a little worried about changing the auth logic without understanding exactly why 😄 . Basically, why these changes are needed? What are their benefits? I'm not sure to follow the goal of the PR. Is it just a refactor?

We are now using .public instead of .for_user, but there is nothing extra returned on .public methods since we don't have privacy levels in organizations, so 🤷🏼 . I'm confused...

humitos · 2022-06-03T12:10:02Z

readthedocs/organizations/managers.py

+        Return all teams the user has access to.
+
+        If ``ALLOW_PRIVATE_REPOS`` is `False`, all teams are public by default.
+        Otherwise, we return only the teams where the user is a member.


I had the same thought as Eric here. Where, or in which situations, are we going to show these teams to anonymous users?

How do other platforms work here? In GitHub for example, I don't think you have access to see all the teams inside an organization. For example, in ours https://github.com/orgs/readthedocs/people, I don't think we have a way to list backend, frontend and advocate teams there. However, on PRs I can see those teams publicly: #929. When logged in, I can access to its members as well https://github.com/orgs/readthedocs/teams/backend

Another thing that GitHub has related to permissions on teams/organizations is "I want to show publicly that I'm member of this organization". Which adds another layer of complexity here.

I'm just describing how another platform works, I'm not saying we need to implement any of this, but just to keep in mind where are we going with this and how do we want it to work.

humitos · 2022-06-03T12:15:26Z

readthedocs/organizations/tests/test_privacy_urls.py

+    def test_private_urls(self):
+        from readthedocs.organizations.urls.private import urlpatterns
+
+        self._test_url(urlpatterns)


Pretty hard to follow all these tests with such magic. None of them seems to do anything different one from the other (just a setting and URLs). I assume that all this magic is inside this method :/

I assume this is fine, tho, to avoid repetition, but... :(

readthedocs/organizations/urls/public.py

humitos · 2022-06-03T12:24:09Z

readthedocs/organizations/querysets.py

+        """
+        Return all organizations the user has access to.
+
+        If ``ALLOW_PRIVATE_REPOS`` is `False`, all organizations are public by default.


I think we should probably rename this to RTD_CORPORATE or something. We have been using this setting to differentiate if we are on .org or .com lately, but it's not what the name suggests.

humitos · 2022-06-03T12:24:22Z

readthedocs/organizations/views/base.py

@@ -72,7 +72,7 @@ def get_organization_queryset(self):
        """
        if self.admin_only:
            return Organization.objects.for_admin_user(user=self.request.user)
-        return Organization.objects.for_user(user=self.request.user)
+        return Organization.objects.public(user=self.request.user)


We don't have the concept of privacy level in Organizations, so I'm not sure these changes make a ton of sense.

humitos · 2022-06-03T12:25:27Z

readthedocs/organizations/views/base.py

@@ -109,7 +109,11 @@ def get_team_queryset(self):
        This will either be team the user is a member of, or teams where the
        user is an owner of the organization.
        """
-        return Team.objects.member(self.request.user).filter(
+        if self.admin_only:


Where are we setting this self.admin_only?

ericholscher · 2022-07-19T21:04:28Z

@stsewd would be good to address the review comments here and get this merged, if possible.

agjohnson · 2022-08-31T17:27:15Z

Ping @stsewd ☝️

We can also push this out some sprints or back on the backlog if it makes sense to return to this later instead.

stsewd · 2022-09-01T16:28:14Z

We can also push this out some sprints or back on the backlog if it makes sense to return to this later instead.

I'm fine with that, this isn't a priority for me at the moment, I do want to finish this some time :)

ericholscher · 2022-09-01T17:02:31Z

Pushed it back a few sprints 👍

humitos · 2023-03-07T10:22:21Z

I'm putting this back into Q2, so we come back to this PR soon.

agjohnson · 2024-02-29T19:32:26Z

Doing some pruning here. How likely are we to get back to this PR? Is it worth prioritizing in the near future, or should we discuss retiring this work?

stsewd · 2024-02-29T19:36:30Z

Doing some pruning here. How likely are we to get back to this PR? Is it worth prioritizing in the near future, or should we discuss retiring this work?

I'm fine closing this, it may be helpful when we decide to introduce organizations in community.

stsewd force-pushed the test-orgs-views-outside-org branch 2 times, most recently from f81720d to 2566107 Compare October 20, 2021 00:13

Organizations: Test access from anonymous users and users outside the…

3409571

… organization Currently I'm checking the global option of `ALLOW_PRIVATE_REPOS` to give access to public views to users outside the org. We could introduce a privacy level for the organization if we want.

stsewd force-pushed the test-orgs-views-outside-org branch from 2566107 to 3409571 Compare October 20, 2021 00:32

stsewd marked this pull request as ready for review October 20, 2021 00:52

stsewd requested a review from a team as a code owner October 20, 2021 00:53

stsewd commented Oct 20, 2021

View reviewed changes

readthedocs/organizations/urls/public.py Show resolved Hide resolved

stale bot added the Status: stale Issue will be considered inactive soon label Mar 2, 2022

Merge branch 'master' into test-orgs-views-outside-org

01398c0

stale bot removed the Status: stale Issue will be considered inactive soon label Mar 2, 2022

stale bot added the Status: stale Issue will be considered inactive soon label May 2, 2022

stale bot removed the Status: stale Issue will be considered inactive soon label May 2, 2022

stsewd added 5 commits May 23, 2022 19:12

Merge branch 'main' into test-orgs-views-outside-org

5a8b8f1

Fix tests

1847e48

Black

d60f7bb

Fix tests

0187116

Merge branch 'main' into test-orgs-views-outside-org

d330122

ericholscher requested review from humitos and removed request for a team May 31, 2022 21:16

ericholscher reviewed May 31, 2022

View reviewed changes

humitos reviewed Jun 3, 2022

View reviewed changes

agjohnson assigned stsewd Aug 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Organizations: Test access from anonymous users and users outside the organization #8603

Organizations: Test access from anonymous users and users outside the organization #8603

stsewd commented Oct 19, 2021 •

edited

Loading

stale bot commented Mar 2, 2022

stale bot commented May 2, 2022

ericholscher commented May 2, 2022

stsewd commented May 24, 2022

ericholscher left a comment

ericholscher May 31, 2022

humitos Jun 3, 2022

ericholscher May 31, 2022

humitos left a comment

humitos Jun 3, 2022

humitos Jun 3, 2022

humitos Jun 3, 2022

humitos Jun 3, 2022

humitos Jun 3, 2022

ericholscher commented Jul 19, 2022

agjohnson commented Aug 31, 2022

stsewd commented Sep 1, 2022

ericholscher commented Sep 1, 2022

humitos commented Mar 7, 2023

agjohnson commented Feb 29, 2024

stsewd commented Feb 29, 2024

Organizations: Test access from anonymous users and users outside the organization #8603

Are you sure you want to change the base?

Organizations: Test access from anonymous users and users outside the organization #8603

Conversation

stsewd commented Oct 19, 2021 • edited Loading

stale bot commented Mar 2, 2022

stale bot commented May 2, 2022

ericholscher commented May 2, 2022

stsewd commented May 24, 2022

ericholscher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humitos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericholscher commented Jul 19, 2022

agjohnson commented Aug 31, 2022

stsewd commented Sep 1, 2022

ericholscher commented Sep 1, 2022

humitos commented Mar 7, 2023

agjohnson commented Feb 29, 2024

stsewd commented Feb 29, 2024

stsewd commented Oct 19, 2021 •

edited

Loading