Skip to content

Commit

Permalink
Merge pull request #271 from bannergress/fix-anonymous-author-search
Browse files Browse the repository at this point in the history
Do not allow search by author for anonymous users.
  • Loading branch information
Poeschl authored Apr 27, 2022
2 parents 782c909 + 0ead590 commit edc250f
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ public ResponseEntity<List<BannerDto>> list(@RequestParam @Parameter(description
@RequestParam(defaultValue = "0") @Parameter(description = "0-based offset for searching.") @Min(0) final int offset,
@RequestParam(defaultValue = "20") @Parameter(description = "Maximum number of results.") @Min(1) @Max(100) final int limit,
Principal principal) {
if ((author.isPresent() || listTypes.isPresent()) && principal == null) {
boolean isAuthenticated = principal != null;
if ((author.isPresent() || listTypes.isPresent()) && !isAuthenticated) {
return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
}
int numberOfBounds = (minLatitude.isPresent() ? 1 : 0) + (maxLatitude.isPresent() ? 1 : 0)
Expand All @@ -128,7 +129,7 @@ public ResponseEntity<List<BannerDto>> list(@RequestParam @Parameter(description
return ResponseEntity.badRequest().build();
}
final Collection<Banner> banners = bannerSearchService.find(placeId, minLatitude, maxLatitude, minLongitude,
maxLongitude, query, missionId, onlyOfficialMissions, author, listTypes,
maxLongitude, query, isAuthenticated, missionId, onlyOfficialMissions, author, listTypes,
Optional.ofNullable(principal).map(Principal::getName), online, orderBy, orderDirection, proximityLatitude,
proximityLongitude, offset, limit);
List<BannerDto> bannerDtos = banners.stream().map(this::toSummary).collect(Collectors.toUnmodifiableList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public interface BannerSearchService {
* @param minLongitude Optional minimum longitude.
* @param maxLongitude Optional maximum longitude.
* @param query Optional query string.
* @param queryAuthor Should the query string be extended to author information?
* @param missionId Optional ID of mission which has to be contained in banner.
* @param onlyOfficialMissions Whether to only include official mission accounts.
* @param author Optional author of one of the banner missions.
Expand All @@ -38,10 +39,10 @@ public interface BannerSearchService {
*/
List<Banner> find(Optional<String> placeSlug, Optional<Double> minLatitude, Optional<Double> maxLatitude,
Optional<Double> minLongitude, Optional<Double> maxLongitude, Optional<String> query,
Optional<String> missionId, boolean onlyOfficialMissions, Optional<String> author,
Optional<Collection<BannerListType>> listTypes, Optional<String> userId, Optional<Boolean> online,
Optional<BannerSortOrder> orderBy, Direction orderDirection, Optional<Double> proximityLatitude,
Optional<Double> proximityLongitude, int offset, int limit);
boolean queryAuthor, Optional<String> missionId, boolean onlyOfficialMissions,
Optional<String> author, Optional<Collection<BannerListType>> listTypes, Optional<String> userId,
Optional<Boolean> online, Optional<BannerSortOrder> orderBy, Direction orderDirection,
Optional<Double> proximityLatitude, Optional<Double> proximityLongitude, int offset, int limit);

/** Updates the search index. */
void updateIndex();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ public class DatabaseBannerSearchServiceImpl extends BaseBannerSearchServiceImpl
@Override
public List<Banner> find(Optional<String> placeSlug, Optional<Double> minLatitude, Optional<Double> maxLatitude,
Optional<Double> minLongitude, Optional<Double> maxLongitude, Optional<String> search,
Optional<String> missionId, boolean onlyOfficialMissions, Optional<String> author,
Optional<Collection<BannerListType>> listTypes, Optional<String> userId,
Optional<Boolean> online, Optional<BannerSortOrder> orderBy, Direction orderDirection,
Optional<Double> proximityLatitude, Optional<Double> proximityLongitude, int offset,
int limit) {
boolean queryAuthor, Optional<String> missionId, boolean onlyOfficialMissions,
Optional<String> author, Optional<Collection<BannerListType>> listTypes,
Optional<String> userId, Optional<Boolean> online, Optional<BannerSortOrder> orderBy,
Direction orderDirection, Optional<Double> proximityLatitude,
Optional<Double> proximityLongitude, int offset, int limit) {
List<Specification<Banner>> specifications = new ArrayList<>();
if (placeSlug.isPresent()) {
specifications.add(BannerSpecifications.hasStartPlaceSlug(placeSlug.get()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import org.hibernate.search.engine.search.common.BooleanOperator;
import org.hibernate.search.engine.search.predicate.dsl.PredicateFinalStep;
import org.hibernate.search.engine.search.predicate.dsl.SearchPredicateFactory;
import org.hibernate.search.engine.search.predicate.dsl.SimpleQueryStringPredicateFieldMoreStep;
import org.hibernate.search.engine.search.sort.dsl.SortOrder;
import org.hibernate.search.mapper.orm.Search;
import org.hibernate.search.mapper.orm.session.SearchSession;
Expand Down Expand Up @@ -56,11 +57,11 @@ public class LuceneBannerSearchServiceImpl extends BaseBannerSearchServiceImpl {
@Override
public List<Banner> find(Optional<String> placeSlug, Optional<Double> minLatitude, Optional<Double> maxLatitude,
Optional<Double> minLongitude, Optional<Double> maxLongitude, Optional<String> search,
Optional<String> missionId, boolean onlyOfficialMissions, Optional<String> author,
Optional<Collection<BannerListType>> listTypes, Optional<String> userId,
Optional<Boolean> online, Optional<BannerSortOrder> orderBy, Direction orderDirection,
Optional<Double> proximityLatitude, Optional<Double> proximityLongitude, int offset,
int limit) {
boolean queryAuthor, Optional<String> missionId, boolean onlyOfficialMissions,
Optional<String> author, Optional<Collection<BannerListType>> listTypes,
Optional<String> userId, Optional<Boolean> online, Optional<BannerSortOrder> orderBy,
Direction orderDirection, Optional<Double> proximityLatitude,
Optional<Double> proximityLongitude, int offset, int limit) {
SearchSession searchSession = Search.session(entityManager);
List<Banner> result = searchSession.search(Banner.class).where(factory -> factory.bool(b -> {
b.filter(factory.matchAll());
Expand All @@ -72,15 +73,17 @@ public List<Banner> find(Optional<String> placeSlug, Optional<Double> minLatitud
minLongitude.get(), minLatitude.get(), maxLongitude.get()));
}
if (search.isPresent()) {
b.must(factory.simpleQueryString() //
SimpleQueryStringPredicateFieldMoreStep<?, ?> step = factory.simpleQueryString() //
.field(FIELD_TITLE).boost(5) //
.field(FIELD_DESCRIPTION).boost(0.1f) //
.field(FIELD_MISSIONS_ID) //
.field(FIELD_MISSIONS_TITLE) //
.field(FIELD_MISSIONS_AUTHOR_NAME) //
.field(FIELD_START_PLACES_INFORMATION_LONG_NAME) //
.field(FIELD_START_PLACES_INFORMATION_FORMATTED_ADDRESS) //
.matching(search.get()).defaultOperator(BooleanOperator.AND));
.field(FIELD_START_PLACES_INFORMATION_FORMATTED_ADDRESS);
if (queryAuthor) {
step = step.field(FIELD_MISSIONS_AUTHOR_NAME);
}
b.must(step.matching(search.get()).defaultOperator(BooleanOperator.AND));
}
if (missionId.isPresent()) {
b.filter(factory.match().field(FIELD_MISSIONS_ID).matching(missionId.get()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@ void list() {
final Banner banner = fixPlaceInformation(a($Banner()));

when(bannerSearchService.find(eq(place), eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()),
eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()), eq(false), eq(Optional.empty()),
eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()), any(),
eq(Optional.empty()), eq(Optional.empty()), eq(0), anyInt())).thenReturn(List.of(banner));
eq(Optional.empty()), eq(Optional.empty()), eq(false), eq(Optional.empty()), eq(false),
eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()), eq(Optional.empty()),
eq(Optional.empty()), any(), eq(Optional.empty()), eq(Optional.empty()), eq(0), anyInt()))
.thenReturn(List.of(banner));

// THEN
final ResponseEntity<List<BannerDto>> result = testController.list(place, Optional.empty(), Optional.empty(),
Expand Down Expand Up @@ -78,7 +79,7 @@ void list_withBoundingBox() {
final Banner banner = fixPlaceInformation(a($Banner()));

when(bannerSearchService.find(eq(Optional.empty()), eq(minLat), eq(maxLat), eq(minLong), eq(maxLong), any(),
any(), eq(false), any(), any(), any(), any(), any(), any(), any(), any(), eq(0), anyInt()))
eq(false), any(), eq(false), any(), any(), any(), any(), any(), any(), any(), any(), eq(0), anyInt()))
.thenReturn(List.of(banner));

// THEN
Expand Down

0 comments on commit edc250f

Please sign in to comment.