From a801cfdb0946cbee3c35b7e917a240f69020f221 Mon Sep 17 00:00:00 2001 From: Jamie Rees Date: Sun, 9 Mar 2025 22:01:10 +0000 Subject: [PATCH 1/2] fix(user-import): Do not import users that do not have access to the server #5064 --- .../Models/Friends/PlexFriends.cs | 12 ++ .../PlexUserImporterTests.cs | 132 ++++++++++++++++-- .../Jobs/Plex/PlexUserImporter.cs | 7 + 3 files changed, 142 insertions(+), 9 deletions(-) diff --git a/src/Ombi.Api.Plex/Models/Friends/PlexFriends.cs b/src/Ombi.Api.Plex/Models/Friends/PlexFriends.cs index 1e597d916..c0442da03 100644 --- a/src/Ombi.Api.Plex/Models/Friends/PlexFriends.cs +++ b/src/Ombi.Api.Plex/Models/Friends/PlexFriends.cs @@ -22,6 +22,18 @@ namespace Ombi.Api.Plex.Models.Friends /// [XmlAttribute(AttributeName = "home")] public bool HomeUser { get; set; } + + [XmlElement(ElementName = "Server")] + public PlexUserServer[] Server { get; set; } + } + + public class PlexUserServer + { + [XmlAttribute(AttributeName = "id")] + public string Id { get; set; } + + [XmlAttribute(AttributeName = "serverId")] + public string ServerId { get; set; } } [XmlRoot(ElementName = "MediaContainer")] diff --git a/src/Ombi.Schedule.Tests/PlexUserImporterTests.cs b/src/Ombi.Schedule.Tests/PlexUserImporterTests.cs index 022190bd2..f03d0e7e8 100644 --- a/src/Ombi.Schedule.Tests/PlexUserImporterTests.cs +++ b/src/Ombi.Schedule.Tests/PlexUserImporterTests.cs @@ -182,8 +182,6 @@ namespace Ombi.Schedule.Tests _mocker.Verify(x => x.UpdateAsync(It.IsAny()), Times.Never); } - - [Test] public async Task Import_Doesnt_Import_Banned_Users() { @@ -247,7 +245,15 @@ namespace Ombi.Schedule.Tests Id = "id", Title = "title", Username = "username", - HomeUser = true + HomeUser = true, + Server = new PlexUserServer[] + { + new PlexUserServer + { + Id = "1", + ServerId = "123" + } + } } } }); @@ -257,7 +263,6 @@ namespace Ombi.Schedule.Tests _mocker.Setup>(x => x.AddToRoleAsync(It.Is(x => x.UserName == "plex"), OmbiRoles.RequestMovie)) .ReturnsAsync(IdentityResult.Success); - await _subject.Execute(null); _mocker.Verify(x => x.CreateAsync(It.IsAny()), Times.Once); @@ -306,7 +311,15 @@ namespace Ombi.Schedule.Tests { Email = "email", Id = "id", - Username = "plex" + Username = "plex", + Server = new PlexUserServer[] + { + new PlexUserServer + { + Id = "1", + ServerId = "123" + } + } } } }); @@ -331,9 +344,9 @@ namespace Ombi.Schedule.Tests ImportPlexAdmin = false, ImportPlexUsers = true, DefaultRoles = new List - { - OmbiRoles.RequestMovie - } + { + OmbiRoles.RequestMovie + } }); _mocker.Setup>(x => x.GetUsers(It.IsAny())).ReturnsAsync(new PlexUsers { @@ -343,7 +356,15 @@ namespace Ombi.Schedule.Tests { Email = "email", Id = "PLEX_ID", - Username = "user" + Username = "user", + Server = new PlexUserServer[] + { + new PlexUserServer + { + Id = "1", + ServerId = "123" + } + } } } }); @@ -440,5 +461,98 @@ namespace Ombi.Schedule.Tests _mocker.Verify(x => x.DeleteUser(It.Is(x => x.ProviderUserId == "ADMIN_ID" && x.Email == "ADMIN@ADMIN.CO" && x.UserName == "Admin")), Times.Never); } + + [Test] + public async Task Import_Skips_Users_Without_Server_Access() + { + _mocker.Setup, Task>(x => x.GetSettingsAsync()) + .ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true }); + _mocker.Setup>(x => x.GetUsers(It.IsAny())).ReturnsAsync(new PlexUsers + { + User = new UserFriends[] + { + new UserFriends + { + Email = "email", + Id = "NoServer", + Title = "title", + Username = "username", + Server = null + } + } + }); + + await _subject.Execute(null); + + _mocker.Verify(x => x.CreateAsync(It.IsAny()), Times.Never); + } + + [Test] + public async Task Import_Skips_Users_With_Empty_Server_Array() + { + _mocker.Setup, Task>(x => x.GetSettingsAsync()) + .ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true }); + _mocker.Setup>(x => x.GetUsers(It.IsAny())).ReturnsAsync(new PlexUsers + { + User = new UserFriends[] + { + new UserFriends + { + Email = "email", + Id = "EmptyServer", + Title = "title", + Username = "username", + Server = new PlexUserServer[0] + } + } + }); + + await _subject.Execute(null); + + _mocker.Verify(x => x.CreateAsync(It.IsAny()), Times.Never); + } + + [Test] + public async Task Import_Creates_User_With_Server_Access() + { + _mocker.Setup, Task>(x => x.GetSettingsAsync()) + .ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true }); + _mocker.Setup>(x => x.GetUsers(It.IsAny())).ReturnsAsync(new PlexUsers + { + User = new UserFriends[] + { + new UserFriends + { + Email = "email", + Id = "HasServer", + Title = "title", + Username = "username", + Server = new PlexUserServer[] + { + new PlexUserServer + { + Id = "1", + ServerId = "123" + } + } + } + } + }); + + _mocker.Setup>(x => x.CreateAsync(It.Is(x => + x.UserName == "username" && + x.Email == "email" && + x.ProviderUserId == "HasServer" && + x.UserType == UserType.PlexUser))) + .ReturnsAsync(IdentityResult.Success); + + await _subject.Execute(null); + + _mocker.Verify(x => x.CreateAsync(It.Is(x => + x.UserName == "username" && + x.Email == "email" && + x.ProviderUserId == "HasServer" && + x.UserType == UserType.PlexUser)), Times.Once); + } } } diff --git a/src/Ombi.Schedule/Jobs/Plex/PlexUserImporter.cs b/src/Ombi.Schedule/Jobs/Plex/PlexUserImporter.cs index 6c90dd20f..bf89e8fa0 100644 --- a/src/Ombi.Schedule/Jobs/Plex/PlexUserImporter.cs +++ b/src/Ombi.Schedule/Jobs/Plex/PlexUserImporter.cs @@ -120,6 +120,13 @@ namespace Ombi.Schedule.Jobs.Plex foreach (var plexUser in users.User) { + // Skip users without server access + if (plexUser.Server == null || !plexUser.Server.Any()) + { + _log.LogInformation($"Skipping user {plexUser.Username ?? plexUser.Id} as they have no server access"); + continue; + } + // Check if we should import this user if (userManagementSettings.BannedPlexUserIds.Contains(plexUser.Id)) { From fe2fe24158067e26a4fda78992efa5c1137c5877 Mon Sep 17 00:00:00 2001 From: Jamie Rees Date: Sun, 9 Mar 2025 22:02:43 +0000 Subject: [PATCH 2/2] update cache --- .github/workflows/automation-tests.yml | 2 +- .github/workflows/build.yml | 6 +++--- .github/workflows/chromatic.yml | 2 +- .github/workflows/pr.yml | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/automation-tests.yml b/.github/workflows/automation-tests.yml index 97572ad25..5c40edd58 100644 --- a/.github/workflows/automation-tests.yml +++ b/.github/workflows/automation-tests.yml @@ -24,7 +24,7 @@ jobs: with: node-version: '18' - - uses: actions/cache@v2 + - uses: actions/cache@v4 with: path: | '**/node_modules' diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 54b623518..998b8387c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,7 +15,7 @@ jobs: node-version: '18' - name: NodeModules Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: '**/node_modules' key: node_modules-${{ hashFiles('**/yarn.lock') }} @@ -42,7 +42,7 @@ jobs: dotnet-version: '8.0.x' - name: Nuget Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.nuget/packages key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }} @@ -112,7 +112,7 @@ jobs: dotnet-version: '5.0.x' - name: Nuget Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.nuget/packages key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }} diff --git a/.github/workflows/chromatic.yml b/.github/workflows/chromatic.yml index f068794be..d142c46c8 100644 --- a/.github/workflows/chromatic.yml +++ b/.github/workflows/chromatic.yml @@ -17,7 +17,7 @@ # fetch-depth: 0 # - name: NodeModules Cache -# uses: actions/cache@v2 +# uses: actions/cache@v4 # with: # path: '**/node_modules' # key: node_modules-${{ hashFiles('**/yarn.lock') }} diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index ea65cfaa6..5a0e0a46d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -20,7 +20,7 @@ jobs: node-version: '18' - name: NodeModules Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: '**/node_modules' key: node_modules-${{ hashFiles('**/yarn.lock') }} @@ -41,7 +41,7 @@ jobs: dotnet-version: '8.0.x' - name: Nuget Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.nuget/packages key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }} @@ -101,7 +101,7 @@ jobs: dotnet-version: '8.0.x' - name: Nuget Cache - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.nuget/packages key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}