mulesoft-labs / raml-generator

Generate files from a RAML document and Handlebars templates
Other
31 stars 11 forks source link

Support secureBy in method #10

Closed WenTingZhu closed 9 years ago

WenTingZhu commented 9 years ago

Hi, secureBy has been marked as a todo item in method, so just wondering if and when this will be added. Thank you.

https://github.com/mulesoft-labs/raml-generator/blob/afee0d85ad20deed4a10b834940a109a0b65f9d8/lib/context/methods.js#L28

blakeembrey commented 9 years ago

@WenTingZhu I don't think there's anything wrong with adding it right now, I just wasn't using it so didn't put any thought on how it should be sanitized. I think it should be an object merged with the securityScheme, but if you have a specific thought let me know :smile:

fjsousa commented 9 years ago

@blakeembrey It can also be handy to include an extra field in the queryParameters object. For instance, I needed to add an 'access_token' field to my queryParameters every time a resource was secured, so I just did this:

https://github.com/fjsousa/raml-generator/blob/master/lib/context/methods.js#L32

Now it's a bit hardcoded and I need to get the parameter name from the securitySchemes.

blakeembrey commented 9 years ago

@fjsousa Are you saying you'd want the describedBy from RAML security scheme to be merged for you? That can definitely be done for you. I think for the most part securedBy should be the merged object from securitySchemes. I'll look at implementing this today (it slid off my list at some point) and see if it works for you.

blakeembrey commented 9 years ago

Released with https://github.com/mulesoft-labs/raml-generator/releases/tag/v0.1.4

blakeembrey commented 9 years ago

It might not be complete with merging the root securedBy actually, so feedback on this will help.

fjsousa commented 9 years ago

Thanks for the release @blakeembrey. Initially we were including securedBy in queryParameters like this, just to get the job done: https://github.com/fjsousa/raml-generator/blob/master/lib/context/methods.js#L30

What you did looks great but I don't understand what you mean by "merging the root securedBy", what are the other options?

blakeembrey commented 9 years ago

@fjsousa The RAML parser is sanitizing this for us, so it shouldn't be an issue, but I was originally thinking about:

#%RAML 0.8

title: test

securitySchemes:
  - foo:
      type: Basic Authentication
  - bar:
      type: Basic Authentication

securedBy: [foo]

/:
  get:
    # Inherits root `securedBy`.
  post:
    securedBy: [bar] # Overrides root `securedBy`.

This is the confirmed output from the RAML parser though, so it's a non-issue and what is written should work perfectly:

{"title":"test","securitySchemes":[{"foo":{"type":"Basic Authentication"}},{"bar":{"type":"Basic Authentication"}}],"securedBy":["foo"],"resources":[{"relativeUri":"/","methods":[{"method":"get","securedBy":["foo"]},{"securedBy":["bar"],"method":"post"}],"relativeUriPathSegments":[]}]}
fjsousa commented 9 years ago

if the parser is already sanitizing, then great