Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Design doc for privacy levels #6194
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
Design doc for privacy levels #6194
Changes from 7 commits
7935167
f40b6dc
caed340
e08654e
7564b10
d5f98a0
0bed2fc
779989d
d6d477a
cca699f
b82b3d3
3fe301f
5198acd
b9cc130
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm on the fence with this. I think it makes sense, but could be confusing or hard to communicate it clearly. I don't have a better proposal, though.
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.
I have the same problem with this. If feels like we're overloading the meaning of the default version privacy level and we're hiding side effects into this version privacy level. Project privacy level actually addresses this quite nicely currently.
Project.privacy_level
for determining a version's privacy level is definitely on the way out, but perhaps we're talking about the need for a comparable setting that has a more narrow use case? That is, a new setting that doesn't affect version privacy level? Focusing on removing the setting entirely is maybe the wrong point, the main problem around project privacy level is that it made version privacy level confusing. We still might need some version ofProject.privacy_level
.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.
robots and sitemap look related, we can have something like
Enable search engine optimization
. But for 404 it kind of makes sense to respect the privacy level of the default version, since we are trying to serve the 404 page of the default version :)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.
The case for 404 maybe makes sense, but it is maybe odd to require auth for a 404 page. Also, currently, I believe our 404 page solution only addresses a 404 error, but we'd ideally have a solution for 404, 403, 4xx, and 5xx pages -- all which would follow similar logic. The 403 page is of most concern here, as we'd use it to describe the default version being private (in your proposal about 404 and default version privacy).
However, we'd have to rethink how we're doing 404 pages. Currently, our extension renders the 404 page with the side bar navigation intact, which would leak information if we used the same pattern for a 403 page. So I think we need to consider our eventual use cases as well. In the case we make 4xx and 5xx pages more of a product feature, 4xx and 5xx pages should probably be under no auth. But we'd have to have this configurable because we've already assumed a different direction with 404 pages so far.
For robots.txt and sitemap.xml, we should probably consider why private versions are included in these at all. It seems that these aren't useful, as search engines will never pass authorization, and these endpoints can simply be unauthed if there is no private version data.
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.
I think we should serve
robots.txt
andsitemap.xml
should be shown if the project has any public versions. Since that isn't exposing any additional information on the domain. If all the projects are private, we should never serve anything from that domain other than 404's I think.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.
I agree with Eric regarding robots and sitemap.
For the 4xx and 5xx, eventually we should do what Anthony mentioned: think about our use cases and adapt our extension to manage those cases as well.
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.
Can we instead limit the data going into both robots.txt and sitemap to just public versions? That is, if there are no public versions, both are mostly empty?
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.
That sounds like a good idea, I'll try to update this soon and see if there is anything missing.
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.
How does somebody set
hidden=True
for versions? Would that just be in the usual version form? Does that make sense on .org? Is the idea perhaps for some beta version that is fine for people to access but they don't want it in search results?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.
@davidfischer we talked about new versions' states some time ago at #5321 . Adding the
hidden
field follow the direction that we talked there.We didn't talk about the UI yet, though, but I imagine just a new checkbox field similar as the
Active
one.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.
Yes, it would be just another field on the form. And it makes sense on .org, below are some uses cases that this attribute will replace (protected versions)
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.
Is the plan that
RTD_ALLOW_PRIVACY_LEVELS=False
would completely remove "privacy level" from all forms everywhere? Would the default beRTD_ALLOW_PRIVACY_LEVELS=False
?For example, "privacy level" would disappear from this form, correct?

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.
Yes, it will disappear from all forms and ui elements, except for api v2, but there will only show
privacy_levet: public
and can't be changed.Yeah, I think that is a nice default.. or maybe not since we'll be hiding things by default
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.
We talked about a few of these above
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.
So, can we safely assume that anyone that is using a protected version will be able to use
hidden
instead then? That is, can we make this a data migration?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.
Yes, it's on the migration section. Also, since we hid the protected level, not sure how users will relate the options... Probably a blog post about the change? And/or a help text like "Are you looking for protected? Go here instead"
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 we write "data migration" as django migration files, we need to be very carefully to consider if that data migration has to be applied in corporate or not. There are some scenarios where we don't want the same behavior (current private versions, for example)
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.
What are we planning on doing with versions that are public project private version? What is the migration path when we remove this version level from community?
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.
I think we are safe to just migrate those to public, and maybe setting the
hidden
option? Since that's the closer option toprivate
in the community site. Let me know what you thinkThere 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.
Using
hidden
kind of makes sense to me, but it's not 100% accurate. We could be leaking data (not in community since the repository is public anyways, but probably in commercial).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.
Anyways, in commercial, we can keep them
private
:)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.
What is the migration path here? Change all protected projects to public projects with hidden versions?
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.
Yeah, I'm making more explicitly what migration to do here.
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.
Actually no, sorry. The migration path for the versions of protected and private projects follow the same rules as the public project. Since at the end what users were hidden is the docs built (versions).
The privacy level for project only controls if the dashboard should be public or not and if the project was listed or not.
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.
What is the migration path here? These projects likely never worked, but we wouldn't want to do anything that would look like a data leak. Making a project go from private and broken to public and working could be really bad -- especially in the case of an old project that was meant to be completely private. At least projects that should be private (ie - commercial projects on community) are broken right now and we aren't leaking anything.
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.
For all versions of a private project follow the same rules as a public project. I think that maybe all private versions we could email users telling them that we are about to make their versions public.
From the project itself we could only be leaking information in the dashboard and on the build page.
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.
I'm not really concern about a Private Project on community since the VCS repository has to be public anyways. So, there is no "data leak" but "serving broken docs" instead.
If we care too much about these cases, we could probably have an intermediate value for the migration only for these cases (
dead
state :) ) where the user can change to a different status but nobody can select this state.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.
Same feedback as above, in case the use case for protected is any different here. If users were trying to accomplish anything else with protected versions, it would help guide the decision on what to do when migrating.
hidden=True
seems mostly safe.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 there are even any of these projects, what should we do with them on migration?
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.
I'd say that the footer should show these links if you are logged in and you are part of a Team with Read access to the project.
Anonymous users should not see these links.
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.
Links to the dashboard should only be visible for admin users, since they are the only ones that can make changes to the project.
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.
Users with Read access can see the dashboard, so they should be able to see these links as well. Even, if they can not edit anything --but can see the Build outputs, for example.
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 like a pretty clear case for
hidden=True
, I'm having trouble considering any other use cases. Do we feel confident about this migration?