From 89a7145c0341a7787ae92fb2a2a48010d034816d Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 17 Nov 2022 11:52:41 +0000 Subject: [PATCH] Polishing contribution Closes gh-367 --- .../sample/graphql/SalaryController.java | 4 ++-- .../io/spring/sample/graphql/SalaryInput.java | 5 +++++ .../spring/sample/graphql/SalaryService.java | 6 +++--- .../spring/sample/graphql/SecurityConfig.java | 4 ++-- .../main/resources/graphql/schema.graphqls | 2 +- .../graphql/WebFluxSecuritySampleTests.java | 17 +++++++++++++++++ .../graphql-test/updateSalary.graphql | 9 +++++++++ .../spring/sample/graphql/SalaryService.java | 2 +- .../spring/sample/graphql/SecurityConfig.java | 19 ++++++++++++++++--- .../WebMvcHttpSecuritySampleTests.java | 5 ++--- 10 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 samples/webflux-security/src/test/resources/graphql-test/updateSalary.graphql diff --git a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryController.java b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryController.java index 7b824184..08834f66 100644 --- a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryController.java +++ b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryController.java @@ -49,10 +49,10 @@ public class SalaryController { } @MutationMapping - public void updateSalary(@Argument("input") SalaryInput salaryInput) { + public Mono updateSalary(@Argument("input") SalaryInput salaryInput) { String employeeId = salaryInput.getEmployeeId(); BigDecimal salary = salaryInput.getNewSalary(); - this.salaryService.updateSalary(employeeId, salary); + return this.salaryService.updateSalary(employeeId, salary); } } diff --git a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryInput.java b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryInput.java index 27869b03..c40652c9 100644 --- a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryInput.java +++ b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryInput.java @@ -23,6 +23,11 @@ public class SalaryInput { private BigDecimal newSalary; + public SalaryInput(String employeeId, BigDecimal newSalary) { + this.employeeId = employeeId; + this.newSalary = newSalary; + } + public String getEmployeeId() { return employeeId; } diff --git a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryService.java b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryService.java index 358d3191..aeddd7f7 100644 --- a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryService.java +++ b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SalaryService.java @@ -32,9 +32,9 @@ public class SalaryService { return Mono.just(new BigDecimal("42")); } - @Secured({ "ROLE_HR" }) - public void updateSalary(String employeeId, BigDecimal newSalary) { - // empty + @Secured("ROLE_HR") + public Mono updateSalary(String employeeId, BigDecimal newSalary) { + return Mono.empty(); } } diff --git a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java index 9fc1ca75..32502df1 100644 --- a/samples/webflux-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java +++ b/samples/webflux-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,7 +35,7 @@ public class SecurityConfig { @Bean SecurityWebFilterChain springWebFilterChain(ServerHttpSecurity http) throws Exception { return http - .csrf(spec -> spec.disable()) + .csrf(c -> c.disable()) // Demonstrate that method security works // Best practice to use both for defense in depth .authorizeExchange(requests -> requests.anyExchange().permitAll()) diff --git a/samples/webflux-security/src/main/resources/graphql/schema.graphqls b/samples/webflux-security/src/main/resources/graphql/schema.graphqls index 7687c854..af5236b6 100644 --- a/samples/webflux-security/src/main/resources/graphql/schema.graphqls +++ b/samples/webflux-security/src/main/resources/graphql/schema.graphqls @@ -14,7 +14,7 @@ type Employee { input UpdateSalaryInput { employeeId: ID! - salary: String! + newSalary: String! } type UpdateSalaryPayload { success: Boolean! diff --git a/samples/webflux-security/src/test/java/io/spring/sample/graphql/WebFluxSecuritySampleTests.java b/samples/webflux-security/src/test/java/io/spring/sample/graphql/WebFluxSecuritySampleTests.java index 43b91673..e3767e6a 100644 --- a/samples/webflux-security/src/test/java/io/spring/sample/graphql/WebFluxSecuritySampleTests.java +++ b/samples/webflux-security/src/test/java/io/spring/sample/graphql/WebFluxSecuritySampleTests.java @@ -15,11 +15,13 @@ */ package io.spring.sample.graphql; +import java.math.BigDecimal; import java.net.URI; import java.time.Duration; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; @@ -105,6 +107,21 @@ class WebFluxSecuritySampleTests { }); } + @Disabled // This does not work currently + @Test + void canNotMutateUpdateSalary() { + SalaryInput salaryInput = new SalaryInput("1", BigDecimal.valueOf(44)); + + this.graphQlTester.documentName("updateSalary") + .variable("salaryInput", salaryInput) + .execute() + .errors() + .satisfy(errors -> { + assertThat(errors).hasSize(1); + assertThat(errors.get(0).getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED); + }); + } + @Test void canQuerySalaryAsAdmin() { diff --git a/samples/webflux-security/src/test/resources/graphql-test/updateSalary.graphql b/samples/webflux-security/src/test/resources/graphql-test/updateSalary.graphql new file mode 100644 index 00000000..02eda7d7 --- /dev/null +++ b/samples/webflux-security/src/test/resources/graphql-test/updateSalary.graphql @@ -0,0 +1,9 @@ +mutation updateSalary($salaryInput: UpdateSalaryInput!) { + updateSalary(input: $salaryInput) { + success + employee { + id + name + } + } +} \ No newline at end of file diff --git a/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SalaryService.java b/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SalaryService.java index 75b1a14f..e62dd516 100644 --- a/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SalaryService.java +++ b/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SalaryService.java @@ -15,7 +15,7 @@ public class SalaryService { return new BigDecimal("42"); } - @Secured({ "ROLE_HR" }) + @Secured("ROLE_HR") public void updateSalary(String employeeId, BigDecimal newSalary) { } diff --git a/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java b/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java index 5dad854d..ecea65cf 100644 --- a/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java +++ b/samples/webmvc-http-security/src/main/java/io/spring/sample/graphql/SecurityConfig.java @@ -1,3 +1,18 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package io.spring.sample.graphql; import org.springframework.context.annotation.Bean; @@ -23,9 +38,7 @@ public class SecurityConfig { .csrf(c -> c.disable()) // Demonstrate that method security works // Best practice to use both for defense in depth - .authorizeRequests(requests -> requests - .anyRequest().permitAll() - ) + .authorizeRequests(requests -> requests.anyRequest().permitAll()) .httpBasic(withDefaults()) .build(); } diff --git a/samples/webmvc-http-security/src/test/java/io/spring/sample/graphql/WebMvcHttpSecuritySampleTests.java b/samples/webmvc-http-security/src/test/java/io/spring/sample/graphql/WebMvcHttpSecuritySampleTests.java index 8a17b5ef..2dab137e 100644 --- a/samples/webmvc-http-security/src/test/java/io/spring/sample/graphql/WebMvcHttpSecuritySampleTests.java +++ b/samples/webmvc-http-security/src/test/java/io/spring/sample/graphql/WebMvcHttpSecuritySampleTests.java @@ -75,11 +75,10 @@ class WebMvcHttpSecuritySampleTests { } @Test - void canNotMutationUpdateSalary() { - WebGraphQlTester tester = this.graphQlTester.mutate().build(); + void canNotMutateUpdateSalary() { SalaryInput salaryInput = new SalaryInput("1", BigDecimal.valueOf(44)); - tester.documentName("updateSalary") + this.graphQlTester.documentName("updateSalary") .variable("salaryInput", salaryInput) .execute() .errors()