Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Ability to Modify Default Headers #724

Closed
OR13 opened this issue Mar 24, 2018 · 8 comments
Closed

Ability to Modify Default Headers #724

OR13 opened this issue Mar 24, 2018 · 8 comments

Comments

@OR13
Copy link
Contributor

OR13 commented Mar 24, 2018

As far as I am aware, best practice for securing IPFS is to use an api gateway or proxy to restrict access to the ipfs api.

ipfs/kubo#1532

Bearer Authentication is common, here are some more details on the practice:

https://swagger.io/docs/specification/authentication/bearer-authentication/

I have tested using Kong to secure IPFS in this way and had success, but I find myself wanting to add a header to every network request made by js-ipfs-api.

We could support adding a header to the config:

const ipfsConfig = {
  host: 'localhost',
  port: 5001,
  protocol: 'https',
  headers: {
    authorization: 'Bearer ' + ACCESS_TOKEN
  }
};

Here where the user agent is added:

https://github.com/ipfs/js-ipfs-api/blob/master/src/utils/send-request.js#L111

We could add the authorization header to every request.

Axios http client supports this:

axios/axios#209

The api gateway (kong/ tyk) would be responsible for validating the jwt token.

I'm happy to submit a PR for this work, but I'm not sure the potential security implications for the rest of the api.

At a minimum it would seem wise to only allow the authorization header (assuming it is formatted correctly).

Interested to hear if this would be useful for others.

@wraithgar
Copy link
Contributor

(Assuming jwt is the bearer schema being used)

Since the API is still on hapi@16 this would be implemented in the server by hapi-auth-jwt@7 (version 8 is when it moved to hapi@17.

If the API were to move to hapi@17 we would probably want to use @now-ims/hapi-now-auth

@prettymuchbryce
Copy link

I think this would be appropriate as I find myself in the same situation as @OR13 . Perhaps as a more general solution would be to allow configuration of custom headers to be sent with any request. This could then extend beyond authentication and a more tailored authentication solution could be implemented if/when the API moves to hapi@17.

Thoughts?

@OR13
Copy link
Contributor Author

OR13 commented Apr 15, 2018

Here is a PR which should support custom headers for all requests made by the library.

#741

@prettymuchbryce
Copy link

I prefer the approach of @OR13 in #741, but I've created a PR #740 as an example of what I'm doing now in my fork. Hopefully we can get a steward of this project to take a look and chime in here. 😄

@eolszewski
Copy link

@diasdavid this pull request allows IPFS consumers to use Bearer Authentication with a half-line of code.

This will allow us and others to remove our forks and stay up to date with the progress y'all are making. Can you please merge this?

@daviddias
Copy link
Contributor

Sounds good to me. Can we get both #741 and #740 to converge and get them documented?

@OR13
Copy link
Contributor Author

OR13 commented Apr 24, 2018

I'm working out some lint errors, but this is my approach:

/* eslint-env mocha */
'use strict';

const chai = require('chai');
const dirtyChai = require('dirty-chai');
const expect = chai.expect;
chai.use(dirtyChai);

const IPFSApi = require('../src');
const f = require('./utils/factory');

describe('custom headers', function() {
  this.timeout(50 * 1000); // slow CI
  let ipfs;
  let ipfsd;
  // initialize ipfs with custom headers
  before(done => {
    f.spawn({ initOptions: { bits: 1024 } }, (err, _ipfsd) => {
      expect(err).to.not.exist();
      ipfsd = _ipfsd;
      ipfs = IPFSApi({
        host: 'localhost',
        port: 6001,
        protocol: 'http',
        headers: {
          authorization: 'Bearer ' + 'YOLO'
        }
      });
      done();
    });
  });

  it('are supported', done => {
    // spin up a test http server to inspect the requests made by the library
    const server = require('http').createServer((req, res) => {
      req.on('data', () => {});
      req.on('end', () => {
        res.writeHead(200);
        res.end();
        // ensure custom headers are present
        expect(req.headers.authorization).to.equal('Bearer ' + 'YOLO');
        server.close();
        done();
      });
    });

    server.listen(6001, () => {
      ipfs.id((err, res) => {
        // this call is used to test that headers are being sent.
      });
    });
  });

  after(done => ipfsd.stop(done));
});

@alanshaw
Copy link
Contributor

Closing as #741 was merged and released :D

@ghost ghost removed the ready label Oct 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants