Skip to content
Permalink
Browse files

[Core, Rust Server, ASP.NET Core] Fix Codegen Operation Scope Consist…

…ency (#3495)

Fix Codegen Operation Scope Consistency

- Filter scopes based on operation

- Partially revert #1984 to not rely on custom attributes as to whether scopes exist

- Fix filtering global authentication schemes
  • Loading branch information...
richardwhiuk committed Nov 8, 2019
1 parent 5b44418 commit de162f7f34e6ac2bf91b289fd7c6ae9f82e03f4b
@@ -17,6 +17,7 @@

package org.openapitools.codegen;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
@@ -38,7 +39,7 @@
// Oauth specific
public String flow, authorizationUrl, tokenUrl;
public List<Map<String, Object>> scopes;
public Boolean isCode, isPassword, isApplication, isImplicit, hasScopes;
public Boolean isCode, isPassword, isApplication, isImplicit;

@Override
public String toString() {
@@ -100,4 +101,47 @@ public int hashCode() {
isImplicit,
scopes);
}

// Return a copy of the security object, filtering out any scopes from the passed-in list.
public CodegenSecurity filterByScopeNames(List<String> filterScopes) {
CodegenSecurity filteredSecurity = new CodegenSecurity();
// Copy all fields except the scopes.
filteredSecurity.name = name;
filteredSecurity.type = type;
filteredSecurity.hasMore = false;
filteredSecurity.isBasic = isBasic;
filteredSecurity.isBasicBasic = isBasicBasic;
filteredSecurity.isBasicBearer = isBasicBearer;
filteredSecurity.isApiKey = isApiKey;
filteredSecurity.isOAuth = isOAuth;
filteredSecurity.keyParamName = keyParamName;
filteredSecurity.isCode = isCode;
filteredSecurity.isImplicit = isImplicit;
filteredSecurity.isApplication = isApplication;
filteredSecurity.isPassword = isPassword;
filteredSecurity.isKeyInCookie = isKeyInCookie;
filteredSecurity.isKeyInHeader = isKeyInHeader;
filteredSecurity.isKeyInQuery = isKeyInQuery;
filteredSecurity.flow = flow;
filteredSecurity.tokenUrl = tokenUrl;
filteredSecurity.authorizationUrl = authorizationUrl;
// It is not possible to deep copy the extensions, as we have no idea what types they are.
// So the filtered method *will* refer to the original extensions, if any.
filteredSecurity.vendorExtensions = new HashMap<String, Object>(vendorExtensions);
List<Map<String, Object>> returnedScopes = new ArrayList<Map<String, Object>>();
Map<String, Object> lastScope = null;
for (String filterScopeName : filterScopes) {
for (Map<String, Object> scope : scopes) {
String name = (String) scope.get("scope");
if (filterScopeName.equals(name)) {
returnedScopes.add(scope);
lastScope = scope;
break;
}
}
}
filteredSecurity.scopes = returnedScopes;

return filteredSecurity;
}
}
@@ -1057,55 +1057,21 @@ private void processOperation(String resourcePath, String httpMethod, Operation
}

Map<String, SecurityScheme> authMethods = getAuthMethods(securities, securitySchemes);
if (authMethods == null || authMethods.isEmpty()) {
authMethods = getAuthMethods(globalSecurities, securitySchemes);
}

if (authMethods != null && !authMethods.isEmpty()) {
codegenOperation.authMethods = config.fromSecurity(authMethods);
List<Map<String, Object>> scopes = new ArrayList<Map<String, Object>>();
if (codegenOperation.authMethods != null) {
for (CodegenSecurity security : codegenOperation.authMethods) {
if (security != null && security.isBasicBearer != null && security.isBasicBearer &&
securities != null) {
for (SecurityRequirement req : securities) {
if (req == null) continue;
for (String key : req.keySet()) {
if (security.name != null && key.equals(security.name)) {
int count = 0;
for (String sc : req.get(key)) {
Map<String, Object> scope = new HashMap<String, Object>();
scope.put("scope", sc);
scope.put("description", "");
count++;
if (req.get(key) != null && count < req.get(key).size()) {
scope.put("hasMore", "true");
} else {
scope.put("hasMore", null);
}
scopes.add(scope);
}
//end this inner for
break;
}
}

}
security.hasScopes = scopes.size() > 0;
security.scopes = scopes;
}
}
}
List<CodegenSecurity> fullAuthMethods = config.fromSecurity(authMethods);
codegenOperation.authMethods = filterAuthMethods(fullAuthMethods, securities);
codegenOperation.hasAuthMethods = true;
}
} else {
authMethods = getAuthMethods(globalSecurities, securitySchemes);

/* TODO need to revise the logic below
Map<String, SecurityScheme> securitySchemeMap = openAPI.getComponents().getSecuritySchemes();
if (securitySchemeMap != null && !securitySchemeMap.isEmpty()) {
codegenOperation.authMethods = config.fromSecurity(securitySchemeMap);
codegenOperation.hasAuthMethods = true;
if (authMethods != null && !authMethods.isEmpty()) {
List<CodegenSecurity> fullAuthMethods = config.fromSecurity(authMethods);
codegenOperation.authMethods = filterAuthMethods(fullAuthMethods, globalSecurities);
codegenOperation.hasAuthMethods = true;
}
}
*/

} catch (Exception ex) {
String msg = "Could not process operation:\n" //
+ " Tag: " + tag + "\n"//
@@ -1311,6 +1277,40 @@ private static OAuthFlow cloneOAuthFlow(OAuthFlow originFlow, List<String> opera
.scopes(newScopes);
}

private List<CodegenSecurity> filterAuthMethods(List<CodegenSecurity> authMethods, List<SecurityRequirement> securities) {
if (securities == null || securities.isEmpty() || authMethods == null) {
return authMethods;
}

List<CodegenSecurity> result = new ArrayList<CodegenSecurity>();

for (CodegenSecurity security : authMethods) {
boolean filtered = false;
if (security != null && security.scopes != null) {
for (SecurityRequirement requirement : securities) {
List<String> opScopes = requirement.get(security.name);
if (opScopes != null) {
// We have operation-level scopes for this method, so filter the auth method to
// describe the operation auth method with only the scopes that it requires.
// We have to create a new auth method instance because the original object must
// not be modified.
CodegenSecurity opSecurity = security.filterByScopeNames(opScopes);
result.add(opSecurity);
filtered = true;
break;
}
}
}

// If we didn't get a filtered version, then we can keep the original auth method.
if (!filtered) {
result.add(security);
}
}

return result;
}

private boolean hasOAuthMethods(List<CodegenSecurity> authMethods) {
for (CodegenSecurity cs : authMethods) {
if (Boolean.TRUE.equals(cs.isOAuth)) {
@@ -33,9 +33,15 @@ namespace {{apiPackage}}
/// <param name="{{paramName}}">{{description}}</param>{{/allParams}}{{#responses}}
/// <response code="{{code}}">{{message}}</response>{{/responses}}
[{{httpMethod}}]
[Route("{{{basePathWithoutHost}}}{{{path}}}")]{{#hasAuthMethods}}{{#authMethods}}{{#isApiKey}}
[Authorize(Policy = "{{name}}")]{{/isApiKey}}{{#isBasicBearer}}
[Authorize{{#hasScopes}}(Roles = "{{#scopes}}{{scope}}{{#hasMore}},{{/hasMore}}{{/scopes}}"){{/hasScopes}}]{{/isBasicBearer}}{{/authMethods}}{{/hasAuthMethods}}
[Route("{{{basePathWithoutHost}}}{{{path}}}")]
{{#authMethods}}
{{#isApiKey}}
[Authorize(Policy = "{{name}}")]
{{/isApiKey}}
{{#isBasicBearer}}
[Authorize{{#scopes}}{{#-first}}(Roles = "{{/-first}}{{scope}}{{^-last}},{{/-last}}{{#-last}}"){{/-last}}{{/scopes}}]
{{/isBasicBearer}}
{{/authMethods}}
[ValidateModelState]{{#useSwashbuckle}}
[SwaggerOperation("{{operationId}}")]{{#responses}}{{#dataType}}
[SwaggerResponse(statusCode: {{code}}, type: typeof({{&dataType}}), description: "{{message}}")]{{/dataType}}{{^dataType}}{{/dataType}}{{/responses}}{{/useSwashbuckle}}{{^useSwashbuckle}}{{#responses}}{{#dataType}}
@@ -101,6 +101,20 @@ paths:
responses:
'200':
description: 'OK'
/readonly_auth_scheme:
get:
security:
- authScheme: ["test.read"]
responses:
200:
description: Check that limiting to a single required auth scheme works
/multiple_auth_scheme:
get:
security:
- authScheme: ["test.read", "test.write"]
responses:
200:
description: Check that limiting to multiple required auth schemes works
/responses_with_headers:
get:
responses:
@@ -123,7 +137,18 @@ paths:
Failure-Info:
schema:
type: String

components:
securitySchemes:
authScheme:
type: oauth2
flows:
authorizationCode:
authorizationUrl: 'http://example.org'
tokenUrl: 'http://example.org'
scopes:
test.read: Allowed to read state.
test.write: Allowed to change state.
schemas:
UuidObject:
description: Test a model containing a UUID
@@ -1 +1 @@
4.1.1-SNAPSHOT
4.1.3-SNAPSHOT

0 comments on commit de162f7

Please sign in to comment.
You can’t perform that action at this time.