Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth NPE on favs and favorite ownership #102

Merged
merged 7 commits into from
Sep 22, 2024
Merged
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
18 changes: 13 additions & 5 deletions src/main/java/com/brennaswitzer/cookbook/config/GraphQLConfig.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.brennaswitzer.cookbook.config;

import com.brennaswitzer.cookbook.graphql.support.OffsetConnectionCursorCoercing;
import com.brennaswitzer.cookbook.security.UserPrincipal;
import com.brennaswitzer.cookbook.util.UserPrincipalAccess;
import graphql.ExceptionWhileDataFetching;
import graphql.ExecutionResult;
import graphql.execution.AsyncExecutionStrategy;
Expand Down Expand Up @@ -88,7 +90,9 @@ public GraphQLScalarType cursor(OffsetConnectionCursorCoercing coercing) {
}

@Bean
public GraphQLServletContextBuilder graphQLServletContextBuilder(DataLoaderRegistry dataLoadRegistry) {
public GraphQLServletContextBuilder graphQLServletContextBuilder(
DataLoaderRegistry dataLoadRegistry,
UserPrincipalAccess principalAccess) {
return new GraphQLServletContextBuilder() {
@Override
public GraphQLKickstartContext build() {
Expand All @@ -100,15 +104,19 @@ public GraphQLKickstartContext build(HttpServletRequest request, HttpServletResp
Map<Object, Object> map = new HashMap<>();
map.put(HttpServletRequest.class, request);
map.put(HttpServletResponse.class, response);
// Building the context happens on the main HTTP thread, so
// Spring Security's holder will always be available. Query
// execution may become asynchronous, rendering Spring's context
// unavailable. So grab the Principal now - though not the User
// object - so it's available to resolvers, without creating any
// mandates about transaction/session demarcation.
map.put(UserPrincipal.class, principalAccess.getUserPrincipal());
return new DefaultGraphQLContext(dataLoadRegistry, map);
}

@Override
public GraphQLKickstartContext build(Session session, HandshakeRequest handshakeRequest) {
Map<Object, Object> map = new HashMap<>();
map.put(Session.class, session);
map.put(HandshakeRequest.class, handshakeRequest);
return new DefaultGraphQLContext(dataLoadRegistry, map);
throw new UnsupportedOperationException("GoBrenna's doesn't yet speak websockets. Again.");
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
"/",
"/error",
"/favicon.ico",
"/favicon.svg",
"/shared/*/*",
"/*/*.png",
"/*/*.gif",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.brennaswitzer.cookbook.domain;

import lombok.Getter;
import lombok.ToString;

@ToString
@Getter
public enum FavoriteType {
RECIPE(Recipe.class);

@Getter
private final String key;

FavoriteType(Class<? extends BaseEntity> clazz) {
Expand All @@ -20,4 +22,13 @@ public boolean matches(String key) {
return this.key.equals(key);
}

public static FavoriteType parse(String key) {
for (var v : values())
if (v.matches(key)) return v;
throw new IllegalArgumentException(String.format(
"No '%s' %s",
key,
FavoriteType.class.getSimpleName()));
}

}
14 changes: 14 additions & 0 deletions src/main/java/com/brennaswitzer/cookbook/domain/Owned.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.brennaswitzer.cookbook.domain;

import com.brennaswitzer.cookbook.security.UserPrincipal;

/**
* @author bboisvert
*/
Expand All @@ -9,4 +11,16 @@ public interface Owned {

void setOwner(User owner);

default boolean isOwner(User user) {
return isOwner(user.getId());
}

default boolean isOwner(UserPrincipal up) {
return isOwner(up.getId());
}

default boolean isOwner(long userId) {
return userId == getOwner().getId();
}

}
16 changes: 4 additions & 12 deletions src/main/java/com/brennaswitzer/cookbook/domain/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,23 @@
import lombok.Getter;
import lombok.Setter;

@Setter
@Getter
@Entity
@Table(name = "users", uniqueConstraints = {
@UniqueConstraint(columnNames = "email")
})
public class User extends BaseEntity {

@Getter
@Setter
private String name;

@Getter
@Setter
private String email;

@Getter
@Setter
private String imageUrl;

@Enumerated(EnumType.STRING)
@Getter
@Setter
private AuthProvider provider;

@Getter
@Setter
private String providerId;

public User() {
Expand All @@ -48,7 +40,7 @@ public String toString() {
return super.toString() + "(" + email + ")";
}

public boolean isOwnerOf(Owned owned) {
return equals(owned.getOwner());
public boolean owns(Owned owned) {
return owned.isOwner(this);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package com.brennaswitzer.cookbook.graphql.loaders;

import com.brennaswitzer.cookbook.domain.FavoriteType;

public record IsFavorite(long ownerId, FavoriteType favType, long objectId) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.brennaswitzer.cookbook.graphql.loaders;

import com.brennaswitzer.cookbook.domain.Favorite;
import com.brennaswitzer.cookbook.domain.FavoriteType;
import com.brennaswitzer.cookbook.repositories.FavoriteRepository;
import com.google.common.annotations.VisibleForTesting;
import org.dataloader.BatchLoader;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import java.util.List;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.stream.Collectors;

import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.mapping;

@Component
public class IsFavoriteBatchLoader implements BatchLoader<IsFavorite, Boolean> {

@Autowired
private FavoriteRepository repo;

private record OwnerType(long ownerId, FavoriteType favType) {

public static OwnerType of(IsFavorite key) {
return new OwnerType(key.ownerId(), key.favType());
}

public static OwnerType of(Favorite fav) {
return new OwnerType(fav.getOwner().getId(), FavoriteType.parse(fav.getObjectType()));
}

}

@Override
public CompletionStage<List<Boolean>> load(List<IsFavorite> keys) {
return CompletableFuture.supplyAsync(() -> loadInternal(keys));
}

@VisibleForTesting
List<Boolean> loadInternal(List<IsFavorite> keys) {
var favIdsByOwnerType = keys.stream()
.collect(groupingBy(OwnerType::of,
mapping(IsFavorite::objectId,
Collectors.toSet())))
.entrySet()
.stream()
.map(e -> repo.findByOwnerIdAndObjectTypeAndObjectIdIn(
e.getKey().ownerId(),
e.getKey().favType().getKey(),
e.getValue()))
.<Favorite>mapMulti(Iterable::forEach)
.collect(groupingBy(OwnerType::of,
mapping(Favorite::getObjectId,
Collectors.toSet())));
return keys.stream()
.map(k -> favIdsByOwnerType.getOrDefault(
OwnerType.of(k),
Set.of())
.contains(k.objectId()))
.toList();
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;

import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;
import java.util.function.Function;
Expand All @@ -23,13 +24,20 @@ public class UnitOfMeasureBatchLoader implements BatchLoader<Long, UnitOfMeasure

@Override
public CompletionStage<List<UnitOfMeasure>> load(List<Long> uomIds) {
Map<Long, UnitOfMeasure> byId = repo.findAllById(new HashSet<>(uomIds)).stream()
.collect(Collectors.toMap(Identified::getId,
Function.identity()));
return CompletableFuture.completedStage(
uomIds.stream()
.map(byId::get)
.toList());
return CompletableFuture.supplyAsync(() -> {
Set<Long> nonNullIds = uomIds.stream()
.filter(Objects::nonNull)
.collect(Collectors.toSet());
Map<Long, UnitOfMeasure> byId;
if (nonNullIds.isEmpty()) byId = Map.of();
else byId = repo.findAllById(nonNullIds)
.stream()
.collect(Collectors.toMap(Identified::getId,
Function.identity()));
return uomIds.stream()
.map(byId::get)
.toList();
});
}

}
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package com.brennaswitzer.cookbook.graphql.resolvers;

import com.brennaswitzer.cookbook.domain.FavoriteType;
import com.brennaswitzer.cookbook.domain.IngredientRef;
import com.brennaswitzer.cookbook.domain.Photo;
import com.brennaswitzer.cookbook.domain.PlanItemStatus;
import com.brennaswitzer.cookbook.domain.PlannedRecipeHistory;
import com.brennaswitzer.cookbook.domain.Recipe;
import com.brennaswitzer.cookbook.graphql.loaders.RecipeIsFavoriteBatchLoader;
import com.brennaswitzer.cookbook.graphql.loaders.IsFavorite;
import com.brennaswitzer.cookbook.graphql.loaders.IsFavoriteBatchLoader;
import com.brennaswitzer.cookbook.mapper.LabelMapper;
import com.brennaswitzer.cookbook.security.UserPrincipal;
import com.brennaswitzer.cookbook.services.favorites.FetchFavorites;
import graphql.kickstart.tools.GraphQLResolver;
import graphql.schema.DataFetchingEnvironment;
Expand Down Expand Up @@ -53,8 +56,12 @@ public List<String> labels(Recipe recipe) {

public CompletableFuture<Boolean> favorite(Recipe recipe,
DataFetchingEnvironment env) {
return env.<Recipe, Boolean>getDataLoader(RecipeIsFavoriteBatchLoader.class.getName())
.load(recipe);
// Spring Security's principal is copied to the GraphQL context when it
// is safe. It's not safe to interrogate Spring here, though it does
// work in some situations.
UserPrincipal up = env.getGraphQlContext().get(UserPrincipal.class);
return env.<IsFavorite, Boolean>getDataLoader(IsFavoriteBatchLoader.class.getName())
.load(new IsFavorite(up.getId(), FavoriteType.RECIPE, recipe.getId()));
}

public Photo photo(Recipe recipe) {
Expand Down
Loading
Loading