Re: [Architecture] [APIM] Admin REST API to check user role existence

2020-05-11 Thread Meruja Selvamanikkam
 Hi all,

Thank you for your suggestions

I have a few concerns regarding validating the subscriber permissions of
> the input application owner using the default subscriber role(Internal
> subscriber). Since the REST API and portal access are based on the
> scope-role mapping which is maintained in tenant-conf.json, a subscriber
> user does not necessarily have the *Internal/subscriber* role. If a new
> role mapping to app management, subscriptions related scopes has been
> introduced with custom roles, then the validation should be done against
> those roles as well.
>

I agree with you. For previous versions, we have a default subscriber role
in the configuration file and checked role-based permission for a
particular feature. From 3.1.0 onwards, we have a scope-role mapping.
In this case, we cannot validate the user role.

Hence, we should be validating whether any of the roles assigned to that
> particular user has bare scope based minimum access to API subscriptions,
> app management resources. So the validation should be based on the
> role-scope mapping.
>
> ie: If the user's role 'roleA' has role-scope mappings for 'apim:subscribe'
> and 'apim:app_manage' scopes, then that particular user is eligible as a
> new owner of a given application.
> WDYT?
>
Yes, we need to validate against the scope.


Thanks & Regards,
*S.Meruja* |Software Engineer | WSO2 Inc.
(m) +94779650506 | Email: mer...@wso2.com
Linkedin:   https://www.linkedin.com/in/meruja

Medium: https://medium.com/@meruja

___
Architecture mailing list
Architecture@wso2.org
https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture
___
Architecture mailing list
Architecture@wso2.org
https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture


Re: [Architecture] [APIM] Admin REST API to check user role existence

2020-05-11 Thread Frank Leymann
Dear Malintha,

my responses below

Best regards,
Frank




Am Mi., 6. Mai 2020 um 08:40 Uhr schrieb Malintha Amarasinghe <
malint...@wso2.com>:

> Dear Frank,
>
> Thank you for looking into this.
>

It is my pleasure :-)


>
> On Tue, May 5, 2020 at 1:48 PM Frank Leymann  wrote:
>
>> Dear Meruja,
>>
>> the URI of the second API (i.e.  /me/roles/{roleName}) is really
>> debatable: the intent of the */me* part of the URI seems to be to
>> identify the logged-in user, and to me, such a user is a resource.
>>
> I may be a little bit deviating from the original question.
>

Let me add to the above:  a URI, e.g. /me, always points to a single, a
unique resource. Thus, /me would point to exactly one person - we can't use
/me as an abstraction for all users implicitly picking the one who is
currently in the context (i.e. logged-in), that would counter the concept
of a URI as an identifier.

If we use a resource like /me, if we consider we do GET for that, we'll get
> the "me" resource which will return the logged in user details: eg:
> {
>"uid": "malintha"
>"fullName": "Malintha Amarasinghe"
>"mobile: "..",
>"roles" : [
>{
>   "name" : "Internal/subscriber",
>   "type": "Internal"
>}, {
>{
>   "name" : "app-manager",
>   "type": "Primary"
>}
>],
>...
> }
>
> In the above resource, I was under the impression that we consider "roles"
> as a sub-resource of /me. For example: if we did a GET for /me/roles we
> could return:
> {
> "count" : 2,
> "list": [
>{
>   "name" : "Internal/subscriber",
>   "type": "Internal"
>}, {
>{
>   "name" : "app-manager",
>   "type": "Primary"
>}
>]
> }
>
> I think /me/roles/{roleName} may not make sense here because we could just
> do /roles/{roleName} to get the role details.
>

Just using  /roles/{roleName} would return the details of the specified
role itself, without any reference to the logged-in user, i.e. the context
has to contain the user itself.


> Instead, I was under impression that we can use  /me/roles?{roleName} to
> check if a particular role is available to the logged-in user.
>

...and, yes, you are absolutely right, "role" is a query parameter, i.e.
.../roles?{roleName} is the correct syntax (I left out the /me part by will
;-)).


>
> Would the above approaches be wrong? If so, if I need to get the roles of
> a particular user, should I only use an approach like /roles?{UserID}?
>

/roles?{UserId} will work too, but it would return all roles of the
specified user.

>
>
> I.e I assume that a user is represented in APIM as a resource (but I
>> didn't check the current API), or has a unique UserID - correct?
>>
>
> Yes, that is correct. But here, if we only need to represent the logged-in
> user, would it be ok if we have a resource /me (which represents a single
> resource for /users/)?. In that case, that doesn't take
> username as an input and we don't need to validate whether the user has
> provided his username instead of providing a different one.
>

As argued above, a URI is not abstract, i.e. it can not point to different
resources (users in this case).  Even worse, if multiple users use the API
at the same time, the "/me" URI would identify (!) different users at the
same time: which shows that it's no longer an identifier.


>
>
>> Thus, the URI of the API should be something like
>> .../users/{UserID}?{roleName}  or  /roles/{roleName}?{UserID}.
>>
>
> In this case, are we adding a filter for /users/{UserID} resource?
>

Yes, the query parameter ?{UserID} would act as a filter


> Which means if we did a GET /users/malintha?roleName=subscriber, should we
> give a 404 or 204?
>

Yes, we'll return a 404 if  GET .../users/{UserID}?{roleName}  did not find
the roleName specified for the UserID.  But should more specific, returning

   - a 403 Forbidden if the user is not allowed to issue this request (e.g.
   because User_1 wants to check the role of User_2 - but I don't know whether
   this is in sope of our application semantics)
   - a 401 Unauthorized if the user has to authenticate him/herself, or
   when authentication failed. But I am not sure whether this may happen
   because of the "security" header, i.e. whether this has been verified
   before.


> Can you kindly elaborate on this?
> I am sorry about asking multiple questions :( but thought of asking all as
> this will be helpful for us when designing new resources in the future too.
>

Continue asking questions: that's how we can jointly learn and build
fantastic products!  If you want, you can send questions whenever you are
in doubt about design decision - I am happy to help  :-))


>
> Thanks!
> Malintha
>
>
>>
>> Best regards,
>> Frank
>>
>>
>
>
>>
>>
>>
>> Am Di., 5. Mai 2020 um 06:17 Uhr schrieb Meruja Selvamanikkam <
>> mer...@wso2.com>:
>>
>>> Hi All,
>>>
>>> We are planning to add a REST API endpoint to APIM 3.2.0 Admin Rest 

Re: [Architecture] [APIM] Admin REST API to check user role existence

2020-05-11 Thread Malintha Amarasinghe
Dear Frank,

Thank you very much.
Please find my inline response.

On Sat, May 9, 2020 at 2:22 PM Frank Leymann  wrote:

> Dear Malintha,
>
> my responses below
>
> Best regards,
> Frank
>
>
>
>
> Am Mi., 6. Mai 2020 um 08:40 Uhr schrieb Malintha Amarasinghe <
> malint...@wso2.com>:
>
>> Dear Frank,
>>
>> Thank you for looking into this.
>>
>
> It is my pleasure :-)
>
>
>>
>> On Tue, May 5, 2020 at 1:48 PM Frank Leymann  wrote:
>>
>>> Dear Meruja,
>>>
>>> the URI of the second API (i.e.  /me/roles/{roleName}) is really
>>> debatable: the intent of the */me* part of the URI seems to be to
>>> identify the logged-in user, and to me, such a user is a resource.
>>>
>> I may be a little bit deviating from the original question.
>>
>
> Let me add to the above:  a URI, e.g. /me, always points to a single, a
> unique resource. Thus, /me would point to exactly one person - we can't use
> /me as an abstraction for all users implicitly picking the one who is
> currently in the context (i.e. logged-in), that would counter the concept
> of a URI as an identifier.
>

I think this was copied from the SCIM spec /Me endpoint (
https://tools.ietf.org/html/rfc7644#page-66).
I did some search regarding this and I found several responses saying "*any
information that can be named can be a resource" *as per Roy Thomas
Fielding's dissertation.
It also says "A resource is a conceptual mapping to a set of entities, not
the entity that corresponds to the mapping at any particular point in time."

So according to this, can we consider that "the logged-in user from the
context" as a conceptual mapping? (I am not sure I have correctly
understood the word conceptual mapping :( ) If that is true, we can
consider that as a resource.
In that case, /me would be the unified locator for that conceptual mapping?

https://stackoverflow.com/questions/36520372/designing-uri-for-current-logged-in-user-in-rest-applications
https://www.ics.uci.edu/~fielding/pubs/dissertation/rest_arch_style.htm#sec_5_2_1_1


> If we use a resource like /me, if we consider we do GET for that, we'll
>> get the "me" resource which will return the logged in user details: eg:
>> {
>>"uid": "malintha"
>>"fullName": "Malintha Amarasinghe"
>>"mobile: "..",
>>"roles" : [
>>{
>>   "name" : "Internal/subscriber",
>>   "type": "Internal"
>>}, {
>>{
>>   "name" : "app-manager",
>>   "type": "Primary"
>>}
>>],
>>...
>> }
>>
>> In the above resource, I was under the impression that we consider
>> "roles" as a sub-resource of /me. For example: if we did a GET
>> for /me/roles we could return:
>> {
>> "count" : 2,
>> "list": [
>>{
>>   "name" : "Internal/subscriber",
>>   "type": "Internal"
>>}, {
>>{
>>   "name" : "app-manager",
>>   "type": "Primary"
>>}
>>]
>> }
>>
>> I think /me/roles/{roleName} may not make sense here because we could
>> just do /roles/{roleName} to get the role details.
>>
>
> Just using  /roles/{roleName} would return the details of the specified
> role itself, without any reference to the logged-in user, i.e. the context
> has to contain the user itself.
>
>
>> Instead, I was under impression that we can use  /me/roles?{roleName} to
>> check if a particular role is available to the logged-in user.
>>
>
> ...and, yes, you are absolutely right, "role" is a query parameter, i.e.
> .../roles?{roleName} is the correct syntax (I left out the /me part by will
> ;-)).
>
>
>>
>> Would the above approaches be wrong? If so, if I need to get the roles of
>> a particular user, should I only use an approach like /roles?{UserID}?
>>
>
> /roles?{UserId} will work too, but it would return all roles of the
> specified user.
>

Well noted above responses.


>>
>> I.e I assume that a user is represented in APIM as a resource (but I
>>> didn't check the current API), or has a unique UserID - correct?
>>>
>>
>> Yes, that is correct. But here, if we only need to represent the
>> logged-in user, would it be ok if we have a resource /me (which represents
>> a single resource for /users/)?. In that case, that doesn't
>> take username as an input and we don't need to validate whether the user
>> has provided his username instead of providing a different one.
>>
>
> As argued above, a URI is not abstract, i.e. it can not point to different
> resources (users in this case).  Even worse, if multiple users use the API
> at the same time, the "/me" URI would identify (!) different users at the
> same time: which shows that it's no longer an identifier.
>
>
>>
>>
>>> Thus, the URI of the API should be something like
>>> .../users/{UserID}?{roleName}  or  /roles/{roleName}?{UserID}.
>>>
>>
>> In this case, are we adding a filter for /users/{UserID} resource?
>>
>
> Yes, the query parameter ?{UserID} would act as a filter
>
>
>> Which means if we did a GET /users/malintha?roleName=subscriber, should
>> we give a 404 or 204?

Re: [Architecture] [APIM] Admin REST API to check user role existence

2020-05-11 Thread Sanjeewa Malalgoda
If we are to check the existence of a specific role, then using a role name
is not a good idea. Using role ID will be a better option.
And roles usually have "/" and we will need to check the impact of role
name resolving when / present in name. Have we checked that part. Reason is
/ have specific meaning in urls.

Thanks
sanjeewa.



On Mon, May 11, 2020 at 3:54 PM Thilini Shanika  wrote:

> Hi Meruja,
>
> I have a few concerns regarding validating the subscriber permissions of
> the input application owner using the default subscriber role(Internal
> subscriber). Since the REST API and portal access are based on the
> scope-role mapping which is maintained in tenant-conf.json, a subscriber
> user does not necessarily have the *Internal/subscriber* role. If a new
> role mapping to app management, subscriptions related scopes has been
> introduced with custom roles, then the validation should be done against
> those roles as well.
>
> Hence, we should be validating whether any of the roles assigned to that
> particular user has bare scope based minimum access to API subscriptions,
> app management resources. So the validation should be based on the
> role-scope mapping.
>
> ie: If the user's role 'roleA' has role-scope mappings for 'apim:subscribe'
> and 'apim:app_manage' scopes, then that particular user is eligible as a
> new owner of a given application.
> WDYT?
>
>
>
> On Wed, May 6, 2020 at 1:29 AM Thilini Shanika  wrote:
>
>>
>>
>> On Tue, May 5, 2020 at 11:36 AM Vithursa Mahendrarajah 
>> wrote:
>>
>>> Hi Meruja,
>>>
>>> The Publisher REST APIs for role validation is used to check whether
>>> the given role exists and the logged-in user has the given role. Here
>>> the role is taken from the user input, AFAIU the requirement, in this case
>>> we need to check whether the new user has the subscriber role before
>>> changing the application owner. Please correct if it is wrong.
>>>
>>> Since we need to validate whether the user has only a particular role,
>>> we do not need to have roleId in the resource path. Shall we have a
>>> resource name like /user/validate-subscriber-role. WDYT?
>>>
>>> Yes, you are correct. The requirement we are going to address through
>> the first API is to check whether the input username of the app owner has a
>> particular role(subscriber in this case). Hence, appowner name should be an
>> input to the first API. So I would suggest modifying the API resource as
>> follows.
>> /roles/{roleName}?{UserID}
>>
>>
>>
>>> Thanks,
>>> Vithursa
>>>
>>>
>>> On Tue, May 5, 2020 at 9:47 AM Meruja Selvamanikkam 
>>> wrote:
>>>
 Hi All,

 We are planning to add a REST API endpoint to APIM 3.2.0 Admin Rest
 APIs and the intention is to check the existence of a particular role name
 ( Internal/subscriber) when transferring ownership of an application to a
 user. We have similar API in the publisher to check the availability
 of the role[1].
 We have to decide the OAuth2 scope which functionalities are used by
 Admin.

 The swagger definition for the new endpoint would be as follows:

 ##
 # The Role Name Existence
 ##
   /roles/{roleName}:
 #-
 # The role name existence check resource
 #-
 head:
   security:
 - OAuth2Security:
 - apim:
   summary:
 Check given role name already exists
   description:
 Using this operation, to check whether given role already exists
   parameters:
 - $ref : '#/parameters/roleName'
   responses:
 200:
   description:
 OK.
 Requested role name is returned.
 404:
   description:
 Not Found.
 Requested role name does not exist.

 ##
 # The Role Name Existence for the logged-in user
 ##
   /me/roles/{roleName}:
 #-
 # Validate role against a user
 #-
 head:
   security:
 - OAuth2Security:
 - apim:
   summary:
 Validate whether the logged-in user has the given role
   description:
 Using this operation, logged-in user can check whether he has 
 given role.
   parameters:
 - $ref : '#/parameters/roleName'
   responses:
 200:
   description:
 OK.
 Logged-in user has the role.
 404:
   description:
 Not Found.
   

Re: [Architecture] [APIM] Admin REST API to check user role existence

2020-05-11 Thread Thilini Shanika
Hi Meruja,

I have a few concerns regarding validating the subscriber permissions of
the input application owner using the default subscriber role(Internal
subscriber). Since the REST API and portal access are based on the
scope-role mapping which is maintained in tenant-conf.json, a subscriber
user does not necessarily have the *Internal/subscriber* role. If a new
role mapping to app management, subscriptions related scopes has been
introduced with custom roles, then the validation should be done against
those roles as well.

Hence, we should be validating whether any of the roles assigned to that
particular user has bare scope based minimum access to API subscriptions,
app management resources. So the validation should be based on the
role-scope mapping.

ie: If the user's role 'roleA' has role-scope mappings for 'apim:subscribe'
and 'apim:app_manage' scopes, then that particular user is eligible as a
new owner of a given application.
WDYT?



On Wed, May 6, 2020 at 1:29 AM Thilini Shanika  wrote:

>
>
> On Tue, May 5, 2020 at 11:36 AM Vithursa Mahendrarajah 
> wrote:
>
>> Hi Meruja,
>>
>> The Publisher REST APIs for role validation is used to check whether the 
>> given
>> role exists and the logged-in user has the given role. Here the role is
>> taken from the user input, AFAIU the requirement, in this case we need to
>> check whether the new user has the subscriber role before changing the
>> application owner. Please correct if it is wrong.
>>
>> Since we need to validate whether the user has only a particular role, we
>> do not need to have roleId in the resource path. Shall we have a resource
>> name like /user/validate-subscriber-role. WDYT?
>>
>> Yes, you are correct. The requirement we are going to address through the
> first API is to check whether the input username of the app owner has a
> particular role(subscriber in this case). Hence, appowner name should be an
> input to the first API. So I would suggest modifying the API resource as
> follows.
> /roles/{roleName}?{UserID}
>
>
>
>> Thanks,
>> Vithursa
>>
>>
>> On Tue, May 5, 2020 at 9:47 AM Meruja Selvamanikkam 
>> wrote:
>>
>>> Hi All,
>>>
>>> We are planning to add a REST API endpoint to APIM 3.2.0 Admin Rest APIs
>>> and the intention is to check the existence of a particular role name (
>>> Internal/subscriber) when transferring ownership of an application to a
>>> user. We have similar API in the publisher to check the availability of
>>> the role[1].
>>> We have to decide the OAuth2 scope which functionalities are used by
>>> Admin.
>>>
>>> The swagger definition for the new endpoint would be as follows:
>>>
>>> ##
>>> # The Role Name Existence
>>> ##
>>>   /roles/{roleName}:
>>> #-
>>> # The role name existence check resource
>>> #-
>>> head:
>>>   security:
>>> - OAuth2Security:
>>> - apim:
>>>   summary:
>>> Check given role name already exists
>>>   description:
>>> Using this operation, to check whether given role already exists
>>>   parameters:
>>> - $ref : '#/parameters/roleName'
>>>   responses:
>>> 200:
>>>   description:
>>> OK.
>>> Requested role name is returned.
>>> 404:
>>>   description:
>>> Not Found.
>>> Requested role name does not exist.
>>>
>>> ##
>>> # The Role Name Existence for the logged-in user
>>> ##
>>>   /me/roles/{roleName}:
>>> #-
>>> # Validate role against a user
>>> #-
>>> head:
>>>   security:
>>> - OAuth2Security:
>>> - apim:
>>>   summary:
>>> Validate whether the logged-in user has the given role
>>>   description:
>>> Using this operation, logged-in user can check whether he has given 
>>> role.
>>>   parameters:
>>> - $ref : '#/parameters/roleName'
>>>   responses:
>>> 200:
>>>   description:
>>> OK.
>>> Logged-in user has the role.
>>> 404:
>>>   description:
>>> Not Found.
>>> Logged-in user does not have the role.
>>>
>>> Appreciate any feedback on this and correct me if I am wrong.
>>>
>>> [1] - [APIM-3.0] Publisher rest API to check a role name existence
>>>
>>> Thanks & Regards,
>>> *S.Meruja* |Software Engineer | WSO2 Inc.
>>> (m) +94779650506 | Email: mer...@wso2.com
>>> Linkedin:   https://www.linkedin.com/in/meruja
>>> 
>>> Medium: https://medium.com/@meruja
>>> 
>>> 

Re: [Architecture] Support importing OpenAPI definitions with external references

2020-05-11 Thread Janakan Sarangan
Hi  Thilini !

As per the current implementation, even the url based references are not
> getting resolved by the OAS Parser. We will need to address that
> requirement and support url based ref support in the default flow of
> swagger import as well.
>

ACK ! I will do the implementation for that.

We will have to read the content of all the files available within the
> archive. Hence, try to come up with an optimized way of parsing these
> contents to identifying the parent
>

ACK


On Mon, May 11, 2020 at 1:49 PM Thilini Shanika  wrote:

> Hi Sarangan,
>
> As per the current implementation, even the url based references are not
> getting resolved by the OAS Parser. We will need to address that
> requirement and support url based ref support in the default flow of
> swagger import as well.
>
> For example, if an imported API definition(file/url) contains url based
> external references, we should resolve them before persisting it in the
> registry. In order to achieve this, *setResolve=true* option can be
> passed to OAS parser when parsing the content.
>
> Following is a sample code snippet you can refer to when modifying the
> default import/validation flow[1](apart from the archive-based approach you
> are implementing).
>
> OpenAPIV3Parser openAPIV3Parser = new OpenAPIV3Parser();
> ParseOptions options = new ParseOptions();
> options.setResolve(true);
> SwaggerParseResult parseAttemptForV3 = 
> openAPIV3Parser.readContents(apiDefinition, null, options);
>
>
> [1]
> https://github.com/wso2/carbon-apimgt/blob/master/components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/definitions/OAS3Parser.java#L563
>
>
> On Mon, May 11, 2020 at 1:09 PM Janakan Sarangan 
> wrote:
>
>> Hi !
>>
>> + @architecture@wso2.org
>>
>> What is the current behavior of the OAS parser with external references?
>>> Does it throw validation exceptions or just ignore them?
>>>
>>
>> @Rukshan PremathungaThere's no exceptions in
>> carbon logs , but it will give a " *OpenAPI content validation failed*"
>> error message in the UI.
>>
>
>> Maybe we can identify the correct swagger by checking its swagger or
>>> openapi tag. There is a function already to check that. It may help in
>>> CI/CD flow if we didn't enforce naming conventions.
>>>
>>
>> Okay !  Then we will move from naming convention to validate through
>> openapi and swagger keywords.
>>
> +1. But in this approach, we will have to read the content of all the
> files available within the archive. Hence, try to come up with an optimized
> way of parsing these contents to identifying the parent file.
>
>>
>> Do we only store the flattened API definition or do we store both the
>>> parameterized definition and flattened definition? If we only store the
>>> flattened definition, how will we support editing of the references?
>>>
>>
>> @Ruwini Wijesiri   When we are saving the Api
>> definition we are only saving the flattened definition. That means Remote
>> References will be converted to local references .
>>
>> If we have a remote reference as below(Referencing Order element in
>> ref.yaml file),
>>
>>
>> *schema:$ref: "ref.yaml#/definitions/Order"*
>>
>> Then, the final swagger definition after resolving remote references will
>> be as below ,
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> *schema:$ref: '#/components/schemas/Order' components:  schemas:
>> Order:  properties:customerName:  type: string
>> delivered:  type: boolean*
>>
>>
>> Thus  we can edit the references without any issues . This is the same as
>> editing local and URL references .
>>
>> On Mon, May 11, 2020 at 12:27 PM Thilini Shanika 
>> wrote:
>>
>>> Hi Sarangan
>>>
>>> This has to be a public mail :). As a practice, please make sure to
>>> initiate technical discussions in public mail threads.
>>>
>>> On Mon, May 11, 2020 at 10:40 AM Ruwini Wijesiri 
>>> wrote:
>>>
 Hi janakan,


>- The extracted archive is processed and the open API definition
>is flattened by resolving the external references via the corresponding
>version of the OpenAPI Parsers.
>- The flattened openAPI definition will be saved as the definition
>of the API.
>
> Do we only store the flattened API definition or do we store both the
 parameterized definition and flattened definition? If we only store the
 flattened definition, how will we support editing of the references?

 What is the current behavior of the OAS parser with external
> references? Does it throw validation exceptions or just ignore them?

 @Rukshan Premathunga  i think it ignores it, doesn't
 throw a validation exception.

 Regards,
 Ruwini

 On Mon, May 11, 2020 at 10:34 AM Rukshan Premathunga 
 wrote:

> Hi Janakan,
>
>
> On Mon, May 11, 2020 at 10:23 AM Janakan Sarangan 
> wrote:
>
>> When importing an existing API with file or url based
>> external 

Re: [Architecture] Support importing OpenAPI definitions with external references

2020-05-11 Thread Thilini Shanika
Hi Sarangan,

As per the current implementation, even the url based references are not
getting resolved by the OAS Parser. We will need to address that
requirement and support url based ref support in the default flow of
swagger import as well.

For example, if an imported API definition(file/url) contains url based
external references, we should resolve them before persisting it in the
registry. In order to achieve this, *setResolve=true* option can be passed
to OAS parser when parsing the content.

Following is a sample code snippet you can refer to when modifying the
default import/validation flow[1](apart from the archive-based approach you
are implementing).

OpenAPIV3Parser openAPIV3Parser = new OpenAPIV3Parser();
ParseOptions options = new ParseOptions();
options.setResolve(true);
SwaggerParseResult parseAttemptForV3 =
openAPIV3Parser.readContents(apiDefinition, null, options);


[1]
https://github.com/wso2/carbon-apimgt/blob/master/components/apimgt/org.wso2.carbon.apimgt.impl/src/main/java/org/wso2/carbon/apimgt/impl/definitions/OAS3Parser.java#L563


On Mon, May 11, 2020 at 1:09 PM Janakan Sarangan  wrote:

> Hi !
>
> + @architecture@wso2.org
>
> What is the current behavior of the OAS parser with external references?
>> Does it throw validation exceptions or just ignore them?
>>
>
> @Rukshan PremathungaThere's no exceptions in carbon
> logs , but it will give a " *OpenAPI content validation failed*"  error
> message in the UI.
>

> Maybe we can identify the correct swagger by checking its swagger or
>> openapi tag. There is a function already to check that. It may help in
>> CI/CD flow if we didn't enforce naming conventions.
>>
>
> Okay !  Then we will move from naming convention to validate through
> openapi and swagger keywords.
>
+1. But in this approach, we will have to read the content of all the files
available within the archive. Hence, try to come up with an optimized way
of parsing these contents to identifying the parent file.

>
> Do we only store the flattened API definition or do we store both the
>> parameterized definition and flattened definition? If we only store the
>> flattened definition, how will we support editing of the references?
>>
>
> @Ruwini Wijesiri   When we are saving the Api definition
> we are only saving the flattened definition. That means Remote References
> will be converted to local references .
>
> If we have a remote reference as below(Referencing Order element in
> ref.yaml file),
>
>
> *schema:$ref: "ref.yaml#/definitions/Order"*
>
> Then, the final swagger definition after resolving remote references will
> be as below ,
>
>
>
>
>
>
>
>
>
>
>
> *schema:$ref: '#/components/schemas/Order' components:  schemas:
> Order:  properties:customerName:  type: string
> delivered:  type: boolean*
>
>
> Thus  we can edit the references without any issues . This is the same as
> editing local and URL references .
>
> On Mon, May 11, 2020 at 12:27 PM Thilini Shanika 
> wrote:
>
>> Hi Sarangan
>>
>> This has to be a public mail :). As a practice, please make sure to
>> initiate technical discussions in public mail threads.
>>
>> On Mon, May 11, 2020 at 10:40 AM Ruwini Wijesiri  wrote:
>>
>>> Hi janakan,
>>>
>>>
- The extracted archive is processed and the open API definition is
flattened by resolving the external references via the corresponding
version of the OpenAPI Parsers.
- The flattened openAPI definition will be saved as the definition
of the API.

 Do we only store the flattened API definition or do we store both the
>>> parameterized definition and flattened definition? If we only store the
>>> flattened definition, how will we support editing of the references?
>>>
>>> What is the current behavior of the OAS parser with external references?
 Does it throw validation exceptions or just ignore them?
>>>
>>> @Rukshan Premathunga  i think it ignores it, doesn't
>>> throw a validation exception.
>>>
>>> Regards,
>>> Ruwini
>>>
>>> On Mon, May 11, 2020 at 10:34 AM Rukshan Premathunga 
>>> wrote:
>>>
 Hi Janakan,


 On Mon, May 11, 2020 at 10:23 AM Janakan Sarangan 
 wrote:

> When importing an existing API with file or url based
> external references , these references do not get resolved .We are
> now supporting  local reference and URL reference , but remote reference
> also needs to be validated and handled in API definition [1]
>
> For file based external references [2],
>
>- If there are external ref files for a given open API definition,
>those files have to be uploaded along with the master definition as an
>archive.
>- The archive will be copied to a temp location
>- The extracted archive is processed and the open API definition
>is flattened by resolving the external references via the corresponding
>version of the OpenAPI Parsers.
>- The flattened 

Re: [Architecture] Support importing OpenAPI definitions with external references

2020-05-11 Thread Janakan Sarangan
Hi !

+ @architecture@wso2.org

What is the current behavior of the OAS parser with external references?
> Does it throw validation exceptions or just ignore them?
>

@Rukshan PremathungaThere's no exceptions in carbon
logs , but it will give a " *OpenAPI content validation failed*"  error
message in the UI.

Maybe we can identify the correct swagger by checking its swagger or
> openapi tag. There is a function already to check that. It may help in
> CI/CD flow if we didn't enforce naming conventions.
>

Okay !  Then we will move from naming convention to validate through
openapi and swagger keywords.

Do we only store the flattened API definition or do we store both the
> parameterized definition and flattened definition? If we only store the
> flattened definition, how will we support editing of the references?
>

@Ruwini Wijesiri   When we are saving the Api definition
we are only saving the flattened definition. That means Remote References
will be converted to local references .

If we have a remote reference as below(Referencing Order element in
ref.yaml file),


*schema:$ref: "ref.yaml#/definitions/Order"*

Then, the final swagger definition after resolving remote references will
be as below ,











*schema:$ref: '#/components/schemas/Order' components:  schemas:
Order:  properties:customerName:  type: string
delivered:  type: boolean*


Thus  we can edit the references without any issues . This is the same as
editing local and URL references .

On Mon, May 11, 2020 at 12:27 PM Thilini Shanika  wrote:

> Hi Sarangan
>
> This has to be a public mail :). As a practice, please make sure to
> initiate technical discussions in public mail threads.
>
> On Mon, May 11, 2020 at 10:40 AM Ruwini Wijesiri  wrote:
>
>> Hi janakan,
>>
>>
>>>- The extracted archive is processed and the open API definition is
>>>flattened by resolving the external references via the corresponding
>>>version of the OpenAPI Parsers.
>>>- The flattened openAPI definition will be saved as the definition
>>>of the API.
>>>
>>> Do we only store the flattened API definition or do we store both the
>> parameterized definition and flattened definition? If we only store the
>> flattened definition, how will we support editing of the references?
>>
>> What is the current behavior of the OAS parser with external references?
>>> Does it throw validation exceptions or just ignore them?
>>
>> @Rukshan Premathunga  i think it ignores it, doesn't
>> throw a validation exception.
>>
>> Regards,
>> Ruwini
>>
>> On Mon, May 11, 2020 at 10:34 AM Rukshan Premathunga 
>> wrote:
>>
>>> Hi Janakan,
>>>
>>>
>>> On Mon, May 11, 2020 at 10:23 AM Janakan Sarangan 
>>> wrote:
>>>
 When importing an existing API with file or url based
 external references , these references do not get resolved .We are now
 supporting  local reference and URL reference , but remote reference also
 needs to be validated and handled in API definition [1]

 For file based external references [2],

- If there are external ref files for a given open API definition,
those files have to be uploaded along with the master definition as an
archive.
- The archive will be copied to a temp location
- The extracted archive is processed and the open API definition is
flattened by resolving the external references via the corresponding
version of the OpenAPI Parsers.
- The flattened openAPI definition will be saved as the definition
of the API.
- We need to identify the master swagger file using a  standard
naming convention or by using swagger validations .(Currently we are 
 hoping
to implement by enforcing some naming convention.*The master
swagger file needs to be named as main.yaml*)

 Maybe we can identify the correct swagger by checking its swagger or
>>> openapi tag. There is a function already to check that. It may help in
>>> CI/CD flow if we didn't enforce naming convention.
>>>

-

 Your thoughts are highly appreciated in this proposed solution !

>>> What is the current behavior of the OAS parser with external references?
>>> Does it throw validation exceptions or just ignore them?
>>>

 [1] https://github.com/wso2/product-apim/issues/4464
 [2] https://swagger.io/docs/specification/using-ref/


 --
 Thanks and Regards,
 *Sarangan*
 *Software Engineer | APIM |** WSO2 Inc*

 *M : 0767882078 | E: saran...@wso2.com *


 *[image: http://wso2.com/signature] *

>>>
>>>
>>> --
>>> Rukshan C. Premathunga | Associate Technical Lead | WSO2 Inc.
>>> (m) +94711822074 | (w) +94112145345 | Email: ruks...@wso2.com
>>> GET INTEGRATION AGILE
>>> Integration Agility for Digitally Driven Business
>>>
>>
>>
>> --
>> Ruwini Wijesiri
>> Senior Software Engineer,
>> WSO2 Inc.
>>
>> Mobile :