Skip to content

Commit

Permalink
Update WLCG policy decision point logic (#45)
Browse files Browse the repository at this point in the history
* HEAD and OPTIONS no more require storage.read
* Any storage.xxx scope can be used to allow HEAD or OPTIONS calls
* Introduced storage.stage as a superset of storage.read
  • Loading branch information
enricovianello authored Oct 3, 2024
1 parent d915ab6 commit 577ec80
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,21 @@ public class WlcgStructuredPathAuthorizationPdp
implements PathAuthorizationPdp, MatcherUtils, TpcUtils {

public static final String WLCG_STORAGE_SCOPE_PATTERN_STRING =
"^storage.(read|modify|create):(\\/.*)$";
"^storage.(read|modify|create|stage):(\\/.*)$";

public static final Pattern WLCG_STORAGE_SCOPE_PATTERN =
Pattern.compile(WLCG_STORAGE_SCOPE_PATTERN_STRING);

public static final String SCOPE_CLAIM = "scope";

public static final String STORAGE_STAGE = "storage.stage";
public static final String STORAGE_READ = "storage.read";
public static final String STORAGE_MODIFY = "storage.modify";
public static final String STORAGE_CREATE = "storage.create";

protected static final Set<String> ALL_STORAGE_SCOPES =
Sets.newHashSet(STORAGE_READ, STORAGE_MODIFY, STORAGE_CREATE, STORAGE_STAGE);

public static final String ERROR_INVALID_AUTHENTICATION =
"Invalid authentication: expected a JwtAuthenticationToken object";
public static final String ERROR_INVALID_TOKEN_NO_SCOPE =
Expand All @@ -68,12 +72,10 @@ public class WlcgStructuredPathAuthorizationPdp

public static final String ERROR_UNKNOWN_TOKEN_ISSUER = "Unknown token issuer: %s";

public static final Set<String> READONLY_METHODS =
Sets.newHashSet("GET", "OPTIONS", "HEAD", "PROPFIND");

public static final Set<String> REPLACE_METHODS = Sets.newHashSet("PUT", "MKCOL");

public static final Set<String> MODIFY_METHODS = Sets.newHashSet("PATCH", "DELETE");
protected static final Set<String> READONLY_METHODS = Sets.newHashSet("GET", "PROPFIND");
protected static final Set<String> REPLACE_METHODS = Sets.newHashSet("PUT", "MKCOL");
protected static final Set<String> MODIFY_METHODS = Sets.newHashSet("PATCH", "DELETE");
protected static final Set<String> CATCHALL_METHODS = Sets.newHashSet("HEAD", "OPTIONS");

public static final String COPY_METHOD = "COPY";
public static final String MOVE_METHOD = "MOVE";
Expand Down Expand Up @@ -125,39 +127,39 @@ public static Set<String> resolveWlcgScopes(JwtAuthenticationToken token) {
boolean filterMatcherByRequest(HttpServletRequest request, String method,
StructuredPathScopeMatcher m, boolean requestedResourceExists) {

String requiredScope = null;
if (CATCHALL_METHODS.contains(method)) {
return ALL_STORAGE_SCOPES.stream().anyMatch(prefix -> m.getPrefix().equals(prefix));
}

if (READONLY_METHODS.contains(method)) {
requiredScope = STORAGE_READ;
} else if (REPLACE_METHODS.contains(method)) {
return m.getPrefix().equals(STORAGE_READ) || m.getPrefix().equals(STORAGE_STAGE);
}
if (REPLACE_METHODS.contains(method)) {
if (requestedResourceExists) {
requiredScope = STORAGE_MODIFY;
} else {
requiredScope = STORAGE_CREATE;
return m.getPrefix().equals(STORAGE_MODIFY);
}
} else if (MODIFY_METHODS.contains(method)) {
requiredScope = STORAGE_MODIFY;
} else if (COPY_METHOD.equals(method)) {

requiredScope = STORAGE_READ;
return m.getPrefix().equals(STORAGE_CREATE);
}
if (MODIFY_METHODS.contains(method)) {
return m.getPrefix().equals(STORAGE_MODIFY);
}
if (COPY_METHOD.equals(method)) {

if (isPullTpc(request, localUrlService)) {
if (requestedResourceExists) {
requiredScope = STORAGE_MODIFY;
} else {
requiredScope = STORAGE_CREATE;
return m.getPrefix().equals(STORAGE_MODIFY);
}
return m.getPrefix().equals(STORAGE_CREATE);
}
return m.getPrefix().equals(STORAGE_READ);

} else if (MOVE_METHOD.equals(method)) {
requiredScope = STORAGE_MODIFY;
}

if (isNull(requiredScope)) {
throw new IllegalArgumentException(format(ERROR_UNSUPPORTED_METHOD_PATTERN, method));
if (MOVE_METHOD.equals(method)) {
return m.getPrefix().equals(STORAGE_MODIFY);
}

return m.getPrefix().equals(requiredScope);
throw new IllegalArgumentException(format(ERROR_UNSUPPORTED_METHOD_PATTERN, method));
}


Expand Down Expand Up @@ -186,7 +188,7 @@ public PathAuthorizationResult authorizeRequest(PathAuthorizationRequest authzRe
if (isNull(sa)) {
return indeterminate(ERROR_SA_NOT_FOUND);
}

final String tokenIssuer = jwtAuth.getToken().getIssuer().toString();

if (!sa.orgs().contains(tokenIssuer)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@
@ExtendWith(MockitoExtension.class)
public class ScopePathAuthzPdpTests {

public static final String[] READ_METHODS = {"GET", "PROPFIND", "OPTIONS", "HEAD"};
public static final String[] CATCHALL_METHODS = {"HEAD", "OPTIONS"};
public static final String[] READ_METHODS = {"GET", "PROPFIND"};
public static final String[] REPLACE_METHODS = {"PUT", "MKCOL"};
public static final String[] MODIFY_METHODS = {"DELETE", "PATCH"};
public static final String COPY_METHOD = "COPY";
Expand Down Expand Up @@ -100,7 +101,7 @@ public void setup() throws MalformedURLException {
}

@Test
public void invalidAuthentication() {
void invalidAuthentication() {

Authentication auth = mock(Authentication.class);
assertThrows(IllegalArgumentException.class, () -> {
Expand All @@ -109,7 +110,7 @@ public void invalidAuthentication() {
}

@Test
public void noScopeClaimYeldsIndeterminate() throws Exception {
void noScopeClaimYeldsIndeterminate() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn(null);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
Expand All @@ -119,7 +120,7 @@ public void noScopeClaimYeldsIndeterminate() throws Exception {
}

@Test
public void noSaYeldsIndeterminate() throws Exception {
void noSaYeldsIndeterminate() {

when(pathResolver.resolveStorageArea("/test/example")).thenReturn(null);
PathAuthorizationResult result =
Expand All @@ -130,7 +131,7 @@ public void noSaYeldsIndeterminate() throws Exception {
}

@Test
public void noStorageScopesYeldsDeny() throws Exception {
void noStorageScopesYeldsDeny() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid profile storage.read");

PathAuthorizationResult result =
Expand All @@ -141,9 +142,62 @@ public void noStorageScopesYeldsDeny() throws Exception {
assertThat(result.getMessage().get(), containsString("Insufficient token scope"));
}

@Test
void noStorageScopesYeldsDenyForCatchallMethods() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid profile");

for (String m : CATCHALL_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));

assertThat(result.getDecision(), is(Decision.INDETERMINATE));
assertThat(result.getMessage().isPresent(), is(true));
assertThat(result.getMessage().get(), containsString("Insufficient token scope"));
}
}

@Test
public void readMethodsRequestsRequireStorageRead() throws Exception {
void catchallMethodsRequestsAtLeastOneStorageScope() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.modify:/");

for (String m : CATCHALL_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}

when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/");

for (String m : CATCHALL_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}

when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.create:/");

for (String m : CATCHALL_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}

when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.stage:/");

for (String m : CATCHALL_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}
}

@Test
void readMethodsRequestsRequireStorageReadOrStage() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.modify:/");

for (String m : READ_METHODS) {
Expand All @@ -163,10 +217,19 @@ public void readMethodsRequestsRequireStorageRead() throws Exception {
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}

when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.stage:/");

for (String m : READ_METHODS) {
when(request.getMethod()).thenReturn(m);
PathAuthorizationResult result =
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
assertThat(result.getDecision(), is(Decision.PERMIT));
}
}

@Test
public void replaceMethodsRequestsRequireStorageModifyOrCreate() throws Exception {
void replaceMethodsRequestsRequireStorageModifyOrCreate() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/");

for (String m : REPLACE_METHODS) {
Expand Down Expand Up @@ -210,7 +273,7 @@ public void replaceMethodsRequestsRequireStorageModifyOrCreate() throws Exceptio
}

@Test
public void modifyMethodsRequestsRequireStorageModifyOrCreate() throws Exception {
void modifyMethodsRequestsRequireStorageModifyOrCreate() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/ storage.create:/");

for (String m : MODIFY_METHODS) {
Expand All @@ -233,7 +296,7 @@ public void modifyMethodsRequestsRequireStorageModifyOrCreate() throws Exception
}

@Test
public void testLocalCopyRequiresStorageCreateOrModify() throws Exception {
void testLocalCopyRequiresStorageCreateOrModify() {

when(request.getMethod()).thenReturn(COPY_METHOD);
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.modify:/");
Expand All @@ -250,7 +313,7 @@ public void testLocalCopyRequiresStorageCreateOrModify() throws Exception {
}

@Test
public void testPullTpcRequiresCreateOrModify() throws Exception {
void testPullTpcRequiresCreateOrModify() {
when(request.getMethod()).thenReturn(COPY_METHOD);
when(request.getHeader("Source")).thenReturn("https://remote.example/test/example");
when(pathResolver.pathExists("/test/example")).thenReturn(true);
Expand Down Expand Up @@ -278,7 +341,7 @@ public void testPullTpcRequiresCreateOrModify() throws Exception {
}

@Test
public void testPushTpcRequiresRead() throws Exception {
void testPushTpcRequiresRead() {
when(request.getMethod()).thenReturn(COPY_METHOD);
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.create:/ storage.modify:/");
PathAuthorizationResult result =
Expand All @@ -294,7 +357,7 @@ public void testPushTpcRequiresRead() throws Exception {
}

@Test
public void testMoveRequiresModify() throws Exception {
void testMoveRequiresModify() {
when(request.getMethod()).thenReturn(MOVE_METHOD);
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/ storage.create:/");
PathAuthorizationResult result =
Expand All @@ -310,7 +373,7 @@ public void testMoveRequiresModify() throws Exception {
}

@Test
public void testModifyImpliesCreate() throws Exception {
void testModifyImpliesCreate() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/ storage.modify:/");
when(pathResolver.pathExists("/test/example")).thenReturn(false);

Expand All @@ -323,15 +386,15 @@ public void testModifyImpliesCreate() throws Exception {
}

@Test
public void testUnsupportedMethod() throws Exception {
void testUnsupportedMethod() {
when(request.getMethod()).thenReturn("TRACE");
assertThrows(IllegalArgumentException.class, () -> {
pdp.authorizeRequest(newAuthorizationRequest(request, jwtAuth));
});
}

@Test
public void testPathAuthzIsEnforced() throws Exception {
void testPathAuthzIsEnforced() {
when(jwt.getClaimAsString(SCOPE_CLAIM)).thenReturn("openid storage.read:/subfolder");
when(request.getMethod()).thenReturn("GET");
PathAuthorizationResult result =
Expand All @@ -347,7 +410,7 @@ public void testPathAuthzIsEnforced() throws Exception {
}

@Test
public void issuerChecksAreEnforced() throws Exception {
void issuerChecksAreEnforced() throws Exception {
when(jwt.getIssuer()).thenReturn(new URL("https://unknown.example"));
when(request.getMethod()).thenReturn("GET");
PathAuthorizationResult result =
Expand Down

0 comments on commit 577ec80

Please sign in to comment.