Skip to content

Commit

Permalink
refactor: delay validation of the payloads
Browse files Browse the repository at this point in the history
We are interested in checking for authorization first, and then we can
go and validate if the body has been properly sent or not.

RHCLOUD-32983
  • Loading branch information
MikelAlejoBR committed Jul 8, 2024
1 parent c51c965 commit 802e5e1
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ public List<Facet> getBundleFacets(@Context SecurityContext sec, @QueryParam("in
@Transactional
public CreateBehaviorGroupResponse createBehaviorGroup(
@Context final SecurityContext sec,
@RequestBody(required = true) @Valid @NotNull final CreateBehaviorGroupRequest request
@RequestBody(required = true) final CreateBehaviorGroupRequest request
) {
if (this.backendConfig.isKesselBackendEnabled()) {
this.kesselAuthorization.hasPermissionOnResource(sec, WorkspacePermission.NOTIFICATIONS_WRITE, ResourceType.WORKSPACE, WORKSPACE_ID_PLACEHOLDER);
Expand All @@ -422,7 +422,7 @@ public CreateBehaviorGroupResponse legacyRBACCreateBehaviorGroup(final SecurityC
return this.internalCreateBehaviorGroup(securityContext, request);
}

public CreateBehaviorGroupResponse internalCreateBehaviorGroup(final SecurityContext sec, final CreateBehaviorGroupRequest request) {
public CreateBehaviorGroupResponse internalCreateBehaviorGroup(final SecurityContext sec, @NotNull @Valid final CreateBehaviorGroupRequest request) {
String accountId = getAccountId(sec);
String orgId = getOrgId(sec);

Expand Down Expand Up @@ -480,7 +480,7 @@ public CreateBehaviorGroupResponse internalCreateBehaviorGroup(final SecurityCon
@Transactional
public Response updateBehaviorGroup(@Context SecurityContext sec,
@Parameter(description = "The UUID of the behavior group to update") @PathParam("id") UUID id,
@RequestBody(description = "New parameters", required = true) @NotNull @Valid UpdateBehaviorGroupRequest request) {
@RequestBody(description = "New parameters", required = true) UpdateBehaviorGroupRequest request) {
if (this.backendConfig.isKesselBackendEnabled()) {
this.kesselAuthorization.hasPermissionOnResource(sec, ResourcePermission.WRITE, ResourceType.BEHAVIOR_GROUP, id.toString());

Expand All @@ -499,7 +499,7 @@ public Response legacyRBACUpdateBehaviorGroup(final SecurityContext securityCont
return this.internalUpdateBehaviorGroup(securityContext, id, request);
}

public Response internalUpdateBehaviorGroup(final SecurityContext securityContext, final UUID id, final UpdateBehaviorGroupRequest request) {
public Response internalUpdateBehaviorGroup(final SecurityContext securityContext, final UUID id, @NotNull @Valid final UpdateBehaviorGroupRequest request) {
String orgId = getOrgId(securityContext);

if (request.displayName != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ static class V2 extends OrgConfigResource {
@Consumes(APPLICATION_JSON)
@Transactional
@Operation(summary = "Set the daily digest time", description = "Sets the daily digest UTC time. The accepted minute values are 00, 15, 30, and 45. Use this endpoint to set the time when daily emails are sent.")
public void saveDailyDigestTimePreference(@Context SecurityContext sec, @NotNull LocalTime expectedTime) {
public void saveDailyDigestTimePreference(@Context SecurityContext sec, LocalTime expectedTime) {
if (this.backendConfig.isKesselBackendEnabled()) {
this.kesselAuthorization.hasPermissionOnResource(sec, WorkspacePermission.NOTIFICATIONS_WRITE, ResourceType.WORKSPACE, WORKSPACE_ID_PLACEHOLDER);

Expand All @@ -86,7 +86,7 @@ public void legacyRBACInternalSaveDailyDigestTimePreference(final SecurityContex
this.internalSaveDailyDigestTimePreference(securityContext, expectedTime);
}

public void internalSaveDailyDigestTimePreference(final SecurityContext securityContext, final LocalTime expectedTime) {
public void internalSaveDailyDigestTimePreference(final SecurityContext securityContext, @NotNull final LocalTime expectedTime) {
String orgId = getOrgId(securityContext);
if (!ALLOWED_MINUTES.contains(expectedTime.getMinute())) {
String errorMessage = "Accepted minute values are: " + ALLOWED_MINUTES.stream().map(min -> String.format("%02d", min)).collect(Collectors.joining(", ")) + ".";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.redhat.cloud.notifications.routers;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.redhat.cloud.notifications.Json;
import com.redhat.cloud.notifications.MockServerConfig;
import com.redhat.cloud.notifications.MockServerConfig.RbacAccess;
Expand Down Expand Up @@ -93,9 +91,6 @@ public class NotificationResourceTest extends DbIsolatedTest {
@InjectMock
BackendConfig backendConfig;

@Inject
ObjectMapper objectMapper;

@BeforeEach
void beforeEach() {
RestAssured.basePath = TestConstants.API_NOTIFICATIONS_V_1_0;
Expand Down Expand Up @@ -840,7 +835,7 @@ void testBundleApplicationEventTypeByName() {
}

@Test
void testInsufficientPrivileges() throws JsonProcessingException {
void testInsufficientPrivileges() {
Header noAccessIdentityHeader = initRbacMock("tenant", "orgId", "noAccess", NO_ACCESS);
Header readAccessIdentityHeader = initRbacMock("tenant", "orgId", "readAccess", READ_ACCESS);

Expand Down Expand Up @@ -885,23 +880,17 @@ void testInsufficientPrivileges() throws JsonProcessingException {
.then()
.statusCode(403);

final CreateBehaviorGroupRequest createBehaviorGroupRequest = new CreateBehaviorGroupRequest();
createBehaviorGroupRequest.bundleId = UUID.randomUUID();
createBehaviorGroupRequest.displayName = "create-behavior-group-test";
given()
.header(readAccessIdentityHeader)
.contentType(JSON)
.body(this.objectMapper.writeValueAsString(createBehaviorGroupRequest))
.when()
.post("/notifications/behaviorGroups")
.then()
.statusCode(403);

final UpdateBehaviorGroupRequest updateBehaviorGroupRequest = new UpdateBehaviorGroupRequest();
given()
.header(readAccessIdentityHeader)
.contentType(JSON)
.body(this.objectMapper.writeValueAsString(updateBehaviorGroupRequest))
.pathParam("id", UUID.randomUUID())
.when()
.put("/notifications/behaviorGroups/{id}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
@QuarkusTest
@QuarkusTestResource(TestLifecycleManager.class)
class OrgConfigResourceTest extends DbIsolatedTest {
@Inject
ObjectMapper objectMapper;

static final LocalTime TIME = LocalTime.of(10, 00);

Expand Down Expand Up @@ -120,7 +118,7 @@ private String recordDailyDigestTimePreference(Header header, LocalTime time, in
}

@Test
void testInsufficientPrivileges() throws JsonProcessingException {
void testInsufficientPrivileges() {
Header noAccessIdentityHeader = initRbacMock("tenant", "orgId", "noAccess", NO_ACCESS);
Header readAccessIdentityHeader = initRbacMock("tenant", "orgId", "readAccess", READ_ACCESS);

Expand All @@ -134,8 +132,6 @@ void testInsufficientPrivileges() throws JsonProcessingException {
given()
.header(noAccessIdentityHeader)
.when()
.contentType(MediaType.APPLICATION_JSON)
.body(this.objectMapper.writeValueAsString(LocalTime.now(Clock.systemUTC())))
.put(ORG_CONFIG_NOTIFICATION_DAILY_DIGEST_TIME_PREFERENCE_URL)
.then()
.statusCode(403);
Expand All @@ -150,8 +146,6 @@ void testInsufficientPrivileges() throws JsonProcessingException {
given()
.header(readAccessIdentityHeader)
.when()
.contentType(MediaType.APPLICATION_JSON)
.body(this.objectMapper.writeValueAsString(LocalTime.now(Clock.systemUTC())))
.put(ORG_CONFIG_NOTIFICATION_DAILY_DIGEST_TIME_PREFERENCE_URL)
.then()
.statusCode(403);
Expand Down

0 comments on commit 802e5e1

Please sign in to comment.