feat(permissions): add granular Advanced Request permission for language profile#3201
Open
Wr0ngName wants to merge 1 commit into
Open
feat(permissions): add granular Advanced Request permission for language profile#3201Wr0ngName wants to merge 1 commit into
Wr0ngName wants to merge 1 commit into
Conversation
…age profile Introduces a new `REQUEST_ADVANCED_LANGUAGE` permission bit so admins can grant access to override only the Sonarr language profile on series requests, without also unlocking quality profile, root folder, tags, or destination server changes. Behavior: - Existing users with `REQUEST_ADVANCED` or `MANAGE_REQUESTS` continue to see and edit every Advanced field unchanged (OR-checks on every gate). - A user holding only `REQUEST_ADVANCED_LANGUAGE` (plus a base request permission) sees the Advanced section on TV requests with only the Language Profile dropdown. - `PermissionEdit` nests the new permission as a child of `REQUEST_ADVANCED`, so toggling the parent in the user-edit UI grants the granular bit via the existing parent-grants-children rule. Partially addresses seerr-team#2248. Follow-up PRs will add the remaining four sub-permissions (server, quality, folder, tags) using the same pattern, and harden the request POST/PUT endpoints which today rely on UI-only gating for Advanced fields.
Contributor
📝 WalkthroughWalkthroughAdds ChangesAdvanced Request Language Permission
Sequence Diagram(s)sequenceDiagram
participant TvRequestModal
participant hasPermission
participant AdvancedRequester
TvRequestModal->>hasPermission: check MANAGE_REQUESTS, REQUEST_ADVANCED, REQUEST_ADVANCED_LANGUAGE
hasPermission-->>TvRequestModal: permission result
TvRequestModal->>AdvancedRequester: render when allowed
AdvancedRequester->>hasPermission: compute canEditLegacyAdvancedFields
hasPermission-->>AdvancedRequester: legacy-compatible result
AdvancedRequester-->>TvRequestModal: show server, profile, root folder, and tags selectors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/components/RequestModal/AdvancedRequester/index.tsx`:
- Around line 365-513: The Advanced section guard in AdvancedRequester is still
using legacy counts even when canEditLegacyAdvancedFields hides those fields, so
compute visibility from only the options the user can actually see. Update the
section-level condition around the server/profile/root folder/tag controls to
derive its “has content” check from the same permission-aware flags used in the
rendered selects, ensuring a language-only user with no language options does
not get an empty Advanced section. Focus on the visibility logic near the
existing canEditLegacyAdvancedFields checks and the currentHasPermission gate.
- Around line 118-121: The permission check in AdvancedRequester should not only
hide legacy fields in the UI; it must also prevent hidden
server/profile/folder/tags values from being emitted in the onChange payload.
Update AdvancedRequester’s submit/change handling (and any helper that builds
the request payload) so that when canEditLegacyAdvancedFields is false, those
legacy fields are stripped before passing data to TvRequestModal, while
preserving language-only values.
🪄 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: e3af3b7b-38bc-4432-960b-45fa7946d73d
📒 Files selected for processing (5)
server/lib/permissions.tssrc/components/PermissionEdit/index.tsxsrc/components/RequestModal/AdvancedRequester/index.tsxsrc/components/RequestModal/TvRequestModal.tsxsrc/i18n/locale/en.json
Comment on lines
+118
to
+121
| const canEditLegacyAdvancedFields = currentHasPermission( | ||
| [Permission.MANAGE_REQUESTS, Permission.REQUEST_ADVANCED], | ||
| { type: 'or' } | ||
| ); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Gate emitted overrides, not only rendered fields.
With language-only access, this component hides server/profile/folder/tags, but the existing onChange payload still emits those hidden values and TvRequestModal forwards them. That undermines the “language only” permission split unless every hidden field is also stripped before submit.
Proposed client-side guard
useEffect(() => {
if (selectedServer !== null || selectedUser) {
onChange({
- folder: selectedFolder !== '' ? selectedFolder : undefined,
- profile: selectedProfile !== -1 ? selectedProfile : undefined,
- server: selectedServer ?? undefined,
+ ...(canEditLegacyAdvancedFields
+ ? {
+ folder: selectedFolder !== '' ? selectedFolder : undefined,
+ profile: selectedProfile !== -1 ? selectedProfile : undefined,
+ server: selectedServer ?? undefined,
+ tags: selectedTags,
+ }
+ : {}),
user: selectedUser ?? undefined,
language: selectedLanguage !== -1 ? selectedLanguage : undefined,
- tags: selectedTags,
ignoreQuota: isIgnoreQuotaVisible && ignoreQuota ? true : undefined,
});
}
}, [
@@
selectedTags,
ignoreQuota,
isIgnoreQuotaVisible,
+ canEditLegacyAdvancedFields,
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const canEditLegacyAdvancedFields = currentHasPermission( | |
| [Permission.MANAGE_REQUESTS, Permission.REQUEST_ADVANCED], | |
| { type: 'or' } | |
| ); | |
| useEffect(() => { | |
| if (selectedServer !== null || selectedUser) { | |
| onChange({ | |
| ...(canEditLegacyAdvancedFields | |
| ? { | |
| folder: selectedFolder !== '' ? selectedFolder : undefined, | |
| profile: selectedProfile !== -1 ? selectedProfile : undefined, | |
| server: selectedServer ?? undefined, | |
| tags: selectedTags, | |
| } | |
| : {}), | |
| user: selectedUser ?? undefined, | |
| language: selectedLanguage !== -1 ? selectedLanguage : undefined, | |
| ignoreQuota: isIgnoreQuotaVisible && ignoreQuota ? true : undefined, | |
| }); | |
| } | |
| }, [ | |
| selectedFolder, | |
| selectedProfile, | |
| selectedServer, | |
| selectedUser, | |
| selectedLanguage, | |
| selectedTags, | |
| ignoreQuota, | |
| isIgnoreQuotaVisible, | |
| canEditLegacyAdvancedFields, | |
| ]); |
🤖 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 `@src/components/RequestModal/AdvancedRequester/index.tsx` around lines 118 -
121, The permission check in AdvancedRequester should not only hide legacy
fields in the UI; it must also prevent hidden server/profile/folder/tags values
from being emitted in the onChange payload. Update AdvancedRequester’s
submit/change handling (and any helper that builds the request payload) so that
when canEditLegacyAdvancedFields is false, those legacy fields are stripped
before passing data to TvRequestModal, while preserving language-only values.
Comment on lines
+365
to
+513
| {canEditLegacyAdvancedFields && | ||
| data.filter((server) => server.is4k === is4k).length > 1 && ( | ||
| <div className="mb-3 w-full flex-shrink-0 flex-grow last:pr-0 md:w-1/4 md:pr-4"> | ||
| <label htmlFor="server"> | ||
| {intl.formatMessage(messages.destinationserver)} | ||
| </label> | ||
| <select | ||
| id="server" | ||
| name="server" | ||
| value={selectedServer} | ||
| onChange={(e) => setSelectedServer(Number(e.target.value))} | ||
| onBlur={(e) => setSelectedServer(Number(e.target.value))} | ||
| className="border-gray-700 bg-gray-800" | ||
| > | ||
| {data | ||
| .filter((server) => server.is4k === is4k) | ||
| .map((server) => ( | ||
| <option | ||
| key={`profile-list${profile.id}`} | ||
| value={profile.id} | ||
| key={`server-list-${server.id}`} | ||
| value={server.id} | ||
| > | ||
| {isAnime && | ||
| serverData.server.activeAnimeProfileId === profile.id | ||
| {server.isDefault | ||
| ? intl.formatMessage(messages.default, { | ||
| name: profile.name, | ||
| name: server.name, | ||
| }) | ||
| : !isAnime && | ||
| serverData.server.activeProfileId === profile.id | ||
| : server.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {canEditLegacyAdvancedFields && | ||
| (isValidating || | ||
| !serverData || | ||
| serverData.profiles.length > 1) && ( | ||
| <div className="mb-3 w-full flex-shrink-0 flex-grow last:pr-0 md:w-1/4 md:pr-4"> | ||
| <label htmlFor="profile"> | ||
| {intl.formatMessage(messages.qualityprofile)} | ||
| </label> | ||
| <select | ||
| id="profile" | ||
| name="profile" | ||
| value={selectedProfile} | ||
| onChange={(e) => setSelectedProfile(Number(e.target.value))} | ||
| onBlur={(e) => setSelectedProfile(Number(e.target.value))} | ||
| className="border-gray-700 bg-gray-800" | ||
| disabled={isValidating || !serverData} | ||
| > | ||
| {(isValidating || !serverData) && ( | ||
| <option value=""> | ||
| {intl.formatMessage(globalMessages.loading)} | ||
| </option> | ||
| )} | ||
| {!isValidating && | ||
| serverData && | ||
| serverData.profiles | ||
| .toSorted((a, b) => | ||
| a.name.localeCompare(b.name, intl.locale, { | ||
| numeric: true, | ||
| sensitivity: 'base', | ||
| }) | ||
| ) | ||
| .map((profile) => ( | ||
| <option | ||
| key={`profile-list${profile.id}`} | ||
| value={profile.id} | ||
| > | ||
| {isAnime && | ||
| serverData.server.activeAnimeProfileId === | ||
| profile.id | ||
| ? intl.formatMessage(messages.default, { | ||
| name: profile.name, | ||
| }) | ||
| : profile.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {(isValidating || | ||
| !serverData || | ||
| serverData.rootFolders.length > 1) && ( | ||
| <div className="mb-3 w-full flex-shrink-0 flex-grow last:pr-0 md:w-1/4 md:pr-4"> | ||
| <label htmlFor="folder"> | ||
| {intl.formatMessage(messages.rootfolder)} | ||
| </label> | ||
| <select | ||
| id="folder" | ||
| name="folder" | ||
| value={selectedFolder} | ||
| onChange={(e) => setSelectedFolder(e.target.value)} | ||
| onBlur={(e) => setSelectedFolder(e.target.value)} | ||
| className="border-gray-700 bg-gray-800" | ||
| disabled={isValidating || !serverData} | ||
| > | ||
| {(isValidating || !serverData) && ( | ||
| <option value=""> | ||
| {intl.formatMessage(globalMessages.loading)} | ||
| </option> | ||
| )} | ||
| {!isValidating && | ||
| serverData && | ||
| serverData.rootFolders.map((folder) => ( | ||
| <option | ||
| key={`folder-list${folder.id}`} | ||
| value={folder.path} | ||
| > | ||
| {isAnime && | ||
| serverData.server.activeAnimeDirectory === folder.path | ||
| ? intl.formatMessage(messages.default, { | ||
| name: intl.formatMessage(messages.folder, { | ||
| path: folder.path, | ||
| space: formatBytes(folder.freeSpace ?? 0), | ||
| }), | ||
| }) | ||
| : !isAnime && | ||
| serverData.server.activeDirectory === folder.path | ||
| : !isAnime && | ||
| serverData.server.activeProfileId === | ||
| profile.id | ||
| ? intl.formatMessage(messages.default, { | ||
| name: profile.name, | ||
| }) | ||
| : profile.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {canEditLegacyAdvancedFields && | ||
| (isValidating || | ||
| !serverData || | ||
| serverData.rootFolders.length > 1) && ( | ||
| <div className="mb-3 w-full flex-shrink-0 flex-grow last:pr-0 md:w-1/4 md:pr-4"> | ||
| <label htmlFor="folder"> | ||
| {intl.formatMessage(messages.rootfolder)} | ||
| </label> | ||
| <select | ||
| id="folder" | ||
| name="folder" | ||
| value={selectedFolder} | ||
| onChange={(e) => setSelectedFolder(e.target.value)} | ||
| onBlur={(e) => setSelectedFolder(e.target.value)} | ||
| className="border-gray-700 bg-gray-800" | ||
| disabled={isValidating || !serverData} | ||
| > | ||
| {(isValidating || !serverData) && ( | ||
| <option value=""> | ||
| {intl.formatMessage(globalMessages.loading)} | ||
| </option> | ||
| )} | ||
| {!isValidating && | ||
| serverData && | ||
| serverData.rootFolders.map((folder) => ( | ||
| <option | ||
| key={`folder-list${folder.id}`} | ||
| value={folder.path} | ||
| > | ||
| {isAnime && | ||
| serverData.server.activeAnimeDirectory === folder.path | ||
| ? intl.formatMessage(messages.default, { | ||
| name: intl.formatMessage(messages.folder, { | ||
| path: folder.path, | ||
| space: formatBytes(folder.freeSpace ?? 0), | ||
| }), | ||
| }) | ||
| : intl.formatMessage(messages.folder, { | ||
| path: folder.path, | ||
| space: formatBytes(folder.freeSpace ?? 0), | ||
| })} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| : !isAnime && | ||
| serverData.server.activeDirectory === | ||
| folder.path | ||
| ? intl.formatMessage(messages.default, { | ||
| name: intl.formatMessage(messages.folder, { | ||
| path: folder.path, | ||
| space: formatBytes(folder.freeSpace ?? 0), | ||
| }), | ||
| }) | ||
| : intl.formatMessage(messages.folder, { | ||
| path: folder.path, | ||
| space: formatBytes(folder.freeSpace ?? 0), | ||
| })} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {type === 'tv' && | ||
| currentHasPermission( | ||
| [ | ||
| Permission.MANAGE_REQUESTS, | ||
| Permission.REQUEST_ADVANCED, | ||
| Permission.REQUEST_ADVANCED_LANGUAGE, | ||
| ], | ||
| { type: 'or' } | ||
| ) && |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the “Advanced” section visibility permission-aware.
The earlier null-return guard still counts profile/root/server/tag options even when canEditLegacyAdvancedFields hides them. A language-only user with no language choices but multiple hidden legacy choices can get an empty Advanced section. Base the section guard on visible options only.
🤖 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 `@src/components/RequestModal/AdvancedRequester/index.tsx` around lines 365 -
513, The Advanced section guard in AdvancedRequester is still using legacy
counts even when canEditLegacyAdvancedFields hides those fields, so compute
visibility from only the options the user can actually see. Update the
section-level condition around the server/profile/root folder/tag controls to
derive its “has content” check from the same permission-aware flags used in the
rendered selects, ensuring a language-only user with no language options does
not get an empty Advanced section. Focus on the visibility logic near the
existing canEditLegacyAdvancedFields checks and the currentHasPermission gate.
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
1 participant
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
Splits the all-or-nothing
REQUEST_ADVANCEDpermission so administrators can grant a user the ability to override only the Sonarr language profile on series requests, without simultaneously unlocking quality profile, root folder, tags, or destination server.The driving use case is a multi-user / family setup where one user needs to fetch foreign-language audio (e.g. French dubs for a child) on TV requests while keeping defaults for everything else.
This PR is a deliberately small first slice and lays a pattern the remaining four sub-permissions can follow in subsequent PRs to fully close the issue.
How it works:
REQUEST_ADVANCED_LANGUAGEpermission bit added to the existing bitmask enum (uses the previously-unused bit536870912, safely within JS's 32-bit signed range).PermissionEdit, the new bit is nested as a child ofREQUEST_ADVANCED, so toggling the parent grants the child via the existing parent-grants-children rule inPermissionOption.AdvancedRequester, every field's render condition gains an OR-check against the new bit, the legacyREQUEST_ADVANCED, andMANAGE_REQUESTS. Legacy users withREQUEST_ADVANCEDkeep the exact same experience; a user holding only the new granular bit sees just the Language Profile dropdown.TvRequestModalis widened to admit the granular-only user.No DB migration, no API contract change. The other four Advanced fields (server / quality / folder / tags) are now gated by the legacy
REQUEST_ADVANCEDinsideAdvancedRequester, so widening the outer gate does not leak those fields to a granular-only user.Out of scope (will be tracked separately): server-side enforcement on the request POST/PUT endpoints. Today,
REQUEST_ADVANCEDis enforced UI-side only on create — a crafted POST already bypasses the gate. That predates this change.How Has This Been Tested?
Tested manually against a local dev instance (
pnpm devvia the project'sDockerfile.local) connected to a Sonarr server with multiple language profiles, multiple quality profiles, multiple root folders, and several tags.Scenarios exercised:
REQUEST_ADVANCEDbit set): Opened a TV request modal. All five Advanced fields render exactly as ondevelop(verified side-by-side). Submitted a request with non-default language; Sonarr received the correctlanguageProfileId.REQUEST+REQUEST_TV+REQUEST_ADVANCED_LANGUAGE): Opened a TV request modal. Advanced section appears with only the Language Profile dropdown — quality, folder, tags, server are all hidden. Submitted a request; Sonarr received the override.REQUEST+REQUEST_TVonly): No Advanced section rendered.REQUEST_ADVANCEDparent checkbox; the newREQUEST_ADVANCED_LANGUAGEchild becomes checked and disabled (existing parent-grants-children semantics). Toggled the child alone with the parent unchecked; works independently.CI checks (
pnpm lint,pnpm format:check,pnpm build,pnpm i18n:extract) run locally in the project's pinned Node 22 alpine container before pushing.Screenshots / Logs (if applicable)
N/A — UI changes are conditional renders only; no visual change for legacy users. Will add screenshots on request.
Checklist:
pnpm buildpnpm i18n:extractAI Disclosure: AI-assisted code generation, fully human-reviewed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation