diff --git a/package-lock.json b/package-lock.json index 17ee3cd9..eacbe899 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1107,6 +1107,7 @@ "version": "2.1.4", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "ajv": "^6.12.4", "debug": "^4.3.2", @@ -1129,6 +1130,7 @@ "version": "8.57.0", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": "^12.22.0 || ^14.17.0 || >=16.0.0" } @@ -1351,6 +1353,7 @@ "version": "0.11.14", "dev": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "@humanwhocodes/object-schema": "^2.0.2", "debug": "^4.3.1", @@ -1375,7 +1378,8 @@ "node_modules/@humanwhocodes/object-schema": { "version": "2.0.3", "dev": true, - "license": "BSD-3-Clause" + "license": "BSD-3-Clause", + "peer": true }, "node_modules/@humanwhocodes/retry": { "version": "0.4.2", @@ -2425,7 +2429,6 @@ "version": "4.1.12", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@types/ms": "*" } @@ -2466,7 +2469,6 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-24.10.0.tgz", "integrity": "sha512-qzQZRBqkFsYyaSWXuEHc2WR9c0a0CXwiE5FWUvn7ZM+vdy1uZLfCunD38UzhuB7YN/J11ndbDBcTmOdxJo9Q7A==", "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -2696,7 +2698,6 @@ "integrity": "sha512-6m1I5RmHBGTnUGS113G04DMu3CpSdxCAU/UvtjNWL4Nuf3MW9tQhiJqRlHzChIkhy6kZSAQmc+I1bcGjE3yNKg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.46.3", "@typescript-eslint/types": "8.46.3", @@ -3199,7 +3200,8 @@ "node_modules/@ungap/structured-clone": { "version": "1.2.0", "dev": true, - "license": "ISC" + "license": "ISC", + "peer": true }, "node_modules/@vitest/expect": { "version": "4.0.8", @@ -3318,7 +3320,6 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", - "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -4009,6 +4010,7 @@ "version": "3.0.0", "dev": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "esutils": "^2.0.2" }, @@ -4271,7 +4273,6 @@ "integrity": "sha512-82GZUjRS0p/jganf6q1rEO25VSoHH0hKPCTrgillPjdI/3bgBhAE1QzHrHTizjpRvy6pGAvKjDJtk2pF9NDq8w==", "dev": true, "license": "MIT", - "peer": true, "bin": { "eslint-config-prettier": "bin/cli.js" }, @@ -4363,6 +4364,7 @@ "version": "7.2.2", "dev": true, "license": "BSD-2-Clause", + "peer": true, "dependencies": { "esrecurse": "^4.3.0", "estraverse": "^5.2.0" @@ -4378,6 +4380,7 @@ "version": "9.6.1", "dev": true, "license": "BSD-2-Clause", + "peer": true, "dependencies": { "acorn": "^8.9.0", "acorn-jsx": "^5.3.2", @@ -4582,6 +4585,7 @@ "version": "6.0.1", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "flat-cache": "^3.0.4" }, @@ -4619,6 +4623,7 @@ "version": "3.0.4", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "flatted": "^3.1.0", "rimraf": "^3.0.2" @@ -4687,7 +4692,8 @@ "node_modules/fs.realpath": { "version": "1.0.0", "dev": true, - "license": "ISC" + "license": "ISC", + "peer": true }, "node_modules/fsevents": { "version": "2.3.3", @@ -4792,6 +4798,7 @@ "version": "7.2.3", "dev": true, "license": "ISC", + "peer": true, "dependencies": { "fs.realpath": "^1.0.0", "inflight": "^1.0.4", @@ -4822,6 +4829,7 @@ "version": "13.24.0", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "type-fest": "^0.20.2" }, @@ -5126,6 +5134,7 @@ "version": "1.0.6", "dev": true, "license": "ISC", + "peer": true, "dependencies": { "once": "^1.3.0", "wrappy": "1" @@ -5338,6 +5347,7 @@ "version": "3.0.3", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=8" } @@ -6271,6 +6281,7 @@ "version": "1.0.1", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -6642,6 +6653,7 @@ "version": "3.0.2", "dev": true, "license": "ISC", + "peer": true, "dependencies": { "glob": "^7.1.3" }, @@ -7272,7 +7284,8 @@ "node_modules/text-table": { "version": "0.2.0", "dev": true, - "license": "MIT" + "license": "MIT", + "peer": true }, "node_modules/through": { "version": "2.3.8", @@ -7336,7 +7349,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7567,6 +7579,7 @@ "version": "0.20.2", "dev": true, "license": "(MIT OR CC0-1.0)", + "peer": true, "engines": { "node": ">=10" }, @@ -7580,7 +7593,6 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", - "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -7838,7 +7850,6 @@ "integrity": "sha512-BxAKBWmIbrDgrokdGZH1IgkIk/5mMHDreLDmCJ0qpyJaAteP8NvMhkwr/ZCQNqNH97bw/dANTE9PDzqwJghfMQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.5.0", @@ -7932,7 +7943,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, diff --git a/packages/ring-client-api/rest-client.ts b/packages/ring-client-api/rest-client.ts index 36a52b40..6134383b 100644 --- a/packages/ring-client-api/rest-client.ts +++ b/packages/ring-client-api/rest-client.ts @@ -355,14 +355,6 @@ export class RingRestClient { refresh_token: this.refreshToken, } } catch (requestError: any) { - if (grantData.refresh_token) { - // failed request with refresh token - this.refreshToken = undefined - this.authConfig = undefined - logError(requestError) - return this.getAuth() - } - const response = requestError.response || {}, responseData: Auth2faResponse = response.body || {}, responseError = @@ -370,6 +362,35 @@ export class RingRestClient { ? responseData.error : '' + // If we were using a refresh token and got an invalid_grant error, + // clear the token and retry with email/password (if available) + if (grantData.refresh_token) { + // Check if this is a permanent authentication failure (invalid token) + // Only clear the refresh token if it's actually invalid + // Common error codes that indicate the token is truly invalid: + // - invalid_grant: token is expired or revoked + // - access_denied: token doesn't have access + const isInvalidToken = + response.status === 401 && + (responseError === 'invalid_grant' || + responseError === 'access_denied') + + if (isInvalidToken) { + // Token is truly invalid, clear it and retry with email/password if available + this.refreshToken = undefined + this.authConfig = undefined + logError('Refresh token is invalid and was cleared') + logError(requestError) + return this.getAuth(twoFactorAuthCode) + } + + // For all other errors (network errors, server errors, rate limiting, etc.), + // do NOT clear the refresh token - just fall through to error handling + // This allows the system to recover from temporary failures + logError('Authentication failed but refresh token preserved') + logError(requestError) + } + if ( response.status === 412 || // need 2fa code (response.status === 400 && diff --git a/packages/ring-client-api/test/rest-client.spec.ts b/packages/ring-client-api/test/rest-client.spec.ts index 118a31e0..87c8886a 100644 --- a/packages/ring-client-api/test/rest-client.spec.ts +++ b/packages/ring-client-api/test/rest-client.spec.ts @@ -211,6 +211,7 @@ afterAll(() => { afterEach(() => { client.clearTimeouts() clearTimeouts() + server.resetHandlers() }) describe('getAuth', () => { @@ -306,6 +307,96 @@ describe('getAuth', () => { newRefreshToken: await wrapRefreshToken(secondRefreshToken), }) }) + + it('should clear refresh token only on invalid_grant error', async () => { + const invalidToken = 'invalid_token' + client = new RingRestClient({ + refreshToken: invalidToken, + }) + + // The invalid token should be cleared when it gets a 401 invalid_grant response + // Because there's no email/password, it will fall back to throwing the error from getGrantData + await expect(() => client.getAuth()).rejects.toThrow( + 'Refresh token is not valid. Unable to authenticate with Ring servers', + ) + + // Verify the token was cleared + expect(client.refreshToken).toBeUndefined() + }) + + it('should NOT clear refresh token on server errors', async () => { + const validToken = await wrapRefreshToken(refreshToken) + client = new RingRestClient({ + refreshToken: validToken, + }) + + // Mock a server error + server.use( + http.post('https://oauth.ring.com/oauth/token', () => { + // Simulate 503 Service Unavailable + return HttpResponse.json( + { error: 'service_unavailable' }, + { status: 503 }, + ) + }), + ) + + // Should throw an error but NOT clear the token + await expect(() => client.getAuth()).rejects.toThrow( + 'Failed to fetch oauth token from Ring', + ) + + // Verify the token was NOT cleared + expect(client.refreshToken).toBe(validToken) + }) + + it('should NOT clear refresh token on rate limiting errors', async () => { + const validToken = await wrapRefreshToken(refreshToken) + client = new RingRestClient({ + refreshToken: validToken, + }) + + // Mock a rate limiting error + server.use( + http.post('https://oauth.ring.com/oauth/token', () => { + // Simulate 429 Too Many Requests + return HttpResponse.json( + { error: 'rate_limit_exceeded' }, + { status: 429 }, + ) + }), + ) + + // Should throw an error but NOT clear the token + await expect(() => client.getAuth()).rejects.toThrow( + 'Failed to fetch oauth token from Ring', + ) + + // Verify the token was NOT cleared - this is critical for recovery from temporary issues + expect(client.refreshToken).toBe(validToken) + }) + + it('should NOT clear refresh token on access_denied from non-401 status', async () => { + const validToken = await wrapRefreshToken(refreshToken) + client = new RingRestClient({ + refreshToken: validToken, + }) + + // Mock an error that has access_denied but not 401 status + server.use( + http.post('https://oauth.ring.com/oauth/token', () => { + return HttpResponse.json({ error: 'access_denied' }, { status: 403 }) + }), + ) + + // Should throw an error but NOT clear the token (only 401 + invalid_grant/access_denied should clear) + await expect(() => client.getAuth()).rejects.toThrow( + 'Failed to fetch oauth token from Ring', + ) + + // Verify the token was NOT cleared + expect(client.refreshToken).toBe(validToken) + }) }) describe('fetch', () => {