From a77d10a11140d7951e1e187e6360232b2444d10c Mon Sep 17 00:00:00 2001 From: Aleksandr Misonizhnik Date: Tue, 13 Oct 2026 15:33:30 +0300 Subject: [PATCH] Model PreAuthorize authentication and ownership in demo rules --- .github/workflows/opentaint.yml | 1 + broken-access-control.yaml | 112 ++++++++++++++++++ build.gradle.kts | 3 + .../spring/auth/AccessControlDemoData.java | 21 ++++ .../auth/AccessControlSecurityConfig.java | 43 +++++++ .../auth/NonconformingNoteController.java | 38 ++++++ .../seqra/spring/auth/NoteAccessService.java | 44 +++++++ .../seqra/spring/auth/NoteAuthorization.java | 21 ++++ .../org/seqra/spring/auth/NoteGateway.java | 56 +++++++++ .../seqra/spring/auth/SafeNoteController.java | 57 +++++++++ .../spring/auth/VulnerableNoteController.java | 66 +++++++++++ .../seqra/spring/persistence/UserNote.java | 67 +++++++++++ .../persistence/UserNoteRepository.java | 24 ++++ 13 files changed, 553 insertions(+) create mode 100644 broken-access-control.yaml create mode 100644 src/main/java/org/seqra/spring/auth/AccessControlDemoData.java create mode 100644 src/main/java/org/seqra/spring/auth/AccessControlSecurityConfig.java create mode 100644 src/main/java/org/seqra/spring/auth/NonconformingNoteController.java create mode 100644 src/main/java/org/seqra/spring/auth/NoteAccessService.java create mode 100644 src/main/java/org/seqra/spring/auth/NoteAuthorization.java create mode 100644 src/main/java/org/seqra/spring/auth/NoteGateway.java create mode 100644 src/main/java/org/seqra/spring/auth/SafeNoteController.java create mode 100644 src/main/java/org/seqra/spring/auth/VulnerableNoteController.java create mode 100644 src/main/java/org/seqra/spring/persistence/UserNote.java create mode 100644 src/main/java/org/seqra/spring/persistence/UserNoteRepository.java diff --git a/.github/workflows/opentaint.yml b/.github/workflows/opentaint.yml index e6a4561..35b414f 100644 --- a/.github/workflows/opentaint.yml +++ b/.github/workflows/opentaint.yml @@ -27,4 +27,5 @@ jobs: - uses: seqra/opentaint/github@github/v0 with: + rules-path: 'broken-access-control.yaml' upload-sarif: 'true' diff --git a/broken-access-control.yaml b/broken-access-control.yaml new file mode 100644 index 0000000..0da5639 --- /dev/null +++ b/broken-access-control.yaml @@ -0,0 +1,112 @@ +# Demo policy: access only your own notes. +# Approved guards bind an unchanged selector with @P; selector mutation is out of scope. +# Method security, the ownership helper, principal-scoped queries and fixed ownership are trusted. +# Authentication uses a method search; ownership rules track request parameters to raw APIs. +# Approved direct guards exempt their bound input; changes after authorization are not modeled. +rules: + - id: spring.missing-note-ownership + languages: + - java + severity: WARNING + message: Raw note access lacks an approved guard for this ID; review runtime ownership checks + metadata: + category: authorization-standard + license: MIT + short-description: Note ownership check missing + mode: taint + options: + primitive-tracking: true + pattern-sources: + - patterns: + - pattern: | + @$MAPPING(...) + $METHOD(..., @$INPUT(...) $TYPE $UNTRUSTED, ...) { ... } + - metavariable-pattern: + metavariable: $MAPPING + pattern-either: + - pattern: RequestMapping + - pattern: GetMapping + - pattern: PostMapping + - pattern: PutMapping + - pattern: PatchMapping + - pattern: DeleteMapping + - metavariable-pattern: + metavariable: $INPUT + pattern-either: + - pattern: PathVariable + - pattern: RequestParam + - pattern: RequestBody + - pattern: RequestHeader + - pattern: CookieValue + - pattern: ModelAttribute + - pattern-not-inside: | + @PreAuthorize("@noteAuthorization.isOwner(#id, authentication)") + $METHOD(..., @P("id") @$INPUT(...) $TYPE $UNTRUSTED, ...) { + ... + } + - pattern-not-inside: | + @PreAuthorize("isAuthenticated() and @noteAuthorization.isOwner(#id, authentication)") + $METHOD(..., @P("id") @$INPUT(...) $TYPE $UNTRUSTED, ...) { + ... + } + - focus-metavariable: $UNTRUSTED + pattern-sinks: + - patterns: + - pattern-either: + - pattern: $SERVICE.readById($UNTRUSTED) + - pattern: $SERVICE.deleteById($UNTRUSTED) + - focus-metavariable: $UNTRUSTED + - id: spring.untrusted-note-owner + languages: [java] + severity: WARNING + message: Raw note access lacks an approved identity binding for this owner; review runtime ownership checks + metadata: + category: authorization-standard + license: MIT + short-description: Note owner is request-controlled + mode: taint + options: + primitive-tracking: true + pattern-sources: + - patterns: + - pattern: | + @$MAPPING(...) + $METHOD(..., @$INPUT(...) $TYPE $UNTRUSTED, ...) { + ... + } + - metavariable-pattern: + metavariable: $MAPPING + pattern-either: + - pattern: RequestMapping + - pattern: GetMapping + - pattern: PostMapping + - pattern: PutMapping + - pattern: PatchMapping + - pattern: DeleteMapping + - metavariable-pattern: + metavariable: $INPUT + pattern-either: + - pattern: PathVariable + - pattern: RequestParam + - pattern: RequestBody + - pattern: RequestHeader + - pattern: CookieValue + - pattern: ModelAttribute + - pattern-not-inside: | + @PreAuthorize("#ownerEmail == authentication.name") + $METHOD(..., @P("ownerEmail") @$INPUT(...) $TYPE $UNTRUSTED, ...) { + ... + } + - pattern-not-inside: | + @PreAuthorize("isAuthenticated() and #ownerEmail == authentication.name") + $METHOD(..., @P("ownerEmail") @$INPUT(...) $TYPE $UNTRUSTED, ...) { + ... + } + - focus-metavariable: $UNTRUSTED + pattern-sinks: + - patterns: + - pattern-either: + - pattern: $SERVICE.listByOwner($UNTRUSTED) + - pattern: $SERVICE.readOwnedById($ID, $UNTRUSTED) + - pattern: $SERVICE.deleteOwnedById($ID, $UNTRUSTED) + - focus-metavariable: $UNTRUSTED diff --git a/build.gradle.kts b/build.gradle.kts index 5357967..127183f 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -19,6 +19,9 @@ repositories { dependencies { implementation("org.springframework.boot:spring-boot-starter-web") implementation("org.springframework.boot:spring-boot-starter-thymeleaf") + implementation("org.springframework.boot:spring-boot-starter-data-jpa") + implementation("org.springframework.boot:spring-boot-starter-security") + runtimeOnly("com.h2database:h2") testImplementation("org.springframework.boot:spring-boot-starter-test") testImplementation(platform("org.junit:junit-bom:5.10.0")) testImplementation("org.junit.jupiter:junit-jupiter") diff --git a/src/main/java/org/seqra/spring/auth/AccessControlDemoData.java b/src/main/java/org/seqra/spring/auth/AccessControlDemoData.java new file mode 100644 index 0000000..c535e02 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/AccessControlDemoData.java @@ -0,0 +1,21 @@ +package org.seqra.spring.auth; + +import org.seqra.spring.persistence.UserNote; +import org.seqra.spring.persistence.UserNoteRepository; +import org.springframework.boot.CommandLineRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class AccessControlDemoData { + + @Bean + CommandLineRunner seedAccessControlNotes(UserNoteRepository notes) { + return args -> { + if (notes.count() == 0) { + notes.save(new UserNote("Alice's note", "Alice's private content", "alice@example.test")); + notes.save(new UserNote("Bob's note", "Bob's private content", "bob@example.test")); + } + }; + } +} diff --git a/src/main/java/org/seqra/spring/auth/AccessControlSecurityConfig.java b/src/main/java/org/seqra/spring/auth/AccessControlSecurityConfig.java new file mode 100644 index 0000000..17f6562 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/AccessControlSecurityConfig.java @@ -0,0 +1,43 @@ +package org.seqra.spring.auth; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.config.Customizer; +import org.springframework.security.config.annotation.method.configuration.EnableMethodSecurity; +import org.springframework.security.config.annotation.web.builders.HttpSecurity; +import org.springframework.security.core.userdetails.User; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.security.provisioning.InMemoryUserDetailsManager; +import org.springframework.security.web.SecurityFilterChain; + +@Configuration +@EnableMethodSecurity +public class AccessControlSecurityConfig { + + @Bean + SecurityFilterChain accessControlSecurity(HttpSecurity http) throws Exception { + return http + .securityMatcher("/api/access-control/**") + .authorizeHttpRequests(auth -> auth.anyRequest().authenticated()) + .httpBasic(Customizer.withDefaults()) + .build(); + } + + @Bean + PasswordEncoder passwordEncoder() { + return new BCryptPasswordEncoder(); + } + + @Bean + UserDetailsService demoUsers(PasswordEncoder encoder) { + return new InMemoryUserDetailsManager( + User.withUsername("alice@example.test") + .password(encoder.encode("alice-demo")) + .roles("USER").build(), + User.withUsername("bob@example.test") + .password(encoder.encode("bob-demo")) + .roles("USER").build()); + } +} diff --git a/src/main/java/org/seqra/spring/auth/NonconformingNoteController.java b/src/main/java/org/seqra/spring/auth/NonconformingNoteController.java new file mode 100644 index 0000000..6fb9674 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/NonconformingNoteController.java @@ -0,0 +1,38 @@ +package org.seqra.spring.auth; + +import java.util.List; +import org.seqra.spring.persistence.UserNote; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.parameters.P; +import org.springframework.http.HttpStatus; +import org.springframework.web.server.ResponseStatusException; +import org.springframework.web.bind.annotation.*; + +// Safe at runtime, but these handlers deliberately violate the annotation standard. +@RestController +@RequestMapping("/api/access-control/nonconforming/notes") +public class NonconformingNoteController { + private final NoteAccessService notes; + private final NoteAuthorization authorization; + + public NonconformingNoteController(NoteAccessService notes, NoteAuthorization authorization) { + this.notes = notes; + this.authorization = authorization; + } + + @GetMapping("/{id}") + @PreAuthorize("isAuthenticated()") + public UserNote read(@PathVariable("id") long id, Authentication authentication) { + if (!authorization.isOwner(id, authentication)) { + throw new ResponseStatusException(HttpStatus.FORBIDDEN); + } + return notes.readById(id); + } + + @GetMapping + @PreAuthorize("#ownerEmail == authentication.name") + public List list(@P("ownerEmail") @RequestParam("ownerEmail") String ownerEmail) { + return notes.listByOwner(ownerEmail); + } +} diff --git a/src/main/java/org/seqra/spring/auth/NoteAccessService.java b/src/main/java/org/seqra/spring/auth/NoteAccessService.java new file mode 100644 index 0000000..18a8e3d --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/NoteAccessService.java @@ -0,0 +1,44 @@ +package org.seqra.spring.auth; + +import java.util.List; +import org.seqra.spring.persistence.UserNote; +import org.seqra.spring.persistence.UserNoteRepository; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.server.ResponseStatusException; + +@Service +public class NoteAccessService { + private final UserNoteRepository notes; + + public NoteAccessService(UserNoteRepository notes) { + this.notes = notes; + } + + public UserNote readById(long id) { + return notes.findById(id).orElseThrow(NoteAccessService::notFound); + } + + @Transactional + public void deleteById(long id) { + notes.delete(readById(id)); + } + + public List listByOwner(String ownerEmail) { + return notes.findByOwnerEmail(ownerEmail); + } + + public UserNote readOwnedById(long id, String ownerEmail) { + return notes.findByIdAndOwnerEmail(id, ownerEmail).orElseThrow(NoteAccessService::notFound); + } + + @Transactional + public void deleteOwnedById(long id, String ownerEmail) { + notes.delete(readOwnedById(id, ownerEmail)); + } + + private static ResponseStatusException notFound() { + return new ResponseStatusException(HttpStatus.NOT_FOUND); + } +} diff --git a/src/main/java/org/seqra/spring/auth/NoteAuthorization.java b/src/main/java/org/seqra/spring/auth/NoteAuthorization.java new file mode 100644 index 0000000..b923bc8 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/NoteAuthorization.java @@ -0,0 +1,21 @@ +package org.seqra.spring.auth; + +import org.seqra.spring.persistence.UserNoteRepository; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.stereotype.Component; + +@Component("noteAuthorization") +public class NoteAuthorization { + private final UserNoteRepository notes; + + public NoteAuthorization(UserNoteRepository notes) { + this.notes = notes; + } + + public boolean isOwner(long id, Authentication authentication) { + return authentication != null && authentication.isAuthenticated() + && !(authentication instanceof AnonymousAuthenticationToken) + && notes.existsByIdAndOwnerEmail(id, authentication.getName()); + } +} diff --git a/src/main/java/org/seqra/spring/auth/NoteGateway.java b/src/main/java/org/seqra/spring/auth/NoteGateway.java new file mode 100644 index 0000000..7f4e118 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/NoteGateway.java @@ -0,0 +1,56 @@ +package org.seqra.spring.auth; + +import java.util.List; +import org.seqra.spring.persistence.UserNote; +import org.seqra.spring.persistence.UserNoteRepository; +import org.springframework.http.HttpStatus; +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.web.server.ResponseStatusException; + +// Reviewed contract: authorize the final selector and return immutable data. +@Service +public class NoteGateway { + private final UserNoteRepository notes; + + public NoteGateway(UserNoteRepository notes) { + this.notes = notes; + } + + public record NoteView(long id, String subject, String body) { + static NoteView from(UserNote note) { + return new NoteView(note.getId(), note.getSubject(), note.getBody()); + } + } + + public NoteView read(long id) { + return notes.findByIdAndOwnerEmail(id, owner()) + .map(NoteView::from).orElseThrow(NoteGateway::notFound); + } + + public List list() { + return notes.findByOwnerEmail(owner()).stream().map(NoteView::from).toList(); + } + + @Transactional + public void delete(long id) { + if (notes.deleteOwned(id, owner()) == 0) { + throw notFound(); + } + } + + private static String owner() { + var authentication = SecurityContextHolder.getContext().getAuthentication(); + if (authentication == null || !authentication.isAuthenticated() + || authentication instanceof AnonymousAuthenticationToken) { + throw new ResponseStatusException(HttpStatus.UNAUTHORIZED); + } + return authentication.getName(); + } + + private static ResponseStatusException notFound() { + return new ResponseStatusException(HttpStatus.NOT_FOUND); + } +} diff --git a/src/main/java/org/seqra/spring/auth/SafeNoteController.java b/src/main/java/org/seqra/spring/auth/SafeNoteController.java new file mode 100644 index 0000000..ee379f1 --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/SafeNoteController.java @@ -0,0 +1,57 @@ +package org.seqra.spring.auth; + +import java.util.List; +import org.seqra.spring.auth.NoteGateway.NoteView; +import org.seqra.spring.persistence.UserNote; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.parameters.P; +import org.springframework.web.bind.annotation.*; + +// Policy: users may access only their own notes. +@RestController +@RequestMapping("/api/access-control/safe/notes") +public class SafeNoteController { + private final NoteGateway notes; + private final NoteAccessService rawNotes; + + public SafeNoteController(NoteGateway notes, NoteAccessService rawNotes) { + this.notes = notes; + this.rawNotes = rawNotes; + } + + @GetMapping("/{id}") + @PreAuthorize("isAuthenticated()") + public NoteView read(@PathVariable("id") long id) { + return notes.read(id); + } + + @DeleteMapping("/{id}") + @PreAuthorize("isAuthenticated()") + public void delete(@PathVariable("id") long id) { + notes.delete(id); + } + + @GetMapping + @PreAuthorize("isAuthenticated()") + public List list() { + return notes.list(); + } + + @GetMapping("/guarded/{id}") + @PreAuthorize("isAuthenticated() and @noteAuthorization.isOwner(#id, authentication)") + public UserNote guardedRead(@P("id") @PathVariable("id") long id) { + return rawNotes.readById(id); + } + + @DeleteMapping("/guarded/{id}") + @PreAuthorize("isAuthenticated() and @noteAuthorization.isOwner(#id, authentication)") + public void guardedDelete(@P("id") @PathVariable("id") long id) { + rawNotes.deleteById(id); + } + + @GetMapping("/guarded") + @PreAuthorize("isAuthenticated() and #ownerEmail == authentication.name") + public List guardedList(@P("ownerEmail") @RequestParam("ownerEmail") String ownerEmail) { + return rawNotes.listByOwner(ownerEmail); + } +} diff --git a/src/main/java/org/seqra/spring/auth/VulnerableNoteController.java b/src/main/java/org/seqra/spring/auth/VulnerableNoteController.java new file mode 100644 index 0000000..31e614e --- /dev/null +++ b/src/main/java/org/seqra/spring/auth/VulnerableNoteController.java @@ -0,0 +1,66 @@ +package org.seqra.spring.auth; + +import java.util.List; +import org.seqra.spring.persistence.UserNote; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.parameters.P; +import org.springframework.web.bind.annotation.*; + +// Authenticated users can select another user's notes. +@RestController +@RequestMapping("/api/access-control/vulnerable/notes") +public class VulnerableNoteController { + private final NoteAccessService notes; + + public VulnerableNoteController(NoteAccessService notes) { + this.notes = notes; + } + + @GetMapping("/{id}") + @PreAuthorize("isAuthenticated()") + public UserNote read(@PathVariable("id") long id) { + return notes.readById(id); + } + + @DeleteMapping("/{id}") + @PreAuthorize("hasRole('USER')") + public void delete(@PathVariable("id") long id) { + notes.deleteById(id); + } + + @GetMapping + public List list(@RequestParam("ownerEmail") String ownerEmail) { + return notes.listByOwner(ownerEmail); + } + + @GetMapping("/boxed") + public UserNote boxedId(@RequestParam("id") Long id) { + return notes.readById(id); + } + + @PostMapping("/body") + public UserNote bodyId(@RequestBody Selection selection) { + return notes.readById(selection.id()); + } + + @GetMapping("/forged-owner/{id}") + public UserNote forgedOwner(@PathVariable("id") long id, + @RequestParam("ownerEmail") String ownerEmail) { + return notes.readOwnedById(id, ownerEmail); + } + + @GetMapping("/wrong-guard/{id}") + @PreAuthorize("@noteAuthorization.isOwner(#id, authentication)") + public UserNote wrongGuard(@P("id") @PathVariable("id") long id, + @RequestParam("otherId") long otherId) { + return notes.readById(otherId); + } + + @GetMapping("/weak-guard/{id}") + @PreAuthorize("@noteAuthorization.isOwner(#id, authentication) or isAuthenticated()") + public UserNote weakGuard(@P("id") @PathVariable("id") long id) { + return notes.readById(id); + } + + public record Selection(long id) {} +} diff --git a/src/main/java/org/seqra/spring/persistence/UserNote.java b/src/main/java/org/seqra/spring/persistence/UserNote.java new file mode 100644 index 0000000..593b3f0 --- /dev/null +++ b/src/main/java/org/seqra/spring/persistence/UserNote.java @@ -0,0 +1,67 @@ +package org.seqra.spring.persistence; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.Table; + +@Entity +@Table(name = "user_notes") +public class UserNote { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @Column(nullable = false) + private String subject; + + @Column(columnDefinition = "TEXT") + private String body; + + @Column(nullable = false) + private String ownerEmail; + + public UserNote() { + } + + public UserNote(String subject, String body, String ownerEmail) { + this.subject = subject; + this.body = body; + this.ownerEmail = ownerEmail; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getSubject() { + return subject; + } + + public void setSubject(String subject) { + this.subject = subject; + } + + public String getBody() { + return body; + } + + public void setBody(String body) { + this.body = body; + } + + public String getOwnerEmail() { + return ownerEmail; + } + + public void setOwnerEmail(String ownerEmail) { + this.ownerEmail = ownerEmail; + } +} diff --git a/src/main/java/org/seqra/spring/persistence/UserNoteRepository.java b/src/main/java/org/seqra/spring/persistence/UserNoteRepository.java new file mode 100644 index 0000000..bc88134 --- /dev/null +++ b/src/main/java/org/seqra/spring/persistence/UserNoteRepository.java @@ -0,0 +1,24 @@ +package org.seqra.spring.persistence; + +import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Modifying; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; +import org.springframework.stereotype.Repository; + +import java.util.List; +import java.util.Optional; + +@Repository +public interface UserNoteRepository extends JpaRepository { + + List findByOwnerEmail(String ownerEmail); + + Optional findByIdAndOwnerEmail(Long id, String ownerEmail); + + boolean existsByIdAndOwnerEmail(Long id, String ownerEmail); + + @Modifying + @Query("delete from UserNote n where n.id = :id and n.ownerEmail = :owner") + int deleteOwned(@Param("id") long id, @Param("owner") String owner); +}