Skip to content

Commit a94b7eb

Browse files
Crowley723Crowley723
authored
fix: missing authz annotations, authz inteceptor default allow (#174)
1 parent 3e133cd commit a94b7eb

10 files changed

Lines changed: 63 additions & 18 deletions

File tree

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/authentication/AuthorizationInterceptor.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,10 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
212212
LOG.debug("JWT validated for user: {}", userContext.getLogIdentifier());
213213
}
214214

215-
if (!isValidRole(roles, handler)) {
216-
return false;
215+
if (!isValidRole(roles, handler, request)) {
216+
throw new CustomExceptionBuilder()
217+
.withStatus(HttpStatus.FORBIDDEN) // 403 instead of 401 since they're authenticated but not authorized
218+
.build();
217219
}
218220

219221
CurrentUser.setUserContext(userContext);
@@ -351,7 +353,7 @@ private List<UserRole> extractRolesFromJwt(Jwt jwt) {
351353

352354
if (groups.contains(authProperties.getGroups().getWrite()))
353355
roles.add(UserRole.WRITE);
354-
if (groups.contains(authProperties.getGroups().getRead()))
356+
if (groups.contains(authProperties.getGroups().getRead()) || roles.contains(UserRole.WRITE))
355357
roles.add(UserRole.READ);
356358

357359
return roles;
@@ -382,7 +384,7 @@ private boolean isPublicEndpoint(Object handler) {
382384
* @param roles the user roles extracted from the jwt
383385
* @param handler the request handler object (most likely the method)
384386
* */
385-
private boolean isValidRole(List<UserRole> roles, Object handler) {
387+
private boolean isValidRole(List<UserRole> roles, Object handler, HttpServletRequest request) {
386388
if (!(handler instanceof HandlerMethod))
387389
return true;
388390

@@ -393,9 +395,13 @@ private boolean isValidRole(List<UserRole> roles, Object handler) {
393395

394396
var annotationOptional = Arrays.stream(method.getAnnotations()).filter(a -> a instanceof RequiredRoles).findFirst();
395397

396-
// If no required role annotation is present, simply being authorized will provide access
397-
if (annotationOptional.isEmpty())
398-
return true;
398+
if (annotationOptional.isEmpty()) {
399+
String httpMethod = request.getMethod().toUpperCase();
400+
if (httpMethod.equals("GET")) {
401+
return roles.contains(UserRole.READ) || roles.contains(UserRole.WRITE);
402+
}
403+
return roles.contains(UserRole.WRITE);
404+
}
399405

400406
RequiredRoles requiredRolesAnnotation = (RequiredRoles) annotationOptional.get();
401407
UserRole[] requiredRoles = requiredRolesAnnotation.roles();

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/config/SecurityConfig.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
145145
.csrf(csrf -> csrf.disable()) // Disabled: API uses stateless JWT Bearer tokens in Authorization header
146146
.authorizeHttpRequests(authz -> authz
147147
.requestMatchers("/api/**", "/login/**", "/oauth2/**", "/auth/**").permitAll() // API authorization handled by AuthorizationInterceptor
148-
.anyRequest().authenticated()
148+
.anyRequest().permitAll()
149149
)
150150
.oauth2Login(oauth2 -> oauth2
151151
.successHandler(oauth2JsonSuccessHandler)

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/BatchesController.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.vsp.endpointinsightsapi.controller;
22

3+
import com.vsp.endpointinsightsapi.authentication.RequiredRoles;
34
import com.vsp.endpointinsightsapi.dto.BatchRequestDTO;
45
import com.vsp.endpointinsightsapi.dto.BatchResponseDTO;
56

@@ -8,6 +9,7 @@
89
import com.vsp.endpointinsightsapi.model.TestBatch;
910
import com.vsp.endpointinsightsapi.model.entity.BatchUpdateRequest;
1011
import com.vsp.endpointinsightsapi.model.entity.TestRun;
12+
import com.vsp.endpointinsightsapi.model.enums.UserRole;
1113
import com.vsp.endpointinsightsapi.repository.TestBatchRepository;
1214

1315
import com.vsp.endpointinsightsapi.service.BatchService;
@@ -46,6 +48,7 @@ public BatchesController(BatchService batchService, TestBatchRepository testBatc
4648

4749
// GET /api/batches
4850
@GetMapping
51+
@RequiredRoles(roles = {UserRole.READ})
4952
@Operation(summary = "List all test batches", description = "Retrieves a list of all test batches with optional filtering")
5053
@ApiResponses(value = {
5154
@ApiResponse(responseCode = "200", description = "Batches retrieved successfully"),
@@ -62,6 +65,7 @@ public ResponseEntity<List<BatchResponseDTO>> listBatches(
6265

6366
// GET /api/batches/{id}
6467
@GetMapping("/{id}")
68+
@RequiredRoles(roles = {UserRole.READ})
6569
@Operation(summary = "Get batch by ID", description = "Retrieves a specific test batch by its unique identifier")
6670
@ApiResponses(value = {
6771
@ApiResponse(responseCode = "200", description = "Batch found"),
@@ -77,6 +81,7 @@ public ResponseEntity<BatchResponseDTO> getBatch(
7781

7882
// POST /api/batches
7983
@PostMapping
84+
@RequiredRoles(roles = {UserRole.WRITE})
8085
@Operation(summary = "Create new test batch", description = "Creates a new test batch with the provided configuration")
8186
@ApiResponses(value = {
8287
@ApiResponse(responseCode = "201", description = "Batch created successfully"),
@@ -90,6 +95,7 @@ public ResponseEntity<BatchResponseDTO> createBatch(@RequestBody BatchRequestDTO
9095
}
9196

9297
@PostMapping("/{batchId}/run")
98+
@RequiredRoles(roles = {UserRole.WRITE})
9399
@Operation(summary = "Run test batch", description = "Executes a test batch and creates a new test run")
94100
@ApiResponses(value = {
95101
@ApiResponse(responseCode = "200", description = "Batch execution started"),
@@ -112,6 +118,7 @@ public ResponseEntity<TestRun> runBatch(
112118

113119
// PUT /api/batches/{id}
114120
@PutMapping("/{id}")
121+
@RequiredRoles(roles = {UserRole.WRITE})
115122
@Operation(summary = "Update test batch", description = "Updates an existing test batch with new information")
116123
@ApiResponses(value = {
117124
@ApiResponse(responseCode = "200", description = "Batch updated successfully"),
@@ -130,7 +137,8 @@ public ResponseEntity<BatchResponseDTO> updateBatch(
130137

131138
// DELETE /api/batches/{id}
132139
@DeleteMapping("/{id}")
133-
@Operation(summary = "Delete test batch", description = "Permanently deletes a test batch by its ID")
140+
@RequiredRoles(roles = {UserRole.WRITE})
141+
@Operation(summary = "Delete test batch", description = "Permanently deletes a test batch by its ID")
134142
@ApiResponses(value = {
135143
@ApiResponse(responseCode = "204", description = "Batch deleted successfully"),
136144
@ApiResponse(responseCode = "404", description = "Batch not found"),

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/DashboardController.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package com.vsp.endpointinsightsapi.controller;
22

3+
import com.vsp.endpointinsightsapi.authentication.RequiredRoles;
34
import com.vsp.endpointinsightsapi.dto.DashboardSummaryResponseDTO;
45
import com.vsp.endpointinsightsapi.dto.DashboardTestActivityDTO;
56
import com.vsp.endpointinsightsapi.dto.charts.ChartResponseDTO;
7+
import com.vsp.endpointinsightsapi.model.enums.UserRole;
68
import com.vsp.endpointinsightsapi.service.DashboardService;
79
import com.vsp.endpointinsightsapi.service.PerformanceChartService;
810
import io.swagger.v3.oas.annotations.Operation;
@@ -30,6 +32,7 @@ public DashboardController(DashboardService dashboardService, PerformanceChartSe
3032
}
3133

3234
@PostMapping("/summary")
35+
@RequiredRoles(roles = {UserRole.WRITE})
3336
@Operation(summary = "Calculate dashboard summary", description = "Calculates aggregated summary statistics for test activities")
3437
@ApiResponses(value = {
3538
@ApiResponse(responseCode = "200", description = "Summary calculated successfully"),
@@ -43,6 +46,7 @@ public ResponseEntity<DashboardSummaryResponseDTO> summary(@RequestBody List<Das
4346

4447
@GetMapping("/charts/performance")
4548
@Operation(summary = "Get API performance chart data", description = "Retrieves performance metrics and chart data for APIs, optionally filtered by job or batch")
49+
@RequiredRoles(roles = {UserRole.READ})
4650
@ApiResponses(value = {
4751
@ApiResponse(responseCode = "200", description = "Chart data retrieved successfully"),
4852
@ApiResponse(responseCode = "400", description = "Invalid parameters - cannot provide both jobId and batchId"),

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/JobsController.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
package com.vsp.endpointinsightsapi.controller;
22

3+
import com.vsp.endpointinsightsapi.authentication.RequiredRoles;
34
import com.vsp.endpointinsightsapi.dto.GitCheckoutResponse;
45
import com.vsp.endpointinsightsapi.dto.JobDTO;
56
import com.vsp.endpointinsightsapi.exception.CustomExceptionBuilder;
67
import com.vsp.endpointinsightsapi.mapper.JobMapper;
78
import com.vsp.endpointinsightsapi.model.*;
89
import com.vsp.endpointinsightsapi.model.entity.TestRun;
910
import com.vsp.endpointinsightsapi.model.enums.TestType;
11+
import com.vsp.endpointinsightsapi.model.enums.UserRole;
1012
import com.vsp.endpointinsightsapi.service.JobService;
1113
import com.vsp.endpointinsightsapi.validation.ErrorMessages;
1214
import io.swagger.v3.oas.annotations.Operation;
@@ -48,7 +50,8 @@ public JobsController(JobService jobService, JobMapper jobMapper) {
4850
* */
4951
@PostMapping
5052
@Operation(summary = "Create new job", description = "Creates a new performance test job with the specified configuration")
51-
@ApiResponses(value = {
53+
@RequiredRoles(roles = {UserRole.WRITE})
54+
@ApiResponses(value = {
5255
@ApiResponse(responseCode = "201", description = "Job created successfully"),
5356
@ApiResponse(responseCode = "400", description = "Invalid input - validation failed"),
5457
@ApiResponse(responseCode = "401", description = "Unauthorized")
@@ -67,7 +70,8 @@ public ResponseEntity<JobDTO> createJob(@RequestBody @Valid JobCreateRequest job
6770

6871

6972
@PostMapping("/{id}/run")
70-
@Operation(summary = "Run job", description = "Executes a performance test job and creates a new test run")
73+
@RequiredRoles(roles = {UserRole.WRITE})
74+
@Operation(summary = "Run job", description = "Executes a performance test job and creates a new test run")
7175
@ApiResponses(value = {
7276
@ApiResponse(responseCode = "200", description = "Job execution started"),
7377
@ApiResponse(responseCode = "404", description = "Job not found"),
@@ -98,7 +102,8 @@ public ResponseEntity<TestRun> runJob(
98102
* @return the updated Job
99103
* */
100104
@PutMapping("/{id}")
101-
@Operation(summary = "Update job", description = "Updates an existing job configuration")
105+
@RequiredRoles(roles = {UserRole.WRITE})
106+
@Operation(summary = "Update job", description = "Updates an existing job configuration")
102107
@ApiResponses(value = {
103108
@ApiResponse(responseCode = "200", description = "Job updated successfully"),
104109
@ApiResponse(responseCode = "400", description = "Invalid input"),
@@ -126,6 +131,7 @@ public ResponseEntity<JobDTO> updateJob(
126131
* @return all job ids as a List of Strings
127132
* */
128133
@GetMapping
134+
@RequiredRoles(roles = {UserRole.READ})
129135
@Operation(summary = "List all jobs", description = "Retrieves a list of all performance test jobs")
130136
@ApiResponses(value = {
131137
@ApiResponse(responseCode = "200", description = "Jobs retrieved successfully"),
@@ -147,6 +153,7 @@ public ResponseEntity<List<JobDTO>> getJobs() {
147153
* @return the Job with the given jobId
148154
* */
149155
@GetMapping("/{id}")
156+
@RequiredRoles(roles = {UserRole.READ})
150157
@Operation(summary = "Get job by ID", description = "Retrieves a specific job by its unique identifier")
151158
@ApiResponses(value = {
152159
@ApiResponse(responseCode = "200", description = "Job found"),
@@ -178,6 +185,7 @@ public ResponseEntity<JobDTO> getJob(
178185
* @return A status message indicating the job was deleted
179186
* */
180187
@DeleteMapping("/{id}")
188+
@RequiredRoles(roles = {UserRole.WRITE})
181189
@Operation(summary = "Delete job", description = "Permanently deletes a job by its ID")
182190
@ApiResponses(value = {
183191
@ApiResponse(responseCode = "204", description = "Job deleted successfully"),
@@ -198,6 +206,7 @@ public ResponseEntity<Void> deleteJob(
198206
* @return A JobRunHistory object for the requested job
199207
* */
200208
@GetMapping("/{id}/history")
209+
@RequiredRoles(roles = {UserRole.WRITE})
201210
@Operation(summary = "Get job run history", description = "Retrieves the run history of a specific job")
202211
@ApiResponses(value = {
203212
@ApiResponse(responseCode = "200", description = "Job history retrieved"),
@@ -220,6 +229,7 @@ public ResponseEntity<JobRunHistory> getJobHistory(
220229
* @return checkout information
221230
* */
222231
@PostMapping("/{id}/checkout")
232+
@RequiredRoles(roles = {UserRole.WRITE})
223233
@Operation(summary = "Checkout job repository", description = "Checks out the Git repository associated with a job")
224234
@ApiResponses(value = {
225235
@ApiResponse(responseCode = "200", description = "Repository checked out successfully"),

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/NotificationGroupsController.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.vsp.endpointinsightsapi.controller;
22

3+
import com.vsp.endpointinsightsapi.authentication.RequiredRoles;
34
import com.vsp.endpointinsightsapi.model.NotificationGroup;
5+
import com.vsp.endpointinsightsapi.model.enums.UserRole;
46
import com.vsp.endpointinsightsapi.service.NotificationGroupService;
57
import jakarta.validation.constraints.NotNull;
68
import org.slf4j.Logger;
@@ -26,13 +28,15 @@ public NotificationGroupsController(NotificationGroupService notificationGroupSe
2628

2729
// GET /api/notification-groups - List all groups
2830
@GetMapping
31+
@RequiredRoles(roles = {UserRole.READ})
2932
public ResponseEntity<List<NotificationGroup>> getAllGroups() {
3033
List<NotificationGroup> groups = notificationGroupService.getAllGroups();
3134
return ResponseEntity.ok(groups);
3235
}
3336

3437
// GET /api/notification-groups/{id} - Get specific group with members
3538
@GetMapping("/{id}")
39+
@RequiredRoles(roles = {UserRole.READ})
3640
public ResponseEntity<NotificationGroup> getGroup(@PathVariable UUID id) {
3741
return notificationGroupService.getGroupById(id)
3842
.map(ResponseEntity::ok)
@@ -41,6 +45,7 @@ public ResponseEntity<NotificationGroup> getGroup(@PathVariable UUID id) {
4145

4246
// POST /api/notification-groups - Create new group
4347
@PostMapping
48+
@RequiredRoles(roles = {UserRole.WRITE})
4449
public ResponseEntity<NotificationGroup> createGroup(@RequestBody CreateGroupRequest request) {
4550
LOG.info("Creating new notification group: {}", request.getName());
4651
NotificationGroup group = notificationGroupService.createGroup(
@@ -53,6 +58,7 @@ public ResponseEntity<NotificationGroup> createGroup(@RequestBody CreateGroupReq
5358

5459
// PUT /api/notification-groups/{id} - Update existing group
5560
@PutMapping("/{id}")
61+
@RequiredRoles(roles = {UserRole.WRITE})
5662
public ResponseEntity<NotificationGroup> updateGroup(
5763
@PathVariable @NotNull UUID id,
5864
@RequestBody UpdateGroupRequest request) {
@@ -67,6 +73,7 @@ public ResponseEntity<NotificationGroup> updateGroup(
6773

6874
// DELETE /api/notification-groups/{id} - Delete group
6975
@DeleteMapping("/{id}")
76+
@RequiredRoles(roles = {UserRole.WRITE})
7077
public ResponseEntity<Void> deleteGroup(@PathVariable UUID id) {
7178
LOG.info("Deleting notification group: {}", id);
7279
notificationGroupService.deleteGroup(id);
@@ -75,6 +82,7 @@ public ResponseEntity<Void> deleteGroup(@PathVariable UUID id) {
7582

7683
// POST /api/notification-groups/{id}/members - Add members to group
7784
@PostMapping("/{id}/members")
85+
@RequiredRoles(roles = {UserRole.WRITE})
7886
public ResponseEntity<Void> addMembers(
7987
@PathVariable @NotNull UUID id,
8088
@RequestBody AddMembersRequest request) {
@@ -85,6 +93,7 @@ public ResponseEntity<Void> addMembers(
8593

8694
// DELETE /api/notification-groups/{id}/members/{email} - Remove member from group
8795
@DeleteMapping("/{id}/members/{email}")
96+
@RequiredRoles(roles = {UserRole.WRITE})
8897
public ResponseEntity<Void> removeMember(
8998
@PathVariable @NotNull UUID id,
9099
@PathVariable String email) {

‎endpoint-insights-api/src/main/java/com/vsp/endpointinsightsapi/controller/TestRunsController.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package com.vsp.endpointinsightsapi.controller;
22

3+
import com.vsp.endpointinsightsapi.authentication.RequiredRoles;
34
import com.vsp.endpointinsightsapi.dto.RecentActivityDTO;
45
import com.vsp.endpointinsightsapi.exception.CustomException;
56
import com.vsp.endpointinsightsapi.exception.CustomExceptionBuilder;
67
import com.vsp.endpointinsightsapi.model.TestRunCreateRequest;
78
import com.vsp.endpointinsightsapi.model.entity.TestRun;
9+
import com.vsp.endpointinsightsapi.model.enums.UserRole;
810
import com.vsp.endpointinsightsapi.service.TestRunService;
911
import com.vsp.endpointinsightsapi.exception.CustomException;
1012
import io.swagger.v3.oas.annotations.Operation;
@@ -38,6 +40,7 @@ public TestRunsController(TestRunService testRunService) {
3840
}
3941

4042
@GetMapping("/recent")
43+
@RequiredRoles(roles = {UserRole.READ})
4144
@Operation(summary = "Get recent test runs", description = "Retrieves the most recent test run executions")
4245
@ApiResponses(value = {
4346
@ApiResponse(responseCode = "200", description = "Recent test runs retrieved"),
@@ -50,6 +53,7 @@ public ResponseEntity<List<TestRun>> getRecentTestRuns(
5053
}
5154

5255
@GetMapping("/recent-activity")
56+
@RequiredRoles(roles = {UserRole.READ})
5357
@Operation(summary = "Get recent activity", description = "Retrieves recent test activity, optionally filtered by job or batch ID")
5458
@ApiResponses(value = {
5559
@ApiResponse(responseCode = "200", description = "Recent activity retrieved"),
@@ -80,6 +84,7 @@ public ResponseEntity<List<RecentActivityDTO>> getRecentActivity(
8084
}
8185

8286
@GetMapping("/{id}")
87+
@RequiredRoles(roles = {UserRole.READ})
8388
@Operation(summary = "Get test run by ID", description = "Retrieves a specific test run by its unique identifier")
8489
@ApiResponses(value = {
8590
@ApiResponse(responseCode = "200", description = "Test run found"),
@@ -93,6 +98,7 @@ public ResponseEntity<TestRun> getTestRunById(
9398
}
9499

95100
@DeleteMapping("/{id}")
101+
@RequiredRoles(roles = {UserRole.WRITE})
96102
@Operation(summary = "Delete test run", description = "Permanently deletes a test run by its ID")
97103
@ApiResponses(value = {
98104
@ApiResponse(responseCode = "200", description = "Test run deleted successfully"),
@@ -107,7 +113,8 @@ public ResponseEntity<Map<String, Object>> deleteTestRun(
107113
}
108114

109115
@DeleteMapping
110-
@Operation(summary = "Delete test runs before a specific date", description = "Permanently deletes all test runs that were finished before the specified purge date")
116+
@RequiredRoles(roles = {UserRole.WRITE})
117+
@Operation(summary = "Delete test runs before a specific date", description = "Permanently deletes all test runs that were finished before the specified purge date")
111118
@ApiResponses(value = {
112119
@ApiResponse(responseCode = "200", description = "Test runs deleted successfully"),
113120
@ApiResponse(responseCode = "400", description = "Invalid purge date - cannot be in the future"),

‎endpoint-insights-ui/src/app/login-component/login-component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ <h2>EndPoint Insights</h2>
99
<div class="welcome-text">Login to Access Dashboard</div>
1010
</div>
1111
<div class="button-container">
12-
<button class="login-button" (click)="login()">Log In With SSO</button>
12+
<button class="login-button" data-test-id="sso-login-button" (click)="login()">Log In With SSO</button>
1313
</div>
1414
</div>
1515
</div>

‎integration-tests/nightwatch/custom-commands/authenticateWithAuthelia.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ module.exports = class AuthenticateWithAuthelia {
99

1010
this.api
1111
.navigateTo(startUrl)
12+
.waitForElementVisible('[data-test-id="sso-login-button"]')
13+
.click('[data-test-id="sso-login-button"]')
1214
.waitForElementVisible('body')
1315
.assert.urlContains('auth.crowleybrynn.com')
1416

@@ -23,9 +25,8 @@ module.exports = class AuthenticateWithAuthelia {
2325

2426
.waitForElementVisible('#openid-consent-accept')
2527
.click('#openid-consent-accept')
26-
27-
.waitForElementVisible('body')
28-
.pause(1000);
28+
29+
.waitForElementVisible('[data-test-id="dashboard-title"]', 10000);
2930

3031
return this;
3132
}

‎integration-tests/test/batches.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('Batch Management Tests', function() {
5151
.assert.textContains('.job-row', testData.email)
5252

5353
.setValue('[data-test-id="search-available-tests-input"]', 'test')
54-
.pause(10)
54+
.waitForElementVisible('[data-test-id="add-test-button"]', 10000)
5555
.click('[data-test-id="add-test-button"]')
5656
.pause(10)
5757

0 commit comments

Comments
 (0)