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

How to secure the rest endpoint #58

Open
hosnas opened this issue Oct 22, 2019 · 11 comments
Open

How to secure the rest endpoint #58

hosnas opened this issue Oct 22, 2019 · 11 comments

Comments

@hosnas
Copy link

hosnas commented Oct 22, 2019

Hello,
How can I secure agenda rest endpoint so that only authorized people can schedule requests?
There should be an apikey but I don't find anything in doc.

@keyvan-m-sadeghi
Copy link
Member

Hi @hosnas, @Arvant has sent a pull request, #59, that I think addresses this issue. Appreciate if you could confirm.

@sampathBlam
Copy link
Contributor

@keyvan-m-sadeghi : I have added another PR #60 with a very cosmetic change regarding the Error message in case of incorrect API key.

@hosnas
Copy link
Author

hosnas commented Oct 27, 2019

@keyvan-m-sadeghi can confirm it works as intended. Great job @Arvant and @sampathBlam 👍

I just want to say its worth mentioning it in the readme doc.

@keyvan-m-sadeghi
Copy link
Member

Agreed, @Arvant and @sampathBlam, any help with documenting this is very much appreciated.

sampathBlam added a commit to sampathBlam/agenda-rest that referenced this issue Oct 27, 2019
- Separate installation and usage sections.
- Addresses issue agenda#61
- Related to issue agenda#58
@sampathBlam
Copy link
Contributor

The current implementation with the API key looks neat and sufficient. A more robust way to authenticate would be to handle in-house OAuth2 based authentication with Client credentials strategy. As agenda-rest might be predominantly used in machine-to-machine communications (users wont directly hit the hosted agenda-rest service), it makes sense for the clients(may be another server or an application) that interact with agenda-rest to get a client ID and secret as part of a registration process, then get an application access token and use it for any further requests. @hosnas , @keyvan-m-sadeghi , @Arvant : Your thoughts on this?

@keyvan-m-sadeghi
Copy link
Member

@sampathBlam you're absolutely right, main use case for agenda-rest is M2M interactions. As such, I'm not clear on the reason for adding complexities like a refresh token. What's the added value? In addition, I'm very much for agenda-rest remaining a microservice, wouldn't adding this be a practice used in monoliths?

@sampathBlam
Copy link
Contributor

sampathBlam commented Nov 9, 2019

@keyvan-m-sadeghi : Yes I agree. Adding OAuth would mean increasing unnecessary complexity and would prove a hindrance in maintaining agenda as a microservice.

@geosp
Copy link
Collaborator

geosp commented Feb 18, 2020

I think is worth considering to adopt the same strategy GitHub uses to secure web-hooks. This strategy is much more secure that passing an none encrypted key. For more documentation please take a look at this https://developer.github.com/webhooks/securing. Here is some example code that shows how to validate the request you could also implement a middle-ware using this approach:

export let validateGithubRequest = ({ secret, req }) => {
  let signature = req.headers['x-hub-signature']
  let hmac = createHmac('sha1', secret)
  let body = Buffer.from(req.rawBody)
  hmac.update(body, 'utf-8')
  return signature === `sha1=${hmac.digest('hex')}`
}
.
.
.
let preserveRawBody = (req, res, bodyBuffer) => {
  req.rawBody = bodyBuffer.toString('utf8')
}

.
.
.
  const app = express(feathers())
  app
    .configure(express.rest(rest()))
    .configure(socketio())
    .configure(prometheus)
    .use(bodyParser.json({ limit: '50mb', verify: preserveRawBody }))
.
.
.

@keyvan-m-sadeghi Let me know what you think and I can implement a PR that exemplifies what I'm proposing.

@keyvan-m-sadeghi
Copy link
Member

@geosp seems like a fine solution, I suggest that we keep the x-api-key as it still might be the preferred method for some. I'll wait for your PR, then publish to npm and bump up the version.

@geosp
Copy link
Collaborator

geosp commented Feb 18, 2020

@keyvan-m-sadeghi Thank you for accepting my suggestion. I will call the header x-agenda-signature and it will be optionally enabled via settings and a flag. While I implement this feature can you please publish and bump the version for me. I have an internal project that depends on PR #70. Thank you so much for your prompt response.

@keyvan-m-sadeghi
Copy link
Member

@geosp done 🎉

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

No branches or pull requests

4 participants