Skip to content

Commit c55dfa9

Browse files
authored
Display data collection permissions in add-on detail page (#13636)
* Display data collection permissions in add-on detail page * Bump subheader to h4, leave it with the default color * Swap tests to be more realistic
1 parent f656b8d commit c55dfa9

File tree

6 files changed

+294
-38
lines changed

6 files changed

+294
-38
lines changed

src/amo/components/PermissionsCard/index.js

Lines changed: 61 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,49 @@ export class PermissionsCardBase extends React.Component<InternalProps> {
3737
});
3838

3939
if (
40-
!addonPermissions.optional.length &&
41-
!addonPermissions.required.length
40+
!addonPermissions.permissions.optional.length &&
41+
!addonPermissions.permissions.required.length &&
42+
!addonPermissions.data_collection_permissions.optional.length &&
43+
!addonPermissions.data_collection_permissions.required.length
4244
) {
4345
return null;
4446
}
4547

46-
const optionalContent = permissionUtils.formatPermissions(
47-
addonPermissions.optional,
48+
const optionalPermissionsContent = permissionUtils.formatPermissions(
49+
addonPermissions.permissions.optional,
4850
);
49-
const requiredContent = permissionUtils.formatPermissions(
50-
addonPermissions.required,
51+
const requiredPermissionsContent = permissionUtils.formatPermissions(
52+
addonPermissions.permissions.required,
5153
);
54+
const optionalDataCollectionPermissionsContent =
55+
permissionUtils.formatPermissions(
56+
addonPermissions.data_collection_permissions.optional,
57+
);
58+
const requiredDataCollectionPermissionsContent =
59+
permissionUtils.formatPermissions(
60+
addonPermissions.data_collection_permissions.required,
61+
);
5262

53-
if (!optionalContent.length && !requiredContent.length) {
63+
if (
64+
!optionalPermissionsContent.length &&
65+
!requiredPermissionsContent.length &&
66+
!optionalDataCollectionPermissionsContent.length &&
67+
!requiredDataCollectionPermissionsContent.length
68+
) {
5469
return null;
5570
}
5671

72+
// 'none' is a special string that can only appear alone in required data
73+
// collection permissions, and the header changes when it appears.
74+
const requiredDataCollectionPermissionsHeader =
75+
addonPermissions.data_collection_permissions.required.length === 1 &&
76+
addonPermissions.data_collection_permissions.required[0] === 'none'
77+
? i18n.gettext('Data collection:')
78+
: i18n.gettext('Required data collection, according to the developer:');
79+
5780
const header = (
5881
<div className="PermissionsCard-header">
59-
{i18n.gettext('Permissions')}
82+
{i18n.gettext('Permissions and data')}
6083
<Link
6184
className="PermissionsCard-learn-more"
6285
href="https://support.mozilla.org/kb/permission-request-messages-firefox-extensions"
@@ -78,23 +101,45 @@ export class PermissionsCardBase extends React.Component<InternalProps> {
78101
id="AddonDescription-permissions-card"
79102
maxHeight={300}
80103
>
81-
{requiredContent.length ? (
104+
{requiredPermissionsContent.length ? (
82105
<>
83-
<p className="PermissionsCard-subhead--required">
106+
<h4 className="PermissionsCard-subhead--required">
84107
{i18n.gettext('Required permissions:')}
85-
</p>
108+
</h4>
86109
<ul className="PermissionsCard-list--required">
87-
{requiredContent}
110+
{requiredPermissionsContent}
88111
</ul>
89112
</>
90113
) : null}
91-
{optionalContent.length ? (
114+
{optionalPermissionsContent.length ? (
92115
<>
93-
<p className="PermissionsCard-subhead--optional">
116+
<h4 className="PermissionsCard-subhead--optional">
94117
{i18n.gettext('Optional permissions:')}
95-
</p>
118+
</h4>
119+
<ul className="PermissionsCard-list--optional">
120+
{optionalPermissionsContent}
121+
</ul>
122+
</>
123+
) : null}
124+
{requiredDataCollectionPermissionsContent.length ? (
125+
<>
126+
<h4 className="PermissionsCard-subhead--required">
127+
{requiredDataCollectionPermissionsHeader}
128+
</h4>
129+
<ul className="PermissionsCard-list--required">
130+
{requiredDataCollectionPermissionsContent}
131+
</ul>
132+
</>
133+
) : null}
134+
{optionalDataCollectionPermissionsContent.length ? (
135+
<>
136+
<h4 className="PermissionsCard-subhead--optional">
137+
{i18n.gettext(
138+
'Optional data collection, according to the developer:',
139+
)}
140+
</h4>
96141
<ul className="PermissionsCard-list--optional">
97-
{optionalContent}
142+
{optionalDataCollectionPermissionsContent}
98143
</ul>
99144
</>
100145
) : null}

src/amo/components/PermissionsCard/permissions.js

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@ export class PermissionUtils {
1717

1818
permissionStrings: Object;
1919

20+
dataCollectionPermissionStrings: Object;
21+
2022
constructor(i18n: I18nType) {
2123
this.i18n = i18n;
2224
// These should be kept in sync with Firefox's strings for webextension
2325
// permissions which can be found in:
24-
// https://searchfox.org/mozilla-central/rev/b0b003e992b199fd8e13999bd5d06d06c84a3fd2/toolkit/components/extensions/ExtensionPermissionMessages.sys.mjs#32
25-
// https://searchfox.org/mozilla-central/rev/b0b003e992b199fd8e13999bd5d06d06c84a3fd2/toolkit/locales/en-US/toolkit/global/extensionPermissions.ftl
26+
// https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/toolkit/components/extensions/ExtensionPermissionMessages.sys.mjs#32
27+
// https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/toolkit/locales/en-US/toolkit/global/extensionPermissions.ftl
2628
this.permissionStrings = {
2729
bookmarks: i18n.gettext('Read and modify bookmarks'),
2830
browserSettings: i18n.gettext('Read and modify browser settings'),
@@ -59,28 +61,67 @@ export class PermissionUtils {
5961
topSites: i18n.gettext('Access browsing history'),
6062
webNavigation: i18n.gettext('Access browser activity during navigation'),
6163
};
64+
65+
this.dataCollectionPermissionStrings = {
66+
authenticationInfo: i18n.gettext('Authentication information'),
67+
bookmarksInfo: i18n.gettext('Bookmarks'),
68+
browsingActivity: i18n.gettext('Browsing activity'),
69+
financialAndPaymentInfo: i18n.gettext(
70+
'Financial and payment information',
71+
),
72+
healthInfo: i18n.gettext('Health information'),
73+
locationInfo: i18n.gettext('Location'),
74+
personalCommunications: i18n.gettext('Personal communications'),
75+
personallyIdentifyingInfo: i18n.gettext(
76+
'Personally identifying information',
77+
),
78+
searchTerms: i18n.gettext('Search terms'),
79+
technicalAndInteraction: i18n.gettext('Technical and interaction data'),
80+
websiteActivity: i18n.gettext('Website activity'),
81+
websiteContent: i18n.gettext('Website content'),
82+
83+
// 'none' is a special string that can only appear alone in required data
84+
// collection permissions, and the header changes when it appears.
85+
none: i18n.gettext(
86+
"The developer says this extension doesn't require data collection.",
87+
),
88+
};
6289
}
6390

6491
// Get lists of optional and required permissions from the correct platform file.
6592
getCurrentPermissions({ file }: GetCurrentPermissionsParams): {
66-
optional: Array<string>,
67-
required: Array<string>,
93+
permissions: {
94+
optional: Array<string>,
95+
required: Array<string>,
96+
},
97+
data_collection_permissions: {
98+
optional: Array<string>,
99+
required: Array<string>,
100+
},
68101
} {
69102
const permissions = {
70103
optional: [],
71104
required: [],
72105
};
106+
const data_collection_permissions = {
107+
optional: [],
108+
required: [],
109+
};
73110

74111
if (!file) {
75112
log.debug('getCurrentPermissions() called with no file');
76113

77-
return permissions;
114+
return { permissions, data_collection_permissions };
78115
}
79116

80117
const hostPermissions = file.host_permissions || [];
81118
permissions.optional = [...file.optional_permissions, ...hostPermissions];
82119
permissions.required = file.permissions;
83-
return permissions;
120+
data_collection_permissions.optional =
121+
file.optional_data_collection_permissions || [];
122+
data_collection_permissions.required =
123+
file.data_collection_permissions || [];
124+
return { permissions, data_collection_permissions };
84125
}
85126

86127
// Classify a permission as a host permission or a regular permission.
@@ -128,12 +169,12 @@ export class PermissionUtils {
128169
continue;
129170
}
130171
// Only output a permission if we have a string defined for it.
131-
if (this.permissionStrings[permission]) {
172+
const permissionString =
173+
this.permissionStrings[permission] ||
174+
this.dataCollectionPermissionStrings[permission];
175+
if (permissionString) {
132176
permissionsToDisplay.push(
133-
<Permission
134-
description={this.permissionStrings[permission]}
135-
key={permission}
136-
/>,
177+
<Permission description={permissionString} key={permission} />,
137178
);
138179
}
139180
}

src/amo/components/PermissionsCard/styles.scss

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
.PermissionsCard-subhead--optional,
1616
.PermissionsCard-subhead--required {
17-
color: $grey-50;
1817
margin: 24px 0 0;
1918

2019
&:first-child {

src/amo/reducers/versions.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@ export type VersionIdType = number;
2828

2929
export type AddonFileType = {|
3030
created: string,
31+
data_collection_permissions?: Array<string>,
3132
hash: string,
3233
id: number,
3334
is_mozilla_signed_extension: boolean,
3435
host_permissions: Array<string>,
36+
optional_data_collection_permissions?: Array<string>,
3537
optional_permissions: Array<string>,
3638
permissions: Array<string>,
3739
platform: 'all' | 'android' | 'mac' | 'linux' | 'windows',

tests/unit/amo/components/TestPermissionUtils.js

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,14 @@ describe(__filename, () => {
2525
};
2626

2727
expect(_getCurrentPermissions({ file })).toEqual({
28-
optional: optionalPermissions,
29-
required: permissions,
28+
permissions: {
29+
optional: optionalPermissions,
30+
required: permissions,
31+
},
32+
data_collection_permissions: {
33+
optional: [],
34+
required: [],
35+
},
3036
});
3137
});
3238

@@ -42,16 +48,51 @@ describe(__filename, () => {
4248
};
4349

4450
expect(_getCurrentPermissions({ file })).toEqual({
45-
optional: [...optionalPermissions, ...hostPermissions],
46-
required: permissions,
51+
permissions: {
52+
optional: [...optionalPermissions, ...hostPermissions],
53+
required: permissions,
54+
},
55+
data_collection_permissions: {
56+
optional: [],
57+
required: [],
58+
},
59+
});
60+
});
61+
62+
it('returns data_collection_permissions from file', () => {
63+
const optionalDataCollectionPermissions = ['authenticationInfo'];
64+
const requiredDataCollectionPermissions = ['searchTerms'];
65+
66+
const file = {
67+
permissions: [],
68+
optional_permissions: [],
69+
optional_data_collection_permissions: optionalDataCollectionPermissions,
70+
data_collection_permissions: requiredDataCollectionPermissions,
71+
};
72+
73+
expect(_getCurrentPermissions({ file })).toEqual({
74+
permissions: {
75+
optional: [],
76+
required: [],
77+
},
78+
data_collection_permissions: {
79+
optional: optionalDataCollectionPermissions,
80+
required: requiredDataCollectionPermissions,
81+
},
4782
});
4883
});
4984

5085
it('returns empty arrays if no file was found', () => {
5186
const file = null;
5287
expect(_getCurrentPermissions({ file })).toEqual({
53-
optional: [],
54-
required: [],
88+
permissions: {
89+
optional: [],
90+
required: [],
91+
},
92+
data_collection_permissions: {
93+
optional: [],
94+
required: [],
95+
},
5596
});
5697
});
5798
});
@@ -115,5 +156,21 @@ describe(__filename, () => {
115156
hostPermissionB,
116157
]);
117158
});
159+
160+
it('formats data collection permission strings', () => {
161+
const testPermissions = [
162+
'technicalAndInteraction',
163+
'personallyIdentifyingInfo',
164+
'foobar',
165+
];
166+
const result = permissionUtils.formatPermissions(testPermissions);
167+
expect(result).toHaveLength(2);
168+
expect(result[0].props.description).toEqual(
169+
'Personally identifying information',
170+
);
171+
expect(result[1].props.description).toEqual(
172+
'Technical and interaction data',
173+
);
174+
});
118175
});
119176
});

0 commit comments

Comments
 (0)