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

Seneca Web does not listen to 'X-HTTP-Method-Override' #89

Closed
devotox opened this issue Sep 15, 2016 · 11 comments
Closed

Seneca Web does not listen to 'X-HTTP-Method-Override' #89

devotox opened this issue Sep 15, 2016 · 11 comments
Labels

Comments

@devotox
Copy link

devotox commented Sep 15, 2016

How can we get Seneca Web to change Paths based on an X-HTTP-Method-Override as there is no way to send a patch request from certain systems

@mcdonnelldean
Copy link
Contributor

Ultimately seneca web just creates routes with Hapi / Express. If it is something that needs to be done per route we can add pass through config.

As of 1.0 the only thing seneca web does so you need to set the above in hapi or express, ala, hapijs/hapi#1217

@devotox
Copy link
Author

devotox commented Sep 16, 2016

Usually you would be able to use the npm module method-override written by
express themselves but this does not seem to work with Seneca when I do

const methodOverride = require('methode-override');
app.use(methodeOverride('X-HTTP-Method-Override'));

On Fri, 16 Sep 2016, 15:09 Dean McDonnell, notifications@github.com wrote:

Ultimately seneca web just creates routes with Hapi / Express. If it is
something that needs to be done per route we can add pass through config.

As of 1.0 the only thing seneca web does so you need to set the above in
hapi or express, ala, hapijs/hapi#1217
hapijs/hapi#1217


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#89 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB-BzmvkN7SOfQWO6RL2JLNnEEg06lUdks5qqqMVgaJpZM4J993v
.

@tswaters
Copy link
Member

This should work - but it depends on when it has been loaded into the middleware chain.

If you follow the quick start example they pass Express() as the context, and later pull it out with seneca.export - if you attach the middleware at this point, it will be too late. All the routes will have been applied to the app, and adding methodOverwride adds it after them, so it is too late.

The following should work:

const Express = require('express')
const Seneca = require('seneca')
const SenecaWeb = require('seneca-web')

const app = Express()
app.use(methodeOverride('X-HTTP-Method-Override'));

const seneca = Seneca()
seneca.use(SenecaWeb, {context: app, adapter: 'express', routes: '...'})
seneca.ready((err) => {
  // app's routes now has:
  // [methodOverride, ...senecaRoutes]
})

Typically what I've been doing is setting up express with all the middleware it needs (i.e., body parser, session, etc., etc.) and passing new Router() to the context, and loading that into the app with app.use('/api/', seneca.export('web/context')())

This allows for dynamic routes to be added later from microservices joining the mesh, while still keeping the last app.all('*', (req, res) => { res.status(404).send('nope!') })

@devotox
Copy link
Author

devotox commented Sep 27, 2016

What is the difference between that and this? Is it the points at which I load the modules?
Should Seneca and Seneca Web be required before i do the const app? or is it the adapter that i need to specify?

// ========================================================= CONFIGURE EXPRESS ======================================================= //

// create the express framework
const app = express();

// override with different headers; last one takes precedence
app.use(methodOverride('X-HTTP-Method', ['POST', 'PUT']));          // Microsoft
app.use(methodOverride('X-Method-Override', ['POST', 'PUT']));      // IBM
app.use(methodOverride('X-HTTP-Method-Override', ['POST', 'PUT'])); // Google / GData / Salesforce

// Protect against known vulnerabilities
app.use(helmet());
app.use(helmet.noCache());

// Set up Passport
app.use(passport.initialize());
app.use(passport.session());

app.use(compression());
app.use(express.query());
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));

app.use(pmx.expressErrorHandler());

// ========================================================= CONFIGURE SENECA ======================================================= //

// create a seneca instance
const seneca = require('seneca')({ timeout: 15000 });

// load seneca modules
seneca.use(require('seneca-web'));

// load custom seneca modules
seneca.use(__dirname + '/seneca_modules/data');                             // data access layer
seneca.use(__dirname + '/seneca_modules/documentation', { app: app });      // documentation
seneca.use(__dirname + '/seneca_modules/core-api', { passport: passport, app: app });   // application layer

// ensure seneca is configured to use passport
require(__dirname + '/seneca_modules/core-api/config/passport.js')(passport, seneca);

// initialize the seneca modules
seneca.act({
    data: 'initialize'
}, (error, output) => {
    console.error(error || output.message);
});

seneca.act({
    model_version: 'v2',
    controller_version: 'v2',
    documentation: 'initialize'
}, (error, output) => {
    console.error(error || output.message);
});

seneca.ready( () => {
    app.use(seneca.export('web'));

    // start seneca!
    seneca.listen();
    seneca.log.info('listen', configuration.node_port );
});

@mcdonnelldean
Copy link
Contributor

This is the old way of using the code let me change it for you.

@mcdonnelldean
Copy link
Contributor

mcdonnelldean commented Sep 27, 2016


const app = express();
app.use(methodOverride('X-HTTP-Method', ['POST', 'PUT']));      
app.use(methodOverride('X-Method-Override', ['POST', 'PUT'])); 
app.use(methodOverride('X-HTTP-Method-Override', ['POST', 'PUT']));
app.use(helmet());
app.use(helmet.noCache());
app.use(passport.initialize());
app.use(passport.session());
app.use(compression());
app.use(express.query());
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.use(pmx.expressErrorHandler());

const seneca = require('seneca')({ timeout: 15000 });
seneca.use(require('seneca-web'), {context: app, adapter: 'express', auth: passport, routes: {}});
seneca.use(__dirname + '/seneca_modules/data'); 
seneca.use(__dirname + '/seneca_modules/documentation', { app: app }); 
seneca.use(__dirname + '/seneca_modules/core-api', { passport: passport, app: app });application layer

// you may not need this, what is it doing? Passport is now properly supported
require(__dirname + '/seneca_modules/core-api/config/passport.js')(passport, seneca);

seneca.ready(() => {
  seneca.act({data: 'initialize'}, (error, output) => {
    console.error(error || output.message);
  });

  seneca.act({model_version: 'v2', controller_version: 'v2', documentation: 'initialize'}, (error, output) => {
    console.error(error || output.message);
  });

  app.listen(configuration.node_port, () => {
    // now started
  })  
});

See https://github.com/senecajs/seneca-web#auth passport is now directly supported on the routes. You need to add in your routes to the empty object or you can add them after seneca.ready too.

The old seneca-web used to return a middleware but this caused issues in load order. The new one just adds routes so it is up to you to start expres/hapi/koa/etc yourself.

PS all acts should happen after ready in seneca.

@devotox
Copy link
Author

devotox commented Sep 27, 2016

so where i used to get my routes from was using this line

seneca.use(__dirname + '/seneca_modules/core-api', { passport: passport, app: app });   // application layer

and that had this in it

'use strong';

var seneca = require('seneca')();

var version = function (options, number, current) {
    return function (app) {
        // route prefix builder
        var version = '/v' + number;
        var prefix  = options.prefix || '/api';

        if (current) {
            require(__dirname + version).routes(app, prefix, options.passport);
        }

        prefix += version;

        seneca.log.info('API prefix', prefix);
        require (__dirname + version).routes(app, prefix, options.passport);
    };
};

module.exports = function (options) {
    var router = this.export('web/httprouter');

    // handle the routes for each version of the API
    // NOTE: /api points to the last version denoted as 'current'
    // this.act('role:web',{use: router(version(options, '1'           )) });  -- V1 DEPRECATED
    this.act('role:web', {
        use: router(version(options, '2', 'current'))
    });

    return { name:'api' };
};

So i no longer need the web/httprouter export and i should just get my routes directly from this line

require(__dirname + version).routes(app, prefix, options.passport);

@mcdonnelldean
Copy link
Contributor

@devotox without seeing your codebase in full it is hard to tell what exactly you require changed. Generally though, you should only have to build a map of the routes and pass it, express, and passport into seneca-web. See the secure routes examples we have in /eg for the changes.

@mcdonnelldean
Copy link
Contributor

@devotox It has been primarily tested with 3.0 It may work with 2.0. Please report any issues and we will fix it up so it works as expected where possible.

@devotox
Copy link
Author

devotox commented Oct 3, 2016

Ok i think i understand now thank you.

@mcdonnelldean
Copy link
Contributor

If you need any more help please add issues, cheers!

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

No branches or pull requests

3 participants