-
-
Notifications
You must be signed in to change notification settings - Fork 891
feat(permissions): add granular Advanced Request permission for language profile #3201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,15 @@ const AdvancedRequester = ({ | |
| currentHasPermission([Permission.MANAGE_REQUESTS]) && | ||
| ((type === 'movie' ? quota?.movie.limit : quota?.tv.limit) ?? 0) > 0; | ||
|
|
||
| // Users with the legacy "all-or-nothing" Advanced Request permission or | ||
| // Manage Requests see every advanced field. Users who only hold a granular | ||
| // sub-permission (e.g. REQUEST_ADVANCED_LANGUAGE) only see the matching | ||
| // field. This keeps backward compatibility while allowing per-field grants. | ||
| const canEditLegacyAdvancedFields = currentHasPermission( | ||
| [Permission.MANAGE_REQUESTS, Permission.REQUEST_ADVANCED], | ||
| { type: 'or' } | ||
| ); | ||
|
|
||
| const { data: serverData, isValidating } = | ||
| useSWR<ServiceCommonServerWithDetails>( | ||
| selectedServer !== null | ||
|
|
@@ -353,141 +362,155 @@ const AdvancedRequester = ({ | |
| <div className="rounded-md"> | ||
| {!!data && selectedServer !== null && ( | ||
| <div className="flex flex-col md:flex-row"> | ||
| {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={`server-list-${server.id}`} | ||
| value={server.id} | ||
| > | ||
| {server.isDefault | ||
| ? intl.formatMessage(messages.default, { | ||
| name: server.name, | ||
| }) | ||
| : server.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
| </div> | ||
| )} | ||
| {(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) => ( | ||
| {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' } | ||
| ) && | ||
|
Comment on lines
+365
to
+513
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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 🤖 Prompt for AI Agents |
||
| (isValidating || | ||
| !serverData || | ||
| (serverData.languageProfiles ?? []).length > 1) && ( | ||
|
|
@@ -540,7 +563,8 @@ const AdvancedRequester = ({ | |
| )} | ||
| </div> | ||
| )} | ||
| {selectedServer !== null && | ||
| {canEditLegacyAdvancedFields && | ||
| selectedServer !== null && | ||
| (isValidating || !serverData || !!serverData?.tags?.length) && ( | ||
| <div className="mb-2"> | ||
| <label htmlFor="tags">{intl.formatMessage(messages.tags)}</label> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 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
onChangepayload still emits those hidden values andTvRequestModalforwards 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
🤖 Prompt for AI Agents