-
Notifications
You must be signed in to change notification settings - Fork 1
Make localhost/port values configurable via env vars #84
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| name: Env File Keys Check | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
|
|
||
| jobs: | ||
| check-env-keys: | ||
| runs-on: ubuntu-latest | ||
| name: Verify required env keys are listed in .env.example | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Verify env keys | ||
| run: | | ||
| set -euo pipefail | ||
| REQUIRED_KEYS=( | ||
| "GMAIL_CLIENT_ID" | ||
| "GMAIL_CLIENT_SECRET" | ||
| "GMAIL_REDIRECT_URI" | ||
| "OUTLOOK_CLIENT_ID" | ||
| "OUTLOOK_CLIENT_SECRET" | ||
| "OUTLOOK_TENANT_ID" | ||
| "OUTLOOK_REDIRECT_URI" | ||
| "REACT_APP_BACKEND_URL" | ||
| "FRONTEND_REDIRECT_URL" | ||
| "FRONTEND_POSTAUTH_ROUTE" | ||
| "SSL_KEY_FILE" | ||
| "SSL_CRT_FILE" | ||
| "SSL_KEY_PATH" | ||
| "SSL_CERT_PATH" | ||
| "ALLOWED_ORIGINS" | ||
| "CLASSIFIER_URL" | ||
| "BACKEND_PORT" | ||
| "BACKEND_HOST" | ||
| "REACT_APP_USE_GROUP_LOGO" | ||
| "CLASSIFICATION_CATEGORIES" | ||
| ) | ||
|
|
||
| MISSING=() | ||
| for key in "${REQUIRED_KEYS[@]}"; do | ||
| if ! grep -q "^${key}=" .env.example; then | ||
| MISSING+=("$key") | ||
| fi | ||
| done | ||
|
|
||
| if [ ${#MISSING[@]} -ne 0 ]; then | ||
| echo "ERROR: The following required env keys are missing from .env.example:" | ||
| for k in "${MISSING[@]}"; do | ||
| echo " - $k" | ||
| done | ||
| echo "\nPlease add them to .env.example (use placeholders, do NOT commit secrets)." | ||
| exit 1 | ||
| fi | ||
| echo "All required env keys present in .env.example" |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,32 +10,36 @@ const app = express(); // Create an Express application instance | |||||||||||||||||||
|
|
||||||||||||||||||||
| app.use(bodyParser.json()); // Use body-parser to parse JSON request bodies | ||||||||||||||||||||
|
|
||||||||||||||||||||
| const allowedOrigins = [ | ||||||||||||||||||||
| // Localhost (HTTPS) | ||||||||||||||||||||
| "https://localhost:3000", | ||||||||||||||||||||
| "https://localhost:5002", | ||||||||||||||||||||
| "https://localhost:5000", | ||||||||||||||||||||
| "https://localhost:5003", | ||||||||||||||||||||
| "https://localhost:5173", | ||||||||||||||||||||
| // Localhost (HTTP) | ||||||||||||||||||||
| "http://localhost:3000", | ||||||||||||||||||||
| "http://localhost:5002", | ||||||||||||||||||||
| "http://localhost:5000", | ||||||||||||||||||||
| "http://localhost:5003", | ||||||||||||||||||||
| "http://localhost:5173", | ||||||||||||||||||||
| // 127.0.0.1 loopback (HTTPS) | ||||||||||||||||||||
| "https://127.0.0.1:3000", | ||||||||||||||||||||
| "https://127.0.0.1:5002", | ||||||||||||||||||||
| "https://127.0.0.1:5000", | ||||||||||||||||||||
| "https://127.0.0.1:5003", | ||||||||||||||||||||
| "https://127.0.0.1:5173", | ||||||||||||||||||||
| // 127.0.0.1 loopback (HTTP) | ||||||||||||||||||||
| "http://127.0.0.1:3000", | ||||||||||||||||||||
| "http://127.0.0.1:5002", | ||||||||||||||||||||
| "http://127.0.0.1:5000", | ||||||||||||||||||||
| "http://127.0.0.1:5003", | ||||||||||||||||||||
| "http://127.0.0.1:5173", | ||||||||||||||||||||
| ]; | ||||||||||||||||||||
| // Allow configuring allowed CORS origins via environment variable `ALLOWED_ORIGINS` as a comma-separated list. | ||||||||||||||||||||
| // If not provided, fall back to a safe default set used for local development. | ||||||||||||||||||||
| const allowedOrigins = process.env.ALLOWED_ORIGINS | ||||||||||||||||||||
| ? process.env.ALLOWED_ORIGINS.split(",").map((s) => s.trim()) | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Consider validating ALLOWED_ORIGINS input for empty strings. Empty strings in allowedOrigins may cause unexpected CORS behavior. Filter them out after splitting to improve robustness.
Suggested change
|
||||||||||||||||||||
| : [ | ||||||||||||||||||||
| // Localhost (HTTPS) | ||||||||||||||||||||
| "https://localhost:3000", | ||||||||||||||||||||
| "https://localhost:5002", | ||||||||||||||||||||
| "https://localhost:5000", | ||||||||||||||||||||
| "https://localhost:5003", | ||||||||||||||||||||
| "https://localhost:5173", | ||||||||||||||||||||
| // Localhost (HTTP) | ||||||||||||||||||||
| "http://localhost:3000", | ||||||||||||||||||||
| "http://localhost:5002", | ||||||||||||||||||||
| "http://localhost:5000", | ||||||||||||||||||||
| "http://localhost:5003", | ||||||||||||||||||||
| "http://localhost:5173", | ||||||||||||||||||||
| // 127.0.0.1 loopback (HTTPS) | ||||||||||||||||||||
| "https://127.0.0.1:3000", | ||||||||||||||||||||
| "https://127.0.0.1:5002", | ||||||||||||||||||||
| "https://127.0.0.1:5000", | ||||||||||||||||||||
| "https://127.0.0.1:5003", | ||||||||||||||||||||
| "https://127.0.0.1:5173", | ||||||||||||||||||||
| // 127.0.0.1 loopback (HTTP) | ||||||||||||||||||||
| "http://127.0.0.1:3000", | ||||||||||||||||||||
| "http://127.0.0.1:5002", | ||||||||||||||||||||
| "http://127.0.0.1:5000", | ||||||||||||||||||||
| "http://127.0.0.1:5003", | ||||||||||||||||||||
| "http://127.0.0.1:5173", | ||||||||||||||||||||
| ]; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| app.use( | ||||||||||||||||||||
| cors({ | ||||||||||||||||||||
|
|
@@ -120,7 +124,7 @@ app.use((req, res) => { | |||||||||||||||||||
| }); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Start server in a way that's safe on Vercel (platform provides HTTPS) | ||||||||||||||||||||
| const PORT = process.env.PORT || 5002; // Ensure backend uses port 5002 | ||||||||||||||||||||
| const PORT = process.env.PORT || process.env.BACKEND_PORT || 5002; // Ensure backend uses BACKEND_PORT or default 5002 | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Detect Vercel or similar serverless hosting environment. Vercel exposes | ||||||||||||||||||||
| // `VERCEL` or `VERCEL_ENV` / `NOW_REGION` environment variables at runtime. | ||||||||||||||||||||
|
|
@@ -144,7 +148,8 @@ if (isVercel) { | |||||||||||||||||||
| cert: fs.readFileSync(certPath), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| https.createServer(options, app).listen(PORT, () => { | ||||||||||||||||||||
| console.log(`HTTPS server running at https://localhost:${PORT}`); | ||||||||||||||||||||
| const host = process.env.BACKEND_HOST || "localhost"; | ||||||||||||||||||||
| console.log(`HTTPS server running at https://${host}:${PORT}`); | ||||||||||||||||||||
|
Comment on lines
146
to
+152
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: BACKEND_HOST may not match actual binding host. If the server binds to a different host than BACKEND_HOST, the log output may be inaccurate. Consider clarifying the log message or ensuring the binding matches BACKEND_HOST.
Suggested change
|
||||||||||||||||||||
| }); | ||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||
| console.error('Failed to start HTTPS server, falling back to HTTP:', err); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,7 +36,13 @@ function App() { | |||||||||||||||||||||||||||||||
| }, []); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Function to handle Gmail login via OAuth | ||||||||||||||||||||||||||||||||
| const backendUrl = process.env.REACT_APP_BACKEND_URL || "https://localhost:5002"; | ||||||||||||||||||||||||||||||||
| // Backend URL is configurable via `REACT_APP_BACKEND_URL`. If not provided, in development we'll point | ||||||||||||||||||||||||||||||||
| // to localhost on `BACKEND_PORT` (default 5002). In production default to the current origin. | ||||||||||||||||||||||||||||||||
| const backendUrl = | ||||||||||||||||||||||||||||||||
| process.env.REACT_APP_BACKEND_URL || | ||||||||||||||||||||||||||||||||
| ((window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1') | ||||||||||||||||||||||||||||||||
| ? `https://${window.location.hostname}:${process.env.BACKEND_PORT || 5002}` | ||||||||||||||||||||||||||||||||
| : `${window.location.protocol}//${window.location.host}`); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Mixing protocol and port logic may cause issues in some dev setups. Defaulting to https for localhost can cause connection errors if the backend uses http. Consider detecting the protocol or making it configurable to support different development setups.
Suggested change
|
||||||||||||||||||||||||||||||||
| const frontendUrl = window.location.origin; | ||||||||||||||||||||||||||||||||
| const handleGmailLogin = () => { | ||||||||||||||||||||||||||||||||
| window.location.href = `${backendUrl}/auth/gmail/login?redirect=${encodeURIComponent(frontendUrl)}`; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
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.
suggestion: Consider validating CLASSIFIER_URL for trailing slashes.
Also, trim whitespace from CLASSIFIER_URL to prevent issues from accidental spaces or formatting.