Updated rusro auth - #21
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesEmail OAuth authentication
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RuSroAuthService
participant 4gameAuth
participant 4gameOAuth
participant GlobalConfig
User->>RuSroAuthService: Start authentication
RuSroAuthService->>GlobalConfig: Read cached token state
RuSroAuthService->>4gameOAuth: Refresh token or begin email authorization
4gameAuth-->>RuSroAuthService: Return email-code authorization result
RuSroAuthService->>4gameOAuth: Exchange authorization code
4gameOAuth-->>RuSroAuthService: Return access and refresh tokens
RuSroAuthService->>GlobalConfig: Persist token and authorization state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Plugins/RSBot.General/Components/RuSroAuthService.cs (1)
479-480: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winParse the round-trip timestamp with invariant culture.
codeRequestedAtis written withToString("O")(ISO-8601 round-trip) but read back with the ambient-cultureTryParse. Use invariant culture and round-trip styles so the value is parsed deterministically regardless of the host locale.♻️ Suggested change
- if (!DateTimeOffset.TryParse(requestedAt, out codeRequestedAt)) + if (!DateTimeOffset.TryParse( + requestedAt, + CultureInfo.InvariantCulture, + DateTimeStyles.RoundtripKind, + out codeRequestedAt)) codeRequestedAt = DateTimeOffset.MinValue;Requires
using System.Globalization;.🤖 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 `@Plugins/RSBot.General/Components/RuSroAuthService.cs` around lines 479 - 480, Update the requested timestamp parsing in the codeRequestedAt assignment to use CultureInfo.InvariantCulture with DateTimeStyles.RoundtripKind, adding the System.Globalization import required for these symbols. Preserve the existing DateTimeOffset.MinValue fallback when parsing fails.
🤖 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 `@Plugins/RSBot.General/Components/RuSroAuthService.cs`:
- Around line 429-431: Update SaveTokens around GlobalConfig.Set for
RefreshTokenConfigKey so the long-lived refresh token is not stored in
plaintext; persist it through the existing encrypted/secure storage mechanism,
or omit persistence and retain only the access token and token owner settings.
---
Nitpick comments:
In `@Plugins/RSBot.General/Components/RuSroAuthService.cs`:
- Around line 479-480: Update the requested timestamp parsing in the
codeRequestedAt assignment to use CultureInfo.InvariantCulture with
DateTimeStyles.RoundtripKind, adding the System.Globalization import required
for these symbols. Preserve the existing DateTimeOffset.MinValue fallback when
parsing fails.
🪄 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: 1a6d4cb8-30fb-4817-affc-895702a43b43
📒 Files selected for processing (1)
Plugins/RSBot.General/Components/RuSroAuthService.cs
| GlobalConfig.Set(AccessTokenConfigKey, tokenResponse.AccessToken); | ||
| GlobalConfig.Set(RefreshTokenConfigKey, refreshToken); | ||
| GlobalConfig.Set(TokenOwnerConfigKey, email); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how GlobalConfig persists values (encryption vs plaintext)
fd -t f -i 'globalconfig' -x cat -n {}
fd -t f -i 'config.cs' Library -x sed -n '1,200p' {}Repository: Silkroad-Developer-Community/OasisBot
Length of output: 19030
Persist the OAuth tokens with protection at rest
SaveTokens writes the access token and long-lived refresh token into GlobalConfig, and that config is serialized as plaintext. Store the refresh token in encrypted storage or avoid persisting it.
🤖 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 `@Plugins/RSBot.General/Components/RuSroAuthService.cs` around lines 429 - 431,
Update SaveTokens around GlobalConfig.Set for RefreshTokenConfigKey so the
long-lived refresh token is not stored in plaintext; persist it through the
existing encrypted/secure storage mechanism, or omit persistence and retain only
the access token and token owner settings.
1 participant
Summary by CodeRabbit