Skip to content

Commit

Permalink
First attempt at having confidential features
Browse files Browse the repository at this point in the history
  • Loading branch information
yanndago committed Dec 16, 2024
1 parent 41d25b0 commit c80f832
Show file tree
Hide file tree
Showing 12 changed files with 81 additions and 6 deletions.
1 change: 1 addition & 0 deletions api/api_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
('enterprise_impact', 'int'),
('shipping_year', 'int'),
('breaking_change', 'bool'),
('confidential', 'bool'),
('bug_url', 'link'),
('category', 'int'),
('cc_emails', 'emails'),
Expand Down
2 changes: 2 additions & 0 deletions api/converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ def feature_entry_to_json_verbose(
'first_enterprise_notification_milestone': fe.first_enterprise_notification_milestone,
'enterprise_impact': fe.enterprise_impact,
'breaking_change': fe.breaking_change,
'confidential': fe.confidential,
'shipping_year': fe.shipping_year,
'flag_name': fe.flag_name,
'finch_name': fe.finch_name,
Expand Down Expand Up @@ -532,6 +533,7 @@ def feature_entry_to_json_basic(fe: FeatureEntry,
'unlisted': fe.unlisted,
'enterprise_impact': fe.enterprise_impact,
'breaking_change': fe.breaking_change,
'confidential': fe.confidential,
'first_enterprise_notification_milestone': fe.first_enterprise_notification_milestone,
'blink_components': fe.blink_components or [],
'resources': {
Expand Down
3 changes: 3 additions & 0 deletions api/features_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ def get_one_feature(self, feature_id: int) -> VerboseFeatureDict:
user = users.get_current_user()
if feature.deleted and not permissions.can_edit_feature(user, feature_id):
self.abort(404, msg='Feature %r not found' % feature_id)
if not permissions.can_view_feature(user, feature):
self.abort(404, msg='Feature %r not found' % feature_id)

return converters.feature_entry_to_json_verbose(feature)

def do_search(self):
Expand Down
2 changes: 2 additions & 0 deletions client-src/elements/form-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ export const ENTERPRISE_NEW_FEATURE_FORM_FIELDS = [
'enterprise_feature_categories',
'first_enterprise_notification_milestone',
'enterprise_impact',
'confidential',
];

// The fields that are available to every feature.
Expand Down Expand Up @@ -233,6 +234,7 @@ export const FLAT_ENTERPRISE_METADATA_FIELDS: MetadataFields = {
'editors',
'enterprise_feature_categories',
'enterprise_impact',
'confidential',
'first_enterprise_notification_milestone',
'screenshot_links',
],
Expand Down
8 changes: 8 additions & 0 deletions client-src/elements/form-field-specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2125,6 +2125,14 @@ export const ALL_FIELDS: Record<string, Field> = {
take action to continue using some existing functionaity.`,
},

confidential: {
type: 'checkbox',
label: 'Confidential',
initial: false,
help_text: html `This is a confidential feature that should not be publicly
visible.`,
},

intent_cc_emails: {
type: 'input',
attrs: MULTI_EMAIL_FIELD_ATTRS,
Expand Down
35 changes: 32 additions & 3 deletions framework/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,41 @@ def is_google_or_chromium_account(user: User) -> bool:
return user.email().endswith(('@chromium.org', '@google.com'))
return False

def can_view_feature(unused_user, unused_feature) -> bool:
def can_view_feature_with_id(user, feature_id) -> bool:
"""Return True if the user is allowed to view the given feature."""
# Note, for now there are no private features, only unlisted ones.
return True
if not feature_id:
return False

return can_view_feature(user, FeatureEntry.get_by_id(feature_id))

def can_view_feature(user, feature) -> bool:
"""Return True if the user is allowed to view the given feature."""
if not feature:
return False

return not feature.confidential or can_view_confidential_feature(user, feature)

def can_view_confidential_feature(user, feature):
''' Check if the user is an owner, editor, spec mentor, or creator
for this feature or has a google.com or chromium.org account.
If yes, they feature can be viewed, otherwise they cannot view
confidential features.'''
if not user:
return False

if is_google_or_chromium_account(user) or can_admin_site(user):
return True

email = user.email()
if (
email in feature.owner_emails or
email in feature.editor_emails or
(feature.spec_mentor_emails and email in feature.spec_mentor_emails) or
email == feature.creator_email):
return True

return False

def can_create_feature(user: User) -> bool:
"""Return True if the user is allowed to create features."""
if not user:
Expand Down
1 change: 1 addition & 0 deletions internals/core_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class FeatureEntry(ndb.Model): # Copy from Feature
first_enterprise_notification_milestone = ndb.IntegerProperty()
enterprise_impact = ndb.IntegerProperty(default=ENTERPRISE_IMPACT_NONE)
breaking_change = ndb.BooleanProperty(default=False)
confidential = ndb.BooleanProperty(default=False)
shipping_year = ndb.IntegerProperty()

# Implementation in Chrome
Expand Down
1 change: 1 addition & 0 deletions internals/data_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ class VerboseFeatureDict(TypedDict):
screenshot_links: list[str]
first_enterprise_notification_milestone: int | None
breaking_change: bool
confidential: bool
enterprise_impact: int
shipping_year: int | None

Expand Down
30 changes: 28 additions & 2 deletions internals/feature_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from api import converters
from framework import rediscache
from framework import users
from framework import permissions
from internals import stage_helpers
from internals.core_enums import *
from internals.core_models import FeatureEntry, Stage
Expand All @@ -45,6 +46,27 @@ def filter_unlisted(feature_list: list[dict]) -> list[dict]:
return listed_features


def filter_confidential_ids(feature_ids: list[int]) -> list[dict]:
"""Filters a feature id list to return only the ids of features the user should see."""
user = users.get_current_user()
visible_ids = []
for id in feature_ids:
if permissions.can_view_feature_with_id(user, id):
visible_ids.append(id)

return visible_ids

def filter_confidential(feature_list: list[dict]) -> list[dict]:
"""Filters a feature list to display only features the user should see."""
user = users.get_current_user()
visible_features = []
for f in feature_list:
if permissions.can_view_feature_with_id(user, f['id']):
visible_features.append(f)

return visible_features


def get_entries_by_id_async(ids) -> Future | None:
if ids:
q = FeatureEntry.query(FeatureEntry.key.IN(
Expand Down Expand Up @@ -90,6 +112,7 @@ def get_features_in_release_notes(milestone: int):
f['feature_type_int'] == FEATURE_TYPE_ENTERPRISE_ID) and
(f['first_enterprise_notification_milestone'] == None or
f['first_enterprise_notification_milestone'] <= milestone)]
features = [f for f in filter_confidential(features)]

rediscache.set(cache_key, features)
return features
Expand Down Expand Up @@ -279,6 +302,8 @@ def get_in_milestone(milestone: int,
if not show_unlisted:
features_by_type[shipping_type] = filter_unlisted(
features_by_type[shipping_type])
features_by_type[shipping_type] = filter_confidential(
features_by_type[shipping_type])

return features_by_type

Expand Down Expand Up @@ -376,7 +401,7 @@ def get_feature_names_by_ids(feature_ids: list[int],
result_list = [
result_dict[feature_id] for feature_id in feature_ids
if feature_id in result_dict]
return result_list
return filter_confidential(result_list)

def get_by_ids(feature_ids: list[int],
update_cache: bool=True) -> list[dict[str, Any]]:
Expand Down Expand Up @@ -436,7 +461,7 @@ def get_by_ids(feature_ids: list[int],
result_list = [
result_dict[feature_id] for feature_id in feature_ids
if feature_id in result_dict]
return result_list
return filter_confidential(result_list)


def get_features_by_impl_status(limit: int | None=None, update_cache: bool=False,
Expand Down Expand Up @@ -483,6 +508,7 @@ def get_features_by_impl_status(limit: int | None=None, update_cache: bool=False
section[0]['first_of_section'] = True
if not show_unlisted:
section = filter_unlisted(section)
section = filter_confidential(section)
feature_list.extend(section)

rediscache.set(cache_key, feature_list)
Expand Down
2 changes: 1 addition & 1 deletion internals/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ def process_query(
result_id_set.intersection_update(permission_ids)
logging.info('got %r result IDs with permissions', len(result_id_set))

result_id_list = list(result_id_set)
result_id_list = feature_helpers.filter_confidential_ids(list(result_id_set))
total_count = len(result_id_list)

# 4. Finish getting the total sort order. Then, sort the IDs according
Expand Down
1 change: 1 addition & 0 deletions internals/search_queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def sorted_by_review_date(descending: bool) -> Future:
'browsers.chrome.bug': FeatureEntry.bug_url,
'launch_bug_url': FeatureEntry.launch_bug_url,
'breaking_change': FeatureEntry.breaking_change,
'confidential': FeatureEntry.confidential,
'enterprise_impact': FeatureEntry.enterprise_impact,
'shipping_year': FeatureEntry.shipping_year,

Expand Down
1 change: 1 addition & 0 deletions scripts/seed_datastore.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ def add_features(server: str, after: datetime, detailsAfter: datetime):
fe.summary = f['summary']
fe.category = MISC
fe.breaking_change = f['breaking_change']
fe.confidential = f['confidential']
fe.first_enterprise_notification_milestone = f[
'first_enterprise_notification_milestone']
fe.blink_components = f['blink_components']
Expand Down

0 comments on commit c80f832

Please sign in to comment.