Skip to content

Commit da606f0

Browse files
montesmocimontesmoci
committed
Add permission check for new endpoints
Signed-off-by: montesm <[email protected]>
1 parent 1648b15 commit da606f0

File tree

5 files changed

+99
-30
lines changed

5 files changed

+99
-30
lines changed

‎UnityAuth/src/main/java/io/unityfoundation/auth/AuthController.java‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,19 @@ public HttpResponse<HasPermissionResponse> hasPermission(@Body HasPermissionRequ
104104
}
105105

106106
@Get("/roles")
107-
public HttpResponse<List<RoleDTO>> getRoles() {
107+
public HttpResponse<List<RoleDTO>> getRoles(Authentication authentication) {
108+
109+
User user = userRepo.findByEmail(authentication.getName()).orElse(null);
110+
if (checkUserStatus(user)) {
111+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user is disabled.");
112+
}
113+
114+
List<String> commonPermissions = permissionsService.checkUserPermissionsAcrossAllTenants(
115+
user, List.of("AUTH_SERVICE_VIEW-SYSTEM", "AUTH_SERVICE_VIEW-TENANT"));
116+
if (commonPermissions.isEmpty()) {
117+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user does not have permission!");
118+
}
119+
108120
return HttpResponse.ok(roleRepo.findAll().stream()
109121
.map(role -> new RoleDTO(role.getId(), role.getName(), role.getDescription()))
110122
.toList());
@@ -114,6 +126,16 @@ public HttpResponse<List<RoleDTO>> getRoles() {
114126
public HttpResponse<List<TenantDTO>> getTenants(Authentication authentication) {
115127

116128
String authenticatedUserEmail = authentication.getName();
129+
User user = userRepo.findByEmail(authenticatedUserEmail).orElse(null);
130+
if (checkUserStatus(user)) {
131+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user is disabled.");
132+
}
133+
134+
List<String> commonPermissions = permissionsService.checkUserPermissionsAcrossAllTenants(
135+
user, List.of("AUTH_SERVICE_VIEW-SYSTEM", "AUTH_SERVICE_VIEW-TENANT"));
136+
if (commonPermissions.isEmpty()) {
137+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user does not have permission!");
138+
}
117139

118140
List<Tenant> tenants = userRepo.existsByEmailAndRoleEqualsUnityAdmin(authenticatedUserEmail) ?
119141
tenantRepo.findAll() : tenantRepo.findAllByUserEmail(authenticatedUserEmail);
@@ -127,14 +149,26 @@ public HttpResponse<List<TenantDTO>> getTenants(Authentication authentication) {
127149
public HttpResponse<List<UserResponse>> getTenantUsers(@PathVariable Long id, Authentication authentication) {
128150

129151
// reject if the declared tenant does not exist
130-
if (!tenantRepo.existsById(id)) {
131-
return HttpResponse.badRequest();
152+
Optional<Tenant> tenantOptional = tenantRepo.findById(id);
153+
if (tenantOptional.isEmpty()) {
154+
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Tenant not found.");
155+
}
156+
157+
User user = userRepo.findByEmail(authentication.getName()).orElse(null);
158+
if (checkUserStatus(user)) {
159+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user is disabled.");
160+
}
161+
162+
List<String> commonPermissions = permissionsService.checkUserPermission(user, tenantOptional.get(),
163+
List.of("AUTH_SERVICE_VIEW-SYSTEM", "AUTH_SERVICE_VIEW-TENANT"));
164+
if (commonPermissions.isEmpty()) {
165+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user does not have permission!");
132166
}
133167

134168
// todo: it would be nice to capture the roles and have them automatically mapped to UserResponse.roles
135-
List<UserResponse> tenantUsers = userRepo.findAllByTenantId(id).stream().map(user ->
136-
new UserResponse(user.getId(), user.getEmail(), user.getFirstName(), user.getLastName(),
137-
userRepo.getUserRolesByUserId(user.getId())
169+
List<UserResponse> tenantUsers = userRepo.findAllByTenantId(id).stream().map(tenantUser ->
170+
new UserResponse(tenantUser.getId(), tenantUser.getEmail(), tenantUser.getFirstName(), tenantUser.getLastName(),
171+
userRepo.getUserRolesByUserId(tenantUser.getId())
138172
)).toList();
139173

140174
return HttpResponse.ok(tenantUsers);

‎UnityAuth/src/main/java/io/unityfoundation/auth/PermissionsService.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import java.util.List;
1111
import java.util.function.BiPredicate;
12+
import java.util.function.Predicate;
1213

1314
@Singleton
1415
public class PermissionsService {
@@ -21,6 +22,11 @@ public class PermissionsService {
2122
|| Permission.PermissionScope.SUBTENANT.equals(tp.permissionScope()))
2223
&& tp.tenantId == t.getId());
2324

25+
private final Predicate<TenantPermission> isTenantOrSystemOrSubtenantScope = (tp) ->
26+
Permission.PermissionScope.SYSTEM.equals(tp.permissionScope()) || (
27+
(Permission.PermissionScope.TENANT.equals(tp.permissionScope())
28+
|| Permission.PermissionScope.SUBTENANT.equals(tp.permissionScope())));
29+
2430
public PermissionsService(UserRepo userRepo) {
2531
this.userRepo = userRepo;
2632
}
@@ -38,6 +44,14 @@ public List<String> getPermissionsFor(User user, Tenant tenant) {
3844
.toList();
3945
}
4046

47+
public List<String> checkUserPermissionsAcrossAllTenants(User user, List<String> permissions) {
48+
return userRepo.getTenantPermissionsFor(user.getId()).stream()
49+
.filter(isTenantOrSystemOrSubtenantScope)
50+
.map(TenantPermission::permissionName)
51+
.filter(permissions::contains)
52+
.toList();
53+
}
54+
4155
@Introspected
4256
public record TenantPermission(
4357
long tenantId,

‎UnityAuth/src/main/java/io/unityfoundation/auth/UserController.java‎

Lines changed: 40 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ public class UserController {
2626
private final TenantRepo tenantRepo;
2727
private final RoleRepo roleRepo;
2828
private final PasswordEncoder passwordEncoder;
29+
private final PermissionsService permissionsService;
2930

30-
public UserController(UserRepo userRepo, TenantRepo tenantRepo, RoleRepo roleRepo, PasswordEncoder passwordEncoder) {
31+
public UserController(UserRepo userRepo, TenantRepo tenantRepo, RoleRepo roleRepo, PasswordEncoder passwordEncoder, PermissionsService permissionsService) {
3132
this.userRepo = userRepo;
3233
this.tenantRepo = tenantRepo;
3334
this.roleRepo = roleRepo;
3435
this.passwordEncoder = passwordEncoder;
36+
this.permissionsService = permissionsService;
3537
}
3638

3739
@Post
@@ -40,23 +42,31 @@ public HttpResponse<UserResponse> createUser(@Body AddUserRequest requestDTO,
4042

4143
Long requestTenantId = requestDTO.tenantId();
4244

43-
// reject if the declared tenant does not exist
44-
if (!tenantRepo.existsById(requestTenantId)) {
45-
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Tenant not found");
45+
Optional<Tenant> tenantOptional = tenantRepo.findById(requestTenantId);
46+
if (tenantOptional.isEmpty()) {
47+
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Tenant not found.");
4648
}
4749

48-
Role unityAdministrator = roleRepo.findByName("Unity Administrator");
50+
Optional<User> adminOptional = userRepo.findByEmail(authentication.getName());
51+
if (adminOptional.isEmpty()) {
52+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user is disabled.");
53+
}
4954

50-
// ignore roles not defined by application
55+
User admin = adminOptional.get();
56+
57+
List<String> commonPermissions = permissionsService.checkUserPermission(admin, tenantOptional.get(),
58+
List.of("AUTH_SERVICE_EDIT-SYSTEM", "AUTH_SERVICE_EDIT-TENANT"));
59+
if (commonPermissions.isEmpty()) {
60+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user does not have permission!");
61+
}
62+
63+
// ignore roles not defined by system
5164
List<Long> rolesIntersection = getRolesIntersection(requestDTO.roles());
5265

5366
// reject if caller is not a unity nor tenant admin of the declared tenant
54-
String authUserEmail = authentication.getName();
55-
if (!userRepo.existsByEmailAndRoleEqualsUnityAdmin(authUserEmail)) {
56-
if (!userRepo.existsByEmailAndTenantEqualsAndIsTenantAdmin(authUserEmail, requestTenantId)) {
57-
return HttpResponse.status(HttpStatus.FORBIDDEN,
58-
"Authenticated user is not authorized to make changes under declared tenant.");
59-
} else if (rolesIntersection.stream().anyMatch(roleId -> roleId.equals(unityAdministrator.getId()))){
67+
if (!commonPermissions.contains("AUTH_SERVICE_EDIT-SYSTEM")) {
68+
Role unityAdministrator = roleRepo.findByName("Unity Administrator");
69+
if (rolesIntersection.stream().anyMatch(roleId -> roleId.equals(unityAdministrator.getId()))){
6070
// authenticated tenant admin user cannot grant unity admin role
6171
return HttpResponse.status(HttpStatus.FORBIDDEN,
6272
"Authenticated user is not authorized to grant Unity Admin");
@@ -97,30 +107,37 @@ public HttpResponse<UserResponse> createUser(@Body AddUserRequest requestDTO,
97107
public HttpResponse<UserResponse> updateUserRoles(@PathVariable Long id, @Body UpdateUserRolesRequest requestDTO,
98108
Authentication authentication) {
99109
Long requestTenantId = requestDTO.tenantId();
110+
Optional<Tenant> tenantOptional = tenantRepo.findById(requestTenantId);
111+
if (tenantOptional.isEmpty()) {
112+
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Tenant not found.");
113+
}
100114

101-
// reject if the declared tenant does not exist
102-
if (!tenantRepo.existsById(requestTenantId)) {
103-
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Tenant not found");
115+
String authUserEmail = authentication.getName();
116+
Optional<User> adminOptional = userRepo.findByEmail(authUserEmail);
117+
if (adminOptional.isEmpty()) {
118+
throw new HttpStatusException(HttpStatus.NOT_FOUND, "Authenticated user does not exist");
104119
}
120+
User admin = adminOptional.get();
105121

106122
Optional<User> userOptional = userRepo.findById(id);
107123
if (userOptional.isEmpty()) {
108124
throw new HttpStatusException(HttpStatus.NOT_FOUND, "User not found");
109125
}
110-
111126
User user = userOptional.get();
112-
Role unityAdministrator = roleRepo.findByName("Unity Administrator");
113127

114128
// ignore roles not defined by application
115129
List<Long> rolesIntersection = getRolesIntersection(requestDTO.roles());
116130

117131
// if unity admin, proceed; otherwise, reject if roles exceed authenticated user's under same tenant.
118-
String authUserEmail = authentication.getName();
119-
if (!userRepo.existsByEmailAndRoleEqualsUnityAdmin(authUserEmail)) {
120-
if (!userRepo.existsByEmailAndTenantEqualsAndIsTenantAdmin(authUserEmail, requestTenantId)) {
121-
return HttpResponse.status(HttpStatus.FORBIDDEN,
122-
"Authenticated user is not authorized to make changes under declared tenant.");
123-
} else if (rolesIntersection.stream().anyMatch(roleId -> roleId.equals(unityAdministrator.getId()))){
132+
List<String> commonPermissions = permissionsService.checkUserPermission(admin, tenantOptional.get(),
133+
List.of("AUTH_SERVICE_EDIT-SYSTEM", "AUTH_SERVICE_EDIT-TENANT"));
134+
if (commonPermissions.isEmpty()) {
135+
throw new HttpStatusException(HttpStatus.FORBIDDEN, "The user does not have permission!");
136+
}
137+
138+
if (!commonPermissions.contains("AUTH_SERVICE_VIEW-SYSTEM")) {
139+
Role unityAdministrator = roleRepo.findByName("Unity Administrator");
140+
if (rolesIntersection.stream().anyMatch(roleId -> roleId.equals(unityAdministrator.getId()))){
124141
// authenticated tenant admin user cannot grant unity admin role
125142
return HttpResponse.status(HttpStatus.FORBIDDEN,
126143
"Authenticated user is not authorized to grant Unity Admin");

‎UnityAuth/src/test/java/io/unityfoundation/UnityIamTest.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ void testGetUserPermissionsHappyPath() {
167167
HttpResponse<UserPermissionsResponse.Success> response = client.toBlocking()
168168
.exchange(hasPermissionRequest, UserPermissionsResponse.Success.class);
169169

170-
assertEquals(response.getBody().get().permissions(), List.of("AUTH_SERVICE_EDIT-SYSTEM"));
170+
assertTrue(response.getBody().get().permissions().contains("AUTH_SERVICE_EDIT-SYSTEM"));
171171
}
172172

173173
@Test
@@ -237,6 +237,8 @@ void testCreateUsers() {
237237
assertFalse(user.roles().isEmpty());
238238

239239
// Case: User does exist in system but not under tenant.
240+
// Given that tenant = 2, this confirms that a Unity Admin can perform actions in a Tenant
241+
// they are not associated to. (See afterMigrate.sql user_role table)
240242
createUserRequest = HttpRequest.POST("/api/users",
241243
new UserController.AddUserRequest(
242244
"[email protected]",

‎UnityAuth/src/test/resources/db/migration/afterMigrate.sql‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,14 @@ INSERT INTO tenant_service (tenant_id, service_id, status) VALUES(2, 1, 'ENABLED
1818
INSERT INTO permission (id, name, description, scope) VALUES(1, 'AUTH_SERVICE_EDIT-SYSTEM', 'Description', 'SYSTEM');
1919
INSERT INTO permission (id, name, description, scope) VALUES(2, 'LIBRE311_REQUEST_EDIT-TENANT', 'Description', 'TENANT');
2020
INSERT INTO permission (id, name, description, scope) VALUES(3, 'LIBRE311_REQUEST_EDIT-SUBTENANT', 'Description', 'SUBTENANT');
21+
INSERT INTO permission (id, name, description, scope) VALUES(4, 'AUTH_SERVICE_VIEW-SYSTEM', 'Description', 'SYSTEM');
2122
INSERT INTO role (id, name, description) VALUES(1, 'Unity Administrator', 'System role');
2223
INSERT INTO role (id, name, description) VALUES(2, 'Tenant role', 'Tenant role');
2324
INSERT INTO role (id, name, description) VALUES(3, 'Subtenant role', 'Subtenant role');
2425
INSERT INTO role_permission (role_id, permission_id) VALUES(1, 1);
2526
INSERT INTO role_permission (role_id, permission_id) VALUES(2, 2);
2627
INSERT INTO role_permission (role_id, permission_id) VALUES(3, 3);
28+
INSERT INTO role_permission (role_id, permission_id) VALUES(1, 4);
2729
INSERT INTO user_role (tenant_id, user_id, role_id) VALUES(1, 1, 1);
2830
INSERT INTO user_role (tenant_id, user_id, role_id) VALUES(2, 1, 2);
2931
INSERT INTO user_role (tenant_id, user_id, role_id) VALUES(2, 1, 3);

0 commit comments

Comments
 (0)