fix(auth): resolve Plex OAuth client ID mismatch#2746
Open
fix(auth): resolve Plex OAuth client ID mismatch#2746
Conversation
📝 WalkthroughWalkthroughAdds a server-generated Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant UserBrowser as User Browser
participant Frontend as Frontend (React)
end
rect rgba(200,255,200,0.5)
participant Backend as Backend (API)
participant SettingsStore as Settings/DB
end
rect rgba(255,220,200,0.5)
participant Plex as Plex Auth
end
UserBrowser->>Frontend: Click "Authenticate with Plex"
Frontend->>Frontend: read currentSettings.plexClientIdentifier
Frontend->>Plex: Open popup & call PlexOAuth.login(plexClientIdentifier)
Plex-->>Frontend: User approves, returns authToken (popup)
Frontend->>Backend: POST /api/v1/plex/linked-accounts { authToken }
Backend->>SettingsStore: Read settings.plexClientIdentifier / sessionSecret
Backend->>Plex: Complete/validate auth using server plexClientIdentifier
Plex-->>Backend: Success -> account linked
Backend-->>Frontend: 200 OK
Frontend->>Frontend: revalidateUser()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@seerr-api.yml`:
- Around line 713-717: Update the OpenAPI schema to make plexClientIdentifier
required: in the PublicSettings schema add "plexClientIdentifier" to its
required array (and ensure the plexClientIdentifier property remains defined
with type: string and format: uuid). Locate the PublicSettings definition and
modify the required list so the contract enforces presence of
plexClientIdentifier used by the Plex OAuth client.
In `@server/interfaces/api/settingsInterfaces.ts`:
- Line 51: The FullPublicSettings type and the fullPublicSettings getter are
missing the required plexClientIdentifier property, causing
/api/v1/settings/public to omit the Plex OAuth client id; update the
FullPublicSettings type to include plexClientIdentifier: string and modify the
fullPublicSettings getter in server/lib/settings/index.ts (function/getter
fullPublicSettings) to read the stored Plex client identifier (e.g., from the
same settings source used for other public fields) and include
plexClientIdentifier in the returned object so the public settings endpoint
always contains the identifier.
In `@server/lib/settings/index.ts`:
- Around line 392-393: Replace uses of randomUUID() for sessionSecret with a
32-byte high-entropy hex string: use crypto.randomBytes(32).toString('hex')
wherever sessionSecret is generated (e.g., the constructor default, the load()
fallback that sets sessionSecret, and the migration that seeds sessionSecret).
Add or ensure an import of randomBytes from 'crypto' (or use
require('crypto').randomBytes) in the modules where you change it so the new
32-byte hex secret is produced instead of a UUID.
In `@src/utils/plex.ts`:
- Around line 82-84: Wrap the call to initializeHeaders inside login in a
try/catch so that if header initialization throws (e.g., missing
plexClientIdentifier) you catch the error, call the same popup cleanup/close
routine used by preparePopup to close the pre-opened popup, then rethrow or
handle the error before returning; specifically update the login method (which
currently calls this.initializeHeaders and this.getPin) to catch initialization
failures, invoke the popup close/cleanup method used elsewhere in this class
(the one that undoes preparePopup), and only proceed to this.getPin on success.
- Around line 31-35: Replace unsafe global checks that use "if (!window)" with a
typeof check to avoid ReferenceError: in initializeHeaders (and the other
similar check in this file) change the condition to use "typeof window ===
'undefined'" (or the negated form) so the code safely detects non-browser
environments; update both the initializeHeaders function and the second
occurrence at line 162 to use this typeof check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccb23e80-37d8-49bc-9bcb-7d989a4cc2a4
📒 Files selected for processing (11)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0009_migrate_session_secret.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes Plex OAuth failures caused by mismatched client identifiers between browser and server, and improves session security by moving cookie signing to a dedicated secret.
Changes:
- Browser-side Plex OAuth now uses a server-provided
plexClientIdentifierinstead of generating/storing its own localStorage ID. - Introduces
sessionSecretin settings and uses it for express-session signing (replacingclientId). - Updates public settings API/types/OpenAPI spec and Cypress settings fixture to include the new fields.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/plex.ts | Removes per-browser generated Plex client ID; requires identifier to be passed in and uses it in headers/login. |
| src/hooks/usePlexLogin.ts | Retrieves plexClientIdentifier from settings and passes it into plexOAuth.login. |
| src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx | Passes settings-provided plexClientIdentifier into Plex OAuth when linking accounts. |
| src/context/SettingsContext.tsx | Extends default settings to include plexClientIdentifier. |
| src/pages/_app.tsx | Extends initial public settings shape to include plexClientIdentifier. |
| server/lib/settings/index.ts | Adds sessionSecret, exposes plexClientIdentifier via public settings, and ensures secrets are generated if missing. |
| server/lib/settings/migrations/0009_migrate_session_secret.ts | Adds a settings migration to populate sessionSecret for existing installs. |
| server/index.ts | Switches session signing secret from clientId to sessionSecret. |
| server/interfaces/api/settingsInterfaces.ts | Updates PublicSettingsResponse to include plexClientIdentifier. |
| seerr-api.yml | Documents plexClientIdentifier in the public settings schema. |
| cypress/config/settings.cypress.json | Adds sessionSecret to test settings fixture. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
Show resolved
Hide resolved
0xSysR3ll
force-pushed
the
fix/0xsysr3ll/plex-oauth-client-id
branch
from
401563c to
c8756e5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/utils/plex.ts (1)
82-89:⚠️ Potential issue | 🟡 MinorClose the pre-opened popup when
getPin()fails too.
preparePopup()runs beforelogin(), but the newtryonly coversinitializeHeaders(). If the PIN request rejects, the popup stays stuck on/login/plex/loading.Suggested fix
public async login(plexClientIdentifier: string): Promise<string> { try { this.initializeHeaders(plexClientIdentifier); + await this.getPin(); } catch (e) { this.closePopup(); throw e; } - await this.getPin();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/plex.ts` around lines 82 - 89, The login method currently only closes the pre-opened popup if initializeHeaders throws; wrap the await this.getPin() call (or add a try/catch around it) so that if getPin() rejects you call this.closePopup() and rethrow the error; update the login function (referencing login, initializeHeaders, getPin, and closePopup) to ensure the popup is closed on getPin failure while leaving successful flows unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/settings/index.ts`:
- Around line 759-760: The getter get sessionSecret currently returns
this.data.sessionSecret! which can be undefined when load(overrideSettings)
returned early; update the override-path in load(overrideSettings) to
normalize/backfill the sessionSecret before returning (e.g., set
this.data.sessionSecret = this.data.sessionSecret ?? generateSessionSecret() or
call the same backfill/normalize helper used for the normal load path), and
ensure the same fix is applied for the other affected getters around 798-800 so
get sessionSecret and related getters never return undefined.
---
Duplicate comments:
In `@src/utils/plex.ts`:
- Around line 82-89: The login method currently only closes the pre-opened popup
if initializeHeaders throws; wrap the await this.getPin() call (or add a
try/catch around it) so that if getPin() rejects you call this.closePopup() and
rethrow the error; update the login function (referencing login,
initializeHeaders, getPin, and closePopup) to ensure the popup is closed on
getPin failure while leaving successful flows unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3507b71f-f311-47e7-bd7e-8b95da132343
📒 Files selected for processing (11)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0009_migrate_session_secret.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (5)
- src/pages/_app.tsx
- server/index.ts
- cypress/config/settings.cypress.json
- src/context/SettingsContext.tsx
- server/interfaces/api/settingsInterfaces.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/hooks/usePlexLogin.ts
- server/lib/settings/migrations/0009_migrate_session_secret.ts
- seerr-api.yml
0xSysR3ll
force-pushed
the
fix/0xsysr3ll/plex-oauth-client-id
branch
from
c8756e5 to
e2626eb
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/plex.ts (1)
82-117:⚠️ Potential issue | 🟠 MajorFail fast when the popup never opens.
If
window.open()is blocked orpreparePopup()was skipped, Lines 111-117 still fall through topinPoll().pinPoll()treatsthis.popup === undefinedas “not closed”, so the login flow polls forever instead of returning an actionable error.💡 Suggested fix
if (!this.plexHeaders || !this.pin) { throw new Error('Unable to call login if class is not initialized.'); } @@ - if (this.popup) { - this.popup.location.href = `https://app.plex.tv/auth/#!?${this.encodeData( - params - )}`; - } + if (!this.popup || this.popup.closed) { + this.closePopup(); + throw new Error( + 'Unable to open Plex login popup. Please allow popups and try again.' + ); + } + + this.popup.location.href = `https://app.plex.tv/auth/#!?${this.encodeData( + params + )}`; return this.pinPoll();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/plex.ts` around lines 82 - 117, The login method can hang because if the popup wasn't created (this.popup is undefined due to window.open being blocked or preparePopup skipped), the method proceeds to call pinPoll which treats undefined popup as "not closed" and polls forever; update login (after getPin and before calling pinPoll) to check that this.popup exists and is not closed, and if it doesn't exist or is blocked throw a descriptive error (e.g., "Popup blocked or not opened") and call closePopup as needed so the caller gets an actionable failure instead of an infinite poll; reference the login, getPin, pinPoll, this.popup and closePopup symbols when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/utils/plex.ts`:
- Around line 82-117: The login method can hang because if the popup wasn't
created (this.popup is undefined due to window.open being blocked or
preparePopup skipped), the method proceeds to call pinPoll which treats
undefined popup as "not closed" and polls forever; update login (after getPin
and before calling pinPoll) to check that this.popup exists and is not closed,
and if it doesn't exist or is blocked throw a descriptive error (e.g., "Popup
blocked or not opened") and call closePopup as needed so the caller gets an
actionable failure instead of an infinite poll; reference the login, getPin,
pinPoll, this.popup and closePopup symbols when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1421cd12-3b1f-4140-82b3-ac73c86447f6
📒 Files selected for processing (11)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0009_migrate_session_secret.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (5)
- server/interfaces/api/settingsInterfaces.ts
- src/pages/_app.tsx
- src/hooks/usePlexLogin.ts
- src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
- server/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- server/lib/settings/migrations/0009_migrate_session_secret.ts
- src/context/SettingsContext.tsx
- seerr-api.yml
fallenbagel
requested changes
Mar 29, 2026
Collaborator
fallenbagel
left a comment
fallenbagel
left a comment
There was a problem hiding this comment.
Overall looks good. A few comments
0xSysR3ll
force-pushed
the
fix/0xsysr3ll/plex-oauth-client-id
branch
from
e2626eb to
a289811
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
server/lib/settings/index.ts (1)
759-761:⚠️ Potential issue | 🟡 Minor
sessionSecretcan still be undefined on the override load path.
load(overrideSettings)returns before the missing-secret backfill runs, so the non-null assertion inget sessionSecret()can still returnundefinedat runtime for override payloads withoutsessionSecret.Suggested fix
public async load( overrideSettings?: AllSettings, raw = false ): Promise<Settings> { if (overrideSettings) { this.data = overrideSettings; + if (!this.data.sessionSecret) { + this.data.sessionSecret = randomBytes(32).toString('hex'); + } return this; }Also applies to: 798-800
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/lib/settings/index.ts` around lines 759 - 761, The sessionSecret getter uses a non-null assertion but load(overrideSettings) can return before the backfill runs, so change get sessionSecret() to safely handle undefined: either throw a clear error if this.data.sessionSecret is missing or synchronously invoke the existing backfill/ensure routine (e.g., call an ensureSessionSecret()/backfillSessionSecret() helper or run the same logic used after load) and then return the value; do the same fix for the other affected getter referenced in the comment (replace the `!` with a guarded check and recovery/throw). Ensure you reference the sessionSecret getter and load(overrideSettings) behavior when implementing the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@server/lib/settings/index.ts`:
- Around line 759-761: The sessionSecret getter uses a non-null assertion but
load(overrideSettings) can return before the backfill runs, so change get
sessionSecret() to safely handle undefined: either throw a clear error if
this.data.sessionSecret is missing or synchronously invoke the existing
backfill/ensure routine (e.g., call an
ensureSessionSecret()/backfillSessionSecret() helper or run the same logic used
after load) and then return the value; do the same fix for the other affected
getter referenced in the comment (replace the `!` with a guarded check and
recovery/throw). Ensure you reference the sessionSecret getter and
load(overrideSettings) behavior when implementing the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85c9e0ee-0d24-4c5d-8375-68c1a5f1dd65
📒 Files selected for processing (11)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tsserver/lib/settings/migrations/0009_migrate_session_secret.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (3)
- src/pages/_app.tsx
- server/index.ts
- server/lib/settings/migrations/0009_migrate_session_secret.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- server/interfaces/api/settingsInterfaces.ts
- src/hooks/usePlexLogin.ts
- src/context/SettingsContext.tsx
fallenbagel
requested changes
Mar 30, 2026
0xSysR3ll
force-pushed
the
fix/0xsysr3ll/plex-oauth-client-id
branch
from
a289811 to
fd3e279
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cypress/config/settings.cypress.json (1)
3-3: Consider using a hex-format test secret for consistency with production.The production code generates
sessionSecretas a 64-character hex string viarandomBytes(32).toString('hex'). While express-session accepts any string and this won't cause functional issues, using a consistent format in test fixtures can help catch format-related bugs earlier.Suggested change
- "sessionSecret": "a1b2c3d4-e5f6-4789-a012-3456789abcde", + "sessionSecret": "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",Note: The static analysis hint flagging this as an "API key" is a false positive — this is an intentional test fixture value, not a leaked production secret.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypress/config/settings.cypress.json` at line 3, Replace the test fixture sessionSecret value with a 64-character hex string to match production format (generated by randomBytes(32).toString('hex')), i.e., update the "sessionSecret" entry so its value is a 64-character hex string (characters 0-9a-f) to ensure format parity with production; reference the "sessionSecret" key when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cypress/config/settings.cypress.json`:
- Line 3: Replace the test fixture sessionSecret value with a 64-character hex
string to match production format (generated by
randomBytes(32).toString('hex')), i.e., update the "sessionSecret" entry so its
value is a 64-character hex string (characters 0-9a-f) to ensure format parity
with production; reference the "sessionSecret" key when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77793b42-4cd6-4255-a3c3-e35e2d4b3e65
📒 Files selected for processing (10)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (3)
- src/pages/_app.tsx
- server/index.ts
- server/interfaces/api/settingsInterfaces.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/hooks/usePlexLogin.ts
- src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
- src/context/SettingsContext.tsx
- seerr-api.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/settings/index.ts`:
- Around line 389-392: The constructor currently pre-populates sessionSecret
which prevents the backfill in load()/mergeSettings() from detecting a missing
value; modify the backfill logic (the check around this.data.sessionSecret
inside load()/mergeSettings()) to generate a new sessionSecret there (e.g., call
randomBytes(32).toString('hex') when sessionSecret is falsy), set change = true,
and call save() so the generated secret is persisted; alternatively remove
sessionSecret generation from the constructor so the backfill detection sees it
as missing—update either the constructor or the load()/mergeSettings() backfill
branch and ensure save() is invoked when a new sessionSecret is created.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2be591da-7f5f-4366-9297-d73cd77a7a5e
📒 Files selected for processing (10)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (6)
- server/index.ts
- src/pages/_app.tsx
- server/interfaces/api/settingsInterfaces.ts
- src/context/SettingsContext.tsx
- src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
- cypress/config/settings.cypress.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/hooks/usePlexLogin.ts
- seerr-api.yml
0xSysR3ll
force-pushed
the
fix/0xsysr3ll/plex-oauth-client-id
branch
from
90c1168 to
a955d65
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/utils/plex.ts (1)
111-119:⚠️ Potential issue | 🟡 MinorTreat a closed popup the same as a missing popup.
preparePopup()andlogin()are separated by a 1.5s timeout insrc/hooks/usePlexLogin.ts:27-31, sothis.popupcan still be defined but already closed by the time this branch runs. In that case we still try to drive a dead window instead of returning the actionable popup error immediately.Suggested change
- if (!this.popup) { + if (!this.popup || this.popup.closed) { + this.closePopup(); throw new Error( 'Unable to open the Plex login window. Please allow popups for this site and try again.' ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/plex.ts` around lines 111 - 119, The login flow can attempt to use a popup that exists but is already closed between preparePopup() and login(); update the login method (where this.popup is referenced) to treat a closed popup like a missing one by checking this.popup.closed (or equivalent) before setting this.popup.location.href and, if closed, throw the same actionable Error ('Unable to open the Plex login window. Please allow popups for this site and try again.') so the caller receives the correct popup error instead of trying to drive a dead window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/utils/plex.ts`:
- Around line 111-119: The login flow can attempt to use a popup that exists but
is already closed between preparePopup() and login(); update the login method
(where this.popup is referenced) to treat a closed popup like a missing one by
checking this.popup.closed (or equivalent) before setting
this.popup.location.href and, if closed, throw the same actionable Error
('Unable to open the Plex login window. Please allow popups for this site and
try again.') so the caller receives the correct popup error instead of trying to
drive a dead window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b71ff1c0-480a-450d-9c23-349c2454d1b0
📒 Files selected for processing (10)
cypress/config/settings.cypress.jsonseerr-api.ymlserver/index.tsserver/interfaces/api/settingsInterfaces.tsserver/lib/settings/index.tssrc/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsxsrc/context/SettingsContext.tsxsrc/hooks/usePlexLogin.tssrc/pages/_app.tsxsrc/utils/plex.ts
✅ Files skipped from review due to trivial changes (5)
- src/pages/_app.tsx
- server/interfaces/api/settingsInterfaces.ts
- cypress/config/settings.cypress.json
- server/index.ts
- src/context/SettingsContext.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/hooks/usePlexLogin.ts
- src/components/UserProfile/UserSettings/UserLinkedAccountsSettings/index.tsx
- seerr-api.yml
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.
Description
This PR fixes an issue where Plex Oauth login could fail because the browser and server were using different client identifiers.
The browser generated its own ID (
plex-client-id, stored in localStorage), while the server usedclientId(stored insettings.json. Now both use the same value:plexClientIdentifierAs part of this PR, I also discovered a security issue:
clientIdwas being used to sign sessions even though it is a public value.To address this, this PR introduces a new
sessionSecretdedicated to session signing.Warning
Introducing this new session secret will obviously invalidate all existing sessions, meaning all users will be logged out.
How Has This Been Tested?
(Not tested yet but in theory should work).
Screenshots / Logs (if applicable)
Checklist:
pnpm buildpnpm i18n:extractSummary by CodeRabbit