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

Extensible request signing #138

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rowanhill
Copy link
Contributor

Hi,

In issue #134 I asked about extension points in iodocs - specifically I was interested in providing my own request signing logic (although I was a bit imprecise and called this an 'authentication mechanism'). I haven't seen anything suggesting this is currently possible, so I've quickly put something together - hopefully it's useful.

This change pulls out some of the existing signing code into a signers directory, and dynamically requires these signers at run time. (I've also commonised some of the existing code). This allows users of iodocs to use a custom signing method by just writing a new signer, popping it in the signers directory, and adding the appropriate signature type in apiconfig.json.

This also helps keep things clean - no need to change app.js, so pulling in upstream updates in the future is easier.

Note that the data I'm passing to signRequest is more than the existing signers want - this is because the new signer I want to write needs a few more bits of data. I can't think of anything else any future signer would want, so hopefully this will do for now. Note also that because I need these extra bits of information, I've moved signing slightly later in the process; I don't believe this will cause any issues, though.

If #137 gets any traction, I'll do something similar for configuring the signers directory (although that seems a bit doubtful, so I've not bothered for now).

Thanks,
Rowan

@rowanhill
Copy link
Contributor Author

I've just rebased onto current master and added the ability to configure the custom signers directory (since it looks like #137 will be accepted).

@mtougeron
Copy link
Contributor

Looks like it'll need to be rebased again after the other PRs I just merged for you. Sorry.

@ghost ghost assigned mtougeron Dec 9, 2013
@rowanhill
Copy link
Contributor Author

No problem - I just have done. Thanks for merging the others!

@mtougeron
Copy link
Contributor

No problem. I'll take a run through this when I get a moment.

@rowanhill
Copy link
Contributor Author

Hi - just checking in on this. Any ETA on when you'll get a chance to take a look at this?

@phairow
Copy link
Contributor

phairow commented Oct 9, 2014

looks like this was planned to be merged so I'll take a look

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

Successfully merging this pull request may close these issues.

4 participants