tsoa icon indicating copy to clipboard operation
tsoa copied to clipboard

Feature request: For Hapi, generate and use authentication schemes

Open icopp opened this issue 5 years ago • 2 comments

Sorting

  • I'm submitting a ...

    • [ ] bug report
    • [x] feature request
    • [ ] support request
  • I confirm that I

    • [x] used the search to make sure that a similar issue hasn't already been submit

Expected Behavior

To play nice with other tooling in the Hapi ecosystem, generated Hapi routes should use native Hapi authentication schemes.

Current Behavior

Generated Hapi routes use pre-route hooks that don't leverage the native Hapi authentication functionality.

Possible Solution

Generated code could look very roughly something like this.

The generated auth scheme would only need to be registered once and then could be referenced multiple times, allowing for other tooling to display auth names for each route matching the OpenAPI generated docs.

server.auth.scheme('tsoa', function (server, options) {
  return {
    authenticate: function (request, reply) {
      // ... code wrapping hapiAuthentication goes here

      // error state

      return reply(Boom.unauthorized())

      // valid state

      return reply(null, { 
        // any user credentials info to use elsewhere
        scope: ['admin'] // user's scopes to compare against routes
      })
    },
  }
})

server.auth.strategy('api_key', 'tsoa')
server.auth.strategy('jwt', 'tsoa')

The generated routes would be slightly simpler and would have scope info from @Security embedded in them, again allowing more functionality with other tooling that looks at route metadata.

server.route({
  method: 'GET',
  path: '/',
  options: {
      auth: {
        strategy: 'api_key'
      }
  }
});

server.route({
  method: 'GET',
  path: '/admins',
  options: {
      auth: {
        strategy: 'jwt',
        access: {
          scope: ['admin', 'superuser']
        }
      }
  }
});

Context (Environment)

Version of the library: 3.6.1 Version of NodeJS: 14

  • Confirm you were using yarn not npm: [x]

Breaking change?

This would be a breaking change in two ways:

  • The resolved value returned from hapiAuthentication would be attached to request.auth.credentials (for example, return { userId: 1234 } and you can access it in a route with request.auth.credentials.userId), rather than creating a request.user property.
  • Leveraging native Hapi scope functionality would require returning the user's available scopes from successful hapiAuthentication responses (rather than the function only returning success/failure).

icopp avatar Apr 08 '21 18:04 icopp

I'd have to see the code, but I think it's unlikely this won't be a pain to integrate (esp. bw-compatible) and I don't really see any benefits (based on your feature request). Can you elaborate on why this change would benefit tsoa users?

WoH avatar Apr 20 '21 16:04 WoH

@WoH The main benefit, as far as I understand, would be less "surprises": when I started using tsoa, I too had a kinda bad feeling about the authentication function - it puts the user field into the request object.

While, in the end, it's not a big problem, it also potentially plays bad with existing middlewares for the various frameworks that perhaps expects authentication data in the "proper place". Those would be, as far as I understand:

(*) this requires some thinking, as they are not exposed in controller methods. Perhaps another @AuthenticatedUser decorator?

Lastly, yes I agree, this would be a non backward-compatible change, and it would require a major bump.

cheng81 avatar Nov 19 '21 13:11 cheng81