-
-
Notifications
You must be signed in to change notification settings - Fork 2
feat(shopping-lists): Share a list by revocable link (version 1) #149
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
Merged
Merged
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
2c6d9e4
feat(shopping-lists): Replace isPublic with a revocable share token
OffCrazyFreak 55175c6
feat(shopping-lists): Wire the frontend to link access and share tokens
OffCrazyFreak b28f729
feat(shopping-lists): Gate item controls on the caller's access level
OffCrazyFreak e750356
feat(shopping-lists): Add the share settings modal
OffCrazyFreak 2626e1b
feat(shopping-lists): Add the public /s/[token] shared list page
OffCrazyFreak 9593f1a
fix(shopping-lists): Drop dead-end controls for logged-out link visitors
OffCrazyFreak 67483f1
Merge remote-tracking branch 'origin/dev' into feat/shopping-list-sha…
OffCrazyFreak 203b8b0
Merge branch 'fix/dev-vs-main-review-2026-07' into feat/shopping-list…
OffCrazyFreak e7e09b0
Merge branch 'dev' into feat/shopping-list-sharing
OffCrazyFreak 4d5866f
chore(agents): Hand back clickable paths and system-themed review HTML
OffCrazyFreak 0dbfc69
Merge branch 'dev' into feat/shopping-list-sharing
OffCrazyFreak f4adef1
fix(shopping-lists): Stop sending account ids to link visitors
OffCrazyFreak 1e4c42e
fix(shopping-lists): Let a stale token fall back to anonymous on shar…
OffCrazyFreak 14eec4a
fix(shopping-lists): Redact the share token from telemetry and referrers
OffCrazyFreak 80313dd
fix(shopping-lists): Key the offline cache by identity and stop purgi…
OffCrazyFreak 7f4ef14
feat(shopping-lists): Make shared lists work offline
OffCrazyFreak e2186dd
fix(shopping-lists): Roll back one item, and own the optimism in onMu…
OffCrazyFreak 1cbdd65
fix(shopping-lists): Spell query keys once and drop a revoked list fr…
OffCrazyFreak f154141
fix(shopping-lists): Let a recipient forward the link they are lookin…
OffCrazyFreak ff5f256
feat(shopping-lists): Rework the share modal and make the level legible
OffCrazyFreak 3ffdbdf
fix(a11y): Restore focus when a modal closes
OffCrazyFreak 22cc862
docs(shopping-lists): Record sharing, and the ordered column drop
OffCrazyFreak 72129a6
fix(shopping-lists): Land the review findings on the sharing fixes
OffCrazyFreak be5509f
Merge remote-tracking branch 'origin/dev' into feat/shopping-list-sha…
OffCrazyFreak a1fae2e
fix(shopping-lists): Adopt the dev share and route helpers in the sha…
OffCrazyFreak bc410b2
docs: Correct the sharing and offline cache details that went stale
OffCrazyFreak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
backend/src/main/java/disscount/config/OptionalBearerAuthenticationFilter.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package disscount.config; | ||
|
|
||
| import jakarta.servlet.FilterChain; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.HttpHeaders; | ||
| import org.springframework.lang.NonNull; | ||
| import org.springframework.security.core.context.SecurityContextHolder; | ||
| import org.springframework.security.oauth2.jwt.Jwt; | ||
| import org.springframework.security.oauth2.jwt.JwtDecoder; | ||
| import org.springframework.security.oauth2.jwt.JwtException; | ||
| import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.filter.OncePerRequestFilter; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Authenticates a bearer token when one is present and usable, and lets the request through | ||
| * anonymously when it is not. | ||
| * | ||
| * <p>This exists because a share link has to work for both. The standard | ||
| * {@code BearerTokenAuthenticationFilter} rejects an expired or malformed token with 401 | ||
| * before authorization rules are consulted, so a permitAll endpoint is not actually reachable | ||
| * by a caller whose cached token has gone stale. Dropping the resource server from the shared | ||
| * chain instead would fix that but break the other half: a signed-in recipient would be seen | ||
| * as anonymous and capped at VIEW however generous the link is. | ||
| * | ||
| * <p>Validation itself is unchanged and is not hand-rolled. This injects the same | ||
| * {@code JwtDecoder} bean the resource server uses, so a token is still checked against the | ||
| * better-auth JWKS, pinned to ES256, and validated for issuer and expiry. Only the response to | ||
| * a failed check differs: continue anonymously instead of committing a 401. Spring's OAuth2 | ||
| * resource-server DSL has no supported way to express that, because its entry point commits | ||
| * the response rather than continuing the chain. | ||
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OptionalBearerAuthenticationFilter extends OncePerRequestFilter { | ||
|
|
||
| private static final String BEARER_PREFIX = "Bearer "; | ||
|
|
||
| private final JwtDecoder jwtDecoder; | ||
| private final JwtAuthenticationConverter converter = new JwtAuthenticationConverter(); | ||
|
|
||
| @Override | ||
| protected void doFilterInternal( | ||
| @NonNull HttpServletRequest request, | ||
| @NonNull HttpServletResponse response, | ||
| @NonNull FilterChain chain | ||
| ) throws ServletException, IOException { | ||
| String header = request.getHeader(HttpHeaders.AUTHORIZATION); | ||
|
|
||
| // RFC 7235 makes the scheme name case-insensitive. Our own client always sends | ||
| // "Bearer", but a recipient arriving from anything else should not be silently | ||
| // demoted to anonymous over capitalisation. | ||
| if (header != null | ||
| && header.regionMatches(true, 0, BEARER_PREFIX, 0, BEARER_PREFIX.length())) { | ||
| try { | ||
| Jwt jwt = jwtDecoder.decode(header.substring(BEARER_PREFIX.length())); | ||
| SecurityContextHolder.getContext().setAuthentication(converter.convert(jwt)); | ||
| } catch (JwtException ignored) { | ||
| // Expired, malformed or issued elsewhere. The caller keeps whatever the link | ||
| // grants an anonymous visitor, which the access resolver caps at VIEW. | ||
| } | ||
| } | ||
|
|
||
| chain.doFilter(request, response); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 23 additions & 0 deletions
23
backend/src/main/java/disscount/shoppingList/domain/LinkAccess.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package disscount.shoppingList.domain; | ||
|
|
||
| /** | ||
| * The levels a share link can actually be set to, which is every {@link ListAccess} value | ||
| * except OWNER. | ||
| * | ||
| * <p>This is a separate enum rather than a validated {@code ListAccess} so that OWNER is not | ||
| * expressible on the wire at all. Jackson rejects it during deserialization, before any | ||
| * service code runs, which means the guard in {@code ShoppingListService.applyLinkAccess} is | ||
| * no longer the only thing standing between a typo and a caller granting themselves the | ||
| * ability to manage sharing. | ||
| */ | ||
| public enum LinkAccess { | ||
|
|
||
| NONE, | ||
| VIEW, | ||
| SHOP, | ||
| EDIT; | ||
|
|
||
| public ListAccess toListAccess() { | ||
| return ListAccess.valueOf(name()); | ||
| } | ||
| } |
35 changes: 35 additions & 0 deletions
35
backend/src/main/java/disscount/shoppingList/domain/ListAccess.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package disscount.shoppingList.domain; | ||
|
|
||
| /** | ||
| * What a caller may do with a shopping list. | ||
| * | ||
| * <p>Only NONE, VIEW, SHOP and EDIT are ever stored in {@code shopping_list.link_access}. | ||
| * OWNER is produced by {@link disscount.shoppingList.service.ShoppingListAccessService} | ||
| * and never persisted. | ||
| */ | ||
| public enum ListAccess { | ||
|
|
||
| NONE, | ||
| VIEW, | ||
| SHOP, | ||
| EDIT, | ||
| OWNER; | ||
|
|
||
| public boolean canView() { | ||
| return this != NONE; | ||
| } | ||
|
|
||
| /** The in-the-shop level: tick an item off, switch its store, snapshot its price. */ | ||
| public boolean canCheck() { | ||
| return this == SHOP || this == EDIT || this == OWNER; | ||
| } | ||
|
|
||
| /** Amount, removal and the list title. Adding items arrives with membership. */ | ||
| public boolean canEditItems() { | ||
| return this == EDIT || this == OWNER; | ||
| } | ||
|
|
||
| public boolean canManageShare() { | ||
| return this == OWNER; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
80 changes: 80 additions & 0 deletions
80
backend/src/main/java/disscount/shoppingList/rest/SharedShoppingListController.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| package disscount.shoppingList.rest; | ||
|
|
||
| import io.swagger.v3.oas.annotations.Operation; | ||
| import io.swagger.v3.oas.annotations.tags.Tag; | ||
| import jakarta.validation.Valid; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| import disscount.shoppingList.dto.ShoppingListDto; | ||
| import disscount.shoppingList.dto.ShoppingListRequest; | ||
| import disscount.shoppingList.service.SharedShoppingListService; | ||
| import disscount.shoppingListItem.dto.ShoppingListItemDto; | ||
| import disscount.shoppingListItem.dto.ShoppingListItemRequest; | ||
| import disscount.util.SecurityUtils; | ||
|
|
||
| import java.util.UUID; | ||
|
|
||
| /** | ||
| * Public entry point for shared lists. Permit-all at the filter chain; the real check is the | ||
| * token plus {@link disscount.shoppingList.service.ShoppingListAccessService}. | ||
| * | ||
| * <p>Callers may be anonymous, so every method reads the user through | ||
| * {@code getCurrentUserIdOptional()} and never {@code getCurrentUserId()}, which throws. | ||
| */ | ||
| @RestController | ||
| @RequestMapping("/api/shared") | ||
| @RequiredArgsConstructor | ||
| @Tag(name = "Shared Shopping Lists", description = "Shopping lists reachable by share token") | ||
| public class SharedShoppingListController { | ||
|
|
||
| private final SharedShoppingListService sharedShoppingListService; | ||
|
|
||
| @Operation(summary = "Get a shared shopping list by its share token") | ||
| @GetMapping("/{token}") | ||
| public ResponseEntity<ShoppingListDto> getSharedShoppingList(@PathVariable String token) { | ||
| UUID userId = currentUserId(); | ||
| return sharedShoppingListService.getByToken(token, userId) | ||
| .map(ResponseEntity::ok) | ||
| .orElse(ResponseEntity.notFound().build()); | ||
| } | ||
|
|
||
| @Operation(summary = "Rename a shared shopping list") | ||
| @PutMapping("/{token}") | ||
| public ResponseEntity<ShoppingListDto> updateSharedShoppingList( | ||
| @PathVariable String token, | ||
| @Valid @RequestBody ShoppingListRequest request) { | ||
| UUID userId = currentUserId(); | ||
| return sharedShoppingListService.updateTitle(token, userId, request) | ||
| .map(ResponseEntity::ok) | ||
| .orElse(ResponseEntity.notFound().build()); | ||
| } | ||
|
|
||
| @Operation(summary = "Update an item on a shared shopping list") | ||
| @PutMapping("/{token}/items/{itemId}") | ||
| public ResponseEntity<ShoppingListItemDto> updateSharedItem( | ||
| @PathVariable String token, | ||
| @PathVariable UUID itemId, | ||
| @Valid @RequestBody ShoppingListItemRequest request) { | ||
| UUID userId = currentUserId(); | ||
| return sharedShoppingListService.updateItem(token, itemId, userId, request) | ||
| .map(ResponseEntity::ok) | ||
| .orElse(ResponseEntity.notFound().build()); | ||
| } | ||
|
|
||
| @Operation(summary = "Delete an item from a shared shopping list") | ||
| @DeleteMapping("/{token}/items/{itemId}") | ||
| public ResponseEntity<Void> deleteSharedItem( | ||
| @PathVariable String token, | ||
| @PathVariable UUID itemId) { | ||
| UUID userId = currentUserId(); | ||
| return sharedShoppingListService.deleteItem(token, itemId, userId) | ||
| ? ResponseEntity.noContent().build() | ||
| : ResponseEntity.notFound().build(); | ||
| } | ||
|
|
||
| private UUID currentUserId() { | ||
| return SecurityUtils.getCurrentUserIdOptional().orElse(null); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: OffCrazyFreak/Disscount
Length of output: 2044
🏁 Script executed:
Repository: OffCrazyFreak/Disscount
Length of output: 235
🏁 Script executed:
Repository: OffCrazyFreak/Disscount
Length of output: 38851
🌐 Web query:
Spring Security SecurityFilterChainBuilder addFilterBefore addFilterAfter equal order same position💡 Result:
In Spring Security, the HttpSecurity methods addFilterBefore, addFilterAfter, and addFilterAt are used to place custom filters relative to existing filters within the SecurityFilterChain [1][2][3]. These methods calculate a specific numerical "order" value to determine the execution sequence [4]. Key behaviors regarding ordering and positioning: 1. How Positions are Calculated: The framework uses a registered list of filter classes and their associated order values (e.g., in FilterOrderRegistration.java) [5][4]. When you call addFilterBefore or addFilterAfter, the framework calculates the new filter's order based on the offset relative to the target filter's order [4]. For example, addFilterAfter generally adds a filter at (targetFilterOrder + 1) [4]. 2. Same Position (Numerical Order) Behavior: It is possible for multiple filters to end up with the same numerical order value [6]. Spring Security does not necessarily prevent this at the API level during configuration [6]. When filters have the same order, their relative execution order within the chain is typically determined by the order in which they were added or by the underlying collection implementation, though this can lead to unpredictable or difficult-to-debug behavior [6][7]. 3. Issues with Same Class Instances: A known complication occurs when the same filter instance or filters of the same class are added multiple times or at different relative positions [8][7]. Because the framework sometimes maps order values by class type internally, subsequent calls for the same filter class can override previous registrations, potentially causing a filter to appear in an unexpected location [8]. 4. Recommendations: To ensure predictable ordering: - Avoid relying on implicit ordering or overlapping positions. - If you need multiple filters to run in a specific sequence, explicitly chain them or use separate classes for each filter [6]. - If you intend to replace a standard filter, use addFilterAt to ensure the custom filter takes the exact position of the original, rather than just adding it before or after [1][3]. - If you encounter order conflicts, verify the generated filter order by enabling debug logging, which helps identify the final placement of filters in the chain [9]. For authoritative details on the default ordering of standard filters, you can inspect the FilterOrderRegistration class in the Spring Security source code [1][5].
Citations:
🌐 Web query:
github spring-security FilterOrderRegistration addFilterBefore addFilterAfter compare insertionOrder sameOrderStableSort💡 Result:
In Spring Security, the FilterOrderRegistration class acts as a central registry that assigns default integer order values to built-in security filters [1][2]. This ensures that when multiple filters are involved, the framework can maintain a predictable sequence (e.g., ensuring authentication filters run before authorization filters) [3]. When you use HttpSecurity methods like addFilterBefore(Filter, Class) or addFilterAfter(Filter, Class), Spring Security uses the order values stored in FilterOrderRegistration to determine where to place your custom filter relative to a known (registered) filter class [4][5]. Key points regarding the registration and ordering mechanics include: 1. Known Filters: The methods addFilterBefore and addFilterAfter operate relative to "known" filter classes [4][5]. A filter is considered "known" if it is a built-in filter already tracked in FilterOrderRegistration or if it has been previously registered via these same HttpSecurity methods [4][5]. 2. Internal Mechanism: While early implementations used a simple map that could lead to overwrites if the same class was registered in multiple positions [6], current versions manage these positions more robustly. However, attempting to register multiple filters in the exact same position (e.g., using addFilterAt) results in non-deterministic ordering among those specific filters, as the framework does not provide an explicit tie-breaking mechanism for same-position registration [4]. 3. Stable Sort: The overall filter chain is typically constructed by sorting these assigned order values [3]. Because the registry uses a step-based increment (e.g., ORDER_STEP = 100) [1], there is generally "room" between standard filters to inject custom filters without collisions. In summary, you should rely on addFilterBefore or addFilterAfter when you need to ensure a specific relative order [3]. If you are concerned about identical positions, avoid multiple registrations at the same target class [4], as the framework is designed to handle relative positioning rather than absolute index-based ordering. [4][3]
Citations:
🌐 Web query:
org.springframework.security.config.annotation.web.SecurityBuilder#doBuild sameOrderStableSort SpringSecurityFilterChainFilterFilter💡 Result:
In Spring Security, the terms "doBuild" and "sameOrderStableSort" relate to the internal mechanism for constructing and ordering the security filter chain [1][2][3]. The doBuild method is part of the AbstractConfiguredSecurityBuilder class, which is a base class for builders like HttpSecurity [2][4][3]. This method is responsible for orchestrating the build process, which includes invoking configuration methods on applied SecurityConfigurers and finally calling performBuild to construct the target object (e.g., DefaultSecurityFilterChain) [2][5]. While "sameOrderStableSort" is not a public API method, it refers to the internal sorting logic used during the construction of the filter chain to ensure that filters are executed in the correct, predictable order [6]. Spring Security uses the FilterOrderRegistration class to maintain a registry of filter classes and their associated order values [1][6]. This ensures that essential security filters (like those for authentication and authorization) are executed in a specific sequence regardless of how they are added to the configuration [6]. If you are investigating the filter order: 1. FilterOrderRegistration: This is the definitive source of truth for the default order of standard Spring Security filters [1][6]. You can check this class in the Spring Security source code to see exactly how filters are ordered [1]. 2. HttpSecurity: When configuring your application using HttpSecurity, you can influence this order using methods like addFilterBefore and addFilterAfter, which allow you to place your custom filters relative to the standard ones [7]. 3. Architecture: The FilterChainProxy is the entry point that uses the ordered SecurityFilterChain to delegate requests to the appropriate filters [6][8]. If you find discrepancies in documentation or need to understand why a filter is placed in a specific position, the FilterOrderRegistration class remains the primary reference [1][9][6].
Citations:
Make the custom filter order explicit.
addFilterAfter(userProvisioningFilter, OptionalBearerAuthenticationFilter.class)assigns the same order as the anchor filter, so the provisioning-before-anonymous sequence is an implementation-detail dependency on stable same-order sorting. Anchor both custom filters with a strict relative order, such as a single chain using an explicit earlier standard anchor or an@Order-backed bean registration, instead of relying on insertion-dependent tie-breaking.Preserve the OAuth2 resource-server path, which still needs
SecurityFilterChain filterChain(...)after this change.🤖 Prompt for AI Agents
Source: Coding guidelines