Skip to content

Commit c214840

Browse files
webprofusion-chriscwebprofusion-chrisc
committed
Access: fix scoped assigned role references, optionally always apply upgrades
1 parent 486046c commit c214840

File tree

8 files changed

+28
-99
lines changed

8 files changed

+28
-99
lines changed

‎src/Certify.Core/Management/Access/AccessControl.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ public async Task<bool> IsSecurityPrincipleAuthorised(string contextUserId, Acce
251251
// TODO: cache results for performance based on last update of access control config, which will be largely static
252252

253253
// get all assigned roles (all users)
254+
// assigned roles are assigned to a security principle (user) and include a specified role plus any restrictions on resource types or identifiers
254255
var allAssignedRoles = await _store.GetItems<AssignedRole>(nameof(AssignedRole));
255256

256257
// get all defined roles
@@ -262,10 +263,10 @@ public async Task<bool> IsSecurityPrincipleAuthorised(string contextUserId, Acce
262263
// get the assigned roles for this specific security principle
263264
var spAssignedRoles = allAssignedRoles.Where(a => a.SecurityPrincipleId == check.SecurityPrincipleId);
264265

265-
// if scoped assigned role ID specified (access token check etc), reduce scope of assigned roles to check
266+
// if scoped AssignedRole.ID (not just the roleID) specified (access token check etc), reduce scope of assigned roles to check
266267
if (check.ScopedAssignedRoles?.Any() == true)
267268
{
268-
spAssignedRoles = spAssignedRoles.Where(a => check.ScopedAssignedRoles.Contains(a.RoleId));
269+
spAssignedRoles = spAssignedRoles.Where(a => check.ScopedAssignedRoles.Contains(a.Id));
269270
}
270271

271272
// get all role definitions included in the principles assigned roles
@@ -407,7 +408,7 @@ public async Task<bool> UpdateSecurityPrinciplePassword(string contextUserId, Se
407408

408409
var updated = false;
409410

410-
var principle = await GetSecurityPrinciple(contextUserId, passwordUpdate.SecurityPrincipleId);
411+
var principle = await GetSecurityPrinciple(contextUserId, passwordUpdate.SecurityPrincipleId, includePassword: true);
411412

412413
if (IsPasswordValid(passwordUpdate.Password, principle.Password))
413414
{

‎src/Certify.Core/Management/CertifyManager/CertifyManager.Maintenance.cs‎

Lines changed: 3 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
using System;
1+
using System;
22
using System.Collections.Concurrent;
33
using System.Collections.Generic;
44
using System.IO;
55
using System.Linq;
66
using System.Threading;
77
using System.Threading.Tasks;
8-
using Certify.Core.Management.Access;
98
using Certify.Models;
109
using Certify.Models.Config;
1110
using Certify.Models.Hub;
@@ -25,7 +24,7 @@ private async Task UpgradeSettings()
2524
var systemVersion = Util.GetAppVersion().ToString();
2625
var previousVersion = CoreAppSettings.Current.CurrentServiceVersion;
2726

28-
if (CoreAppSettings.Current.CurrentServiceVersion != systemVersion)
27+
if (CoreAppSettings.Current.CurrentServiceVersion != systemVersion || Environment.GetEnvironmentVariable("CERTIFY_UPGRADE_SETTINGS") == "true")
2928
{
3029
_tc?.TrackEvent("ServiceUpgrade", new Dictionary<string, string> {
3130
{ "previousVersion", previousVersion },
@@ -48,90 +47,11 @@ private async Task UpgradeSettings()
4847

4948
if (CoreAppSettings.Current.IsManagementHubService)
5049
{
51-
if (await accessControl.IsInitialized() == false)
52-
{
53-
await BootstrapAdminUserAndRoles(accessControl);
54-
}
55-
else
56-
{
57-
await UpdateStandardRoles(accessControl);
58-
}
50+
await AccessControlConfig.ConfigureStandardUsersAndRoles(accessControl);
5951
}
6052
}
6153
}
6254

63-
private static async Task BootstrapAdminUserAndRoles(IAccessControl access)
64-
{
65-
// setup roles with policies
66-
await UpdateStandardRoles(access);
67-
68-
var adminSp = new SecurityPrinciple
69-
{
70-
Id = "admin_01",
71-
Description = "Primary default admin",
72-
PrincipleType = SecurityPrincipleType.User,
73-
Username = Environment.GetEnvironmentVariable("CERTIFY_ADMIN_DEFAULTUSERNAME") ?? "admin",
74-
Password = Environment.GetEnvironmentVariable("CERTIFY_ADMIN_DEFAULTPWD") ?? "changeme!",
75-
Provider = StandardIdentityProviders.INTERNAL
76-
};
77-
78-
await access.AddSecurityPrinciple(adminSp.Id, adminSp, bypassIntegrityCheck: true);
79-
80-
// assign security principles to roles
81-
var assignedRoles = new List<AssignedRole> {
82-
// administrator
83-
new AssignedRole{
84-
Id= Guid.NewGuid().ToString(),
85-
RoleId=StandardRoles.Administrator.Id,
86-
SecurityPrincipleId=adminSp.Id
87-
}
88-
};
89-
90-
foreach (var r in assignedRoles)
91-
{
92-
// add roles and policy assignments to store
93-
await access.AddAssignedRole(adminSp.Id, r, bypassIntegrityCheck: true);
94-
}
95-
}
96-
97-
/// <summary>
98-
/// Add/update standard system roles, policies and resource actions
99-
/// </summary>
100-
/// <param name="access"></param>
101-
/// <returns></returns>
102-
private static async Task UpdateStandardRoles(IAccessControl access)
103-
{
104-
// setup roles with policies
105-
106-
var adminSvcPrinciple = "admin_01";
107-
108-
var actions = Policies.GetStandardResourceActions();
109-
110-
foreach (var action in actions)
111-
{
112-
await access.AddResourceAction(adminSvcPrinciple, action, bypassIntegrityCheck: true);
113-
}
114-
115-
// setup policies with actions
116-
117-
var policies = Policies.GetStandardPolicies();
118-
119-
// add policies to store
120-
foreach (var r in policies)
121-
{
122-
_ = await access.AddResourcePolicy(adminSvcPrinciple, r, bypassIntegrityCheck: true);
123-
}
124-
125-
// setup roles with policies
126-
var roles = Policies.GetStandardRoles();
127-
128-
foreach (var r in roles)
129-
{
130-
// add roles and policy assignments to store
131-
await access.AddRole(adminSvcPrinciple, r, bypassIntegrityCheck: true);
132-
}
133-
}
134-
13555
/// <summary>
13656
/// When called, perform daily cache cleanup, cert cleanup, diagnostics and maintenance
13757
/// </summary>

‎src/Certify.Models/Hub/AccessControl.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public class SecurityPrinciple : ConfigurationStoreItem
4040
public string? AuthKey { get; set; }
4141

4242
public string AvatarUrl { get; set; } = string.Empty;
43-
public bool IsBuiltIn { get; set; } = false;
43+
public bool IsBuiltIn { get; set; }
4444
}
4545

4646
/// <summary>
@@ -124,7 +124,7 @@ public class AssignedAccessToken : ConfigurationStoreItem
124124
public string SecurityPrincipleId { get; set; } = default!;
125125

126126
/// <summary>
127-
/// Optional list of Assigned Roles this access token is scoped to
127+
/// Optional list of Assigned Roles this access token is scoped to. Note this is not the RoleID but the AssignedRoleID.
128128
/// </summary>
129129
public List<string> ScopedAssignedRoles { get; set; } = [];
130130

‎src/Certify.Models/Hub/AccessControlConfig.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ public static async Task ConfigureStandardUsersAndRoles(IAccessControl access)
482482
}
483483
},
484484
ScopedAssignedRoles = new List<string> {
485-
StandardRoles.ManagedInstance.Id
485+
// scope assigned role is the id for AssignedRole (not the role id itself)
486+
assignedRoles.First(a=>a.RoleId==StandardRoles.ManagedInstance.Id).Id
486487
},
487488
};
488489

‎src/Certify.Server/Certify.Server.Core/Certify.Server.Core/Controllers/AccessController.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,12 @@ public async Task<ICollection<AssignedAccessToken>> GetAssignedAccessTokens()
122122
{
123123
var accessControl = await _certifyManager.GetCurrentAccessControl();
124124

125-
token.AccessTokens?.ForEach(a => a.DateCreated = DateTime.UtcNow);
125+
token.AccessTokens?.ForEach(a =>
126+
{
127+
a.DateCreated = DateTime.UtcNow;
128+
a.Id = Guid.NewGuid().ToString();
129+
a.Secret = Guid.NewGuid().ToString();
130+
});
126131

127132
var addResultOk = await accessControl.AddAssignedAccessToken(GetContextUserId(), token);
128133

‎src/Certify.Server/Certify.Server.Core/Certify.Server.Core/Properties/launchSettings.json‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
"environmentVariables": {
1717
"ASPNETCORE_ENVIRONMENT": "Development",
1818
"ASPNETCORE_URLS": "http://127.0.0.2:9696",
19-
"CERTIFY_MANAGEMENT_HUB": "https://localhost:44361/api/internal/managementhub",
19+
//"CERTIFY_MANAGEMENT_HUB": "https://localhost:44361/api/internal/managementhub",
2020
"CERTIFY_ENABLE_MANAGEMENT_HUB": "true",
21-
"CERTIFY_SERVICE_AUTH_MODE": "none"
21+
"CERTIFY_SERVICE_AUTH_MODE": "none",
22+
"CERTIFY_UPGRADE_SETTINGS": "true"
2223
}
2324
},
2425
"Certify.Server.Core (windows auth)": {
@@ -27,9 +28,10 @@
2728
"environmentVariables": {
2829
"ASPNETCORE_ENVIRONMENT": "Development",
2930
"ASPNETCORE_URLS": "http://127.0.0.2:9696",
30-
"CERTIFY_MANAGEMENT_HUB": "https://localhost:44361/api/internal/managementhub",
31+
// "CERTIFY_MANAGEMENT_HUB": "https://localhost:44361/api/internal/managementhub",
3132
"CERTIFY_ENABLE_MANAGEMENT_HUB": "true",
32-
"CERTIFY_SERVICE_AUTH_MODE": "windows"
33+
"CERTIFY_SERVICE_AUTH_MODE": "windows",
34+
"CERTIFY_UPGRADE_SETTINGS": "true"
3335
}
3436
},
3537
"IIS Express": {

‎src/Certify.Tests/Certify.Core.Tests.Integration/DataStores/AccessControlDataStoreTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public async Task TestStoreGeneralAccessControl(string storeType)
176176
Assert.IsTrue(list.Any(), "Should have security principles in store");
177177

178178
// get updated sp so that password is hashed for comparison check
179-
consumerSp = await access.GetSecurityPrinciple(adminSp.Id, consumerSp.Id);
179+
consumerSp = await access.GetSecurityPrinciple(adminSp.Id, consumerSp.Id, includePassword: true);
180180

181181
Assert.IsTrue(access.IsPasswordValid("oldpassword", consumerSp.Password));
182182
}

‎src/Certify.Tests/Certify.Core.Tests.Unit/Tests/AccessControlTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ public async Task TestUpdateSecurityPrinciplePassword()
414414
assignedRoles.ForEach(async r => await access.AddAssignedRole(contextUserId, r, bypassIntegrityCheck: true));
415415

416416
// Validate password of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() before update
417-
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
417+
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id, includePassword: true);
418418
var firstPasswordHashed = access.HashPassword(firstPassword, storedSecurityPrinciple.Password.Split('.')[1]);
419419
Assert.AreEqual(storedSecurityPrinciple.Password, firstPasswordHashed, $"Expected SecurityPrinciple returned by GetSecurityPrinciple() to match Password '{firstPasswordHashed}' of SecurityPrinciple passed into AddSecurityPrinciple()");
420420

@@ -424,7 +424,7 @@ public async Task TestUpdateSecurityPrinciplePassword()
424424
Assert.IsTrue(securityPrincipleUpdated, $"Expected security principle password update for {adminSecurityPrinciples[0].Id} to succeed");
425425

426426
// Validate password of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() after update
427-
storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
427+
storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id, includePassword: true);
428428
var newPasswordHashed = access.HashPassword(newPassword, storedSecurityPrinciple.Password.Split('.')[1]);
429429

430430
Assert.AreNotEqual(storedSecurityPrinciple.Password, firstPasswordHashed, $"Expected SecurityPrinciple returned by GetSecurityPrinciple() to not match previous Password '{firstPasswordHashed}' of SecurityPrinciple passed into AddSecurityPrinciple()");
@@ -445,7 +445,7 @@ public async Task TestUpdateSecurityPrinciplePasswordNoRoles()
445445
Assert.IsFalse(securityPrincipleUpdated, $"Expected security principle password update for {adminSecurityPrinciples[0].Id} to fail without roles");
446446

447447
// Validate password of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() after failed update
448-
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
448+
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id, includePassword: true);
449449
var firstPasswordHashed = access.HashPassword(firstPassword, storedSecurityPrinciple.Password.Split('.')[1]);
450450

451451
Assert.AreEqual(storedSecurityPrinciple.Password, firstPasswordHashed, $"Expected SecurityPrinciple returned by GetSecurityPrinciple() to match Password '{firstPasswordHashed}' of SecurityPrinciple passed into AddSecurityPrinciple()");
@@ -481,7 +481,7 @@ public async Task TestUpdateSecurityPrinciplePasswordBadPassword()
481481
Assert.IsFalse(securityPrincipleUpdated, $"Expected security principle password update for {adminSecurityPrinciples[0].Id} to fail with wrong password");
482482

483483
// Validate password of SecurityPrinciple object returned by AccessControl.GetSecurityPrinciple() after failed update
484-
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id);
484+
var storedSecurityPrinciple = await access.GetSecurityPrinciple(contextUserId, adminSecurityPrinciples[0].Id, includePassword: true);
485485
var firstPasswordHashed = access.HashPassword(firstPassword, storedSecurityPrinciple.Password.Split('.')[1]);
486486
Assert.AreEqual(storedSecurityPrinciple.Password, firstPasswordHashed, $"Expected SecurityPrinciple returned by GetSecurityPrinciple() to match Password '{firstPasswordHashed}' of SecurityPrinciple passed into AddSecurityPrinciple()");
487487
}

0 commit comments

Comments
 (0)