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

oAuth 1.0a Support #571

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
2 participants
@besolov
Contributor

besolov commented Nov 5, 2017

Closes #197

It's a basic support for oAuth 1.0a authentication. oauth-1.0a library is used for authentication support. At this moment only HMAC-SHA1 and PLAINTEXT signature methods are supported.

image

@besolov besolov force-pushed the besolov:oauth1.0a branch from 34e6413 to 0d8636f Nov 5, 2017

@gschier

This comment has been minimized.

Collaborator

gschier commented Nov 5, 2017

Thanks for doing this! Will take a closer look this week.

One thing I noticed before I start the actual review. There is no need for the external crypto package right? Node has it built in now.

@gschier

This comment has been minimized.

Collaborator

gschier commented Nov 5, 2017

@besolov

This comment has been minimized.

Contributor

besolov commented Nov 5, 2017

@gschier Yeah, it was from example for oauth-1.0a library. It is used for create signature. I'll dig it tomorrow, for replacement with built-in Node module.

@besolov

This comment has been minimized.

Contributor

besolov commented Nov 5, 2017

@gschier Oh, it was easy, I've removed 'crypto' from dependencies and it still works :)

@gschier

This comment has been minimized.

Collaborator

gschier commented Nov 5, 2017

Could have been an old example. Thanks for the quick update 😀

@besolov besolov force-pushed the besolov:oauth1.0a branch from d48ff7a to 4b2a8b4 Nov 6, 2017

@besolov besolov force-pushed the besolov:oauth1.0a branch from 4b2a8b4 to f78442d Nov 6, 2017

@gschier

gschier approved these changes Nov 6, 2017 edited

No complaints here. Looks great! One thing, though. Since this PR touches the UI, can you attach a screenshot of the OAuth form to the PR description? I'll get it merged in after that.

Thanks.

@besolov

This comment has been minimized.

Contributor

besolov commented Nov 6, 2017

Here it is:
screen shot 2017-11-06 at 21 22 33

@gschier

This comment has been minimized.

Collaborator

gschier commented Nov 6, 2017

Nice! Excellent work @besolov 😄

@gschier gschier merged commit d3e8a05 into getinsomnia:develop Nov 6, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment