From be2ff1558843d6081d17529875f960095abb58fe Mon Sep 17 00:00:00 2001 From: Yann Dago Date: Mon, 16 Dec 2024 11:59:06 -0500 Subject: [PATCH] First attempt at having confidential features --- api/api_specs.py | 1 + api/converters.py | 2 ++ api/features_api.py | 3 +++ client-src/elements/form-definition.ts | 2 ++ client-src/elements/form-field-specs.ts | 8 ++++++ framework/permissions.py | 34 ++++++++++++++++++++++--- internals/core_models.py | 1 + internals/data_types.py | 1 + internals/feature_helpers.py | 32 ++++++++++++++++++++--- internals/search.py | 2 +- internals/search_queries.py | 1 + scripts/seed_datastore.py | 1 + 12 files changed, 81 insertions(+), 7 deletions(-) diff --git a/api/api_specs.py b/api/api_specs.py index ba27552d0892..d1964339cff6 100644 --- a/api/api_specs.py +++ b/api/api_specs.py @@ -33,6 +33,7 @@ ('enterprise_impact', 'int'), ('shipping_year', 'int'), ('breaking_change', 'bool'), + ('confidential', 'bool'), ('bug_url', 'link'), ('category', 'int'), ('cc_emails', 'emails'), diff --git a/api/converters.py b/api/converters.py index 645d6b63b540..63a3e43b7c11 100644 --- a/api/converters.py +++ b/api/converters.py @@ -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, @@ -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': { diff --git a/api/features_api.py b/api/features_api.py index bca42add8b3b..1535b9fc1581 100644 --- a/api/features_api.py +++ b/api/features_api.py @@ -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): diff --git a/client-src/elements/form-definition.ts b/client-src/elements/form-definition.ts index 9c96cefc4280..376df2c9b5f0 100644 --- a/client-src/elements/form-definition.ts +++ b/client-src/elements/form-definition.ts @@ -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. @@ -233,6 +234,7 @@ export const FLAT_ENTERPRISE_METADATA_FIELDS: MetadataFields = { 'editors', 'enterprise_feature_categories', 'enterprise_impact', + 'confidential', 'first_enterprise_notification_milestone', 'screenshot_links', ], diff --git a/client-src/elements/form-field-specs.ts b/client-src/elements/form-field-specs.ts index 38f82edaf59b..2119c7817907 100644 --- a/client-src/elements/form-field-specs.ts +++ b/client-src/elements/form-field-specs.ts @@ -2125,6 +2125,14 @@ export const ALL_FIELDS: Record = { 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, diff --git a/framework/permissions.py b/framework/permissions.py index 8d07688b64b9..97487bd04db0 100644 --- a/framework/permissions.py +++ b/framework/permissions.py @@ -44,12 +44,40 @@ 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 + 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: diff --git a/internals/core_models.py b/internals/core_models.py index 43a667c395fd..ef5027702c0e 100644 --- a/internals/core_models.py +++ b/internals/core_models.py @@ -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 diff --git a/internals/data_types.py b/internals/data_types.py index 9bba69419e4a..8e4caa253333 100644 --- a/internals/data_types.py +++ b/internals/data_types.py @@ -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 diff --git a/internals/feature_helpers.py b/internals/feature_helpers.py index 3840f08a93e6..b11583bbec9b 100644 --- a/internals/feature_helpers.py +++ b/internals/feature_helpers.py @@ -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 @@ -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( @@ -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 @@ -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 @@ -330,7 +355,7 @@ def get_all(limit: Optional[int]=None, rediscache.set(KEY, feature_list) - return feature_list + return filter_confidential(feature_list) def get_feature_names_by_ids(feature_ids: list[int], update_cache: bool=True) -> list[dict[str, Any]]: @@ -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]]: @@ -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, @@ -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) diff --git a/internals/search.py b/internals/search.py index 1961274a38d7..b74ba8fc45fa 100644 --- a/internals/search.py +++ b/internals/search.py @@ -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 diff --git a/internals/search_queries.py b/internals/search_queries.py index d45d00f2ed2c..f86d7a70523d 100644 --- a/internals/search_queries.py +++ b/internals/search_queries.py @@ -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, diff --git a/scripts/seed_datastore.py b/scripts/seed_datastore.py index f61b6b263a02..93d85336db1e 100755 --- a/scripts/seed_datastore.py +++ b/scripts/seed_datastore.py @@ -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']