-
Notifications
You must be signed in to change notification settings - Fork 773
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
Relax redirect URL matching for the forward slash path case #710
base: master
Are you sure you want to change the base?
Conversation
Thanks for the contribution @AlexPlekhov, could you sign the contributor agreements so we can get this merged? |
Hi @petea, I've already done it immediately after creating PR. I hope you got email notifications. If didn't I'm ready to send those PDFs via email. |
Hi @petea, could you confirm that everything OK? I followed the steps in the contributor agreements. Please, write me back if I missed anything and still have to do something. |
Would be great to have this PR merged as it fixes an issue with Azure AD we have been experiencing. The Azure Portal gives us a redirect URI which looks like |
Hi all, @petea, I've resigned all docs as required by Mike Leszcz, was added to a WG, got a subscription email. |
I think this should be merged, as this is mission critical for functioning apps that rely on third party services and thus cannot influence how the backend behaves. |
@developerfromjokela note that there are workarounds for this situation discussed in #446. |
Yep, we already did the same. But wants to help the community to not face with it again. |
…"/" as equal to an empty path of "".
Hi @petea, will this get merged? We encountered this issue just today. We have support for multiple IdPs that support OIDC and customer wanted to use ADFS but it is not working cause ADFS appends |
@davidkaya note that there is a straightforward workaround for this issue that is detailed in #446. |
Add a comment for changes.
Can we get this PR moving again? I think the only remaining question is a missing comment? Would help in our case (using the Flutter AppAuth library where this is an issue as well (and mentioned workaround does not apply in our case) |
@DelcoigneYves |
Then poking @petea for a merge 💯 |
@DelcoigneYves @petea @davidkaya |
@petea LGTM! Can we please now merge it? 🙂 https://github.com/openid/AppAuth-iOS/blob/master/CONTRIBUTING.md#pull-request-reviews |
Hi @petea, is this going to get merged? |
Based on issue #446 sometimes a trailing slash added to path for redirect url.
Here a fix to make comparison regardless that trailing slash.
This issue affects react-native-app-auth lib:
FormidableLabs/react-native-app-auth#123
FormidableLabs/react-native-app-auth#336
and a few others.