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

feat: add DISABLE_ANONYMOUS_USER_CREATE_EMAIL which only allow logi… #545

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

dreamhunter2333
Copy link
Owner

@dreamhunter2333 dreamhunter2333 commented Jan 4, 2025

User description

…n user create email address

#540


PR Type

Enhancement, Documentation


Description

  • Add DISABLE_ANONYMOUS_USER_CREATE_EMAIL configuration option

  • Update frontend logic to respect new configuration

  • Update API endpoints to enforce new configuration

  • Document new configuration in CLI documentation


Changes walkthrough 📝

Relevant files
Enhancement
8 files
Login.vue
Add logic to conditionally show email creation tab             
+11/-3   
worker_config.ts
Add `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` to worker config
+1/-0     
commom_api.ts
Include `disableAnonymousUserCreateEmail` in open API settings
+1/-0     
index.ts
Enforce `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` in new address endpoint
+5/-0     
types.d.ts
Add `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` to type definitions
+1/-0     
index.js
Fetch `disableAnonymousUserCreateEmail` setting from API 
+1/-0     
index.js
Add `disableAnonymousUserCreateEmail` to global state       
+1/-0     
wrangler.toml.template
Add `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` to wrangler template
+2/-0     
Documentation
3 files
CHANGELOG.md
Document new feature in changelog                                               
+1/-0     
cli.md
Document `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` in English CLI docs
+2/-0     
worker.md
Document `DISABLE_ANONYMOUS_USER_CREATE_EMAIL` in Chinese CLI docs
+2/-0     

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

Copy link

github-actions bot commented Jan 4, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

540 - Fully compliant

Fully compliant requirements:

  • Prevent abuse by requiring users to register before creating email addresses.
  • Implement a configuration option to disable anonymous email creation.
  • Update frontend and backend logic to respect this new configuration.
  • Document the new configuration option.

Not compliant requirements:
None

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Ensure that the showNewAddressTab computed property correctly handles all edge cases where anonymous users should not be able to create email addresses.

const showNewAddressTab = computed(() => {
    if (openSettings.value.disableAnonymousUserCreateEmail
        && !userSettings.value.user_email
    ) {
        return false;
    }
    return openSettings.value.enableUserCreateEmail;
});

Copy link

github-actions bot commented Jan 4, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible issue
Add optional chaining to prevent runtime errors when accessing properties of potentially undefined objects

Ensure that openSettings.value and userSettings.value are properly initialized
before accessing their properties to avoid potential runtime errors.

frontend/src/views/common/Login.vue [188-189]

-if (openSettings.value.disableAnonymousUserCreateEmail
-    && !userSettings.value.user_email
+if (openSettings.value?.disableAnonymousUserCreateEmail
+    && !userSettings.value?.user_email
 ) {
Suggestion importance[1-10]: 8

Why: This suggestion adds optional chaining to prevent potential runtime errors if openSettings.value or userSettings.value are undefined. This is a valuable improvement for ensuring code robustness.

8

@dreamhunter2333 dreamhunter2333 merged commit 92620cd into main Jan 5, 2025
1 check passed
@dreamhunter2333 dreamhunter2333 deleted the feature/dev branch January 5, 2025 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant