CWE-863: Incorrect Authorization - Java

Overview

In Spring applications, Incorrect Authorization commonly appears as @PreAuthorize("hasRole('ADMIN')") used alone on an endpoint that also needs an ownership check, a denylist role comparison in a custom filter (if (!role.equals("ADMIN")) combined incorrectly with other conditions), or a @PreAuthorize/@Secured check present on one service method but missing from a newer controller path that calls the repository directly. Method security silently doing nothing - because @EnableMethodSecurity was never added, or the SpEL expression evaluates to a permissive default - is also a recurring cause. Fix flawed logic by combining role checks with a SpEL expression that calls a bean-backed ownership check, and ensure every entry point to the resource goes through the same authorization method.

Common Vulnerable Patterns

Role-Only Check Missing Ownership

// VULNERABLE - hasRole() proves the caller has a role, not that they own this order
@RestController
@RequestMapping("/orders")
public class OrderController {

    private final OrderRepository orderRepository;

    public OrderController(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    @PreAuthorize("hasRole('USER')")
    @PutMapping("/{id}")
    public ResponseEntity<Order> updateOrder(@PathVariable Long id, @RequestBody OrderDto dto) {
        // Any authenticated USER can update any order - the role check
        // passes for every logged-in user, and nothing compares the
        // order's owner to the caller.
        Order order = orderRepository.findById(id).orElseThrow();
        order.setStatus(dto.getStatus());
        return ResponseEntity.ok(orderRepository.save(order));
    }
}

// Attack: an authenticated low-privilege user sends PUT /orders/{someoneElsesId}
// Result: hasRole('USER') is satisfied by every logged-in user, so the
// update succeeds regardless of who owns the order

Why this is vulnerable: A role answers what kind of thing the caller may do, never which instance they may do it to. @PreAuthorize("hasRole('EDITOR')") is satisfied identically by every editor in the system, so the identifier in the path is the only thing choosing a record and the caller supplies it.

Spring Security has the missing half, and which half depends on whether the method writes. @PostAuthorize("returnObject.owner == authentication.name") evaluates against the loaded object, which is the right shape for a read. On the method above it is the wrong one: the expression runs after save(), so the update has already happened by the time the check fails, and it is undone only if the method is transactional and the AccessDeniedException escapes the transaction boundary. Spring's own reference says so - "@PostAuthorize is not recommended for classes that perform database writes since that typically means that a database change was made before the security invariants were checked" - and names @Transactional and @PostAuthorize on the same method as the common case. For an update or a delete, use @PreAuthorize against a bean that loads the record - @PreAuthorize("@orderSecurity.isOwner(#id, authentication.name)"), the shape the method-security patterns below take - or read the record under @PostAuthorize first and write only once that read is authorized. Where a write genuinely has to stay behind @PostAuthorize, Spring notes that @EnableTransactionManagement must come before @EnableMethodSecurity for the rollback to cover it. Reaching for the object-level form is the fix; adding another role to the role check is not.

Inverted Boolean in a Custom Filter

// VULNERABLE - short-circuit OR makes either condition alone sufficient
public class OrderAccessFilter extends OncePerRequestFilter {

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
                                     FilterChain chain) throws IOException, ServletException {
        String role = getRoleFromContext();
        boolean isOwner = checkOwnership(request);

        // Intended: admin OR owner. Actual: role != null is true for
        // *any* authenticated caller, so isOwner is never required.
        if (role != null || isOwner) {
            chain.doFilter(request, response);
            return;
        }
        response.sendError(HttpServletResponse.SC_FORBIDDEN);
    }
}

Why this is vulnerable: The condition was meant to require ownership unless the caller is an admin, but role != null is true for every authenticated request, so the isOwner check on the right side of || never gates anything.

Check Missing on a Duplicate Path

// VULNERABLE - the service layer enforces ownership, but a newer controller
// bypasses the service and calls the repository directly
@Service
public class OrderService {
    @PreAuthorize("hasRole('ADMIN') or @orderSecurity.isOwner(#id, authentication.name)")
    public void deleteOrder(Long id) {
        orderRepository.deleteById(id);
    }
}

@RestController
@RequestMapping("/admin/orders")
public class AdminOrderController {

    private final OrderRepository orderRepository; // bypasses OrderService entirely

    @DeleteMapping("/bulk")
    public void bulkDelete(@RequestBody List<Long> ids) {
        // No @PreAuthorize, no ownership check - added directly against
        // the repository when the "admin" bulk endpoint was built.
        orderRepository.deleteAllById(ids);
    }
}

Why this is vulnerable: Two mappings reach the same rows and only one carries the annotation, so the effective control on the data is the weaker path. Nothing fails: Spring registers both mappings happily, and a test suite that exercises the protected one passes.

Method-level annotations are what make this recurrent, because the security decision lives on each method rather than on the data. That is why closing one of these means searching by repository or entity rather than by URL - the sibling is typically a bulk operation, an internal admin controller, or a v1 mapping kept for a client that was never migrated. Moving the check into the service layer, where both controllers converge, removes the possibility rather than fixing this instance of it.

Secure Patterns

Role Check Combined with a Bean-Backed Ownership Check

// SECURE - role check combined with a bean-backed ownership check
@Service("orderSecurity")
public class OrderSecurityService {

    private final OrderRepository orderRepository;

    public OrderSecurityService(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    public boolean isOwner(Long orderId, String username) {
        return orderRepository.findById(orderId)
            .map(order -> order.getOwnerUsername().equals(username))
            .orElse(false);
    }
}

@RestController
@RequestMapping("/orders")
public class OrderController {

    private final OrderRepository orderRepository;

    public OrderController(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    // Admins pass on role alone; other authenticated users must own the order.
    @PreAuthorize("hasRole('ADMIN') or @orderSecurity.isOwner(#id, authentication.name)")
    @PutMapping("/{id}")
    public ResponseEntity<Order> updateOrder(@PathVariable Long id, @RequestBody OrderDto dto) {
        // Bare orElseThrow() raises NoSuchElementException, which Spring maps
        // to 500 - a missing order is a client error, not a server fault.
        Order order = orderRepository.findById(id)
            .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND));
        order.setStatus(dto.getStatus());
        return ResponseEntity.ok(orderRepository.save(order));
    }

    @PreAuthorize("hasRole('ADMIN') or @orderSecurity.isOwner(#id, authentication.name)")
    @DeleteMapping("/{id}")
    public ResponseEntity<Void> deleteOrder(@PathVariable Long id) {
        orderRepository.deleteById(id);
        return ResponseEntity.noContent().build();
    }
}

Why this works: isOwner() loads the resource from the repository and compares its owner field against authentication.name, resolved by Spring Security from the verified principal - not from any client-supplied field - so a valid role alone is no longer sufficient to act on someone else's order. The same SpEL expression is applied to both updateOrder and deleteOrder, so the ownership rule cannot drift between the two actions the way it could if each wrote its own inline check.

The two denials are also indistinguishable, which is worth keeping. A non-admin naming an order that does not exist gets 403, because isOwner() returns false through orElse(false); a non-admin naming someone else's order gets 403 too. Nothing in the response tells the caller which order IDs are real, so the endpoint cannot be walked to enumerate the table - a property that is easy to lose the moment someone "improves" the missing-order case to a 404.

Ownership Enforced Across a Batch

// SECURE - the bulk endpoint runs the same ownership rule as the single-order
// route, over every ID in the batch, and rejects the request if any fails
@Service("orderSecurity")
public class OrderSecurityService {

    private final OrderRepository orderRepository;

    public OrderSecurityService(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    public boolean isOwner(Long orderId, String username) {
        return orderRepository.findById(orderId)
            .map(order -> order.getOwnerUsername().equals(username))
            .orElse(false);
    }

    // #ids is a List, so the single-order isOwner() cannot be applied to it -
    // a SpEL expression that passes the whole list to isOwner(Long, String)
    // does not match the method signature and fails at evaluation time.
    public boolean ownsAll(Collection<Long> ids, String username) {
        if (ids == null || ids.isEmpty()) {
            return false;
        }
        List<Order> found = orderRepository.findAllById(ids);
        // findAllById silently omits IDs that do not exist, so compare the
        // counts: a missing ID is refused exactly like one owned by
        // someone else, and the caller learns nothing from the difference.
        // Set.copyOf deduplicates first - against ids.size(), a request
        // naming the same owned order twice would be refused for no reason.
        return found.size() == Set.copyOf(ids).size()
            && found.stream().allMatch(order -> order.getOwnerUsername().equals(username));
    }
}

@RestController
@RequestMapping("/admin/orders")
public class AdminOrderController {

    private final OrderRepository orderRepository;

    public AdminOrderController(OrderRepository orderRepository) {
        this.orderRepository = orderRepository;
    }

    @PreAuthorize("hasRole('ADMIN') or @orderSecurity.ownsAll(#ids, authentication.name)")
    @DeleteMapping("/bulk")
    public ResponseEntity<Void> bulkDelete(@RequestBody List<Long> ids) {
        orderRepository.deleteAllById(ids);
        return ResponseEntity.noContent().build();
    }
}

Why this works: the batch is authorized as a single decision before any row is deleted, so a request naming one order the caller does not own deletes nothing at all - rather than deleting the ones they do own and failing partway through, which leaves the caller unable to tell what happened and the data in a state neither side intended. ownsAll() is a separate method rather than a loop over isOwner() in SpEL because @PreAuthorize evaluates one expression: there is no iteration construct that would let isOwner(#id, ...) be applied to each element of #ids, and passing the list to a method typed Long fails at evaluation rather than denying cleanly.

The empty-list case returns false deliberately. allMatch on an empty stream is true - the vacuous-truth default that turns "every order is owned by the caller" into "there are no orders to check, so proceed" - and a bulk endpoint that authorizes an empty batch is one refactor away from authorizing a batch it failed to load.

Explicit Deny for Unmatched Conditions

// SECURE - authorization filter denies by default; ownership is required
// unless the role check independently succeeds
public class OrderAccessFilter extends OncePerRequestFilter {

    @Override
    protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
                                     FilterChain chain) throws IOException, ServletException {
        boolean isAdmin = "ADMIN".equals(getRoleFromContext());
        boolean isOwner = checkOwnership(request);

        if (isAdmin || isOwner) {
            chain.doFilter(request, response);
            return;
        }

        // Any combination not explicitly matched above is denied.
        response.sendError(HttpServletResponse.SC_FORBIDDEN);
    }
}

Why this works: isAdmin is now a concrete boolean derived from an equality check against a known role, not a null-check on "any authenticated caller," so the || correctly requires either the admin role or verified ownership - never neither. Writing the intended rule ("admin OR owner") as a plain-language comment above the condition and comparing it term-by-term against the code is what catches this class of inversion during review.

Framework-Specific Guidance

Spring Security Method Security

// SECURE - method security must be explicitly enabled, and the default-deny
// fallback protects endpoints added without an explicit annotation
@Configuration
@EnableMethodSecurity  // Required for @PreAuthorize to be evaluated at all
public class MethodSecurityConfig {
}

@Configuration
public class WebSecurityConfig {

    @Bean
    public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
        http.authorizeHttpRequests(authorize -> authorize
            .anyRequest().authenticated()  // fallback: no route is unauthenticated by omission
        );
        return http.build();
    }
}

Why this works: without @EnableMethodSecurity, @PreAuthorize annotations are silently never evaluated and every method executes as if unprotected - a common root cause of an authorization check that "exists in the code" but never runs. The anyRequest().authenticated() fallback in the filter chain means a new controller that forgets a method-security annotation still requires authentication, even though it does not yet enforce ownership.

Testing

• Role boundary: use @WithMockUser(roles = "SUPPORT") (a role the check does not recognize) against the endpoint and confirm a 403, not a successful response.
• Cross-owner access: use @WithMockUser for one user's identity and request another user's resource ID through every method that touches it, confirming each is independently denied.
• Boolean logic regression: unit-test OrderSecurityService.isOwner() and any custom filter condition directly against the exact combination of role and ownership that a prior inversion would have allowed through.
• Bypass paths: write an integration test that calls the repository-backed admin/bulk endpoint the same way the flagged endpoint was tested, confirming the fix was not scoped to only the originally reported path.
• Use @SpringBootTest with MockMvc to exercise the full filter chain and method security together, since unit-testing the bean method alone will not catch a missing @EnableMethodSecurity or filter-chain misconfiguration.

Common Pitfalls

• Forgetting @EnableMethodSecurity: @PreAuthorize annotations that appear correct in code review do nothing at runtime if method security was never enabled - verify the annotation actually executes, not just that it is present.
• Fixing the flagged controller but not a service-layer or repository-direct bypass: A newer controller that calls orderRepository directly instead of going through the already-fixed OrderService reintroduces the same gap under a different entry point.
• Treating hasRole() as sufficient for object-level access: A role check confirms the caller's role class, not their relationship to the specific resource id in the path - combine it with a bean-backed ownership check for any action that reads or mutates a specific instance.
• Reusing a single-resource SpEL expression on a bulk endpoint: @PreAuthorize("@orderSecurity.isOwner(#ids, authentication.name)") on a method taking List<Long> ids does not check each order - SpEL has no iteration construct here, and the argument does not match a method typed Long. Write a separate collection-typed method that decides the whole batch.
• Trusting authentication.name populated from a mutable client claim: If the Authentication object is populated from a JWT, confirm the token's signature and claims are verified before the principal is trusted - an unverified or improperly validated token defeats the ownership check downstream.

Additional Resources

• CWE-863: Incorrect Authorization
• Spring Security Method Security Reference
• Spring Security Authorization Architecture
• OWASP Authorization Cheat Sheet