Skip to content

Commit c878dd1

Browse files
nscuronscuro
authored
Merge pull request #1060 from DependencyTrack/acl-hierarchical
2 parents e335763 + b2a4bdb commit c878dd1

File tree

13 files changed

+502
-112
lines changed

13 files changed

+502
-112
lines changed

‎pom.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,7 @@
568568
<directory>src/main/resources</directory>
569569
<filtering>true</filtering>
570570
<includes>
571+
<include>META-INF/MANIFEST.MF</include>
571572
<include>application.version</include>
572573
<include>openapi-configuration.yaml</include>
573574
</includes>

‎src/main/java/org/dependencytrack/persistence/ProjectQueryManager.java

Lines changed: 52 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import com.fasterxml.jackson.databind.json.JsonMapper;
3333
import com.github.packageurl.PackageURL;
3434
import org.apache.commons.collections4.CollectionUtils;
35-
import org.apache.commons.lang3.StringUtils;
3635
import org.datanucleus.api.jdo.JDOQuery;
3736
import org.dependencytrack.auth.Permissions;
3837
import org.dependencytrack.event.kafka.KafkaEventDispatcher;
@@ -71,6 +70,7 @@
7170
import java.util.UUID;
7271
import java.util.concurrent.atomic.AtomicReference;
7372

73+
import static java.util.Objects.requireNonNullElse;
7474
import static org.dependencytrack.util.PersistenceUtil.assertPersistent;
7575
import static org.dependencytrack.util.PersistenceUtil.assertPersistentAll;
7676

@@ -940,45 +940,54 @@ public Project updateLastBomImport(Project p, Date date, String bomFormat) {
940940

941941
@Override
942942
public boolean hasAccess(final Principal principal, final Project project) {
943-
if (isEnabled(ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED)) {
944-
if (principal instanceof UserPrincipal userPrincipal) {
945-
if (super.hasAccessManagementPermission(userPrincipal)) {
946-
return true;
947-
}
948-
if (userPrincipal.getTeams() != null) {
949-
for (final Team userInTeam : userPrincipal.getTeams()) {
950-
for (final Team accessTeam : project.getAccessTeams()) {
951-
if (userInTeam.getId() == accessTeam.getId()) {
952-
return true;
953-
}
954-
}
955-
}
956-
}
957-
} else if (principal instanceof ApiKey apiKey) {
958-
if (super.hasAccessManagementPermission(apiKey)) {
959-
return true;
960-
}
961-
if (apiKey.getTeams() != null) {
962-
for (final Team userInTeam : apiKey.getTeams()) {
963-
for (final Team accessTeam : project.getAccessTeams()) {
964-
if (userInTeam.getId() == accessTeam.getId()) {
965-
return true;
966-
}
967-
}
968-
}
969-
}
970-
} else if (principal == null) {
971-
// This is a system request being made (e.g. MetricsUpdateTask, etc) where there isn't a principal
972-
return true;
973-
}
974-
return false;
975-
} else {
943+
if (principal == null) {
944+
// This is a system request being made (e.g. MetricsUpdateTask, etc) where there isn't a principal
945+
return true;
946+
}
947+
948+
if (!isEnabled(ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED)) {
949+
return true;
950+
}
951+
952+
// TODO: After upgrading to Alpine >= 3.2.0, this should become:
953+
// request.getEffectivePermission().contains(Permissions.ACCESS_MANAGEMENT.name())
954+
// https://github.com/stevespringett/Alpine/pull/764
955+
if (super.hasAccessManagementPermission(principal)) {
976956
return true;
977957
}
958+
959+
final Set<Long> teamIds = getTeamIds(principal);
960+
if (teamIds.isEmpty()) {
961+
return false;
962+
}
963+
964+
final Query<?> query = pm.newQuery(Query.SQL, "SELECT HAS_PROJECT_ACCESS(:projectId, :teamIds)");
965+
query.setNamedParameters(Map.ofEntries(
966+
Map.entry("projectId", project.getId()),
967+
Map.entry("teamIds", teamIds.toArray(new Long[0]))));
968+
return executeAndCloseResultUnique(query, Boolean.class);
978969
}
979970

980971
@Override
981972
void preprocessACLs(final Query<?> query, final String inputFilter, final Map<String, Object> params, final boolean bypass) {
973+
if (bypass
974+
|| principal == null
975+
|| !isEnabled(ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED)
976+
|| hasAccessManagementPermission(principal)) {
977+
query.setFilter(inputFilter);
978+
return;
979+
}
980+
981+
final Set<Long> teamIds = getTeamIds(principal);
982+
if (teamIds.isEmpty()) {
983+
if (inputFilter != null && !inputFilter.isBlank()) {
984+
query.setFilter(inputFilter + " && false");
985+
} else {
986+
query.setFilter("false");
987+
}
988+
return;
989+
}
990+
982991
String projectMemberFieldName = null;
983992
final org.datanucleus.store.query.Query<?> internalQuery = ((JDOQuery<?>)query).getInternalQuery();
984993
if (!Project.class.equals(internalQuery.getCandidateClass())) {
@@ -1006,45 +1015,16 @@ void preprocessACLs(final Query<?> query, final String inputFilter, final Map<St
10061015
.formatted(internalQuery.getCandidateClassName(), Project.class.getName()));
10071016
}
10081017
}
1009-
if (super.principal != null && isEnabled(ConfigPropertyConstants.ACCESS_MANAGEMENT_ACL_ENABLED) && !bypass) {
1010-
final List<Team> teams;
1011-
if (super.principal instanceof UserPrincipal userPrincipal) {
1012-
teams = userPrincipal.getTeams();
1013-
if (super.hasAccessManagementPermission(userPrincipal)) {
1014-
query.setFilter(inputFilter);
1015-
return;
1016-
}
1017-
} else {
1018-
final ApiKey apiKey = ((ApiKey) super.principal);
1019-
teams = apiKey.getTeams();
1020-
if (super.hasAccessManagementPermission(apiKey)) {
1021-
query.setFilter(inputFilter);
1022-
return;
1023-
}
1024-
}
1025-
if (teams != null && !teams.isEmpty()) {
1026-
final StringBuilder sb = new StringBuilder();
1027-
for (int i = 0, teamsSize = teams.size(); i < teamsSize; i++) {
1028-
final Team team = super.getObjectById(Team.class, teams.get(i).getId());
1029-
sb.append(" ");
1030-
if (projectMemberFieldName != null) {
1031-
sb.append(projectMemberFieldName).append(".");
1032-
}
1033-
sb.append(" accessTeams.contains(:team").append(i).append(") ");
1034-
params.put("team" + i, team);
1035-
if (i < teamsSize - 1) {
1036-
sb.append(" || ");
1037-
}
1038-
}
1039-
if (inputFilter != null && !inputFilter.isBlank()) {
1040-
query.setFilter(inputFilter + " && (" + sb + ")");
1041-
} else {
1042-
query.setFilter(sb.toString());
1043-
}
1044-
}
1045-
} else if (StringUtils.trimToNull(inputFilter) != null) {
1046-
query.setFilter(inputFilter);
1018+
1019+
final var aclCondition = "%s.isAccessibleBy(:projectAclTeamIds)".formatted(
1020+
requireNonNullElse(projectMemberFieldName, "this"));
1021+
if (inputFilter != null && !inputFilter.isBlank()) {
1022+
query.setFilter("%s && (%s)".formatted(inputFilter, aclCondition));
1023+
} else {
1024+
query.setFilter("(%s)".formatted(aclCondition));
10471025
}
1026+
1027+
params.put("projectAclTeamIds", teamIds.toArray(new Long[0]));
10481028
}
10491029

10501030
/**

‎src/main/java/org/dependencytrack/persistence/QueryManager.java

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2052,34 +2052,22 @@ public Map.Entry<String, Map<String, Object>> getProjectAclSqlCondition() {
20522052
*/
20532053
public Map.Entry<String, Map<String, Object>> getProjectAclSqlCondition(final String projectTableAlias) {
20542054
if (request == null) {
2055-
return Map.entry(/* true */ "1=1", Collections.emptyMap());
2055+
return Map.entry("TRUE", Collections.emptyMap());
20562056
}
20572057

20582058
if (principal == null || !isEnabled(ACCESS_MANAGEMENT_ACL_ENABLED) || hasAccessManagementPermission(principal)) {
2059-
return Map.entry(/* true */ "1=1", Collections.emptyMap());
2059+
return Map.entry("TRUE", Collections.emptyMap());
20602060
}
20612061

20622062
final var teamIds = new ArrayList<>(getTeamIds(principal));
20632063
if (teamIds.isEmpty()) {
2064-
return Map.entry(/* false */ "1=2", Collections.emptyMap());
2064+
return Map.entry("FALSE", Collections.emptyMap());
20652065
}
20662066

2067-
2068-
// NB: Need to work around the fact that the RDBMSes can't agree on how to do member checks. Oh joy! :)))
20692067
final var params = new HashMap<String, Object>();
2070-
final var teamIdChecks = new ArrayList<String>();
2071-
for (int i = 0; i < teamIds.size(); i++) {
2072-
teamIdChecks.add("\"PROJECT_ACCESS_TEAMS\".\"TEAM_ID\" = :teamId" + i);
2073-
params.put("teamId" + i, teamIds.get(i));
2074-
}
2068+
params.put("projectAclTeamIds", teamIds.toArray(new Long[0]));
20752069

2076-
return Map.entry("""
2077-
EXISTS (
2078-
SELECT 1
2079-
FROM "PROJECT_ACCESS_TEAMS"
2080-
WHERE "PROJECT_ACCESS_TEAMS"."PROJECT_ID" = "%s"."ID"
2081-
AND (%s)
2082-
)""".formatted(projectTableAlias, String.join(" OR ", teamIdChecks)), params);
2070+
return Map.entry("HAS_PROJECT_ACCESS(\"%s\".\"ID\", :projectAclTeamIds)".formatted(projectTableAlias), params);
20832071
}
20842072

20852073
/**

‎src/main/java/org/dependencytrack/persistence/datanucleus/method/ProjectIsAccessibleByMethod.java

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* This file is part of Dependency-Track.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
* Copyright (c) OWASP Foundation. All Rights Reserved.
18+
*/
19+
package org.dependencytrack.persistence.datanucleus.method;
20+
21+
import org.datanucleus.store.query.expression.Expression;
22+
import org.datanucleus.store.rdbms.mapping.java.JavaTypeMapping;
23+
import org.datanucleus.store.rdbms.sql.SQLStatement;
24+
import org.datanucleus.store.rdbms.sql.expression.ArrayLiteral;
25+
import org.datanucleus.store.rdbms.sql.expression.BooleanExpression;
26+
import org.datanucleus.store.rdbms.sql.expression.BooleanLiteral;
27+
import org.datanucleus.store.rdbms.sql.expression.ObjectExpression;
28+
import org.datanucleus.store.rdbms.sql.expression.SQLExpression;
29+
import org.datanucleus.store.rdbms.sql.expression.StringExpression;
30+
import org.datanucleus.store.rdbms.sql.expression.StringLiteral;
31+
import org.datanucleus.store.rdbms.sql.method.SQLMethod;
32+
import org.dependencytrack.model.Project;
33+
34+
import java.util.List;
35+
import java.util.StringJoiner;
36+
37+
/**
38+
* @since 5.6.0
39+
*/
40+
public class ProjectIsAccessibleByMethod implements SQLMethod {
41+
42+
@Override
43+
public SQLExpression getExpression(
44+
final SQLStatement stmt,
45+
final SQLExpression expr,
46+
final List<SQLExpression> args) {
47+
if (!(expr instanceof final ObjectExpression objectExpr)) {
48+
// DataNucleus should prevent this from ever happening since
49+
// the method is explicitly registered for java.lang.Object.
50+
throw new IllegalStateException(
51+
"Expected expression to be of type %s, but got: %s".formatted(
52+
ObjectExpression.class.getName(), expr.getClass().getName()));
53+
}
54+
55+
final String objectTypeName = objectExpr.getJavaTypeMapping().getType();
56+
if (!Project.class.getName().equals(objectTypeName)) {
57+
throw new IllegalStateException(
58+
"isAccessibleBy is only allowed for objects of type %s, but was called on %s".formatted(
59+
Project.class.getName(), objectTypeName));
60+
}
61+
62+
if (args == null) {
63+
throw new IllegalArgumentException();
64+
} else if (args.size() != 1) {
65+
throw new IllegalArgumentException("Expected exactly one argument, but got " + args.size());
66+
}
67+
68+
// TODO: When a list, set, etc. is passed as argument, it will be of type CollectionLiteral.
69+
// Array literals are easier to verify the type of, hence we're focusing on that for now.
70+
71+
if (!(args.getFirst() instanceof final ArrayLiteral arrayLiteralArg)) {
72+
throw new IllegalArgumentException(
73+
"Expected argument to be of type %s, but got %s".formatted(
74+
ArrayLiteral.class.getName(), args.getFirst().getClass().getName()));
75+
}
76+
if (!(arrayLiteralArg.getValue() instanceof final Long[] teamIds)) {
77+
throw new IllegalArgumentException(
78+
"Expected array argument to be of type %s, but got %s".formatted(
79+
Long[].class.getName(), arrayLiteralArg.getValue().getClass().getName()));
80+
}
81+
82+
final JavaTypeMapping booleanTypeMapping = stmt.getSQLExpressionFactory().getMappingForType(Boolean.class);
83+
final JavaTypeMapping stringTypeMapping = stmt.getSQLExpressionFactory().getMappingForType(String.class);
84+
85+
// Transform the array literal to have the correct type for Postgres.
86+
// Will result in the following expression: cast('{1,2,3}' as bigint[])
87+
final StringJoiner joiner = new StringJoiner(",", "{", "}");
88+
for (final Long teamId : teamIds) {
89+
joiner.add(String.valueOf(teamId));
90+
}
91+
final var teamIdsLiteral = new StringLiteral(
92+
stmt, stringTypeMapping, joiner.toString(), null);
93+
final var teamIdsExpr = new StringExpression(
94+
stmt, stringTypeMapping, "cast", List.of(teamIdsLiteral), List.of("bigint[]"));
95+
96+
// NB: objectExpr will compile to a reference of the object table's ID column, e.g.:
97+
// * "A0"."ID"
98+
// * "B0"."PROJECT_ID"
99+
final var hasProjectAccessFunctionExpr = new StringExpression(
100+
stmt, stringTypeMapping, "has_project_access", List.of(objectExpr, teamIdsExpr));
101+
102+
// Wrap the function call in a boolean expression. Final result(s) will be:
103+
// * has_project_access("A0"."ID", cast('{1,2,3}' as bigint[])) = TRUE
104+
// * has_project_access("B0"."PROJECT_ID", cast('{1,2,3}' as bigint[])) = TRUE
105+
final var booleanTrueLiteral = new BooleanLiteral(stmt, booleanTypeMapping, Boolean.TRUE, null);
106+
return new BooleanExpression(hasProjectAccessFunctionExpr, Expression.OP_EQ, booleanTrueLiteral);
107+
}
108+
109+
}

‎src/main/java/org/dependencytrack/persistence/jdbi/ApiRequestStatementCustomizer.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,7 @@
7171
class ApiRequestStatementCustomizer implements StatementCustomizer {
7272

7373
static final String PARAMETER_PROJECT_ACL_TEAM_IDS = "projectAclTeamIds";
74-
static final String TEMPLATE_PROJECT_ACL_CONDITION = """
75-
EXISTS (
76-
SELECT 1
77-
FROM "PROJECT_ACCESS_TEAMS"
78-
WHERE "PROJECT_ACCESS_TEAMS"."PROJECT_ID" = "%s"."ID"
79-
AND "PROJECT_ACCESS_TEAMS"."TEAM_ID" = ANY(:projectAclTeamIds)
80-
)""";
74+
static final String TEMPLATE_PROJECT_ACL_CONDITION = "HAS_PROJECT_ACCESS(\"%s\".\"ID\", :projectAclTeamIds)";
8175

8276
private final AlpineRequest apiRequest;
8377

@@ -214,9 +208,9 @@ private boolean isAclEnabled(final StatementContext ctx) throws SQLException {
214208
}
215209

216210
private boolean hasAccessManagementPermission(final StatementContext ctx, final Principal principal) throws SQLException {
217-
// NB: This could be simplified if Alpine's AuthenticationFilter would load all
218-
// effective permissions of the authenticated user, and make them available in
219-
// the Principal object. Then, we wouldn't need to perform any database queries here.
211+
// TODO: After upgrading to Alpine >= 3.2.0, this should become:
212+
// apiRequest.getEffectivePermission().contains(Permissions.ACCESS_MANAGEMENT.name())
213+
// https://github.com/stevespringett/Alpine/pull/764
220214

221215
return switch (principal) {
222216
case ApiKey apiKey -> hasAccessManagementPermission(ctx, apiKey);

‎src/main/resources/META-INF/MANIFEST.MF

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
Manifest-Version: 1.0
2+
Bundle-Description: DataNucleus plugin for Dependency-Track-specific functionality
3+
Bundle-License: https://apache.org/licenses/LICENSE-2.0.txt
4+
Bundle-ManifestVersion: 2
5+
Bundle-Name: datanucleus-dependencytrack
6+
Bundle-SymbolicName: org.dependencytrack.persistence.datanucleus;singleton:=true
7+
Bundle-Version: ${project.version}

‎src/main/resources/migration/changelog-procedures.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,7 @@
2323
<changeSet id="procedure_update-portfolio-metrics" author="nscuro@protonmail.com" runOnChange="true">
2424
<createProcedure path="procedures/procedure_update-portfolio-metrics.sql" relativeToChangelogFile="true"/>
2525
</changeSet>
26+
<changeSet id="function_has-project-access" author="nscuro" runOnChange="true">
27+
<createProcedure path="procedures/function_has-project-access.sql" relativeToChangelogFile="true"/>
28+
</changeSet>
2629
</databaseChangeLog>

‎src/main/resources/migration/procedures/function_has-project-access.sql

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
create or replace function has_project_access(
2+
project_id bigint
3+
, team_ids bigint[]
4+
) returns bool
5+
language "sql"
6+
parallel safe
7+
stable
8+
as
9+
$$
10+
with recursive project_hierarchy(id, parent_id) as(
11+
select "ID" as id
12+
, "PARENT_PROJECT_ID" as parent_id
13+
from "PROJECT"
14+
where "ID" = project_id
15+
union all
16+
select "PROJECT"."ID" as id
17+
, "PROJECT"."PARENT_PROJECT_ID" as parent_id
18+
from "PROJECT"
19+
inner join project_hierarchy
20+
on project_hierarchy.parent_id = "PROJECT"."ID"
21+
)
22+
select exists(
23+
select 1
24+
from project_hierarchy
25+
inner join "PROJECT_ACCESS_TEAMS"
26+
on "PROJECT_ACCESS_TEAMS"."PROJECT_ID" = project_hierarchy.id
27+
where "PROJECT_ACCESS_TEAMS"."TEAM_ID" = any(team_ids)
28+
)
29+
$$;

0 commit comments

Comments
 (0)