add support for using either public_name or public_identifier #4490

jlbyrne · 2024-08-01T00:50:19Z

Resolves https://github.com/brave/roadmap/issues/1544
Resolves https://github.com/brave/reviews/issues/1709
Resolves https://github.com/brave-intl/creators-private-issues/issues/1885

Contribution page change:

Admin Panel:
https://github.com/user-attachments/assets/4b37a378-0fbc-4503-b52b-73e46284e59b

Contribution Page Editor UI:
https://github.com/user-attachments/assets/2121fa28-6e8f-4041-bb18-52929734f9a5

github-actions · 2025-02-05T05:07:43Z

[puLL-Merge] - brave-intl/publishers@4490

Description

This PR introduces custom vanity URLs for contribution pages and a system to manage reserved public names. It allows creators to set custom URLs for their contribution pages while maintaining security and uniqueness constraints. The changes also include an admin interface to manage reserved names and a temporary hold period for previously used names.

Security Hotspots

Public URL Validation: The validation of public names (validate_public_url_unique in Channel model) needs careful review to ensure no malicious URLs can be created. Currently implements format restrictions and uniqueness checks.
Admin Controller Access: The new ReservedPublicNamesController should be properly secured with admin authentication (appears to be handled by inheriting from AdminController).
URL Path Traversal: The public_identifier lookup in PublicChannelController now checks two fields which could potentially lead to path traversal if not properly sanitized.

Changes

Changes

New Files:

app/controllers/admin/reserved_public_names_controller.rb: Admin interface for managing reserved names
app/models/reserved_public_name.rb: Model for storing reserved public names
app/views/admin/reserved_public_names/index.html.slim: Admin UI for reserved names
nextjs/src/app/[locale]/publishers/contribution_page/PublicUrlConfirmationModal.jsx: Confirmation modal for URL changes

Modified Files:

Channel Model (app/models/channel.rb):
- Added validation for public names
- Implemented reserved name system
- Added whitespace stripping for public names
- Added uniqueness constraints across public_name and public_identifier
Controllers:
- Updated PublicChannelController to support vanity URLs
- Modified ContributionPageController to handle public name updates
Frontend:
- Added UI for managing custom URLs
- Added validation messages and error handling
- Updated translations
Database:
- Added new reserved_public_names table
- Added necessary indexes and constraints

sequenceDiagram
    participant User
    participant Frontend
    participant API
    participant Channel
    participant ReservedNames

    User->>Frontend: Enter custom URL
    Frontend->>Frontend: Show confirmation modal
    Frontend->>API: Request URL update
    API->>Channel: Validate URL
    Channel->>ReservedNames: Check if reserved
    ReservedNames-->>Channel: Return status
    Channel-->>API: Return validation result
    API-->>Frontend: Return success/error
    Frontend-->>User: Show result

github-actions · 2025-02-05T05:11:56Z

app/controllers/admin/reserved_public_names_controller.rb

+    private
+
+    def set_reserved_public_name
+      @reserved_public_name = ReservedPublicName.find(params[:id])


_{reported by reviewdog 🐶}
[semgrep] Found an unscoped find(...) with user-controllable input. If the ActiveRecord model being searched against is sensitive, this may lead to Insecure Direct Object Reference (IDOR) behavior and allow users to read arbitrary records. Scope the find to the current user, e.g. current_user.accounts.find(params[:id]).

Source: https://semgrep.dev/r/ruby.rails.security.brakeman.check-unscoped-find.check-unscoped-find

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/c/[public_identifier]/PublicChannelPage.jsx

@@ -95,6 +95,9 @@ export default function PublicChannelPage({publicIdentifier, previewMode}) {
              <div className={`${styles['privacy-disclaimer']}`}>
                {t('publicChannelPage.privacyDisclaimer')}
              </div>
+              <div className={`${styles['privacy-disclaimer']}`}>
+                {t('publicChannelPage.report_sus_urls')}<a target="_blank" rel="noopener noreferrer" href="https://support.brave.com/hc/en-us/requests/new">https://support.brave.com/hc/en-us/requests/new</a>


_{reported by reviewdog 🐶}
[semgrep] Translation key 'publicChannelPage.report_sus_urls' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/PublicUrlConfirmationModal.jsx

+
+  return (
+    <div>
+      <p className={styles['privacy-text']}>{t('contribution_pages.confirmation_text')}</p>


_{reported by reviewdog 🐶}
[semgrep] Translation key 'contribution_pages.confirmation_text' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/PublicUrlConfirmationModal.jsx

+            style={{ margin: '10px 0px', width: '320px' }}
+            kind='outline'
+          >
+            {t('shared.cancel')}


_{reported by reviewdog 🐶}
[semgrep] Translation key 'shared.cancel' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/PublicUrlConfirmationModal.jsx

+            style={{ margin: '10px 0px', width: '320px' }}
+            kind='filled'
+          >
+            {t('shared.continue')}


_{reported by reviewdog 🐶}
[semgrep] Translation key 'shared.continue' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/page.jsx

+                <span slot='errors'>{publicNameError}</span>
+              </Input>
+              <div className='pb-3 color-tertiary'>
+                {t('contribution_pages.public_url_note')}


_{reported by reviewdog 🐶}
[semgrep] Translation key 'contribution_pages.public_url_note' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/page.jsx

+                  rel="noreferrer"
+                  href="https://support.brave.com/hc/en-us/articles/33646848629901-Creators-Custom-URLs-for-Contribution-Pages"
+                >
+                  {t('contribution_pages.public_url_link')}


_{reported by reviewdog 🐶}
[semgrep] Translation key 'contribution_pages.public_url_link' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T05:11:57Z

nextjs/src/app/[locale]/publishers/contribution_page/page.jsx

+                >
+                  {t('contribution_pages.public_url_link')}
+                </Link>
+                {t('contribution_pages.public_url_note_2')}


_{reported by reviewdog 🐶}
[semgrep] Translation key 'contribution_pages.public_url_note_2' should match format 'MODULE.FEATURE.*'

Source: https://semgrep.dev/r/typescript.react.portability.i18next.i18next-key-format.i18next-key-format

Cc @thypon @kdenhartog

github-actions · 2025-02-05T06:26:51Z

The following commits were not verified:
d3fc373 (unsigned)
6c0bdc2 (unsigned)

Miyayes · 2025-02-05T17:59:09Z

@jlbyrne Text:

Choose a custom URL for your contribution page. See our support article for more information about custom URLs.

Modal:

Are you sure you would like to change the URL for your contribution page? Note:

You can only change your URL once every 12 months.

If you change your URL, your old URL will become unavailable for awhile before it can be used again.

On the contribution page itself:

Make the first two paragraphs into 1 paragraph. (Remove the newline.) Remember to put a period in between the two sentences.
Does this page have an inappropriate URL? Report it to Brave. ("Report it to Brave" is hyperlinked to the support form.)

Miyayes · 2025-02-05T18:05:55Z

Can we remove the space between .../c/ and VANITYURLHERE?

for VANITYURLHERE, you can bold the text in the field.

@jlbyrne Question: Given the cooldown on changing your URL, we should gray out the field and put different text if the user is still in cooldown period. Are we able to show exactly how many days are remaining? If so, then we should gray out the field and say:

Your shareable URL can be changed in x days.

jlbyrne · 2025-02-05T20:02:22Z

Can we remove the space between .../c/ and VANITYURLHERE?

for VANITYURLHERE, you can bold the text in the field.

@jlbyrne Question: Given the cooldown on changing your URL, we should gray out the field and put different text if the user is still in cooldown period. Are we able to show exactly how many days are remaining? If so, then we should gray out the field and say:

Your shareable URL can be changed in x days.

So the cooldown period is for use of the custom url itself, across all users. A user can still change their url to something else.

Miyayes · 2025-02-06T17:48:03Z

So the cooldown period is for use of the custom url itself, across all users. A user can still change their url to something else.

If a user has no cooldown on changing their URL, then can't they just run an attack and basically lock out every single possible URL? For example, every 1 second, I change my custom URL to something else, locking out all the custom URLs I touch for 1 month. @jlbyrne @kdenhartog

jlbyrne · 2025-02-06T18:33:16Z

If a user has no cooldown on changing their URL, then can't they just run an attack and basically lock out every single possible URL? For example, every 1 second, I change my custom URL to something else, locking out all the custom URLs I touch for 1 month. @jlbyrne @kdenhartog

That's correct. I could add in an additional cooldown per user if necessary?

Miyayes · 2025-02-10T16:06:03Z

If a user has no cooldown on changing their URL, then can't they just run an attack and basically lock out every single possible URL? For example, every 1 second, I change my custom URL to something else, locking out all the custom URLs I touch for 1 month. @jlbyrne @kdenhartog

That's correct. I could add in an additional cooldown per user if necessary?

Yes, I think that is necessary. How about we make it 60 days? cc: @kdenhartog

jlbyrne force-pushed the feat/enable-public-name branch from a25ed69 to 0604330 Compare August 1, 2024 00:52

jlbyrne added the needs-security-review label Aug 1, 2024

add support for using either public_name or public_identifier

d3fc373

jlbyrne force-pushed the feat/enable-public-name branch from 0604330 to d3fc373 Compare February 5, 2025 05:07

github-actions bot added the puLL-Merge label Feb 5, 2025

github-actions bot reviewed Feb 5, 2025

View reviewed changes

github-actions bot assigned kdenhartog and thypon Feb 5, 2025

fix for lint and tests

6c0bdc2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for using either public_name or public_identifier #4490

add support for using either public_name or public_identifier #4490

jlbyrne commented Aug 1, 2024 •

edited by Miyayes

Loading

github-actions bot commented Feb 5, 2025

Changes

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot Feb 5, 2025

github-actions bot commented Feb 5, 2025

Miyayes commented Feb 5, 2025 •

edited

Loading

Miyayes commented Feb 5, 2025

jlbyrne commented Feb 5, 2025

Miyayes commented Feb 6, 2025 •

edited

Loading

jlbyrne commented Feb 6, 2025

Miyayes commented Feb 10, 2025

add support for using either public_name or public_identifier #4490

Are you sure you want to change the base?

add support for using either public_name or public_identifier #4490

Conversation

jlbyrne commented Aug 1, 2024 • edited by Miyayes Loading

github-actions bot commented Feb 5, 2025

Description

Security Hotspots

Changes

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot Feb 5, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2025

Miyayes commented Feb 5, 2025 • edited Loading

Miyayes commented Feb 5, 2025

jlbyrne commented Feb 5, 2025

Miyayes commented Feb 6, 2025 • edited Loading

jlbyrne commented Feb 6, 2025

Miyayes commented Feb 10, 2025

jlbyrne commented Aug 1, 2024 •

edited by Miyayes

Loading

Miyayes commented Feb 5, 2025 •

edited

Loading

Miyayes commented Feb 6, 2025 •

edited

Loading