Implement OAuth 2.0 + PKCE authentication flow#1750
Open
tsightler wants to merge 2 commits into
Open
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR modernizes Ring authentication by replacing the legacy password grant with an OAuth 2.0 Authorization Code + PKCE flow, while preserving refresh-token authentication and updating device/location endpoint usage.
Changes:
- Adds PKCE flow orchestration, CSRF extraction, 2FA verification, authorization-code exchange, and token persistence.
- Switches device and location fetching to newer
device_info/v3andlocation_info/v3endpoints. - Updates auth tests and Ring response types for the new OAuth and location payload shapes.
Show a summary per file
| File | Description |
|---|---|
packages/ring-client-api/rest-client.ts |
Implements OAuth PKCE authentication, cookie handling, 2FA continuation, token exchange, and new API URL helpers. |
packages/ring-client-api/api.ts |
Fetches devices/locations from new endpoints and classifies flat device responses into existing device groups. |
packages/ring-client-api/ring-types.ts |
Expands location typing and adds a 2FA verification response interface. |
packages/ring-client-api/test/rest-client.spec.ts |
Reworks auth mocks/tests around the PKCE + 2FA flow and refresh-token exchange. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 10
Comment on lines
+208
to
+219
| // Verify PKCE: SHA256(code_verifier) should equal the stored code_challenge | ||
| if (codeVerifier && storedCodeChallenge) { | ||
| const computedChallenge = createHash('sha256') | ||
| .update(codeVerifier) | ||
| .digest('base64url') | ||
| if (computedChallenge !== storedCodeChallenge) { | ||
| return HttpResponse.json( | ||
| { error: 'invalid_grant', error_description: 'PKCE verification failed' }, | ||
| { status: 400 }, | ||
| ) | ||
| } | ||
| } |
Comment on lines
+489
to
+491
| logDebug(`Cookies after initiatePkceFlow: ${cookieJar.getCookieHeader()}`) | ||
| const csrfToken = this.extractCsrfToken(html, cookieJar) | ||
| logDebug(`Extracted CSRF token: ${csrfToken.substring(0, 20)}...`) |
| remember_me: 'false', | ||
| }) | ||
|
|
||
| logDebug(`2FA verify request cookies: ${cookieJar.getCookieHeader()}`) |
| cookieJar.extractFromResponse(response) | ||
|
|
||
| logDebug(`2FA verify response status: ${response.status}`) | ||
| logDebug(`2FA verify response cookies: ${cookieJar.getCookieHeader()}`) |
| // Read the response body to get redirect_url | ||
| const verifyBody = await response.json().catch(() => ({})) as any | ||
| logDebug(`2FA verification successful: ${JSON.stringify(verifyBody)}`) | ||
| logDebug(`All cookies after 2FA: ${cookieJar.getCookieHeader()}`) |
| ) | ||
|
|
||
| cookieJar.extractFromResponse(response) | ||
| logDebug(`Cookies after submitCredentials: ${cookieJar.getCookieHeader()}`) |
|
|
||
| if (response.status >= 300 && response.status < 400) { | ||
| const location = response.headers.get('location') | ||
| logDebug(`Authorize redirect ${i}: ${location}`) |
| // Get the authorization code via redirect | ||
| logDebug('Getting authorization code...') | ||
| const code = await this.getAuthorizationCode() | ||
| logDebug(`Got authorization code: ${code.substring(0, 10)}...`) |
Comment on lines
+408
to
+409
| logDebug( | ||
| 'HTML preview (first 2000 chars): ' + html.substring(0, 2000), |
Comment on lines
+405
to
+407
| logDebug( | ||
| 'CSRF extraction failed. Cookies: ' + cookieJar.getCookieHeader(), | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the second in a series of PRs to bring ring-client-api on par with modern Ring API specs. Please note that, while I've done my best to keep the PRs independent, it's very difficult to develop these changes without some depedency on prior PRs.
While I have tested these changes with ring-mqtt, the modifications are in core functions that have remained largely unchaged for many years so this PR has more risk than many.
The official Ring app has implemented the standard OAuth 2.0 with PKCE flow as the default authentication method for some time, but it still has a fall back to the legacy direct password grant API method which is still used by ring-client-api today. However, more recently it appears Ring has tightened the security around the legacy API endpoint and some users have expeirenced various issues performing authentication with the API.
This PR implements this new authentication workflow. I have tested this flow extensively and it works, however, this flow is more fragile than the previous API flow and could be impacted by changes to the Ring auth web page, although such changes would likely also implement their own app so I'm hopeful it will not be common. This should make auth requests less picky to things like the user agent string, since the oauth flow is generally performed via a browser instead of natively in the app.
Important Note: This code was written 99% by Claude Code. I did have to do a some manual debugging as the OAuth process is very picky with regards to token handling and headers. The overall flow is signfiicantly more complex than the previous method, but I'm not sure if it's avoidable as it's the same patter for many modern apps and I've had to do the same for other projects as well.