Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ ext {
elasticVersion = '9.4.4'
testContainersVersion = '2.0.5'
cucumber = '7.34.6'
beftaFwVersion = '9.2.7'
beftaFwVersion = '9.2.9'
ccdTestDefinitionVersion = '7.26.7'
limits = [
'instruction': 90,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,19 @@ Scenario: must return the list of cases and status code 200 for correct inputs
And the response has all other details as expected.

#-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@S-026.03 # Access control parameters are namespaced (abac$...) so that no case field name can collide with,
# and therefore rebind, an access control predicate value. This covers the second entry point into
# the sanitiser (the aggregated/UI search) and pins that the namespace is unreachable from the API.
Scenario: must return 400 when a case field name targets the reserved access control parameter namespace

Given a user with [an active profile in CCD],

When a request is prepared with appropriate values,
And the request [contains a case field name inside the reserved access control parameter namespace],
And it is submitted to call the [Get case data with UI layout] operation of [CCD Data Store],

Then a negative response is received,
And the response [contains an error message : Field Names Invalid],
And the response has all other details as expected.

#-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"_guid_": "S-026.03",
"_extends_": "F-026_Test_Data_Base",
"title": "must return 400 when a case field name targets the reserved access control parameter namespace",

"specs": [
"an active profile in CCD",
"contains a case field name inside the reserved access control parameter namespace",
"contains an error message : Field Names Invalid"
],

"request": {
"queryParams": {
"case.abac$region_1_basic": "GB-WEST"
}
},

"expectedResponse": {
"_extends_": "Common_400_Response",
"body": {
"exception": "uk.gov.hmcts.ccd.endpoint.exceptions.BadRequestException",
"timestamp": "[[ANYTHING_PRESENT]]",
"message": "Field Names Invalid",
"path": "[[ANYTHING_PRESENT]]",
"details": null,
"callbackErrors": null,
"callbackWarnings": null
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,36 @@ Scenario: must return pagination metadata successfully for correct Last State Mo
And the response has all the details as expected.

#-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@S-048.02 # Access control parameters are namespaced (abac$...) so that no case field name can collide with,
# and therefore rebind, an access control predicate value. This pins that the namespace is
# unreachable from the API: the reserved character is rejected before the query is ever built.
Scenario: must return 400 when a case field name targets the reserved access control parameter namespace

Given a user with [a detailed profile in CCD],

When a request is prepared with appropriate values,
And the request [contains a case field name inside the reserved access control parameter namespace],
And it is submitted to call the [Get the pagination metadata for a case data search for Case Worker] operation of [CCD Data Store],

Then a negative response is received,
And the response [contains an error message : Field Names Invalid],
And the response has all the details as expected.

#-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@S-048.03 # A case field criterion and the access control predicates are bound into the same query, from two
# separate parameter namespaces. This pins that they coexist: the search is accepted, the criterion
# is bound and applied, and no parameter fails to resolve at execution time.
Scenario: must apply a case field search criterion alongside the access control predicates

Given a user with [a detailed profile in CCD],

When a request is prepared with appropriate values,
And the request [filters on a case field value that no case holds],
And it is submitted to call the [Get the pagination metadata for a case data search for Case Worker] operation of [CCD Data Store],

Then a positive response is received,
And the response [returns the pagination metadata],
And the response [contains pagination results count as 0],
And the response has all the details as expected.

#-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{
"_guid_": "S-048.02",
"_extends_": "Base_Expecting_400",
"title": "must return 400 when a case field name targets the reserved access control parameter namespace",

"productName": "CCD Data Store",
"operationName": "Get the pagination metadata for a case data search for Case Worker",

"method": "GET",
"uri": "/caseworkers/{uid}/jurisdictions/{jid}/case-types/{ctid}/cases/pagination_metadata",

"specs": [
"a detailed profile in CCD",
"contains a case field name inside the reserved access control parameter namespace",
"contains an error message : Field Names Invalid"
],

"request": {
"pathVariables": {
"uid": "[[DEFAULT_AUTO_VALUE]]",
"jid": "AUTOTEST1",
"ctid": "AAT"
},
"queryParams": {
"case.abac$region_1_basic": "GB-WEST"
}
},

"expectedResponse": {
"_extends_": "Common_400_Response",
"headers": {
"Content-Encoding": "gzip",
"Content-Length": "[[ANYTHING_PRESENT]]"
},
"body": {
"exception": "uk.gov.hmcts.ccd.endpoint.exceptions.BadRequestException",
"timestamp": "[[ANYTHING_PRESENT]]",
"message": "Field Names Invalid",
"path": "[[ANYTHING_PRESENT]]",
"details": null,
"callbackErrors": null,
"callbackWarnings": null
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"_guid_": "S-048.03",
"_extends_": "Base_Expecting_200",
"title": "must apply a case field search criterion alongside the access control predicates",

"productName": "CCD Data Store",
"operationName": "Get the pagination metadata for a case data search for Case Worker",

"method": "GET",
"uri": "/caseworkers/{uid}/jurisdictions/{jid}/case-types/{ctid}/cases/pagination_metadata",

"specs": [
"a detailed profile in CCD",
"filters on a case field value that no case holds",
"returns the pagination metadata",
"contains pagination results count as 0"
],

"request": {
"pathVariables": {
"uid": "[[DEFAULT_AUTO_VALUE]]",
"jid": "AUTOTEST1",
"ctid": "AAT"
},
"queryParams": {
"case.TextField": "S-048.03 no case holds this value"
}
},

"expectedResponse": {
"headers": {
"Content-Encoding": "gzip",
"Content-Length": "[[ANYTHING_PRESENT]]"
},
"body": {
"total_results_count": 0,
"total_pages_count": 0
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
import uk.gov.hmcts.ccd.domain.service.casedataaccesscontrol.CaseDataAccessControl;
import uk.gov.hmcts.ccd.domain.service.common.CaseTypeService;
import uk.gov.hmcts.ccd.domain.service.security.AuthorisedCaseDefinitionDataService;
import uk.gov.hmcts.ccd.endpoint.exceptions.BadRequestException;
import uk.gov.hmcts.ccd.infrastructure.user.UserAuthorisation;

import static uk.gov.hmcts.ccd.data.casedetails.search.builder.GrantTypeSqlQueryBuilder.ACCESS_CONTROL_PARAM_PREFIX;
import static uk.gov.hmcts.ccd.domain.service.common.AccessControlService.CAN_READ;

@Named
Expand All @@ -32,6 +34,11 @@ public class SearchQueryFactoryOperation {
private static final String OPERATION_EQ = " = ";
private static final String OPERATION_LIKE = " LIKE ";

// Access control parameters live in a namespace user supplied search fields cannot reach
// see GrantTypeSqlQueryBuilder#ACCESS_CONTROL_PARAM_PREFIX.
static final String USER_ID_PARAM = ACCESS_CONTROL_PARAM_PREFIX + "user_id";
static final String CASE_STATES_PARAM = ACCESS_CONTROL_PARAM_PREFIX + "states";

@PersistenceContext
private final EntityManager entityManager;

Expand Down Expand Up @@ -75,6 +82,7 @@ public Optional<Query> build(MetaData metadata, Map<String, String> params, bool
if (whereClausePart.isEmpty()) {
return Optional.empty();
}
rejectAccessControlParameterCollisions(criteria, parametersToBind);

String sortClause = sortOrderQueryBuilder.buildSortOrderClause(metadata);
String queryToFormat = isCountQuery ? MAIN_COUNT_QUERY : MAIN_QUERY;
Expand All @@ -88,7 +96,7 @@ public Optional<Query> build(MetaData metadata, Map<String, String> params, bool
}
parametersToBind.forEach(query::setParameter);
addParameters(query, criteria);
log.debug("[SQL Query ]] : " + queryString);
log.debug("[SQL Query ]] : {}", queryString);
return Optional.of(query);
}

Expand All @@ -111,8 +119,9 @@ private String addUserCaseAccessClause(Map<String, Object> params, MetaData meta
return accessControlGrantTypeQueryBuilder.createQuery(roleAssignments, params, caseTypeDefinition);
}
} else if (UserAuthorisation.AccessLevel.GRANTED.equals(userAuthorisation.getAccessLevel())) {
params.put("user_id", userAuthorisation.getUserId());
return " AND id IN (SELECT cu.case_data_id FROM case_users AS cu WHERE user_id = :user_id)";
params.put(USER_ID_PARAM, userAuthorisation.getUserId());
return " AND id IN (SELECT cu.case_data_id FROM case_users AS cu WHERE user_id = :"
+ USER_ID_PARAM + ")";
}
return "";
}
Expand All @@ -125,13 +134,32 @@ private String addUserCaseStateAccessClause(MetaData metadata, Map<String, Objec
metadata.getCaseTypeId(),
CAN_READ);
if (!caseStateIds.isEmpty()) {
params.put("states", caseStateIds);
return " AND state IN (:states)";
params.put(CASE_STATES_PARAM, caseStateIds);
return " AND state IN (:" + CASE_STATES_PARAM + ")";
}
}
return "";
}

/**
* Criteria are bound after the access control parameters, so a criterion whose parameter id matched an
* access control parameter would silently replace that predicate's value and widen the user's access.
* The reserved prefix makes this unreachable; this check keeps the guarantee enforced
* if the field name validation or the prefix ever changes.
*/
private void rejectAccessControlParameterCollisions(List<Criterion> criteria,
Map<String, Object> accessControlParams) {
criteria.stream()
.map(Criterion::buildParameterId)
.filter(accessControlParams::containsKey)
.findFirst()
.ifPresent(parameterId -> {
// This is a security risk, so we should log it and throw an exception.
log.warn("Search field parameter '{}' collides with an access control parameter", parameterId);
throw new BadRequestException("Field Names Invalid");
});
}

private void addParameters(final Query query, List<Criterion> criteria) {
criteria.forEach(criterion -> query.setParameter(criterion.buildParameterId(), criterion.getSoughtValue()));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package uk.gov.hmcts.ccd.data.casedetails.search;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.Strings;
import org.springframework.stereotype.Component;
import uk.gov.hmcts.ccd.data.casedetails.search.MetaData.CaseField;
import uk.gov.hmcts.ccd.endpoint.exceptions.BadRequestException;
Expand All @@ -15,7 +16,7 @@ public class SortOrderQueryBuilder {
private static final String CREATED_DATE = "created_date";
private static final String SPACE = " ";
private static final String COMMA = ",";
private static final String CASE_FIELD_ID_PATTERN = "^['a-zA-Z0-9\\[\\]\\#%\\&()\\.?_\\£\\s\\xA0-]+$";
private static final String CASE_FIELD_ID_PATTERN = "^[a-zA-Z0-9_.\\[\\]]+$";


public String buildSortOrderClause(MetaData metaData) {
Expand All @@ -37,7 +38,7 @@ public String buildSortOrderClause(MetaData metaData) {
});
// always sort with creation_date as a last order so that it supports cases where
// no values at all for the configured fields and also default fallback.
return sb.append(CREATED_DATE + SPACE + fromOptionalString(metaData.getSortDirection())).toString();
return sb.append(CREATED_DATE + SPACE).append(fromOptionalString(metaData.getSortDirection())).toString();
}

private String getMataFieldName(String fieldName) {
Expand All @@ -47,7 +48,7 @@ private String getMataFieldName(String fieldName) {
}

private static String convertFieldNameToJsonbSqlFormat(final String in) {
return DATA_FIELD + " #>> '{" + StringUtils.replace(in, ".", ",") + "}'";
return DATA_FIELD + " #>> '{" + Strings.CS.replace(in, ".", ",") + "}'";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
@SuppressWarnings("checkstyle:AbbreviationAsWordInName")
public class ExcludedGrantTypeQueryBuilder extends GrantTypeSqlQueryBuilder {

public static final String CASE_IDS_EXCLUDED_PARAM = ACCESS_CONTROL_PARAM_PREFIX + "case_ids_excluded";

@Autowired
public ExcludedGrantTypeQueryBuilder(AccessControlService accessControlService,
CaseDataAccessControl caseDataAccessControl,
Expand Down Expand Up @@ -56,9 +58,8 @@ private String getCaseReferences(Supplier<Stream<RoleAssignment>> streamSupplier
.collect(Collectors.toSet());

if (!caseReferences.isEmpty()) {
String paramName = "case_ids_excluded";
params.put(paramName, caseReferences);
return String.format(QUERY, REFERENCE, paramName);
params.put(CASE_IDS_EXCLUDED_PARAM, caseReferences);
return String.format(QUERY, REFERENCE, CASE_IDS_EXCLUDED_PARAM);
}
return EMPTY;
}
Expand Down
Loading