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

[Feature] OpenID response type selection for OAuth 2.0 implicit mode #681

Merged
merged 13 commits into from Jan 16, 2018

Conversation

Projects
None yet
2 participants
@emdfonseca
Contributor

emdfonseca commented Dec 27, 2017

This PR adds support for OpenID Connect by allowing the configuration of the OAuth 2 (implicit grant) response type. In the advanced options a new dropdown was added requesting the user to select whether he wants to receive the id_token, the access_token or both. By default the access_token is chosen (oauth 2 default). However, the user can also select to receive only the id_token or both the id_token and the access_token to be OpenID Connect compliant.

This PR closes an already closed issue (#501). The issue was previously closed due to inactivity but the solution is not yet available. This PR should reopen the issue for further discussions and, hopefully, help improving insomnia with this new feature.

@welcome

This comment has been minimized.

welcome bot commented Dec 27, 2017

💖 Thanks for opening this pull request! 💖

To help make this a smooth process, please be sure you have first read the
contributing guidelines.

@emdfonseca emdfonseca changed the title from Enable response type selection for oauth2 implicit mode to [Feature] Enable response type selection for oauth2 implicit mode Dec 27, 2017

emdfonseca added some commits Dec 28, 2017

@gschier

Awesome, just one minor issue @emdfonseca. Thanks for contributing! 😄

@@ -9,30 +9,33 @@ export default async function (
clientId: string,
redirectUri: string = '',
scope: string = '',
state: string = ''
state: string = '',
responseType: string = ''

This comment has been minimized.

@gschier

gschier Dec 30, 2017

Collaborator

response_type is a required OAuth 2.0 parameter so this argument default should probably be changed from '' to c.RESPONSE_TYPE_TOKEN;

This comment has been minimized.

@gschier

gschier Dec 30, 2017

Collaborator

And line 15 can be reverted.

_parseUrl(currentUrl);
});
child.webContents.on('did-fail-load', (e, errorCode, errorDescription, url) => {

This comment has been minimized.

@gschier

gschier Dec 30, 2017

Collaborator

Nice 👍

emdfonseca added some commits Dec 30, 2017

@emdfonseca

This comment has been minimized.

Contributor

emdfonseca commented Dec 30, 2017

Hi @gschier, thanks for the feedback! I made the changes as requested.

I left the name of the constant as RESPONSE_TYPE_ACCESS instead of RESPONSE_TYPE_TOKEN. This helps avoiding confusion since you can get both the 'id_token' and 'access_token' on openid connect servers. Is that ok?

emdfonseca added some commits Dec 30, 2017

@emdfonseca

This comment has been minimized.

Contributor

emdfonseca commented Jan 7, 2018

Hi @gschier, any additional changes needed in order to get this PR merged?

@gschier

This comment has been minimized.

Collaborator

gschier commented Jan 7, 2018

Sorry, haven't had time to look at it again. It's probably good enough to merge at this point. I'll try to get it merge this week.

@gschier

looks good @emdfonseca! Thanks 😄

@gschier gschier merged commit 3f46f58 into getinsomnia:develop Jan 16, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@welcome

This comment has been minimized.

welcome bot commented Jan 16, 2018

Congrats on merging your first pull request! 🎉🎉🎉 You're helping make Insomnia awesome! 🙌

@gschier gschier changed the title from [Feature] Enable response type selection for oauth2 implicit mode to [Feature] OpenID response type selection for OAuth 2.0 implicit mode Jan 17, 2018

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