Skip to content
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

chore: renamed non-inclusive language #18319

Merged
merged 12 commits into from
Nov 2, 2023
Merged
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,4 @@ gen/
.antlr
upgrade/
hogvm/typescript/dist

.wokeignore
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 2 additions & 2 deletions frontend/src/initKea.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { formsPlugin } from 'kea-forms'
Actions for which we don't want to show error alerts,
mostly to avoid user confusion.
*/
const ERROR_FILTER_WHITELIST = [
const ERROR_FILTER_ALLOW_LIST = [
'loadPreflight', // Gracefully handled if it fails
'loadUser', // App won't load (unless loading from shared dashboards)
'loadFunnels', // Special error handling on insights
Expand Down Expand Up @@ -79,7 +79,7 @@ export function initKea({ routerHistory, routerLocation, beforePlugins }: InitKe
onFailure({ error, reducerKey, actionKey }: { error: any; reducerKey: string; actionKey: string }) {
// Toast if it's a fetch error or a specific API update error
if (
!ERROR_FILTER_WHITELIST.includes(actionKey) &&
!ERROR_FILTER_ALLOW_LIST.includes(actionKey) &&
(error?.message === 'Failed to fetch' || // Likely CORS headers errors (i.e. request failing without reaching Django)
(error?.status !== undefined && ![200, 201, 204].includes(error.status)))
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export enum ConfigMode {
export type InstanceStatusTabName = 'overview' | 'metrics' | 'settings' | 'staff_users' | 'kafka_inspector'

/**
* We whitelist the specific instance settings that can be edited via the /instance/status page.
* We allow the specific instance settings that can be edited via the /instance/status page.
* Even if some settings are editable in the frontend according to the API, we may don't want to expose them here.
* For example: async migrations settings are handled in their own page.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export function VerifiedDomains(): JSX.Element {
<>
<div className="flex items-center">
<div style={{ flexGrow: 1 }}>
<div id="domain-whitelist" /> {/** For backwards link compatibility. Remove after 2022-06-01. */}
<h2 id="authentication-domains" className="subtitle">
Authentication domains
</h2>
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/project/Settings/DataAttributes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export function DataAttributes(): JSX.Element {
<p>
For example, when creating an action on your CTA button, the best selector could be something like:{' '}
<code>div &gt; form &gt; button:nth-child(2)</code>. However all buttons in your app have a{' '}
<code>data-custom-id</code> attribute. If you whitelist it here, the selector for your button will
instead be <code>button[data-custom-id='cta-button']</code>.
<code>data-custom-id</code> attribute. If you allow it here, the selector for your button will instead
be <code>button[data-custom-id='cta-button']</code>.
</p>
<div className="space-y-4 max-w-160">
<LemonSelectMultiple
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/project/Settings/WebhookIntegration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export function WebhookIntegration(): JSX.Element {
}
}, [currentTeam])

const webhooks_blacklisted = featureFlags[FEATURE_FLAGS.WEBHOOKS_DENYLIST]
if (webhooks_blacklisted) {
const webhooks_disallowed = featureFlags[FEATURE_FLAGS.WEBHOOKS_DENYLIST]
if (webhooks_disallowed) {
return (
<div>
<p>
Expand Down
10 changes: 5 additions & 5 deletions plugin-server/src/worker/ingestion/event-pipeline/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class EventPipelineRunner {
this.originalEvent = event
}

isEventBlacklisted(event: PipelineEvent): boolean {
isEventDisallowed(event: PipelineEvent): boolean {
// During incidents we can use the the env DROP_EVENTS_BY_TOKEN_DISTINCT_ID
// to drop events here before processing them which would allow us to catch up
const key = event.token || event.team_id?.toString()
Expand All @@ -87,14 +87,14 @@ export class EventPipelineRunner {
this.hub.statsd?.increment('kafka_queue.event_pipeline.start', { pipeline: 'event' })

try {
if (this.isEventBlacklisted(event)) {
if (this.isEventDisallowed(event)) {
eventDroppedCounter
.labels({
event_type: 'analytics',
drop_cause: 'blacklisted',
drop_cause: 'disallowed',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this impact analytics that are relied on?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question.

@tiina303 Git blame tells me you were the last around these parts of the code. Do you know whether there are any implications for changing this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh indeed great catch, it is changing the metric label, so yes filtering for it in grafana-prod-us would break / need updating

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good with this one. For any metrics I'd just search in the charts repo, where we have dashboards/alerts https://github.com/search?q=repo%3APostHog%2Fcharts%20blacklisted&type=code

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm rather new with Grafana and I'm not too sure what I'm looking for in there. Considering the search in the charts repo returns nothing for me, are we all good, or are those two things not connected?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the search returns nothing (for neither repo), so we don't use the label explicitly in alerts nor dashboards.
example alerts conf: https://github.com/PostHog/charts/blob/main/config/vmalert/alerts/pipeline.yml
example dashboard conf: https://github.com/PostHog/posthog-cloud-infra/blob/fb2a10888e5c8ac2cb7c2268f1c710e38e49c9f2/terraform/modules/grafana/dashboards/webhooks.json#L1165

})
.inc()
return this.registerLastStep('eventBlacklistingStep', null, [event])
return this.registerLastStep('eventDisallowedStep', null, [event])
}
let result: EventPipelineResult
const eventWithTeam = await this.runStep(populateTeamDataStep, [this, event], event.team_id || -1)
Expand Down Expand Up @@ -204,7 +204,7 @@ export class EventPipelineRunner {
// for a reason that we control and that is transient.
return true
}
// TODO: Blacklist via env of errors we're going to put into DLQ instead of taking Kafka lag
// TODO: Disallow via env of errors we're going to put into DLQ instead of taking Kafka lag
return false
}

Expand Down
4 changes: 2 additions & 2 deletions plugin-server/src/worker/vm/lazy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ export class LazyPluginVM {
const team = await this.hub.teamManager.fetchTeam(this.pluginConfig.team_id)
// There's a single team with replication for the same api key from US to EU
// otherwise we're just checking that token differs to better safeguard against forwarding
const isWhitelisted = team?.uuid == '017955d2-b09f-0000-ec00-2116c7e8a605' && host == 'eu.posthog.com'
if (!isWhitelisted && team?.api_token.trim() == apiKey.trim()) {
const isAllowed = team?.uuid == '017955d2-b09f-0000-ec00-2116c7e8a605' && host == 'eu.posthog.com'
if (!isAllowed && team?.api_token.trim() == apiKey.trim()) {
throw Error('Self replication is not allowed')
}
// Only default org can use higher than 1x replication
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ describe('EventPipelineRunner', () => {
expect(runner.stepsWithArgs).toMatchSnapshot()
})

it('drops blacklisted events', async () => {
it('drops disallowed events', async () => {
const event = {
...pipelineEvent,
token: 'drop_token',
Expand All @@ -131,7 +131,7 @@ describe('EventPipelineRunner', () => {
expect(runner.steps).toEqual([])
})

it('does not drop blacklisted token mismatching distinct_id events', async () => {
it('does not drop disallowed token mismatching distinct_id events', async () => {
const event = {
...pipelineEvent,
token: 'drop_token',
Expand All @@ -146,7 +146,7 @@ describe('EventPipelineRunner', () => {
])
})

it('drops blacklisted events by *', async () => {
it('drops disallowed events by *', async () => {
const event = {
...pipelineEvent,
token: 'drop_token_all',
Expand Down
2 changes: 1 addition & 1 deletion posthog/api/signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ def process_social_invite_signup(strategy: DjangoStrategy, invite_id: str, email
def process_social_domain_jit_provisioning_signup(
email: str, full_name: str, user: Optional[User] = None
) -> Optional[User]:
# Check if the user is on a whitelisted domain
# Check if the user is on a allowed domain
domain = email.split("@")[-1]
try:
logger.info(f"process_social_domain_jit_provisioning_signup", domain=domain)
Expand Down
12 changes: 6 additions & 6 deletions posthog/api/survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ def validate_appearance(self, value):

thank_you_message = value.get("thankYouMessageHeader")
if thank_you_message and nh3.is_html(thank_you_message):
value["thankYouMessageHeader"] = nh3_clean_with_whitelist(thank_you_message)
value["thankYouMessageHeader"] = nh3_clean_with_allow_list(thank_you_message)

thank_you_description = value.get("thankYouMessageDescription")
if thank_you_description and nh3.is_html(thank_you_description):
value["thankYouMessageDescription"] = nh3_clean_with_whitelist(thank_you_description)
value["thankYouMessageDescription"] = nh3_clean_with_allow_list(thank_you_description)

return value

Expand All @@ -131,9 +131,9 @@ def validate_questions(self, value):

description = raw_question.get("description")
if nh3.is_html(question_text):
cleaned_question["question"] = nh3_clean_with_whitelist(question_text)
cleaned_question["question"] = nh3_clean_with_allow_list(question_text)
if description and nh3.is_html(description):
cleaned_question["description"] = nh3_clean_with_whitelist(description)
cleaned_question["description"] = nh3_clean_with_allow_list(description)

cleaned_questions.append(cleaned_question)

Expand Down Expand Up @@ -347,7 +347,7 @@ def surveys(request: Request):
return cors_response(request, JsonResponse({"surveys": surveys}))


def nh3_clean_with_whitelist(to_clean: str):
def nh3_clean_with_allow_list(to_clean: str):
return nh3.clean(
to_clean,
link_rel="noopener",
Expand Down Expand Up @@ -431,7 +431,7 @@ def nh3_clean_with_whitelist(to_clean: str):
attributes={
"*": {"style", "lang", "title", "width", "height"},
# below are mostly defaults to ammonia, but we need to add them explicitly
# because this python binding doesn't allow additive whitelisting
# because this python binding doesn't allow additive allowing
"a": {"href", "hreflang"},
"bdo": {"dir"},
"blockquote": {"cite"},
Expand Down
22 changes: 10 additions & 12 deletions posthog/api/test/test_signup.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ def test_api_social_login_cannot_create_second_organization(self, mock_sso_provi
response, "/login?error_code=no_new_organizations"
) # show the user an error; operation not permitted

def run_test_for_whitelisted_domain(self, mock_sso_providers, mock_request, mock_capture):
def run_test_for_allowed_domain(self, mock_sso_providers, mock_request, mock_capture):
# Make sure Google Auth is valid for this test instance
mock_sso_providers.return_value = {"google-oauth2": True}

Expand Down Expand Up @@ -536,18 +536,18 @@ def run_test_for_whitelisted_domain(self, mock_sso_providers, mock_request, mock
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@mock.patch("posthog.tasks.user_identify.identify_task")
@pytest.mark.ee
def test_social_signup_with_whitelisted_domain_on_self_hosted(
def test_social_signup_with_allowed_domain_on_self_hosted(
self, mock_identify, mock_sso_providers, mock_request, mock_capture
):
self.run_test_for_whitelisted_domain(mock_sso_providers, mock_request, mock_capture)
self.run_test_for_allowed_domain(mock_sso_providers, mock_request, mock_capture)

@patch("posthoganalytics.capture")
@mock.patch("ee.billing.billing_manager.BillingManager.update_billing_distinct_ids")
@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@mock.patch("posthog.tasks.user_identify.identify_task")
@pytest.mark.ee
def test_social_signup_with_whitelisted_domain_on_cloud(
def test_social_signup_with_allowed_domain_on_cloud(
self,
mock_identify,
mock_sso_providers,
Expand All @@ -556,13 +556,13 @@ def test_social_signup_with_whitelisted_domain_on_cloud(
mock_capture,
):
with self.is_cloud(True):
self.run_test_for_whitelisted_domain(mock_sso_providers, mock_request, mock_capture)
self.run_test_for_allowed_domain(mock_sso_providers, mock_request, mock_capture)
assert mock_update_distinct_ids.called_once()

@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@pytest.mark.ee
def test_social_signup_with_whitelisted_domain_on_cloud_reverse(self, mock_sso_providers, mock_request):
def test_social_signup_with_allowed_domain_on_cloud_reverse(self, mock_sso_providers, mock_request):
with self.is_cloud(True):
# user already exists
User.objects.create(email="[email protected]", distinct_id=str(uuid.uuid4()))
Expand Down Expand Up @@ -608,9 +608,7 @@ def test_social_signup_with_whitelisted_domain_on_cloud_reverse(self, mock_sso_p
@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@pytest.mark.ee
def test_cannot_social_signup_with_whitelisted_but_jit_provisioning_disabled(
self, mock_sso_providers, mock_request
):
def test_cannot_social_signup_with_allowed_but_jit_provisioning_disabled(self, mock_sso_providers, mock_request):
mock_sso_providers.return_value = {"google-oauth2": True}
new_org = Organization.objects.create(name="Test org")
OrganizationDomain.objects.create(
Expand Down Expand Up @@ -639,7 +637,7 @@ def test_cannot_social_signup_with_whitelisted_but_jit_provisioning_disabled(
@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@pytest.mark.ee
def test_cannot_social_signup_with_whitelisted_but_unverified_domain(self, mock_sso_providers, mock_request):
def test_cannot_social_signup_with_allowed_but_unverified_domain(self, mock_sso_providers, mock_request):
mock_sso_providers.return_value = {"google-oauth2": True}
new_org = Organization.objects.create(name="Test org")
OrganizationDomain.objects.create(
Expand Down Expand Up @@ -668,7 +666,7 @@ def test_cannot_social_signup_with_whitelisted_but_unverified_domain(self, mock_
@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@pytest.mark.ee
def test_api_cannot_use_whitelist_for_different_domain(self, mock_sso_providers, mock_request):
def test_api_cannot_use_allow_list_for_different_domain(self, mock_sso_providers, mock_request):
mock_sso_providers.return_value = {"google-oauth2": True}
new_org = Organization.objects.create(name="Test org")
OrganizationDomain.objects.create(
Expand Down Expand Up @@ -697,7 +695,7 @@ def test_api_cannot_use_whitelist_for_different_domain(self, mock_sso_providers,
@mock.patch("social_core.backends.base.BaseAuth.request")
@mock.patch("posthog.api.authentication.get_instance_available_sso_providers")
@pytest.mark.ee
def test_social_signup_to_existing_org_without_whitelisted_domain_on_cloud(self, mock_sso_providers, mock_request):
def test_social_signup_to_existing_org_without_allowed_domain_on_cloud(self, mock_sso_providers, mock_request):
with self.is_cloud(True):
mock_sso_providers.return_value = {"google-oauth2": True}
Organization.objects.create(name="Hogflix Movies")
Expand Down
4 changes: 2 additions & 2 deletions posthog/api/test/test_survey.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rest_framework import status
from django.core.cache import cache
from django.test.client import Client
from posthog.api.survey import nh3_clean_with_whitelist
from posthog.api.survey import nh3_clean_with_allow_list

from posthog.models.feedback.survey import Survey
from posthog.test.base import (
Expand Down Expand Up @@ -1232,4 +1232,4 @@ def test_responses_count_zero_responses(self):
],
)
def test_nh3_clean_configuration(test_input, expected):
assert nh3_clean_with_whitelist(test_input).replace(" ", "") == expected.replace(" ", "")
assert nh3_clean_with_allow_list(test_input).replace(" ", "") == expected.replace(" ", "")
Loading