-
Notifications
You must be signed in to change notification settings - Fork 78
Add email (OTP) credentials setup to init scripts #112
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe initialization scripts now collect email credentials for OTP functionality. Both Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes The changes introduce credential handling across two scripts with different logic paths. Review should verify proper environment variable assignment, secure credential handling (especially password conversion to plaintext), and that the reorganized Livekit workflow in the Bash script maintains correct execution order. Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
init.ps1 (2)
183-183: Remove or clarify the commented line.This commented-out
appwrite push functions --with-variablescommand appears to be leftover from refactoring, as the actual command is executed at Line 202. If this comment serves a specific purpose (e.g., documenting the flow), consider adding context; otherwise, remove it to avoid confusion.-# appwrite push functions --with-variables
186-188: Add input validation for email credentials.The script accepts email and password without validation. Consider verifying that the email follows a basic format and that the password is not empty.
Apply this diff to add basic validation:
$senderMail = Read-Host "Please provide sender email address (e.g., [email protected])" +if (-not ($senderMail -match "^[^@]+@[^@]+\.[^@]+$")) { + Write-Host "Invalid email format. Please run the script again with a valid email." + exit 1 +} + $senderPassword = Read-Host "Please provide sender email app password" -AsSecureString +if ($senderPassword.Length -eq 0) { + Write-Host "Password cannot be empty. Please run the script again." + exit 1 +} $senderPasswordPlain = [Runtime.InteropServices.Marshal]::PtrToStringAuto([Runtime.InteropServices.Marshal]::SecureStringToBSTR($senderPassword))init.sh (2)
130-130: Remove or clarify the misplaced comment.The comment
## appwrite push functions --with-variablesappears to be leftover from refactoring, as the actual command is executed at Line 190. If this comment serves as a section marker, consider rephrasing it to clarify its purpose; otherwise, remove it.-## appwrite push functions --with-variables
133-134: Add input validation for email credentials.The script accepts email and password without validation. Empty inputs or invalid email formats could cause runtime issues in the OTP functionality.
Apply this diff to add basic validation:
read -p "Please provide sender email address (e.g., [email protected]): " senderMail +if [[ ! "$senderMail" =~ ^[^@]+@[^@]+\.[^@]+$ ]]; then + echo "Invalid email format. Please run the script again with a valid email." + exit 1 +fi + read -sp "Please provide sender email app password: " senderPassword echo "" +if [[ -z "$senderPassword" ]]; then + echo "Password cannot be empty. Please run the script again." + exit 1 +fi
| appwrite project create-variable --key SENDER_MAIL --value $senderMail | ||
| appwrite project create-variable --key SENDER_PASSWORD --value $senderPasswordPlain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that environment variables are created successfully.
The script doesn't check whether the appwrite project create-variable commands succeed. If these commands fail, the functions may not have access to the required credentials.
Run the following script to check if similar error handling exists elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check if other appwrite commands in init scripts have error handling
# Search for appwrite commands and their error handling patterns
rg -n -A2 -B2 'appwrite project create-variable' init.ps1 init.shLength of output: 4143
Add error handling to verify environment variable creation succeeds before continuing.
The review comment is valid. Neither init.sh nor init.ps1 implement any error checks after executing appwrite project create-variable commands. Both scripts print "Email credentials configured successfully!" regardless of whether the preceding commands actually succeeded. This pattern is systematic throughout the codebase—all appwrite environment variable assignments lack validation.
If these commands fail silently, critical credentials won't be set, yet the script will report success and continue, leaving cloud functions without required authentication. Add conditional checks (e.g., $? in bash, $LASTEXITCODE in PowerShell) to verify each command succeeds before proceeding, or combine them with && operators to halt on failure.
🤖 Prompt for AI Agents
In init.ps1 around lines 192-193, the two appwrite project create-variable calls
for SENDER_MAIL and SENDER_PASSWORD do not check for success; add explicit
PowerShell error handling after each command by checking $LASTEXITCODE (or $?)
and if non-zero write a clear error to stderr and call exit 1 (or throw) so the
script stops on failure; alternatively chain commands with -and or use try/catch
around Invoke-Expression to validate creation, and ensure the final "Email
credentials configured successfully!" message only prints when both creations
succeeded.
| appwrite project create-variable --key SENDER_MAIL --value "$senderMail" | ||
| appwrite project create-variable --key SENDER_PASSWORD --value "$senderPassword" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify that environment variables are created successfully.
The script doesn't check whether the appwrite project create-variable commands succeed. This is consistent with other variable creation calls in the script, but failed credential pushes could break OTP functionality.
Run the following script to verify the pattern used elsewhere in init.sh:
🏁 Script executed:
#!/bin/bash
# Description: Check error handling patterns for appwrite commands in init.sh
# Search for appwrite commands and check if any have error handling
rg -n -C3 'appwrite project create-variable.*\|\||\&\& \[|if \[' init.shLength of output: 2361
I need to examine the actual code context around lines 139-140 and search for all appwrite project create-variable calls to verify the pattern.
Now let me examine the actual init.sh file to see the code context:
🏁 Script executed:
#!/bin/bash
# Get the content around lines 139-140 and show all appwrite project create-variable calls
# Show lines around 139-140
echo "=== Lines around 139-140 in init.sh ==="
sed -n '130,150p' init.sh
echo -e "\n=== All appwrite project create-variable calls in init.sh ==="
rg -n 'appwrite project create-variable' init.sh -A1 -B1Length of output: 201
I need to locate init.sh in the repository first:
🏁 Script executed:
#!/bin/bash
# Find init.sh location in repository
fd 'init\.sh' --type f
# Once found, get content around lines 139-140 and all appwrite project create-variable calls
echo -e "\n=== Searching for init.sh and extracting content ==="
init_file=$(fd 'init\.sh' --type f | head -1)
if [ -n "$init_file" ]; then
echo "Found init.sh at: $init_file"
echo -e "\n=== Lines around 139-140 ==="
sed -n '130,150p' "$init_file"
echo -e "\n=== All appwrite project create-variable calls ==="
rg -n 'appwrite project create-variable' "$init_file" -A1 -B1
else
echo "init.sh not found"
fiLength of output: 2676
Add error handling to email credential setup commands at lines 139-140.
Your observation is verified and accurate. None of the 10 appwrite project create-variable calls in init.sh have error handling, making lines 139-140 consistent with the pattern elsewhere in the file. However, this is problematic for OTP functionality since SENDER_MAIL and SENDER_PASSWORD are critical credentials. The script checks appwrite login exit code (line 43) but not these credential pushes, creating an inconsistency. If either command fails silently, OTP verification will break without user awareness.
Add checks after both commands:
appwrite project create-variable --key SENDER_MAIL --value "$senderMail" || { echo "Failed to set SENDER_MAIL"; exit 1; }
appwrite project create-variable --key SENDER_PASSWORD --value "$senderPassword" || { echo "Failed to set SENDER_PASSWORD"; exit 1; }
Consider applying the same pattern to other critical variable creation calls (lines 71, 75, 127-128, 186-189) for consistency and robustness.
🤖 Prompt for AI Agents
In init.sh around lines 139-140, the appwrite project create-variable calls for
SENDER_MAIL and SENDER_PASSWORD lack error handling; add checks immediately
after each command so the script prints a clear failure message and exits
non-zero if the create-variable command fails (i.e., run the create-variable,
then test its exit status and echo "Failed to set SENDER_MAIL" / "Failed to set
SENDER_PASSWORD" and exit 1 on failure). Also apply the same exit-on-failure
pattern to other critical variable creation calls noted (lines 71, 75, 127-128,
186-189) to ensure the script halts and reports which credential failed to
create.
| $senderMail = Read-Host "Please provide sender email address (e.g., [email protected])" | ||
| $senderPassword = Read-Host "Please provide sender email app password" -AsSecureString |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As suggested by CodeRabbit, please add checks to ensure that the senderMail and senderPassword inputs were entered before moving forward. stay in a read loop until they are entered.
|
Hey @M4dhav, sorry for the delay, I was busy with exams. |
Changes Made
Added email setup section in init.sh and init.ps1, and Added prompts for sender email and app password during initialization.
Configured the credentials to be stored as project environment variables.
Followed the existing structure used for MeiliSearch and Livekit setup.
Testing
Verified that email credentials are prompted and stored correctly.
Confirmed that the initialization flow remains smooth and consistent.
Ensured no regressions or breaking changes in existing functionality.
Summary by CodeRabbit
New Features
Chores