[Issue#392] Correct issue with OAuth scopes not propogated correctly (#1982)

If a path defined security to an OAuth type, and defined scopes, the scopes from the
components definition were still being used, rather than the (most likely shorter) list
of specific scopes for the path.

This copies all the component security information over EXCEPT for the scopes. The scopes
to be included are determined by the path's security settings.

NOTE: Modified the petstore.yaml file so the GET operations only have read:pets scope and
utilized the Kotlin server sample to verify output. Sample output updated only for this scenario
This commit is contained in:
Mike Wilkes 2019-04-05 06:49:29 -04:00 committed by William Cheng
parent b5ede4b339
commit 67b3766332
6 changed files with 216 additions and 48 deletions

View File

@ -29,8 +29,7 @@ import io.swagger.v3.oas.models.info.Info;
import io.swagger.v3.oas.models.info.License;
import io.swagger.v3.oas.models.media.Schema;
import io.swagger.v3.oas.models.parameters.Parameter;
import io.swagger.v3.oas.models.security.SecurityRequirement;
import io.swagger.v3.oas.models.security.SecurityScheme;
import io.swagger.v3.oas.models.security.*;
import io.swagger.v3.oas.models.tags.Tag;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.ObjectUtils;
@ -1177,16 +1176,77 @@ public class DefaultGenerator extends AbstractGenerator implements Generator {
}
final Map<String, SecurityScheme> authMethods = new HashMap<>();
for (SecurityRequirement requirement : securities) {
for (String key : requirement.keySet()) {
for (Map.Entry<String, List<String>> entry : requirement.entrySet()) {
final String key = entry.getKey();
SecurityScheme securityScheme = securitySchemes.get(key);
if (securityScheme != null) {
if (securityScheme.getType().equals(SecurityScheme.Type.OAUTH2)) {
OAuthFlows oautUpdatedFlows = new OAuthFlows();
oautUpdatedFlows.extensions(securityScheme.getFlows().getExtensions());
SecurityScheme oauthUpdatedScheme = new SecurityScheme()
.type(securityScheme.getType())
.description(securityScheme.getDescription())
.name(securityScheme.getName())
.$ref(securityScheme.get$ref())
.in(securityScheme.getIn())
.scheme(securityScheme.getScheme())
.bearerFormat(securityScheme.getBearerFormat())
.openIdConnectUrl(securityScheme.getOpenIdConnectUrl())
.extensions(securityScheme.getExtensions())
.flows(oautUpdatedFlows);
// Ensure inserted AuthMethod only contains scopes of actual operation, and not all of them defined in the Security Component
// have to iterate through and create new SecurityScheme objects with the scopes 'fixed/updated'
final OAuthFlows securitySchemeFlows = securityScheme.getFlows();
if (securitySchemeFlows.getAuthorizationCode() != null) {
OAuthFlow updatedFlow = cloneOAuthFlow(securitySchemeFlows.getAuthorizationCode(), entry.getValue());
oautUpdatedFlows.setAuthorizationCode(updatedFlow);
}
if (securitySchemeFlows.getImplicit() != null) {
OAuthFlow updatedFlow = cloneOAuthFlow(securitySchemeFlows.getImplicit(), entry.getValue());
oautUpdatedFlows.setImplicit(updatedFlow);
}
if (securitySchemeFlows.getPassword() != null) {
OAuthFlow updatedFlow = cloneOAuthFlow(securitySchemeFlows.getPassword(), entry.getValue());
oautUpdatedFlows.setPassword(updatedFlow);
}
if (securitySchemeFlows.getClientCredentials() != null) {
OAuthFlow updatedFlow = cloneOAuthFlow(securitySchemeFlows.getClientCredentials(), entry.getValue());
oautUpdatedFlows.setClientCredentials(updatedFlow);
}
authMethods.put(key, oauthUpdatedScheme);
} else {
authMethods.put(key, securityScheme);
}
}
}
}
return authMethods;
}
private static OAuthFlow cloneOAuthFlow(OAuthFlow originFlow, List<String> operationScopes) {
Scopes newScopes = new Scopes();
for (String operationScope : operationScopes) {
newScopes.put(operationScope, originFlow.getScopes().get(operationScope));
}
return new OAuthFlow()
.authorizationUrl(originFlow.getAuthorizationUrl())
.tokenUrl(originFlow.getTokenUrl())
.refreshUrl(originFlow.getRefreshUrl())
.extensions(originFlow.getExtensions())
.scopes(newScopes);
}
private boolean hasOAuthMethods(List<CodegenSecurity> authMethods) {
for (CodegenSecurity cs : authMethods) {
if (cs.isOAuth) {

View File

@ -36,6 +36,9 @@ import java.nio.file.Files;
import java.util.*;
import java.util.stream.Collectors;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
public class JavaClientCodegenTest {
@Test
@ -102,9 +105,9 @@ public class JavaClientCodegenTest {
javaClientCodegen.postProcessOperationsWithModels(objs, Collections.emptyList());
Assert.assertEquals(Arrays.asList(pathParam1, pathParam2, queryParamRequired, queryParamOptional), codegenOperation.allParams);
Assert.assertTrue(pathParam1.hasMore);
Assert.assertTrue(pathParam2.hasMore);
Assert.assertTrue(queryParamRequired.hasMore);
assertTrue(pathParam1.hasMore);
assertTrue(pathParam2.hasMore);
assertTrue(queryParamRequired.hasMore);
Assert.assertFalse(queryParamOptional.hasMore);
}
@ -293,7 +296,7 @@ public class JavaClientCodegenTest {
String defaultApiFilename = new File(output, "src/main/java/xyz/abcdef/api/DefaultApi.java").getAbsolutePath().replace("\\", "/");
String defaultApiConent = generatedFiles.get(defaultApiFilename);
Assert.assertTrue(defaultApiConent.contains("public class DefaultApi"));
assertTrue(defaultApiConent.contains("public class DefaultApi"));
WrittenTemplateBasedFile templateBasedFile = TestUtils.getTemplateBasedFile(generator, output, "src/main/java/xyz/abcdef/api/DefaultApi.java");
Assert.assertEquals(templateBasedFile.getTemplateData().get("classname"), "DefaultApi");
@ -316,6 +319,38 @@ public class JavaClientCodegenTest {
Assert.assertEquals("Request", header.baseName);
}
@Test
public void testAuthorizationScopeValues_Issue392() {
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue392.yaml");
final DefaultGenerator defaultGenerator = new DefaultGenerator();
final ClientOptInput clientOptInput = new ClientOptInput();
clientOptInput.setOpenAPI(openAPI);
clientOptInput.setConfig(new JavaClientCodegen());
final ClientOpts clientOpts = new ClientOpts();
clientOpts.setProperties(Collections.emptyMap());
clientOptInput.setOpts(clientOpts);
defaultGenerator.opts(clientOptInput);
final List<CodegenOperation> codegenOperations = defaultGenerator.processPaths(openAPI.getPaths()).get("Pet");
// Verify GET only has 'read' scope
final CodegenOperation getCodegenOperation = codegenOperations.stream().filter(it -> it.httpMethod.equals("GET")).collect(Collectors.toList()).get(0);
assertTrue(getCodegenOperation.hasAuthMethods);
assertEquals(getCodegenOperation.authMethods.size(), 1);
final List<Map<String, Object>> getScopes = getCodegenOperation.authMethods.get(0).scopes;
assertEquals(getScopes.size(), 1, "GET scopes don't match. actual::" + getScopes);
// POST operation should have both 'read' and 'write' scope on it
final CodegenOperation postCodegenOperation = codegenOperations.stream().filter(it -> it.httpMethod.equals("POST")).collect(Collectors.toList()).get(0);
assertTrue(postCodegenOperation.hasAuthMethods);
assertEquals(postCodegenOperation.authMethods.size(), 1);
final List<Map<String, Object>> postScopes = postCodegenOperation.authMethods.get(0).scopes;
assertEquals(postScopes.size(), 2, "POST scopes don't match. actual:" + postScopes);
}
@Test
public void testFreeFormObjects() {
final OpenAPI openAPI = TestUtils.parseSpec("src/test/resources/3_0/issue796.yaml");
@ -356,7 +391,7 @@ public class JavaClientCodegenTest {
Assert.fail("Could not find '" + absoluteFilename + "' file in list:\n" +
generatedFiles.keySet().stream().sorted().collect(Collectors.joining(",\n")));
}
Assert.assertTrue(generatedFiles.containsKey(absoluteFilename), "File '" + absoluteFilename + "' was not fould in the list of generated files");
assertTrue(generatedFiles.containsKey(absoluteFilename), "File '" + absoluteFilename + "' was not fould in the list of generated files");
}
private CodegenProperty codegenPropertyWithArrayOfIntegerValues() {

View File

@ -0,0 +1,99 @@
openapi: 3.0.0
servers:
- url: 'http://petstore.swagger.io/v2'
info:
description: Used for verification of AuthorizationScope resolution issue
version: 1.0.0
title: OpenAPI Petstore
license:
name: Apache-2.0
url: 'http://www.apache.org/licenses/LICENSE-2.0.html'
paths:
/pet:
post:
tags:
- pet
summary: Add a new pet to the store
description: ''
operationId: addPet
responses:
'405':
description: Invalid input
security:
- petstore_auth:
- 'write:pets'
- 'read:pets'
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/Pet'
'/pet/{petId}':
get:
tags:
- pet
summary: Find pet by ID
description: Returns a single pet
operationId: getPetById
parameters:
- name: petId
in: path
description: ID of pet to return
required: true
schema:
type: integer
format: int64
responses:
'200':
description: successful operation
content:
application/json:
schema:
$ref: '#/components/schemas/Pet'
'400':
description: Invalid ID supplied
'404':
description: Pet not found
security:
- petstore_auth:
- 'read:pets'
components:
securitySchemes:
petstore_auth:
type: oauth2
flows:
implicit:
authorizationUrl: 'http://petstore.swagger.io/api/oauth/dialog'
scopes:
'write:pets': modify pets in your account
'read:pets': read your pets
api_key:
type: apiKey
name: api_key
in: header
schemas:
Pet:
title: a Pet
description: A pet for sale in the pet store
type: object
required:
- name
- photoUrls
properties:
id:
type: integer
format: int64
name:
type: string
example: doggie
photoUrls:
type: array
items:
type: string
status:
type: string
description: pet status in the store
enum:
- available
- pending
- sold

View File

@ -94,7 +94,6 @@ paths:
description: Invalid status value
security:
- petstore_auth:
- 'write:pets'
- 'read:pets'
/pet/findByTags:
get:
@ -141,7 +140,6 @@ paths:
description: Invalid tag value
security:
- petstore_auth:
- 'write:pets'
- 'read:pets'
deprecated: true
'/pet/{petId}':

View File

@ -66,7 +66,7 @@ class PetApiController(@Autowired(required = true) val service: PetApiService) {
notes = "Multiple status values can be provided with comma separated strings",
response = Pet::class,
responseContainer = "List",
authorizations = [Authorization(value = "petstore_auth", scopes = [AuthorizationScope(scope = "write:pets", description = "modify pets in your account"), AuthorizationScope(scope = "read:pets", description = "read your pets")])])
authorizations = [Authorization(value = "petstore_auth", scopes = [AuthorizationScope(scope = "read:pets", description = "read your pets")])])
@ApiResponses(
value = [ApiResponse(code = 200, message = "successful operation", response = Pet::class, responseContainer = "List"),ApiResponse(code = 400, message = "Invalid status value")])
@RequestMapping(
@ -83,7 +83,7 @@ class PetApiController(@Autowired(required = true) val service: PetApiService) {
notes = "Multiple tags can be provided with comma separated strings. Use tag1, tag2, tag3 for testing.",
response = Pet::class,
responseContainer = "List",
authorizations = [Authorization(value = "petstore_auth", scopes = [AuthorizationScope(scope = "write:pets", description = "modify pets in your account"), AuthorizationScope(scope = "read:pets", description = "read your pets")])])
authorizations = [Authorization(value = "petstore_auth", scopes = [AuthorizationScope(scope = "read:pets", description = "read your pets")])])
@ApiResponses(
value = [ApiResponse(code = 200, message = "successful operation", response = Pet::class, responseContainer = "List"),ApiResponse(code = 400, message = "Invalid tag value")])
@RequestMapping(

View File

@ -1,24 +0,0 @@
package org.openapitools.model
import java.util.Objects
import com.fasterxml.jackson.annotation.JsonProperty
import javax.validation.Valid
import javax.validation.constraints.*
import io.swagger.annotations.ApiModelProperty
/**
* A category for a pet
* @param id
* @param name
*/
data class Category (
@ApiModelProperty(example = "null", value = "")
@JsonProperty("id") val id: Long? = null,
@ApiModelProperty(example = "null", value = "")
@JsonProperty("name") val name: String? = null
) {
}