Development has been made to allow X users to generate and use their own APIKEY and Access Token #489
Conversation
…own APIKEY and Access Token. By setting ENABLE_X_SELF="true" in the .env file, each user can use their own API Key.
|
Someone is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce a new integration feature with self-generated tokens, encapsulated in a new environment variable Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Env as Environment
participant Int as Integration Manager
participant XSP as XSelfProvider
participant API as Twitter API
Dev->>Env: Set ENABLE_X_SELF=true
Env->>Int: Initialize socialIntegrationList
Int->>XSP: Instantiate XSelfProvider
XSP->>API: Interact with Twitter API
API-->>XSP: Return API Response
XSP-->>Int: Provide functionality
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (3)
libraries/nestjs-libraries/src/integrations/social/xself.provider.ts (2)
186-187: Remove unused variables to improve code clarityThe variables
code,codeVerifier,oauth_token, andoauth_token_secretare declared but not used. Removing them can improve code readability and prevent confusion.Apply this diff:
-const { code, codeVerifier } = params; -const [oauth_token, oauth_token_secret] = codeVerifier.split(':');
290-291: Remove debugging statements from production codeThe
console.log('GGG DATA', data);statement is a leftover debugging statement. It should be removed to keep the code clean and avoid unnecessary console output.Apply this diff:
- console.log('GGG DATA', data);libraries/nestjs-libraries/src/integrations/integration.manager.ts (1)
28-28: Document and validate the feature flag configuration.Since this feature affects user authentication and API access patterns, consider:
- Adding validation in the application startup to ensure the environment variable is properly configured
- Documenting the feature flag in the README or configuration guide
- Adding monitoring/metrics to track usage patterns with self-generated tokens
- Implementing rate limiting or quota management for self-generated tokens
Would you like assistance in implementing any of these suggestions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/public/icons/platforms/xself.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.env.example(1 hunks)apps/frontend/src/components/launches/providers/show.all.providers.tsx(2 hunks)apps/frontend/src/components/launches/providers/xself/xself.provider.tsx(1 hunks)libraries/nestjs-libraries/src/integrations/integration.manager.ts(1 hunks)libraries/nestjs-libraries/src/integrations/social/xself.provider.ts(1 hunks)
🔇 Additional comments (4)
.env.example (1)
92-93: LGTM
The addition of ENABLE_X_SELF="true" is appropriate and correctly placed under the "Developer Settings" section.
apps/frontend/src/components/launches/providers/show.all.providers.tsx (1)
20-41: LGTM
The integration of XSelfProvider into the Providers array is correctly implemented, and the import statement is accurate.
libraries/nestjs-libraries/src/integrations/integration.manager.ts (2)
24-24: LGTM!
The import statement follows the established pattern and is correctly placed with other provider imports.
28-28: 🛠️ Refactor suggestion
Enhance environment variable handling and testability.
The current implementation has several potential issues:
- Direct use of environment variable without validation
- No handling of unexpected values
- Increased testing complexity due to runtime configuration
Consider refactoring to improve robustness and testability:
- process.env.ENABLE_X_SELF === 'true' ? new XSelfProvider() : new XProvider(),
+ getXProvider(),
+ function getXProvider(): SocialProvider {
+ const enableXSelf = process.env.ENABLE_X_SELF?.toLowerCase();
+ if (enableXSelf !== 'true' && enableXSelf !== 'false') {
+ console.warn('ENABLE_X_SELF not properly set, defaulting to XProvider');
+ return new XProvider();
+ }
+ return enableXSelf === 'true' ? new XSelfProvider() : new XProvider();
+ }This refactoring:
- Adds proper validation for the environment variable
- Provides a clear default behavior
- Makes the code more testable by isolating the provider selection logic
- Adds logging for misconfiguration
Let's verify the environment variable usage across the codebase:
✅ Verification successful
Let me gather more context about the provider implementations to ensure the refactoring suggestion is appropriate.
Let me check for any existing environment variable validation patterns or configuration management in the codebase to ensure our suggestion aligns with the project's practices.
Let me check the configuration management patterns in the codebase to ensure our suggestion aligns with the project's practices.
Based on the codebase analysis, I can now provide the final response:
Consider using the existing ConfigurationChecker pattern for environment variable validation
The codebase already has a robust configuration management system in ConfigurationChecker that handles environment variable validation. Instead of adding inline validation, we should extend this pattern:
- process.env.ENABLE_X_SELF === 'true' ? new XSelfProvider() : new XProvider(),
+ getXProvider(),
+ // Add to ConfigurationChecker class
+ checkXSelfProvider() {
+ const value = this.get('ENABLE_X_SELF')?.toLowerCase();
+ if (value !== 'true' && value !== 'false') {
+ this.issues.push('ENABLE_X_SELF must be either "true" or "false"');
+ }
+ }
+ // Add to check() method
+ this.checkXSelfProvider();
+ // In integration.manager.ts
+ function getXProvider(): SocialProvider {
+ return process.env.ENABLE_X_SELF?.toLowerCase() === 'true'
+ ? new XSelfProvider()
+ : new XProvider();
+ }This approach:
- Maintains consistency with existing codebase patterns
- Centralizes configuration validation
- Provides early validation during application startup
- Preserves the simple provider selection logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of ENABLE_X_SELF to ensure consistent handling
rg "ENABLE_X_SELF" --type ts
Length of output: 175
Script:
#!/bin/bash
# Search for XProvider and XSelfProvider implementations
ast-grep --pattern 'class $_ extends SocialProvider {
$$$
}'
# Also check for any interface or type definitions related to these providers
rg "interface.*Provider|type.*Provider" --type ts
Length of output: 966
Script:
#!/bin/bash
# Search for environment variable validation patterns
rg "process\.env\." -A 3 --type ts
# Look for configuration management files
fd -e ts "config|environment" --type f
Length of output: 64347
Script:
#!/bin/bash
# Check configuration management patterns
cat libraries/helpers/src/configuration/configuration.checker.ts
# Look for any existing environment variable validation patterns
ast-grep --pattern 'function $_(process.env.$_) {
$$$
}'
Length of output: 2435
| for (const load of posts.flatMap((p) => p.flatMap((a) => a.path))) { | ||
| if (load.indexOf('mp4') > -1) { | ||
| const isValid = await checkVideoDuration(load); | ||
| if (!isValid) { | ||
| return 'Video duration must be less than or equal to 140 seconds.'; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle potential CORS issues when accessing video metadata
Accessing video.duration may fail due to CORS restrictions when loading videos from external URLs. This could cause the validation to fail or behave inconsistently. Consider performing video duration validation on the server side or ensure that videos come from same-origin sources where CORS policies allow metadata access.
| const checkVideoDuration = async (url: string): Promise<boolean> => { | ||
| return new Promise((resolve, reject) => { | ||
| const video = document.createElement('video'); | ||
| video.src = url; | ||
| video.preload = 'metadata'; | ||
|
|
||
| video.onloadedmetadata = () => { | ||
| // Check if the duration is less than or equal to 140 seconds | ||
| const duration = video.duration; | ||
| if (duration <= 140) { | ||
| resolve(true); // Video duration is acceptable | ||
| } else { | ||
| resolve(false); // Video duration exceeds 140 seconds | ||
| } | ||
| }; | ||
|
|
||
| video.onerror = () => { | ||
| reject(new Error('Failed to load video metadata.')); | ||
| }; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Ensure proper error handling in checkVideoDuration
The checkVideoDuration function may encounter errors if the video fails to load or due to CORS issues. Ensure that these errors are properly handled and communicated to the user. Additionally, consider implementing a timeout to handle cases where onloadedmetadata may not fire.
| if ( | ||
| posts.some( | ||
| (p) => p.some((m) => m.path.indexOf('mp4') > -1) && p.length > 1 | ||
| ) | ||
| ) { | ||
| return 'There can be maximum 1 video in a post.'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve media type detection and validation logic
Using indexOf('mp4') to detect media types may produce false positives and is not reliable. Consider using file extensions with endsWith('.mp4') or checking MIME types. Also, clarify the validation logic to ensure that posts containing a video do not include other media items.
Apply this diff to improve media type checking and validation logic:
-if (
- posts.some(
- (p) => p.some((m) => m.path.indexOf('mp4') > -1) && p.length > 1
- )
-) {
- return 'There can be maximum 1 video in a post.';
-}
+if (
+ posts.some(
+ (p) =>
+ p.some((m) => m.path.endsWith('.mp4')) &&
+ p.length > 1
+ )
+) {
+ return 'Posts containing a video cannot include other media items.';
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| posts.some( | |
| (p) => p.some((m) => m.path.indexOf('mp4') > -1) && p.length > 1 | |
| ) | |
| ) { | |
| return 'There can be maximum 1 video in a post.'; | |
| } | |
| if ( | |
| posts.some( | |
| (p) => | |
| p.some((m) => m.path.endsWith('.mp4')) && | |
| p.length > 1 | |
| ) | |
| ) { | |
| return 'Posts containing a video cannot include other media items.'; | |
| } |
| ] = integration.token.split(':'); | ||
| const client = new TwitterApi({ | ||
| appKey: apiKeySplit, | ||
| appSecret: apiSecretKeySplit, | ||
| accessToken: accessTokenSplit, | ||
| accessSecret: accessTokenSecretSplit, | ||
| }); |
There was a problem hiding this comment.
Use structured data for tokens instead of splitting strings
Splitting tokens using split(':') is fragile and can lead to errors if any component contains a colon. It's safer to store tokens as structured data, such as JSON, to ensure reliable parsing and prevent potential security issues.
Apply this suggestion:
- Store token components as a JSON string when saving the token, and parse it with
JSON.parsewhen needed.
Also applies to: 129-134, 222-227
| const body = JSON.parse(Buffer.from(params.code, 'base64').toString()); | ||
|
|
||
| const { code, codeVerifier } = params; | ||
| const [oauth_token, oauth_token_secret] = codeVerifier.split(':'); |
There was a problem hiding this comment.
Validate and sanitize authentication parameters
Decoding params.code as base64 and parsing it as JSON without validation can be risky. Ensure that the input is properly validated and sanitized to prevent security vulnerabilities, such as injection attacks.
| m.url.indexOf('mp4') > -1 | ||
| ? Buffer.from(await readOrFetch(m.url)) | ||
| : await sharp(await readOrFetch(m.url), { | ||
| animated: lookup(m.url) === 'image/gif', | ||
| }) | ||
| .resize({ | ||
| width: 1000, | ||
| }) | ||
| .gif() | ||
| .toBuffer(), | ||
| { | ||
| mimeType: lookup(m.url) || '', | ||
| } | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve media type detection when uploading media
Using m.url.indexOf('mp4') to determine the media type is unreliable. It's better to use the file extension with endsWith('.mp4') or check the MIME type using lookup(m.url) to accurately detect the media type.
Apply this diff to improve media type detection:
- return {
- id: await client.v1.uploadMedia(
- m.url.indexOf('mp4') > -1
+ const isVideo = m.url.endsWith('.mp4') || lookup(m.url) === 'video/mp4';
+ return {
+ id: await client.v1.uploadMedia(
+ isVideoCommittable suggestion skipped: line range outside the PR's diff.
|
Thank you for the pull request. |
…own APIKEY and Access Token. By setting ENABLE_X_SELF="true" in the .env file, each user can use their own API Key.
What kind of change does this PR introduce?
A feature
Why was this change needed?
Some self-hosters may prefer each user to generate their own API Token and Access Key instead of using a shared X API. This is because Twitter APIs are quite expensive.
Other information:
eg: Did you discuss this change with anybody before working on it?
No, I needed it and developed.
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit
New Features
XSelfProviderto enhance functionality related to social media interactions.Bug Fixes
Style