Skip to content

fix sign in bug on dev#1119

Merged
BilalG1 merged 5 commits intodevfrom
fix-dev-signin
Jan 19, 2026
Merged

fix sign in bug on dev#1119
BilalG1 merged 5 commits intodevfrom
fix-dev-signin

Conversation

@BilalG1
Copy link
Contributor

@BilalG1 BilalG1 commented Jan 19, 2026

Summary by CodeRabbit

  • Refactor
    • Tightened OAuth environment handling: non-HTTPS token requests are now permitted only in development or test runtimes and only when the token endpoint uses plain HTTP. Public APIs and signatures are unchanged; this restricts when insecure token exchanges are allowed without affecting normal end-user behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Jan 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
stack-backend Ready Ready Preview, Comment Jan 19, 2026 8:00pm
stack-dashboard Ready Ready Preview, Comment Jan 19, 2026 8:00pm
stack-demo Ready Ready Preview, Comment Jan 19, 2026 8:00pm
stack-docs Ready Ready Preview, Comment Jan 19, 2026 8:00pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

Replaced a framework-specific test-environment check with a Node.js NODE_ENV-based check in the OAuth client interface; removed the getNodeEnvironment import and updated allowInsecure to permit insecure HTTP only when process.env.NODE_ENV includes "dev" or equals "test" and the token endpoint starts with "http://".

Changes

Cohort / File(s) Summary
OAuth environment check update
packages/stack-shared/src/interface/client-interface.ts
Removed getNodeEnvironment import; changed allowInsecure logic in fetchNewAccessToken and callOAuthCallback to use process.env.NODE_ENV (allow when it includes "dev" or equals 'test') and only when the token endpoint starts with http://. No public signatures changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through code at break of day,

dev and test now show the way,
Only http when I give a wink,
Production stays secure, I think,
I nibble bugs and bound away 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix sign in bug on dev' is vague and does not clearly describe the technical change. It lacks specificity about what the bug is or what was changed. Use a more descriptive title that explains the specific change, such as 'Update allowInsecure logic to check NODE_ENV for dev/test environments' or 'Fix OAuth token endpoint security check for development mode'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/stack-shared/src/interface/client-interface.ts`:
- Around line 1043-1044: The comment above the allowInsecure declaration is
inaccurate: it says "only in test environment" while the code uses
process.env.NODE_ENV !== 'production' so insecure HTTP is permitted in any
non-production environment; update the comment to accurately reflect this
behavior (referencing the allowInsecure constant, tokenEndpoint check, and
NODE_ENV) — e.g., note that insecure HTTP is allowed when NODE_ENV is not
'production' and tokenEndpoint starts with 'http://', rather than "test
environment" only.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 19, 2026

Greptile Summary

Fixed sign-in bug in development environment by updating the insecure HTTP check for OAuth token endpoints.

  • Changed from getNodeEnvironment() === 'test' to process.env.NODE_ENV !== 'production'
  • Removed import of getNodeEnvironment utility function
  • Applied to both fetchNewAccessToken (line 168) and callOAuthCallback (line 1044)

The original code only allowed insecure HTTP requests (localhost testing) when NODE_ENV === 'test', which blocked sign-in in development environments where NODE_ENV === 'development'. The fix now allows insecure HTTP in all non-production environments (development, test, etc.).

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is a straightforward bug fix that correctly expands the allowInsecure check from test-only to all non-production environments, which is the appropriate behavior for local development. The logic is sound and the change is minimal.
  • No files require special attention

Important Files Changed

Filename Overview
packages/stack-shared/src/interface/client-interface.ts Fixed OAuth token endpoint insecure HTTP check to allow development environment (was only allowing test environment)

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant Interface as StackClientInterface
    participant Library as OAuth4WebAPI
    participant Server as API Server
    
    Note over App,Server: Token Refresh Flow
    
    App->>Interface: Request token refresh
    Interface->>Interface: Evaluate environment setting
    alt Not in production
        Interface->>Library: Grant request (insecure allowed)
    else In production
        Interface->>Library: Grant request (secure only)
    end
    Library->>Server: Token request
    Server-->>Library: Response
    Library-->>Interface: Token data
    Interface-->>App: Access token
    
    Note over App,Server: Callback flow uses same logic
Loading

@BilalG1 BilalG1 enabled auto-merge (squash) January 19, 2026 18:20
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@packages/stack-shared/src/interface/client-interface.ts`:
- Line 168: The current expression for allowInsecure uses
process.env.NODE_ENV.includes(...) which can be undefined and causes TS/runtime
errors; normalize NODE_ENV first (e.g., const env = String(process.env.NODE_ENV
|| '') or process.env.NODE_ENV ?? '') and then replace occurrences so
allowInsecure is computed using env.includes('dev') and env === 'test' with
tokenEndpoint.startsWith('http://'); update any other sites (e.g., the other
occurrence around line 1044) to use the same normalized env variable to avoid
TS18048 and runtime crashes.
♻️ Duplicate comments (1)
packages/stack-shared/src/interface/client-interface.ts (1)

1043-1044: Update the comment to reflect dev/test behavior.

The comment says “test environment only,” but the condition also allows dev. Align the comment with the actual logic.

📝 Suggested comment update
-    // Allow insecure HTTP requests only in test environment (for localhost testing)
+    // Allow insecure HTTP requests in dev/test environments (for localhost testing)
     const allowInsecure = (process.env.NODE_ENV.includes("dev") || process.env.NODE_ENV === 'test') && tokenEndpoint.startsWith('http://');

@BilalG1 BilalG1 merged commit 2e13eb1 into dev Jan 19, 2026
26 of 29 checks passed
@BilalG1 BilalG1 deleted the fix-dev-signin branch January 19, 2026 20:35
N2D4 pushed a commit that referenced this pull request Jan 19, 2026
<!--

Make sure you've read the CONTRIBUTING.md guidelines:
https://github.com/stack-auth/stack-auth/blob/dev/CONTRIBUTING.md

-->


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Updated internal environment detection mechanism for OAuth flows.
Insecure HTTP requests are now allowed when running outside of
production environments, rather than only during testing scenarios. No
changes to public APIs.

<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants