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

Fix to invalidate the second redirect URI when the first URI is the native URI #976

Conversation

eitoball
Copy link
Contributor

This PR should fix to invalidates the second (or further) redirect URI when multiple redirect URIs are entered and the first URI is native URI like urn:ietf:wg:oauth:2.0:oob.

@@ -75,4 +75,11 @@
expect(error).to eq('must be an HTTPS/SSL URI.')
end
end

context 'multiple redirect uri' do
it 'invalidates the second uri when the first uri is native uri' do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -75,4 +75,11 @@
expect(error).to eq('must be an HTTPS/SSL URI.')
end
end

context 'multiple redirect uri' do

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@eitoball eitoball force-pushed the fix-when-native-redirect-uri-first-in-multiple-redirect-uri branch from b82555e to 67c9520 Compare July 31, 2017 00:47
@nbulaj nbulaj added the bug? label Feb 4, 2018
@nbulaj
Copy link
Member

nbulaj commented Feb 16, 2018

Hi @eitoball. Yep, it's a nice point! Could you please resolve the conflicts, add an entry to NEWS.md file and squash commits to a single one? Thank you!

@eitoball eitoball force-pushed the fix-when-native-redirect-uri-first-in-multiple-redirect-uri branch from 67c9520 to da1116f Compare February 16, 2018 16:34
@eitoball
Copy link
Contributor Author

All Done. Thank you for checking in.

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

Successfully merging this pull request may close these issues.

3 participants