Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make route configurable #11

Closed
sfabriece opened this issue Oct 24, 2016 · 13 comments
Closed

Make route configurable #11

sfabriece opened this issue Oct 24, 2016 · 13 comments
Assignees

Comments

@sfabriece
Copy link

SInce my hapi instance has auth enabled on all routes, i had to add the config at the end to make it work. It would be nice to make it configurable, so that I can pass an option when registering the plugin.¨

server.route({
    method: 'GET',
    path: options.path,
    handler: (request, reply) => {
      const renderedHtml =
        fs.readFileSync(path.join(__dirname, '/public/index.html'))
          .toString()
          .replace(/{{title}}/g, options.title)
          .replace(/{{script}}/g, fs.readFileSync(path.join(__dirname, '/public/javascripts/app.js')))
          .replace(/{{style}}/g, fs.readFileSync(path.join(__dirname, '/public/stylesheets/style.css')));

      reply(renderedHtml)
        .header('Content-Type', 'text/html')
        .code(200)
    },
    config: {
      auth: false
    }
  })
@danielbayerlein danielbayerlein changed the title make config.auth configurable Make route configurable Oct 24, 2016
@danielbayerlein
Copy link
Collaborator

@sfabriece Thanks for your feature request. It will be available in the next version.

@danielbayerlein danielbayerlein self-assigned this Oct 24, 2016
@danielbayerlein
Copy link
Collaborator

WIP ➡️ branch route-configurable

@danielbayerlein
Copy link
Collaborator

@sfabriece
Copy link
Author

I would rename options.route to options.routeConfig to avoid any potential confusions.
Otherwise everything looks good.

@bugthesystem
Copy link
Owner

hey @danielbayerlein that looks good!

@danielbayerlein
Copy link
Collaborator

Done, see e20172e
It will be available with the next version 😉

@millette
Copy link

I tried that, wanting to allow access only to logged in users, so:

      plugin: {
        register: 'hapijs-status-monitor',
        options: {
          title: 'Status monitor',
          path: '/status',
          // FIXME: Doesn't work, why?
          routeConfig: { auth: { mode: 'required' } }
        }
      }

I'm using v0.4.0. I tried in the manifest and another try in the plugin after I register an auth strategy, but I can always access /status. It might have to do with my setup (I'm not 100% sure of myself dealing with plugins and dependencies), but if someone could test that the routeConfig can actually disallow access, that would be great.

@danielbayerlein
Copy link
Collaborator

@millette Thanks for your feedback. v0.4.0 does not include the feature. We release a new release very soon. Sorry...

@ziyasal Can you add me to the contributors at npm?

@bugthesystem
Copy link
Owner

Hi @danielbayerlein fyi, I just added.

@millette
Copy link

Doh, I should have checked the source I was using, hey ;-)

If you could also tag your git releases (npm version patch instead of hand editing the version in package.json will update the version and tag it in git) that would be great!

@danielbayerlein
Copy link
Collaborator

@millette Sorry for the trouble. With the next version i'll also tag releases. 😺

@bugthesystem
Copy link
Owner

@danielbayerlein thank you for taking over this 👍

@danielbayerlein
Copy link
Collaborator

hapijs-status-monitor 0.5.0 published 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants