swagger-node-runner icon indicating copy to clipboard operation
swagger-node-runner copied to clipboard

security handlers

Open osher opened this issue 9 years ago • 6 comments

EDITED: clarifications added here and there...

I was trying to implement security handlers.

I found the example in the test:

swagger:
    bagpipes:
        _swagger_security:
           name: swagger_security
           securityHandlersModule: api/helpers/securityHandlers

Ok. that's a start. Imature for my flavor - but still a start. 2 problems are:

  1. What if I want to initiate plummings on server start-up, and fail fast unless I have all I need? In the current setup, the server may start accept requests while async initiation is still on progress. If the initiation is incomplete - I don't want the server to even join the load-balancer. If there is a problem - I want it to fail fast. I want to leave the developer the decision to lazy-load his dependency, but also to let him pull the breaks.
  2. What if I want to wrap handler of each method in it's own package, and let my developers cherry-pick them to their micro-services?

So I thought to create a PR that will accept multiple modules and consolidate them together to the securityHandlers collection.

Something Like that:

swagger:
    bagpipes:
        _swagger_security:
            name: swagger_security
            securityHandlers: 
                basic: openapi-seciruty-myorg-basic
                oauth: openapi-seciruty-myorg-oauth
                req-ip: openapi-seciruty-myorg-req-ip

Keys are types matching api.securityDefinitions like the openapi spec demonstrates. About values - I thought to let them be

  • require module names, Expecting each module to export a factory(config, cb) that yields a handler(req,res,cb) or handler(req,def,keyOrScope,cb), pre-loaded once on server-startup. If he wants to yield the handlers right away and implement deferring internally - it's his decision. He can also not yield until all peers are connected. In this case, the configs of these modules should be found on config, as siblings to swagger, or as siblings to bagpipes, I have not decided yet.
  • pipe-names, that expect (fittingDef,bagpipes) like any pipe, and return a fitting. The pipe will reject a context by nexting an Error with all rejection-info loaded on it, and accept a context by nexting a null. I assume the error will end up with the error_json_handler. The pipe may enrich the context with more data like middlewares often enrich request objects with user data or roles collection.

These are were the early thoughts.

So I thought to have a look inside and see where it takes me. I opened fitting/swagger_security, which calls yet again for double-checking with you...

It seems like it wants to support:

swagger:
    bagpipes:
        ...
    securityHandlers: 
       <but what's here? I have no idea> 

After reading the code it looks to me that if config.swagger.securityHandlers is present - no security handlers are loaded...

  1. What am I missing? really... 😲
  2. What are your thoughts about the ideas above?

osher avatar Nov 08 '16 14:11 osher

P.S. The current situation will force me to handle the cherry-picking and consolidating to a single module in code of the securityHandlersModule, and pull the breaks abruptly with log.fatal(..) && process.exit(1) - which isn't nice 😢

We'll soon see how I progress with that

osher avatar Nov 08 '16 14:11 osher

I'll have to look into it when I get time. Hopefully it's not as dire as you expect.

Regarding security handler configs, have you had a look at this? https://github.com/swagger-api/swagger-node/issues/228

It covers a lot, so maybe that could help answer some of your questions?

theganyo avatar Nov 08 '16 16:11 theganyo

Well, that helped a bit. After reading that - I found that runner.create(options) can be injected with security hanlders:

  • the user may inject a ready-to-work module to the runner.create(options) as options.securityHandlers or using the legacy name options.swaggerSecurityHandlers.

That's great.

I also believe I figured the OR and AND logic between elements in operation.security section: The following section will accept basic auth only from the given IP ranges, OR requests with valid api-key in header OR in qs param.

  security:
     - ipRange: [ "10.138.0.*" ] 
       basicAuth: true
     - apiKeyHeader: []
     - apiKeyQS: []

So that's good to put that explicitly too 😉 (pls correct me if I'm wrong about it 😮).

That's also great. .

But that still doesn't help to clarify what the following line means:

if (fittingDef.securityHandlersModule && !runner.config.securityHandlers) {

If it was

if (fittingDef.securityHandlersModule && !runner.securityHandlers) {

Then it would make sense to me like:

  • the module injected in options.securityHandlers will cascade whatever securityHandlersModule is provided in config (plus, you can initiate it in your own time, and create the runner whenever you're ready, in the other way - it has to load synchronously and be ready to work).

.

Also I cannot understand whats runner.config.securityHandlers - i.e where it comes from, and if it's provided what values should be in it.

Reverse engineering the way it's put now - then I understand it as:

  • fittingDef.secutityHandlersModule will cascade the injected options.securityHandlers, unless config contains a root-level key securityHandlers with a truthful value. ...really? :face_with_head_bandage: sounds too sophisticated to me... too many moving parts in this version, and that makes me reluctant to believe it.

The fact that there are no specs/tests for this fitting also makes it harder to guess the original intent... 😿

osher avatar Nov 08 '16 18:11 osher

Sorry for the nit-picking, I'm MUCH more interested about your thoughts regarding cherry-picking authentication handlers in configuration. Now, even if the implementations come from reusable cherry-picked packages - I still have to compose them together and I have to do that in code... 😛

osher avatar Nov 09 '16 09:11 osher

Mmm. Looks like my security example is not correct. I see that operation.security.{key} may only have Array<string> as value, although the array may be empty.

Plus - the vaildator in swagger-ui will not accept custom types for custom handlers (like my ipRange checker). I now see it permits only basic, apiKey or oauth2, so I cannot implement custom handlers (for application-level ip checks, for example).

This leaves me puzzled regarding why we need such a complex AND/OR logic, but that's another story...

Perhaps I can cheat the system, chose one of the supported types, but implement whatever checks in my securityHandlersModule. meh. 😛

osher avatar Nov 09 '16 12:11 osher

I think I got a setup, but it feels scattered and abusive. Anyway:

in swagger.yaml

paths:
  <path>:
    <operation>:
      security:
        - basic: []
          internalNetwork: []
        - my-oauth2: ["user-read"]

securityDefinitions:
  basic:
    type: basic
  internalNetwork:
    type: basic
  my-oauth2:
     type: oauth2
     flow: implicit
     tokenUrl: "http://foo.bar/token"
     authorizationUrl: "http://foo.bar/auth"
     scopes: 
       user-read: "read permissions"

securityHandlersModule:

const allowedIpRanges = getAllowedIpRanges()
module.exports =  {
  internalNetwork: function(req,res,next) {
      const ip = ipOf(req);
      if ( isIpInRange( ip, allowedIpRanges ) ) return next();
      const e = new Error('Request IP is not in permitted range');
      e.status = 401;
      e.ip = ip;
      next(e);
  },
  basic: function(req,res,next) { ... },
  'my-oauth2': function( req, secDef, scopes, next) { ... }
}
function getAllowedIpRanges() { ... } //pull from some other config :P
function ipOf(req) { ... } //pull from XFF http header or remoteAddress
function isIpInRange(ip, ranges) { ... } //returns true | false

osher avatar Nov 09 '16 13:11 osher