Skip to content

chore: only send credentials on prod - #2273

Merged
apsantiso merged 2 commits into
masterfrom
feat/auth-credentials-production-only
Nov 4, 2026
Merged

apsantiso merged 2 commits into
masterfrom
feat/auth-credentials-production-only

Conversation

@apsantiso

@apsantiso apsantiso commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Description

Related Issues

Related Pull Requests

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.
  • QA Passed

Testing Process

Additional Notes

Summary by CodeRabbit

  • New Features
    • Authentication requests can now be configured to send or omit cookies using an environment setting. The setting defaults to false, so cookies are omitted unless explicitly enabled. This behavior applies to both regular and desktop sign-in flows.

@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

Both auth clients now set withCredentials according to the sendCookiesOnAuthRequests environment variable. The environment example sets this variable to false. A test verifies that both clients disable credentials when the setting is false.

Changes: Auth credential configuration

Layer / File(s) Summary
Configure and apply auth credentials
.env.example, src/services/env.service.ts, src/app/core/factory/sdk/index.ts, src/app/core/factory/sdk/index.test.ts
The environment service maps REACT_APP_SEND_COOKIES_ON_AUTH_REQUESTS. Both auth clients use a shared helper that enables withCredentials only when the setting equals 'true'. A test verifies that both clients set withCredentials to false when the setting is 'false'.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Possibly related PRs

Suggested reviewers: xabg2

Merge Risk: Minimal · up to 21fd1

Auth requests now send cookies only when the REACT_APP_SEND_COOKIES_ON_AUTH_REQUESTS build flag is 'true'. No concrete defect was found in the change. Confirm that the production build sets the flag before deploying.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description contains only the repository template. It does not provide a change summary, related issue or pull request details, checklist status, testing process, or additional notes. Complete the Description, Checklist, Testing Process, and Additional Notes sections. Explain the environment flag behavior, identify related work or state that none exists, record completed checks, and document the tests that verify credent…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise and describes the main behavior change: credentials are sent only when enabled for production. It does not mention that the behavior uses an environment flag, but it remains suffi…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title is concise and describes the main behavior change: credentials are sent only when enabled for production. It does not mention that the behavior uses an environment flag, but it remains sufficiently related to the changeset.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files. (1 skipped: 1 unsupported.)

Full details: Description check

Resolution

Complete the Description, Checklist, Testing Process, and Additional Notes sections. Explain the environment flag behavior, identify related work or state that none exists, record completed checks, and document the tests that verify credential handling.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2819f0d2-8f91-4ad1-b25c-56aa3c82eca3
Commits

Reviewing files that changed from the base of the PR and between f17bdad and 21fd1fd.

Files selected for processing: 4
  • .env.example
  • src/app/core/factory/sdk/index.test.ts
  • src/app/core/factory/sdk/index.ts
  • src/services/env.service.ts

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 21fd1fd
Status: ✅  Deploy successful!
Preview URL: https://c1417d51.drive-web.pages.dev
Branch Preview URL: https://feat-auth-credentials-produc.drive-web.pages.dev

View logs

@apsantiso apsantiso self-assigned this Nov 3, 2026
@apsantiso
apsantiso marked this pull request as ready for review November 3, 2026 15:05
CandelR
CandelR previously approved these changes Nov 3, 2026
larryrider
larryrider previously approved these changes Nov 3, 2026
@apsantiso
apsantiso dismissed stale reviews from larryrider and CandelR via 21fd1fd November 3, 2026 15:11
@apsantiso

apsantiso commented Nov 3, 2026 •

Copy link
Copy Markdown
Member Author

@larryrider @CandelR ended up setting this on a flag instead of relying on prod so we can toggle it in case it's required. Also, this should work with preview too

Can you help me to set the envar? REACT_APP_SEND_COOKIES_ON_AUTH_REQUESTS = true

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@CandelR

CandelR commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

@larryrider @CandelR ended up setting this on a flag instead of relying on prod so we can toggle it in case it's required. Also, this should work with preview too

Can you help me to set the envar? REACT_APP_SEND_COOKIES_ON_AUTH_REQUESTS = true

Done

@apsantiso
apsantiso merged commit 5853441 into master Nov 4, 2026
12 checks passed
@apsantiso
apsantiso deleted the feat/auth-credentials-production-only branch November 4, 2026 08:45
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.

3 participants