diff --git a/src/main/java/net/javahippie/fitpub/config/SecurityConfig.java b/src/main/java/net/javahippie/fitpub/config/SecurityConfig.java index aff2bae..ca906ea 100644 --- a/src/main/java/net/javahippie/fitpub/config/SecurityConfig.java +++ b/src/main/java/net/javahippie/fitpub/config/SecurityConfig.java @@ -168,7 +168,7 @@ public class SecurityConfig { .requestMatchers(HttpMethod.GET, "/api/users/me").authenticated() .requestMatchers(HttpMethod.PUT, "/api/users/me").authenticated() .requestMatchers(HttpMethod.DELETE, "/api/users/me").authenticated() - .requestMatchers(HttpMethod.GET, "/api/users/{username}").permitAll() + .requestMatchers(HttpMethod.GET, "/api/users/*").permitAll() .requestMatchers(HttpMethod.GET, "/api/users/id/*").permitAll() .requestMatchers(HttpMethod.GET, "/api/users/search").permitAll() // User search .requestMatchers(HttpMethod.GET, "/api/users/browse").permitAll() // Browse all users diff --git a/src/main/java/net/javahippie/fitpub/controller/UserController.java b/src/main/java/net/javahippie/fitpub/controller/UserController.java index e64023c..e31508c 100644 --- a/src/main/java/net/javahippie/fitpub/controller/UserController.java +++ b/src/main/java/net/javahippie/fitpub/controller/UserController.java @@ -15,17 +15,20 @@ import net.javahippie.fitpub.repository.RemoteActorRepository; import net.javahippie.fitpub.repository.UserRepository; import net.javahippie.fitpub.service.FederationService; import net.javahippie.fitpub.service.TextValidationService; +import net.javahippie.fitpub.service.ProfileAccessService; import net.javahippie.fitpub.service.WebFingerClient; import net.javahippie.fitpub.service.UserService; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.annotation.AuthenticationPrincipal; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.security.core.userdetails.UsernameNotFoundException; import org.springframework.web.bind.annotation.*; +import org.springframework.web.server.ResponseStatusException; import java.util.ArrayList; import java.util.List; @@ -49,6 +52,7 @@ public class UserController { private final FederationService federationService; private final UserService userService; private final TextValidationService textValidationService; + private final ProfileAccessService profileAccessService; private final net.javahippie.fitpub.repository.ActivityPeakRepository activityPeakRepository; @Value("${fitpub.base-url}") @@ -70,6 +74,14 @@ public class UserController { dto.setFollowingCount((long) followingCount); } + private User getCurrentUserOrNull(UserDetails userDetails) { + if (userDetails == null) { + return null; + } + + return userRepository.findByUsername(userDetails.getUsername()).orElse(null); + } + /** * Get current user's profile. * @@ -115,6 +127,9 @@ public class UserController { if (request.getBio() != null) { user.setBio(request.getBio().trim()); } + if (request.getProfileVisibility() != null) { + user.setProfileVisibility(request.getProfileVisibility()); + } if (request.getAvatarUrl() != null) { user.setAvatarUrl(request.getAvatarUrl().trim()); } @@ -181,13 +196,21 @@ public class UserController { * @return user profile */ @GetMapping("/{username}") - public ResponseEntity getUserByUsername(@PathVariable String username) { + public ResponseEntity getUserByUsername( + @PathVariable String username, + @AuthenticationPrincipal UserDetails userDetails + ) { log.debug("Retrieving profile for username: {}", username); User user = userRepository.findByUsername(username) .orElseThrow(() -> new UsernameNotFoundException("User not found: " + username)); - UserDTO dto = UserDTO.fromEntity(user); + User viewer = getCurrentUserOrNull(userDetails); + profileAccessService.requireProfileAccess(user, viewer); + + UserDTO dto = viewer != null && viewer.getId().equals(user.getId()) + ? UserDTO.fromEntity(user) + : UserDTO.fromEntityPublic(user); populateSocialCounts(dto, user); return ResponseEntity.ok(dto); @@ -200,13 +223,21 @@ public class UserController { * @return user profile */ @GetMapping("/id/{id}") - public ResponseEntity getUserById(@PathVariable UUID id) { + public ResponseEntity getUserById( + @PathVariable UUID id, + @AuthenticationPrincipal UserDetails userDetails + ) { log.debug("Retrieving profile for user ID: {}", id); User user = userRepository.findById(id) .orElseThrow(() -> new UsernameNotFoundException("User not found")); - UserDTO dto = UserDTO.fromEntity(user); + User viewer = getCurrentUserOrNull(userDetails); + profileAccessService.requireProfileAccess(user, viewer); + + UserDTO dto = viewer != null && viewer.getId().equals(user.getId()) + ? UserDTO.fromEntity(user) + : UserDTO.fromEntityPublic(user); populateSocialCounts(dto, user); return ResponseEntity.ok(dto); @@ -627,13 +658,17 @@ public class UserController { */ @GetMapping("/{username}/peaks") public ResponseEntity>> getUserPeaks( - @PathVariable String username + @PathVariable String username, + @AuthenticationPrincipal UserDetails userDetails ) { User user = userRepository.findByUsername(username).orElse(null); if (user == null) { return ResponseEntity.notFound().build(); } + User viewer = getCurrentUserOrNull(userDetails); + profileAccessService.requireProfileAccess(user, viewer); + var projections = activityPeakRepository.findPeaksVisitedByUser(user.getId()); var result = projections.stream() .map(p -> { @@ -649,4 +684,13 @@ public class UserController { return ResponseEntity.ok(result); } + + @ExceptionHandler(ResponseStatusException.class) + public ResponseEntity handleResponseStatusException(ResponseStatusException e) { + HttpStatus status = HttpStatus.valueOf(e.getStatusCode().value()); + String message = e.getReason() != null ? e.getReason() : status.getReasonPhrase(); + return ResponseEntity.status(status).body(new ErrorResponse(status.name(), message)); + } + + record ErrorResponse(String error, String message) {} } diff --git a/src/main/java/net/javahippie/fitpub/model/dto/UserDTO.java b/src/main/java/net/javahippie/fitpub/model/dto/UserDTO.java index fc08a61..a069baa 100644 --- a/src/main/java/net/javahippie/fitpub/model/dto/UserDTO.java +++ b/src/main/java/net/javahippie/fitpub/model/dto/UserDTO.java @@ -24,6 +24,7 @@ public class UserDTO { private String email; // Only shown to the user themselves private String displayName; private String bio; + private User.ProfileVisibility profileVisibility; private String avatarUrl; private LocalDateTime createdAt; private LocalDateTime updatedAt; @@ -52,6 +53,7 @@ public class UserDTO { .email(user.getEmail()) .displayName(user.getDisplayName()) .bio(user.getBio()) + .profileVisibility(user.getProfileVisibility()) .avatarUrl(user.getAvatarUrl()) .homeLatitude(user.getHomeLatitude()) .homeLongitude(user.getHomeLongitude()) @@ -72,6 +74,7 @@ public class UserDTO { .username(user.getUsername()) .displayName(user.getDisplayName()) .bio(user.getBio()) + .profileVisibility(user.getProfileVisibility()) .avatarUrl(user.getAvatarUrl()) .createdAt(user.getCreatedAt()) .updatedAt(user.getUpdatedAt()) diff --git a/src/main/java/net/javahippie/fitpub/model/dto/UserUpdateRequest.java b/src/main/java/net/javahippie/fitpub/model/dto/UserUpdateRequest.java index 13fb1f7..decbb81 100644 --- a/src/main/java/net/javahippie/fitpub/model/dto/UserUpdateRequest.java +++ b/src/main/java/net/javahippie/fitpub/model/dto/UserUpdateRequest.java @@ -7,6 +7,7 @@ import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Data; import lombok.NoArgsConstructor; +import net.javahippie.fitpub.model.entity.User; import org.hibernate.validator.constraints.URL; /** @@ -23,6 +24,8 @@ public class UserUpdateRequest { private String bio; + private User.ProfileVisibility profileVisibility; + @URL(message = "Avatar URL must be a valid URL") private String avatarUrl; diff --git a/src/main/java/net/javahippie/fitpub/model/entity/User.java b/src/main/java/net/javahippie/fitpub/model/entity/User.java index 9efb1f8..36b22be 100644 --- a/src/main/java/net/javahippie/fitpub/model/entity/User.java +++ b/src/main/java/net/javahippie/fitpub/model/entity/User.java @@ -43,6 +43,11 @@ public class User { @Column(columnDefinition = "TEXT") private String bio; + @Enumerated(EnumType.STRING) + @Column(name = "profile_visibility", nullable = false, length = 20) + @Builder.Default + private ProfileVisibility profileVisibility = ProfileVisibility.FOLLOWERS; + @Column(name = "avatar_url") private String avatarUrl; @@ -112,4 +117,10 @@ public class User { public String getWebFingerAccount(String domain) { return String.format("acct:%s@%s", username, domain); } + + public enum ProfileVisibility { + PUBLIC, + FOLLOWERS, + PRIVATE + } } diff --git a/src/main/java/net/javahippie/fitpub/service/ProfileAccessService.java b/src/main/java/net/javahippie/fitpub/service/ProfileAccessService.java new file mode 100644 index 0000000..c1e801f --- /dev/null +++ b/src/main/java/net/javahippie/fitpub/service/ProfileAccessService.java @@ -0,0 +1,59 @@ +package net.javahippie.fitpub.service; + +import lombok.RequiredArgsConstructor; +import net.javahippie.fitpub.model.entity.Follow; +import net.javahippie.fitpub.model.entity.User; +import net.javahippie.fitpub.repository.FollowRepository; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.stereotype.Service; +import org.springframework.web.server.ResponseStatusException; + +import static org.springframework.http.HttpStatus.FORBIDDEN; + +/** + * Central access policy for profile visibility checks. + */ +@Service +@RequiredArgsConstructor +public class ProfileAccessService { + + private final FollowRepository followRepository; + + @Value("${fitpub.base-url}") + private String baseUrl; + + public boolean canViewProfile(User profileOwner, User viewer) { + if (viewer != null && viewer.getId().equals(profileOwner.getId())) { + return true; + } + + User.ProfileVisibility visibility = profileOwner.getProfileVisibility() != null + ? profileOwner.getProfileVisibility() + : User.ProfileVisibility.PUBLIC; + + if (visibility == User.ProfileVisibility.PUBLIC) { + return true; + } + + if (visibility == User.ProfileVisibility.PRIVATE || viewer == null) { + return false; + } + + String actorUri = profileOwner.getActorUri(baseUrl); + return followRepository.findByFollowerIdAndFollowingActorUri(viewer.getId(), actorUri) + .filter(follow -> follow.getStatus() == Follow.FollowStatus.ACCEPTED) + .isPresent(); + } + + public String getAccessDeniedMessage(User profileOwner) { + return profileOwner.getProfileVisibility() == User.ProfileVisibility.FOLLOWERS + ? "This profile is only visible to followers." + : "This profile is private."; + } + + public void requireProfileAccess(User profileOwner, User viewer) { + if (!canViewProfile(profileOwner, viewer)) { + throw new ResponseStatusException(FORBIDDEN, getAccessDeniedMessage(profileOwner)); + } + } +} diff --git a/src/main/resources/db/migration/V32_1__add_profile_visibility_to_users.sql b/src/main/resources/db/migration/V32_1__add_profile_visibility_to_users.sql new file mode 100644 index 0000000..8d92a9a --- /dev/null +++ b/src/main/resources/db/migration/V32_1__add_profile_visibility_to_users.sql @@ -0,0 +1,2 @@ +ALTER TABLE users + ADD COLUMN profile_visibility VARCHAR(20) NOT NULL DEFAULT 'FOLLOWERS'; diff --git a/src/main/resources/static/js/auth.js b/src/main/resources/static/js/auth.js index 78755a0..10c9735 100644 --- a/src/main/resources/static/js/auth.js +++ b/src/main/resources/static/js/auth.js @@ -323,6 +323,12 @@ const FitPubAuth = { return; } + // Public profile pages are public (visibility is enforced by the profile API) + // Pattern: /users/{username} + if (currentPath.startsWith('/users/') && currentPath.split('/').length === 3) { + return; + } + if (currentPath.startsWith('/terms')) { return; } diff --git a/src/main/resources/templates/profile/edit.html b/src/main/resources/templates/profile/edit.html index 617286a..a00a2bd 100644 --- a/src/main/resources/templates/profile/edit.html +++ b/src/main/resources/templates/profile/edit.html @@ -61,6 +61,17 @@ + +
+ + +
Controls who can view your profile page.
+
+
@@ -255,6 +266,7 @@ const formData = { displayName: document.getElementById('displayName').value.trim(), bio: document.getElementById('bio').value.trim(), + profileVisibility: document.getElementById('profileVisibility').value, avatarUrl: document.getElementById('avatarUrl').value.trim(), homeLatitude: document.getElementById('homeLatitude').value ? parseFloat(document.getElementById('homeLatitude').value) : null, homeLongitude: document.getElementById('homeLongitude').value ? parseFloat(document.getElementById('homeLongitude').value) : null, @@ -321,6 +333,7 @@ function populateForm(user) { document.getElementById('displayName').value = user.displayName || ''; document.getElementById('bio').value = user.bio || ''; + document.getElementById('profileVisibility').value = user.profileVisibility || 'FOLLOWERS'; document.getElementById('avatarUrl').value = user.avatarUrl || ''; document.getElementById('email').value = user.email || ''; document.getElementById('username').value = user.username || ''; diff --git a/src/main/resources/templates/profile/public.html b/src/main/resources/templates/profile/public.html index 828877d..f0fe3f4 100644 --- a/src/main/resources/templates/profile/public.html +++ b/src/main/resources/templates/profile/public.html @@ -24,6 +24,11 @@
+ +
@@ -156,10 +161,21 @@ function loadProfile() { // For now, we'll fetch from the user API endpoint // In the future, this should use /api/users/{username} + fetch(`/api/users/${targetUsername}`, { + headers: { + 'Accept': 'application/json' + } + }) fetch(`/api/users/${targetUsername}`) .then(response => { if (!response.ok) { - throw new Error('User not found'); + return response.json() + .catch(() => ({})) + .then(errorData => { + const error = new Error(errorData.message || 'User not found'); + error.status = response.status; + throw error; + }); } return response.json(); }) @@ -171,8 +187,13 @@ .catch(error => { console.error('Error loading profile:', error); document.getElementById('loadingIndicator').classList.add('d-none'); - document.getElementById('errorMessage').textContent = 'User not found or profile could not be loaded.'; - document.getElementById('errorAlert').classList.remove('d-none'); + if (error.status === 403) { + document.getElementById('accessNoticeMessage').textContent = error.message; + document.getElementById('accessNotice').classList.remove('d-none'); + } else { + document.getElementById('errorMessage').textContent = 'User not found or profile could not be loaded.'; + document.getElementById('errorAlert').classList.remove('d-none'); + } }); } diff --git a/src/test/java/net/javahippie/fitpub/service/ProfileAccessServiceTest.java b/src/test/java/net/javahippie/fitpub/service/ProfileAccessServiceTest.java new file mode 100644 index 0000000..716571c --- /dev/null +++ b/src/test/java/net/javahippie/fitpub/service/ProfileAccessServiceTest.java @@ -0,0 +1,203 @@ +package net.javahippie.fitpub.service; + +import net.javahippie.fitpub.model.entity.Follow; +import net.javahippie.fitpub.model.entity.User; +import net.javahippie.fitpub.repository.FollowRepository; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.server.ResponseStatusException; + +import java.lang.reflect.Proxy; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.springframework.http.HttpStatus.FORBIDDEN; + +@DisplayName("ProfileAccessService Tests") +class ProfileAccessServiceTest { + + @Test + @DisplayName("PUBLIC profile should be visible anonymously") + void publicProfileShouldBeVisibleAnonymously() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService(Optional.empty(), lookupCount); + User owner = user("owner-public", User.ProfileVisibility.PUBLIC); + + assertTrue(service.canViewProfile(owner, null)); + assertEquals(0, lookupCount.get()); + } + + @Test + @DisplayName("PUBLIC profile should be visible to another authenticated user") + void publicProfileShouldBeVisibleToAnotherAuthenticatedUser() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService(Optional.empty(), lookupCount); + User owner = user("owner-public-auth", User.ProfileVisibility.PUBLIC); + User viewer = user("viewer-public-auth", User.ProfileVisibility.PUBLIC); + + assertTrue(service.canViewProfile(owner, viewer)); + assertEquals(0, lookupCount.get()); + } + + @Test + @DisplayName("FOLLOWERS profile should be forbidden anonymously") + void followersProfileShouldBeForbiddenAnonymously() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + User owner = user("owner-followers-anon", User.ProfileVisibility.FOLLOWERS); + + assertFalse(service.canViewProfile(owner, null)); + } + + @Test + @DisplayName("FOLLOWERS profile should be forbidden to non followers") + void followersProfileShouldBeForbiddenToNonFollowers() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService(Optional.empty(), lookupCount); + User owner = user("owner-followers-nonf", User.ProfileVisibility.FOLLOWERS); + User viewer = user("viewer-followers-nonf", User.ProfileVisibility.PUBLIC); + + assertFalse(service.canViewProfile(owner, viewer)); + assertEquals(1, lookupCount.get()); + } + + @Test + @DisplayName("FOLLOWERS profile should be visible to accepted followers") + void followersProfileShouldBeVisibleToAcceptedFollowers() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService( + Optional.of(Follow.builder().status(Follow.FollowStatus.ACCEPTED).build()), + lookupCount + ); + User owner = user("owner-followers-accepted", User.ProfileVisibility.FOLLOWERS); + User viewer = user("viewer-followers-accepted", User.ProfileVisibility.PUBLIC); + + assertTrue(service.canViewProfile(owner, viewer)); + assertEquals(1, lookupCount.get()); + } + + @Test + @DisplayName("FOLLOWERS profile should be forbidden to pending followers") + void followersProfileShouldBeForbiddenToPendingFollowers() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService( + Optional.of(Follow.builder().status(Follow.FollowStatus.PENDING).build()), + lookupCount + ); + User owner = user("owner-followers-pending", User.ProfileVisibility.FOLLOWERS); + User viewer = user("viewer-followers-pending", User.ProfileVisibility.PUBLIC); + + assertFalse(service.canViewProfile(owner, viewer)); + assertEquals(1, lookupCount.get()); + } + + @Test + @DisplayName("PRIVATE profile should be forbidden anonymously") + void privateProfileShouldBeForbiddenAnonymously() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + User owner = user("owner-private-anon", User.ProfileVisibility.PRIVATE); + + assertFalse(service.canViewProfile(owner, null)); + } + + @Test + @DisplayName("PRIVATE profile should be forbidden to another authenticated user") + void privateProfileShouldBeForbiddenToAnotherAuthenticatedUser() { + AtomicInteger lookupCount = new AtomicInteger(); + ProfileAccessService service = createService(Optional.empty(), lookupCount); + User owner = user("owner-private-other", User.ProfileVisibility.PRIVATE); + User viewer = user("viewer-private-other", User.ProfileVisibility.PUBLIC); + + assertFalse(service.canViewProfile(owner, viewer)); + assertEquals(0, lookupCount.get()); + } + + @Test + @DisplayName("Owner should always be able to view own profile") + void ownerShouldAlwaysBeAbleToViewOwnProfile() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + + for (User.ProfileVisibility visibility : User.ProfileVisibility.values()) { + User owner = user("self-" + visibility.name().toLowerCase(), visibility); + assertTrue(service.canViewProfile(owner, owner)); + } + } + + @Test + @DisplayName("Require profile access should throw forbidden with followers message") + void requireProfileAccessShouldThrowForbiddenWithFollowersMessage() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + User owner = user("owner-followers-msg", User.ProfileVisibility.FOLLOWERS); + User viewer = user("viewer-followers-msg", User.ProfileVisibility.PUBLIC); + + ResponseStatusException exception = assertThrows( + ResponseStatusException.class, + () -> service.requireProfileAccess(owner, viewer) + ); + + assertEquals(FORBIDDEN, exception.getStatusCode()); + assertEquals("This profile is only visible to followers.", exception.getReason()); + } + + @Test + @DisplayName("Require profile access should throw forbidden with private message") + void requireProfileAccessShouldThrowForbiddenWithPrivateMessage() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + User owner = user("owner-private-msg", User.ProfileVisibility.PRIVATE); + User viewer = user("viewer-private-msg", User.ProfileVisibility.PUBLIC); + + ResponseStatusException exception = assertThrows( + ResponseStatusException.class, + () -> service.requireProfileAccess(owner, viewer) + ); + + assertEquals(FORBIDDEN, exception.getStatusCode()); + assertEquals("This profile is private.", exception.getReason()); + } + + @Test + @DisplayName("Require profile access should allow visible profiles") + void requireProfileAccessShouldAllowVisibleProfiles() { + ProfileAccessService service = createService(Optional.empty(), new AtomicInteger()); + User owner = user("owner-public-visible", User.ProfileVisibility.PUBLIC); + + assertDoesNotThrow(() -> service.requireProfileAccess(owner, null)); + } + + private ProfileAccessService createService(Optional followLookupResult, AtomicInteger lookupCount) { + FollowRepository repository = (FollowRepository) Proxy.newProxyInstance( + FollowRepository.class.getClassLoader(), + new Class[]{FollowRepository.class}, + (proxy, method, args) -> { + if ("findByFollowerIdAndFollowingActorUri".equals(method.getName())) { + lookupCount.incrementAndGet(); + return followLookupResult; + } + + throw new UnsupportedOperationException("Unexpected repository method: " + method.getName()); + } + ); + + ProfileAccessService service = new ProfileAccessService(repository); + ReflectionTestUtils.setField(service, "baseUrl", "http://localhost:8080"); + return service; + } + + private User user(String username, User.ProfileVisibility visibility) { + return User.builder() + .id(UUID.randomUUID()) + .username(username) + .email(username + "@example.com") + .passwordHash("hash") + .publicKey("pub") + .privateKey("priv") + .profileVisibility(visibility) + .build(); + } +}