Skip to content

Commit

Permalink
dcache-frontend: remove authz checks in Quota GET methods
Browse files Browse the repository at this point in the history
Motivation:

All of the Frontend REST GET methods allow
anonymous read (if the global property so
allows it), except for `quota` (but see below).

The extra check is actually not necessary,
as authenticated users can see all quotas
if `frontend.authz.anonymous-operations` is
not `NONE`.

Modification:

Remove the authorization check on the `quota`
GET methods.

The `event` resources seem to constitute a
special case, so we have not altered them;
however, the method which enforces authenticated
user has been moved to that class and out of
`RequestUser`.

Result:

All of the REST GET methods now allow anonymous
users to get the data if the global property
allows (in the case of `NONE`, 401 not authorized
is returned at login).

Target: master
Request: 9.1
Request: 9.0
Request: 8.2
Patch: https://rb.dcache.org/r/14014
Requires-notes: yes
Acked-by: Tigran
Acked-by: Dmitry
  • Loading branch information
alrossi committed Jul 6, 2023
1 parent 2c528cf commit 4175d9b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 22 deletions.
Expand Up @@ -21,6 +21,8 @@
import static com.google.common.base.Preconditions.checkArgument;
import static org.dcache.restful.util.Preconditions.checkNotForbidden;
import static org.dcache.restful.util.Preconditions.checkRequestNotBad;
import static org.dcache.restful.util.RequestUser.isAnonymous;
import static org.dcache.util.Exceptions.genericCheck;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.JsonNode;
Expand All @@ -47,6 +49,7 @@
import javax.ws.rs.GET;
import javax.ws.rs.HeaderParam;
import javax.ws.rs.InternalServerErrorException;
import javax.ws.rs.NotAuthorizedException;
import javax.ws.rs.NotFoundException;
import javax.ws.rs.PATCH;
import javax.ws.rs.POST;
Expand Down Expand Up @@ -183,7 +186,7 @@ private static String canonicaliseClientId(String in) {
}

private Channel channelForId(String id) {
RequestUser.checkAuthenticated();
checkAuthenticated();
Channel channel = registrar.get(id)
.orElseThrow(() -> new NotFoundException("No such channel"));
checkNotForbidden(channel.isAccessAllowed(RequestUser.getSubject()),
Expand Down Expand Up @@ -216,7 +219,7 @@ public ServiceMetadata serviceMetadata() {
})
@Path("/eventTypes")
public List<String> getEventTypes() {
RequestUser.checkAuthenticated();
checkAuthenticated();
return repository.listEventTypes();
}

Expand All @@ -234,7 +237,7 @@ public List<String> getEventTypes() {
@Path("/eventTypes/{type}")
public EventStreamMetadata getEventType(@ApiParam("The specific event type to be described.")
@NotNull @PathParam("type") String type) {
RequestUser.checkAuthenticated();
checkAuthenticated();
return repository.metadataForEventType(type)
.orElseThrow(() -> new NotFoundException("No such event type"));
}
Expand All @@ -252,7 +255,7 @@ public EventStreamMetadata getEventType(@ApiParam("The specific event type to be
@Path("/eventTypes/{type}/selector")
public ObjectNode getSelectorSchema(@ApiParam("The specific event type to be described.")
@NotNull @PathParam("type") String type) {
RequestUser.checkAuthenticated();
checkAuthenticated();
return repository.selectorSchemaForEventType(type)
.orElseThrow(() -> new NotFoundException("No such event type"));
}
Expand All @@ -269,7 +272,7 @@ public ObjectNode getSelectorSchema(@ApiParam("The specific event type to be des
@Path("/eventTypes/{type}/event")
public ObjectNode getEventSchema(@ApiParam("The specific event type to be described.")
@NotNull @PathParam("type") String type) {
RequestUser.checkAuthenticated();
checkAuthenticated();
return repository.eventSchemaForEventType(type)
.orElseThrow(() -> new NotFoundException("No such event type"));
}
Expand All @@ -296,7 +299,7 @@ public ObjectNode getEventSchema(@ApiParam("The specific event type to be descri
public List<String> getChannels(@Context UriInfo uriInfo,
@ApiParam("Limit channels by client-id")
@QueryParam("client-id") String clientId) {
RequestUser.checkAuthenticated();
checkAuthenticated();

UriBuilder idToUri = uriInfo.getBaseUriBuilder().path(getClass())
.path(getClass(), "events");
Expand Down Expand Up @@ -348,7 +351,7 @@ public List<String> getChannels(@Context UriInfo uriInfo,
@Path("/channels")
public Response register(@Context UriInfo uriInfo,
NewChannelMetadata metadata) {
RequestUser.checkAuthenticated();
checkAuthenticated();

String clientId = metadata == null ? null : canonicaliseClientId(metadata.clientId);

Expand Down Expand Up @@ -590,4 +593,15 @@ public void deleteSubscription(@NotNull @PathParam("channel_id") String channelI
.orElseThrow(() -> new NotFoundException("Subscription not found"))
.close();
}

/* REVISIT This contradicts the global property `frontend.authz.anonymous-operations=READONLY`.
Currently this exception pertains only to Event resources. If this exception is
necessary, the method belongs here (it has been moved from RequestUser).
Otherwise, the checks in this class should be changed to allow GET in conformity
with the global behavior indicated by the property.
*/
private static void checkAuthenticated() throws NotAuthorizedException {
genericCheck(!isAnonymous(), NotAuthorizedException::new,
"anonymous access not allowed");
}
}
Expand Up @@ -62,7 +62,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
import static diskCacheV111.util.CacheException.ATTRIBUTE_EXISTS;
import static diskCacheV111.util.CacheException.NO_ATTRIBUTE;
import static diskCacheV111.util.CacheException.SERVICE_UNAVAILABLE;
import static org.dcache.restful.util.RequestUser.checkAuthenticated;

import com.google.common.base.Strings;
import diskCacheV111.util.CacheException;
Expand Down Expand Up @@ -140,8 +139,6 @@ public List<QuotaInfo> getUserQuotas(@ApiParam(value = "Return user quota associ
+ "calling user only.")
@DefaultValue("false")
@QueryParam("user") boolean user) {
checkAuthenticated();

PnfsManagerGetQuotaMessage message;

if (user) {
Expand Down Expand Up @@ -169,8 +166,6 @@ public List<QuotaInfo> getGroupQuotas(@ApiParam(value = "Return group quota asso
+ "calling user only.")
@DefaultValue("false")
@QueryParam("user") boolean user) {
checkAuthenticated();

PnfsManagerGetQuotaMessage message;

/*
Expand Down Expand Up @@ -200,8 +195,6 @@ public List<QuotaInfo> getUserQuota(
@ApiParam(value = "The user id to which the quota corresponds.",
required = true)
@PathParam("id") int id) {
checkAuthenticated();

return getQuotas(new PnfsManagerGetQuotaMessage(id, QuotaType.USER));
}

Expand All @@ -218,8 +211,6 @@ public List<QuotaInfo> getGroupQuota(
@ApiParam(value = "The group id to which the quota corresponds.",
required = true)
@PathParam("id") int id) {
checkAuthenticated();

return getQuotas(new PnfsManagerGetQuotaMessage(id, QuotaType.GROUP));
}

Expand Down
Expand Up @@ -18,7 +18,6 @@
package org.dcache.restful.util;

import static com.google.common.base.Preconditions.checkState;
import static org.dcache.util.Exceptions.genericCheck;

import java.io.IOException;
import java.security.AccessController;
Expand Down Expand Up @@ -87,11 +86,6 @@ public static Long getSubjectUidForFileOperations(boolean isUnlimitedVisibility)
return Subjects.getUid(user);
}

public static void checkAuthenticated() throws NotAuthorizedException {
genericCheck(!isAnonymous(), NotAuthorizedException::new,
"anonymous access not allowed");
}

@Override
public void filter(ContainerRequestContext requestContext) throws IOException {
RESTRICTIONS.set(HttpServletRequests.getRestriction(request));
Expand Down

0 comments on commit 4175d9b

Please sign in to comment.