Skip to content

Commit d0e27a7

Browse files
committed
chore: restore old script based tests, good for quick local testing
1 parent ac4e85d commit d0e27a7

40 files changed

Lines changed: 2180 additions & 135 deletions

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,6 @@ public UrlRuleAggregateFilter(
5353
this(order, ALL_OPERATIONS, provider, urlRuleFilters, pathPrefix, null, null);
5454
}
5555

56-
public UrlRuleAggregateFilter(
57-
int order,
58-
GitProxyProvider provider,
59-
List<UrlRuleFilter> urlRuleFilters,
60-
String pathPrefix,
61-
FetchStore fetchStore) {
62-
this(order, ALL_OPERATIONS, provider, urlRuleFilters, pathPrefix, fetchStore, null);
63-
}
64-
6556
public UrlRuleAggregateFilter(
6657
int order,
6758
GitProxyProvider provider,
@@ -72,16 +63,6 @@ public UrlRuleAggregateFilter(
7263
this(order, ALL_OPERATIONS, provider, urlRuleFilters, pathPrefix, fetchStore, repoRegistry);
7364
}
7465

75-
public UrlRuleAggregateFilter(
76-
int order,
77-
Set<HttpOperation> applicableOperations,
78-
GitProxyProvider provider,
79-
List<UrlRuleFilter> urlRuleFilters,
80-
String pathPrefix,
81-
FetchStore fetchStore) {
82-
this(order, applicableOperations, provider, urlRuleFilters, pathPrefix, fetchStore, null);
83-
}
84-
8566
public UrlRuleAggregateFilter(
8667
int order,
8768
Set<HttpOperation> applicableOperations,

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleEvaluator.java

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.nio.file.FileSystems;
44
import java.nio.file.PathMatcher;
55
import java.nio.file.Paths;
6+
import java.util.Comparator;
67
import java.util.List;
78
import java.util.concurrent.ConcurrentHashMap;
89
import java.util.regex.Pattern;
@@ -19,34 +20,28 @@
1920
* <p>Evaluation order:
2021
*
2122
* <ol>
22-
* <li>Config deny rules — first match returns {@link Result.Denied}.
23+
* <li>Config rules — evaluated in ascending {@code order}, first match wins (allow or deny). This is firewall /
24+
* iptables semantics: a lower-numbered allow rule beats a higher-numbered deny rule and vice versa.
2325
* <li>DB deny rules — first match returns {@link Result.Denied}.
24-
* <li>If no allow rules are configured anywhere — returns {@link Result.OpenMode} (implicitly allowed).
25-
* <li>Config allow rules — first match returns {@link Result.Allowed}.
2626
* <li>DB allow rules — first match returns {@link Result.Allowed}.
27-
* <li>Allow rules exist but none matched — returns {@link Result.NotAllowed}.
27+
* <li>No rule matched — returns {@link Result.NotAllowed} (fail-closed).
2828
* </ol>
2929
*
30-
* <p>DB rules are fetched once per evaluation (not twice), then split into deny/allow lists in memory.
30+
* <p>DB rules are fetched once per evaluation (not twice), then split into deny/allow lists in memory. DB rules do not
31+
* carry an {@code order} field and are therefore evaluated after all config rules.
3132
*/
3233
@Slf4j
3334
public class UrlRuleEvaluator {
3435

3536
/** Outcome of a single rule evaluation pass. */
36-
public sealed interface Result permits Result.Denied, Result.Allowed, Result.OpenMode, Result.NotAllowed {
37+
public sealed interface Result permits Result.Denied, Result.Allowed, Result.NotAllowed {
3738

3839
/** A deny rule matched — request must be rejected. */
3940
record Denied(String ruleId) implements Result {}
4041

4142
/** An allow rule matched — request may proceed. */
4243
record Allowed(String ruleId) implements Result {}
4344

44-
/**
45-
* No allow rules are configured anywhere (neither config nor DB). The proxy is in open/permissive mode and the
46-
* request may proceed.
47-
*/
48-
record OpenMode() implements Result {}
49-
5045
/** Allow rules are configured but none matched — request must be rejected. */
5146
record NotAllowed() implements Result {}
5247
}
@@ -58,7 +53,11 @@ record NotAllowed() implements Result {}
5853
private final GitProxyProvider provider;
5954

6055
public UrlRuleEvaluator(List<UrlRuleFilter> configRules, RepoRegistry repoRegistry, GitProxyProvider provider) {
61-
this.configRules = configRules != null ? configRules : List.of();
56+
this.configRules = configRules != null
57+
? configRules.stream()
58+
.sorted(Comparator.comparingInt(UrlRuleFilter::getOrder))
59+
.toList()
60+
: List.of();
6261
this.repoRegistry = repoRegistry;
6362
this.provider = provider;
6463
}
@@ -79,13 +78,23 @@ public Result evaluate(String slug, String owner, String name, HttpOperation ope
7978
? repoRegistry.findEnabledForProvider(provider.getProviderId())
8079
: List.of();
8180

82-
// ── Step 1: deny rules ─────────────────────────────────────────────────
81+
// ── Step 1: config rules — single ordered pass, first match wins ───────
8382
for (UrlRuleFilter f : configRules) {
84-
if (f.getAccess() == AccessRule.Access.DENY && f.appliesTo(operation) && f.matchesRepo(slug, owner, name)) {
85-
log.debug("Denied by config rule: {}", f);
83+
if (!f.appliesTo(operation) || !f.matchesRepo(slug, owner, name)) continue;
84+
if (f.getAccess() == AccessRule.Access.DENY) {
85+
log.debug("Denied by config rule (order {}): {}", f.getOrder(), f);
8686
return new Result.Denied(f.toString());
87+
} else {
88+
log.debug("Allowed by config rule (order {}): {}", f.getOrder(), f);
89+
return new Result.Allowed(f.toString());
8790
}
8891
}
92+
93+
// ── Step 2: DB rules — deny first, then allow ──────────────────────────
94+
List<AccessRule> dbAllow = dbRules.stream()
95+
.filter(r -> r.getAccess() == AccessRule.Access.ALLOW && operationMatches(r, operation))
96+
.toList();
97+
8998
for (AccessRule rule : dbRules) {
9099
if (rule.getAccess() == AccessRule.Access.DENY
91100
&& operationMatches(rule, operation)
@@ -95,25 +104,7 @@ && matchesRepo(rule, slug, owner, name)) {
95104
}
96105
}
97106

98-
// ── Step 2: allow rules ────────────────────────────────────────────────
99-
List<UrlRuleFilter> configAllow = configRules.stream()
100-
.filter(f -> f.getAccess() == AccessRule.Access.ALLOW && f.appliesTo(operation))
101-
.toList();
102-
List<AccessRule> dbAllow = dbRules.stream()
103-
.filter(r -> r.getAccess() == AccessRule.Access.ALLOW && operationMatches(r, operation))
104-
.toList();
105-
106-
if (configAllow.isEmpty() && dbAllow.isEmpty()) {
107-
log.debug("No allow rules configured for operation {} — open mode", operation);
108-
return new Result.OpenMode();
109-
}
110-
111-
for (UrlRuleFilter f : configAllow) {
112-
if (f.matchesRepo(slug, owner, name)) {
113-
log.debug("Allowed by config rule: {}", f);
114-
return new Result.Allowed(f.toString());
115-
}
116-
}
107+
// ── Step 3: allow rules matched ────────────────────────
117108
for (AccessRule rule : dbAllow) {
118109
if (matchesRepo(rule, slug, owner, name)) {
119110
log.debug("Allowed by DB rule: id={}", rule.getId());

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleFilter.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ public enum Target {
4545

4646
private static final String REGEX_PREFIX = "regex:";
4747

48-
// URL rule filters must be in the authorization range 50-199
49-
private static final int MIN_ORDER = 50;
50-
private static final int MAX_ORDER = 199;
48+
private static final int MIN_ORDER = 1;
5149

5250
@Getter
5351
private final AccessRule.Access access;
@@ -122,10 +120,8 @@ private static boolean isRegexPattern(String s) {
122120
}
123121

124122
private static int validateOrder(int order) {
125-
if (order < MIN_ORDER || order > MAX_ORDER) {
126-
throw new IllegalArgumentException(String.format(
127-
"UrlRuleFilter order must be in the authorization range %d-%d (inclusive), but was %d",
128-
MIN_ORDER, MAX_ORDER, order));
123+
if (order < MIN_ORDER) {
124+
throw new IllegalArgumentException("UrlRuleFilter order must be >= " + MIN_ORDER + ", but was " + order);
129125
}
130126
return order;
131127
}

git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/UrlRuleFilterTest.java

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,14 @@ private GitRequestDetails makeDetails(String owner, String name, String slug) {
122122
void urlRule_orderBelowMinimum_throws() {
123123
assertThrows(
124124
IllegalArgumentException.class,
125-
() -> new UrlRuleFilter(49, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
126-
}
127-
128-
@Test
129-
void urlRule_orderAboveMaximum_throws() {
130-
assertThrows(
131-
IllegalArgumentException.class,
132-
() -> new UrlRuleFilter(200, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
125+
() -> new UrlRuleFilter(0, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
133126
}
134127

135128
@Test
136129
void urlRule_validOrder_succeeds() {
137-
assertDoesNotThrow(() -> new UrlRuleFilter(50, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
138-
assertDoesNotThrow(() -> new UrlRuleFilter(199, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
130+
assertDoesNotThrow(() -> new UrlRuleFilter(1, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
131+
assertDoesNotThrow(
132+
() -> new UrlRuleFilter(Integer.MAX_VALUE, GITHUB, List.of("owner"), UrlRuleFilter.Target.OWNER));
139133
}
140134

141135
@Test

git-proxy-java-dashboard/frontend/src/pages/Repos.tsx

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ interface ActiveRepo {
1010
blockedFetchCount: number
1111
}
1212

13-
interface AccessRule {
13+
interface Rule {
1414
id: string
1515
provider: string | null
1616
slug: string | null
@@ -123,7 +123,7 @@ function AddRuleModal({
123123
onCreated,
124124
}: {
125125
onClose: () => void
126-
onCreated: (rule: AccessRule) => void
126+
onCreated: (rule: Rule) => void
127127
}) {
128128
const [form, setForm] = useState<AddRuleForm>(DEFAULT_FORM)
129129
const [error, setError] = useState<string | null>(null)
@@ -207,7 +207,7 @@ function AddRuleModal({
207207
<div className="fixed inset-0 bg-black/40 flex items-center justify-center z-50">
208208
<div className="bg-white rounded-lg shadow-xl w-full max-w-md p-6 space-y-4">
209209
<div className="flex items-center justify-between">
210-
<h3 className="text-lg font-semibold text-gray-800">Add Access Rule</h3>
210+
<h3 className="text-lg font-semibold text-gray-800">Add Rule</h3>
211211
<button
212212
onClick={onClose}
213213
className="text-gray-400 hover:text-gray-600 text-xl leading-none"
@@ -388,7 +388,7 @@ function AddRuleModal({
388388
export function Repos() {
389389
const [tab, setTab] = useState<Tab>('active')
390390
const [activeRepos, setActiveRepos] = useState<ActiveRepo[]>([])
391-
const [rules, setRules] = useState<AccessRule[]>([])
391+
const [rules, setRules] = useState<Rule[]>([])
392392
const [loadedTab, setLoadedTab] = useState<Tab | null>(null)
393393
const [showAddRule, setShowAddRule] = useState(false)
394394
const [providers, setProviders] = useState<{ name: string; id: string; host: string }[]>([])
@@ -442,7 +442,7 @@ export function Repos() {
442442
: 'border-transparent text-gray-500 hover:text-gray-700'
443443
}`}
444444
>
445-
{t === 'active' ? 'Active' : 'Access Rules'}
445+
{t === 'active' ? 'Active' : 'Rules'}
446446
</button>
447447
))}
448448
</div>
@@ -508,11 +508,11 @@ export function Repos() {
508508
</>
509509
)}
510510

511-
{/* Access rules tab */}
511+
{/* Rules tab */}
512512
{!loading && tab === 'rules' && (
513513
<>
514514
{rules.length === 0 ? (
515-
<p className="text-sm text-gray-400">No access rules configured.</p>
515+
<p className="text-sm text-gray-400">No rules configured.</p>
516516
) : (
517517
<div className="space-y-2">
518518
{rules.map((rule) => (

0 commit comments

Comments
 (0)