fix(auth): honour MS_SCOPES in the OAuth consent request - #75
Conversation
The auth server hardcoded its scope list while token-storage.js reads MS_SCOPES when refreshing. Setting MS_SCOPES therefore had no effect on consent: the consented token never carried the requested scopes, and every write tool (delete-email, move-emails, draft-email, mark-as-read) failed with insufficient privileges even though the Azure app registration granted Mail.ReadWrite. Read MS_SCOPES in AUTH_CONFIG with the previous list as fallback, so consent and refresh can no longer drift apart.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
User does not have a PR Review subscription. Go to Team management and add this email to the PR Review subscription. |
📝 WalkthroughWalkthroughAdds optional ChangesOAuth scope configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@outlook-auth-server.js`:
- Around line 46-56: Unify the OAuth scope fallback used by the consent
configuration in outlook-auth-server.js, specifically the scopes definition,
with the shared default list already used by the token refresh path; continue
honoring MS_SCOPES when set. In README.md lines 195-204, retain the
documentation stating that an unset MS_SCOPES preserves defaults only after both
flows reference the same fallback list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5a618544-327c-4e23-9720-c8e5b84ee94f
📒 Files selected for processing (2)
README.mdoutlook-auth-server.js
| // MS_SCOPES honours the same env var token-storage.js uses for refresh, so the | ||
| // consented scopes and the refreshed scopes can't drift apart. | ||
| scopes: (process.env.MS_SCOPES || [ | ||
| 'offline_access', | ||
| 'User.Read', | ||
| 'Mail.Read', | ||
| 'Mail.Send', | ||
| 'Calendars.Read', | ||
| 'Calendars.ReadWrite', | ||
| 'Contacts.Read' | ||
| ], | ||
| ].join(' ')).split(' ').filter(Boolean), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Unify the fallback OAuth scopes across the consent and refresh paths. MS_SCOPES keeps the paths aligned only when explicitly set; their unset defaults still differ.
outlook-auth-server.js#L46-L56: derive the fallback from the same shared default used by token refresh.README.md#L195-L204: retain the “unset preserves defaults” documentation only after both flows use the same fallback list.
📍 Affects 2 files
outlook-auth-server.js#L46-L56(this comment)README.md#L195-L204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@outlook-auth-server.js` around lines 46 - 56, Unify the OAuth scope fallback
used by the consent configuration in outlook-auth-server.js, specifically the
scopes definition, with the shared default list already used by the token
refresh path; continue honoring MS_SCOPES when set. In README.md lines 195-204,
retain the documentation stating that an unset MS_SCOPES preserves defaults only
after both flows reference the same fallback list.
Problem
outlook-auth-server.jshardcodes its scope list, whileauth/token-storage.jsreadsMS_SCOPESwhen refreshing tokens. The two disagree, and the consent request always wins — so settingMS_SCOPEShas no effect on what the user actually consents to.The practical failure: an Azure app registration with
Mail.ReadWriteconfigured still produces a token whose granted scope isMail.Readonly. Every write tool then fails, without any obvious cause:delete-emailmove-emailsdraft-emailmark-as-readReproduced on a personal Microsoft account (
MS_TENANT_ID=consumers), commit95d6ff2:Note the second half of the bug:
Contacts.Readis consented even though nothing asks for it, because it is in the hardcoded list. Users grant more access than the app needs and less than it uses.Fix
Read
MS_SCOPESinAUTH_CONFIG, keeping the existing list as the fallback, so consent and refresh can no longer drift apart. Behaviour is unchanged whenMS_SCOPESis unset.Also documents
MS_SCOPESin the README env section, where it was previously undocumented.Verification
After the fix, with the same
.env:End-to-end over stdio against a real mailbox:
list-emailsdraft-emaildelete-email(The test draft was deleted as part of the run.)
Credit
Found and fixed while reviewing this server for a Microsoft 365 integration on Jerico — the security review flagged that the consent scopes and the refresh scopes were reading from different sources, which turned out to be a real functional bug rather than a hardening nit.
Summary by CodeRabbit
New Features
MS_SCOPESenvironment variable.Documentation
MS_SCOPESusage and defaults.