Skip to content

Commit 9227bc0

Browse files
tidusjartidusjar
authored
Merge pull request #5219 from Ombi-app/fix-users
fix(user-import): Do not import users that do not have access to the server
2 parents 72af4f7 + fe2fe24 commit 9227bc0

File tree

7 files changed

+150
-17
lines changed

7 files changed

+150
-17
lines changed

‎.github/workflows/automation-tests.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
with:
2525
node-version: '18'
2626

27-
- uses: actions/cache@v2
27+
- uses: actions/cache@v4
2828
with:
2929
path: |
3030
'**/node_modules'

‎.github/workflows/build.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
node-version: '18'
1616

1717
- name: NodeModules Cache
18-
uses: actions/cache@v2
18+
uses: actions/cache@v4
1919
with:
2020
path: '**/node_modules'
2121
key: node_modules-${{ hashFiles('**/yarn.lock') }}
@@ -42,7 +42,7 @@ jobs:
4242
dotnet-version: '8.0.x'
4343

4444
- name: Nuget Cache
45-
uses: actions/cache@v2
45+
uses: actions/cache@v4
4646
with:
4747
path: ~/.nuget/packages
4848
key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}
@@ -112,7 +112,7 @@ jobs:
112112
dotnet-version: '5.0.x'
113113

114114
- name: Nuget Cache
115-
uses: actions/cache@v2
115+
uses: actions/cache@v4
116116
with:
117117
path: ~/.nuget/packages
118118
key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}

‎.github/workflows/chromatic.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
# fetch-depth: 0
1818

1919
# - name: NodeModules Cache
20-
# uses: actions/cache@v2
20+
# uses: actions/cache@v4
2121
# with:
2222
# path: '**/node_modules'
2323
# key: node_modules-${{ hashFiles('**/yarn.lock') }}

‎.github/workflows/pr.yml

+3-3
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ jobs:
2020
node-version: '18'
2121

2222
- name: NodeModules Cache
23-
uses: actions/cache@v2
23+
uses: actions/cache@v4
2424
with:
2525
path: '**/node_modules'
2626
key: node_modules-${{ hashFiles('**/yarn.lock') }}
@@ -41,7 +41,7 @@ jobs:
4141
dotnet-version: '8.0.x'
4242

4343
- name: Nuget Cache
44-
uses: actions/cache@v2
44+
uses: actions/cache@v4
4545
with:
4646
path: ~/.nuget/packages
4747
key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}
@@ -101,7 +101,7 @@ jobs:
101101
dotnet-version: '8.0.x'
102102

103103
- name: Nuget Cache
104-
uses: actions/cache@v2
104+
uses: actions/cache@v4
105105
with:
106106
path: ~/.nuget/packages
107107
key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}

‎src/Ombi.Api.Plex/Models/Friends/PlexFriends.cs

+12
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ public class UserFriends
2222
/// </summary>
2323
[XmlAttribute(AttributeName = "home")]
2424
public bool HomeUser { get; set; }
25+
26+
[XmlElement(ElementName = "Server")]
27+
public PlexUserServer[] Server { get; set; }
28+
}
29+
30+
public class PlexUserServer
31+
{
32+
[XmlAttribute(AttributeName = "id")]
33+
public string Id { get; set; }
34+
35+
[XmlAttribute(AttributeName = "serverId")]
36+
public string ServerId { get; set; }
2537
}
2638

2739
[XmlRoot(ElementName = "MediaContainer")]

‎src/Ombi.Schedule.Tests/PlexUserImporterTests.cs

+123-9
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,6 @@ public async Task Import_Only_Imports_Plex_Admin_Username_Clash()
182182
_mocker.Verify<OmbiUserManager>(x => x.UpdateAsync(It.IsAny<OmbiUser>()), Times.Never);
183183
}
184184

185-
186-
187185
[Test]
188186
public async Task Import_Doesnt_Import_Banned_Users()
189187
{
@@ -247,7 +245,15 @@ public async Task Imports_Unmanaged_Home_User()
247245
Id = "id",
248246
Title = "title",
249247
Username = "username",
250-
HomeUser = true
248+
HomeUser = true,
249+
Server = new PlexUserServer[]
250+
{
251+
new PlexUserServer
252+
{
253+
Id = "1",
254+
ServerId = "123"
255+
}
256+
}
251257
}
252258
}
253259
});
@@ -257,7 +263,6 @@ public async Task Imports_Unmanaged_Home_User()
257263
_mocker.Setup<OmbiUserManager, Task<IdentityResult>>(x => x.AddToRoleAsync(It.Is<OmbiUser>(x => x.UserName == "plex"), OmbiRoles.RequestMovie))
258264
.ReturnsAsync(IdentityResult.Success);
259265

260-
261266
await _subject.Execute(null);
262267

263268
_mocker.Verify<OmbiUserManager>(x => x.CreateAsync(It.IsAny<OmbiUser>()), Times.Once);
@@ -306,7 +311,15 @@ public async Task Import_Created_Plex_User()
306311
{
307312
Email = "email",
308313
Id = "id",
309-
Username = "plex"
314+
Username = "plex",
315+
Server = new PlexUserServer[]
316+
{
317+
new PlexUserServer
318+
{
319+
Id = "1",
320+
ServerId = "123"
321+
}
322+
}
310323
}
311324
}
312325
});
@@ -331,9 +344,9 @@ public async Task Import_Update_Plex_User()
331344
ImportPlexAdmin = false,
332345
ImportPlexUsers = true,
333346
DefaultRoles = new List<string>
334-
{
335-
OmbiRoles.RequestMovie
336-
}
347+
{
348+
OmbiRoles.RequestMovie
349+
}
337350
});
338351
_mocker.Setup<IPlexApi, Task<PlexUsers>>(x => x.GetUsers(It.IsAny<string>())).ReturnsAsync(new PlexUsers
339352
{
@@ -343,7 +356,15 @@ public async Task Import_Update_Plex_User()
343356
{
344357
Email = "email",
345358
Id = "PLEX_ID",
346-
Username = "user"
359+
Username = "user",
360+
Server = new PlexUserServer[]
361+
{
362+
new PlexUserServer
363+
{
364+
Id = "1",
365+
ServerId = "123"
366+
}
367+
}
347368
}
348369
}
349370
});
@@ -440,5 +461,98 @@ public async Task Import_Cleanup_Missing_Plex_Admin_Dont_Delete()
440461

441462
_mocker.Verify<IUserDeletionEngine>(x => x.DeleteUser(It.Is<OmbiUser>(x => x.ProviderUserId == "ADMIN_ID" && x.Email == "ADMIN@ADMIN.CO" && x.UserName == "Admin")), Times.Never);
442463
}
464+
465+
[Test]
466+
public async Task Import_Skips_Users_Without_Server_Access()
467+
{
468+
_mocker.Setup<ISettingsService<UserManagementSettings>, Task<UserManagementSettings>>(x => x.GetSettingsAsync())
469+
.ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true });
470+
_mocker.Setup<IPlexApi, Task<PlexUsers>>(x => x.GetUsers(It.IsAny<string>())).ReturnsAsync(new PlexUsers
471+
{
472+
User = new UserFriends[]
473+
{
474+
new UserFriends
475+
{
476+
Email = "email",
477+
Id = "NoServer",
478+
Title = "title",
479+
Username = "username",
480+
Server = null
481+
}
482+
}
483+
});
484+
485+
await _subject.Execute(null);
486+
487+
_mocker.Verify<OmbiUserManager>(x => x.CreateAsync(It.IsAny<OmbiUser>()), Times.Never);
488+
}
489+
490+
[Test]
491+
public async Task Import_Skips_Users_With_Empty_Server_Array()
492+
{
493+
_mocker.Setup<ISettingsService<UserManagementSettings>, Task<UserManagementSettings>>(x => x.GetSettingsAsync())
494+
.ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true });
495+
_mocker.Setup<IPlexApi, Task<PlexUsers>>(x => x.GetUsers(It.IsAny<string>())).ReturnsAsync(new PlexUsers
496+
{
497+
User = new UserFriends[]
498+
{
499+
new UserFriends
500+
{
501+
Email = "email",
502+
Id = "EmptyServer",
503+
Title = "title",
504+
Username = "username",
505+
Server = new PlexUserServer[0]
506+
}
507+
}
508+
});
509+
510+
await _subject.Execute(null);
511+
512+
_mocker.Verify<OmbiUserManager>(x => x.CreateAsync(It.IsAny<OmbiUser>()), Times.Never);
513+
}
514+
515+
[Test]
516+
public async Task Import_Creates_User_With_Server_Access()
517+
{
518+
_mocker.Setup<ISettingsService<UserManagementSettings>, Task<UserManagementSettings>>(x => x.GetSettingsAsync())
519+
.ReturnsAsync(new UserManagementSettings { ImportPlexAdmin = false, ImportPlexUsers = true });
520+
_mocker.Setup<IPlexApi, Task<PlexUsers>>(x => x.GetUsers(It.IsAny<string>())).ReturnsAsync(new PlexUsers
521+
{
522+
User = new UserFriends[]
523+
{
524+
new UserFriends
525+
{
526+
Email = "email",
527+
Id = "HasServer",
528+
Title = "title",
529+
Username = "username",
530+
Server = new PlexUserServer[]
531+
{
532+
new PlexUserServer
533+
{
534+
Id = "1",
535+
ServerId = "123"
536+
}
537+
}
538+
}
539+
}
540+
});
541+
542+
_mocker.Setup<OmbiUserManager, Task<IdentityResult>>(x => x.CreateAsync(It.Is<OmbiUser>(x =>
543+
x.UserName == "username" &&
544+
x.Email == "email" &&
545+
x.ProviderUserId == "HasServer" &&
546+
x.UserType == UserType.PlexUser)))
547+
.ReturnsAsync(IdentityResult.Success);
548+
549+
await _subject.Execute(null);
550+
551+
_mocker.Verify<OmbiUserManager>(x => x.CreateAsync(It.Is<OmbiUser>(x =>
552+
x.UserName == "username" &&
553+
x.Email == "email" &&
554+
x.ProviderUserId == "HasServer" &&
555+
x.UserType == UserType.PlexUser)), Times.Once);
556+
}
443557
}
444558
}

‎src/Ombi.Schedule/Jobs/Plex/PlexUserImporter.cs

+7
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,13 @@ private async Task<List<OmbiUser>> ImportPlexUsers(UserManagementSettings userMa
120120

121121
foreach (var plexUser in users.User)
122122
{
123+
// Skip users without server access
124+
if (plexUser.Server == null || !plexUser.Server.Any())
125+
{
126+
_log.LogInformation($"Skipping user {plexUser.Username ?? plexUser.Id} as they have no server access");
127+
continue;
128+
}
129+
123130
// Check if we should import this user
124131
if (userManagementSettings.BannedPlexUserIds.Contains(plexUser.Id))
125132
{

0 commit comments

Comments
 (0)