Closed thisbarker closed 7 years ago
What is the specific URL?
I gave the example above. Generally the will be the bundle identifier/package name/etc as the scheme then a path that the app will know how to handle the request from the system.
For example on android, but iOS and Windows UWP work similarly, I may register an intent filter for the browsable category with a data scheme of, com.mycompanyname.myappname, and a data path of, /bungieOAuthResult. When the redirect from the browser occurs it will be forwarded to my application (or any other application with the same scheme registered, but user will be prompted to select which one to use).
Is there a specific error that is given? And are you using myapp:/oauth
exactly? This shouldn't be an issue as far as I can tell. There are multiple mobile apps (Ishtar Commander for example) that use this type of URL scheme to do authorization on device.
It appears if the '.''s are not used it will allow it, but common OAuth2 providers, specifically Google, allow for the package name/bundle identifier/etc to be part of the scheme. This allows you to uniquely identify your scheme from others, and prevent conflict between applications.
The problem is that isn't how a mobile url scheme works. Say your app is called 'fakeDIM', then you want to enter on the portal 'fakedim://authResult'. You then register 'fakedim://' scheme in the settings plist of iOS. You can google how to do the same for Android.
'.' and '-' are both permitted characters in the URI spec, so that's a valid URI per RFC 3986, anyways:
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
(Hi, @vthornheart-bng!)
OK so you are talking about the https://www.bungie.net/en/Application/ pages then. That is what I was asking. You just said "the page". These additional comments provide the context that I was wondering about.
@thisbarker If you experimentally try a callback URI that doesn't have + - . in it, does the page accept it?
I did try plain myapp:/oauth and it worked. It is only for domain included scheme's that the error occurs. I believe this must be added as it is common practice to use this format for a mobile application. Generic schemes like @designker pointed out is generally used when you want multiple applications to register for the same scheme.
I believe the bungie site has a defect that prevents periods in it. See this thread
https://www.bungie.net/en/Groups/Post?groupId=39966&postId=221293685&sort=0&page=0
@pendsley Exactly.
This is one of the reasons I never converted my application to use OAuth from Bungie, because it isn't valid. I am forced to now, as the /mobi/en/User/LogIn/{platform} paths do redirects to non-https now, which breaks web-view security.
Nevertheless, we can override it by submitting the form data, bypassing the javascript on the page, but these seems like a work-around to a bug.
Here is the related code for that input box.
<input type="text" placeholder="URL to receive the authorization code" maxlength="200" data-anyurl="1" data-errormessage-anyurl="The redirect URL must be a fully qualified URL using any scheme except http." data-regex="^http:" data-regex-flags="i" data-regex-invert="1" data-errormessage-regex="The redirect URL must be a fully qualified URL using any scheme except http." value="">
`f.is("[data-anyurl]") && (t = i.validateAnyUrl(f),
n.validateAnyUrl = function(n) { var i = n.val(), r = !1, u; return i.length > 0 && (u = /^[a-z-]+:\/?\/?([\w-_.]+)+([\w-.()!,@?^=%&:\/~+#]*[\w-\@?^!=%&\/~+#])?$/i, r = !u.test(i)), this.createValidationResult(r, n, t.anyurl) }`
So the specific regex is:
/^[a-z\-]+:\/?\/?([\w\-_\.]+)+([\w\-\.\(\)\!,@?^=%&:\/~\+#]*[\w\-\@?^\!=%&\/~\+#])?$/i
@vthornheart-bng ^[a-z\-]+:
needs to be changed to ^[a-z\-.+]+:
in this regex. (Or, if you want to be super ~pedantic~ correct, ^[a-z][a-z0-9\-.+]*:
@floatingatoll that doesn't look correct. Did you mean [a-z\-\.]+:
?
Well, actually, I meant +]+, fixed! Thanks. You don't need any of those backslashes, it's a character class. Technically, this also permits invalid URIs that start with . - + because of an existing bug in the current regex, so the formally more correct would be:
/^[a-z][a-z-.+]*:\/\/
[rest of regex here]
Regex stripped apart for explanation (we're fixing the 'scheme' component here, the :// follows it for all callback handlers.) Oh, I found another bug, it excludes digits incorrectly! Let's fix that too.
^ [a-z] [ a-z 0-9 + \- . ]* ://
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
Removing the spaces, you get:
^[a-z][a-z0-9+\-.]*://
And then escaping the forward slashes to stick it back into the code:
u = /^[a-z][a-z0-9+\-.]*:\/\/
As you posted:
scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
Another way to write it is:
; the scheme is in lower case; interpreters should use case-ignore scheme = 1*[ lowalpha | digit | "+" | "-" | "." ]
So with that in mind [a-z0-9\+\-\.]+
looks like the correct block (+, -, and . have special meaning to regex and must be escaped). In other words the current block is not allowing numbers, periods, or plus signs and it should.
(+, -, and . have special meaning to regex and must be escaped)
Incorrect. In character classes, neither +
nor .
are special, and -
is only special when preceded by a character that is not part of a preceding a-z
-type range. So [abc-]
is invalid, but [ab-c-]
is valid, because c
terminates the preceding range, and therefore the -
cannot be a range operator and must mean the standalone -
. I still prefer to backslash -
for the unwary, but not the others.
Note that not all regex parsers will accept the standalone -
in the middle of the character class, but they all seem to accept it as the first character in the character class — I have no idea why, sorry — and that's more in favor of backslashing it for sure.
$ echo 'u-+r.i0://' | egrep '^[a-z][-a-z0-9.+]*://'
u-+r.i0://
$ echo 'u-+r.i0://' | perl -nE 'say "OK" if m{^[a-z][-a-z0-9.+]*://}'
OK
@thisbarker Also, you might need ://
instead of :/
.
@floatingatoll This does not work. Further more even com.example.bungieauth:oauth2redirect is a real path. The / is only used to simulate a route
Hmm - I'll get the right people looped in on this shortly. We may be able to accommodate depending on the situation - we'll get back to you!
I created this simple Plunker to allow for testing of the redirect URI which has to meet the standards of an absolute URI. It uses the URI.js package and links to the RFC's. Both of @thisbarker 's lines returned true (as far as they meet absolute URI syntax according to URI.js).
http://embed.plnkr.co/440NwRHTEJUjsD0P0c8q/
com.example.bungieauth:oauth2redirect true myapp:/oauth true
I indicated :// instead of :/ because callback: URIs can't be tested on mobile device browsers, due to not having the :// in them. They might tolerate :/, but they definitely support ://.
On Fri, Aug 25, 2017 at 12:30 PM, vpzed notifications@github.com wrote:
I created this simple Plunker to allow for testing of the redirect URI which has to meet the standards of an absolute URI. It uses the URI.js package and links to the RFC's. Both of @thisbarker https://github.com/thisbarker 's lines returned true (as far as they meet absolute URI syntax according to URI.js).
http://embed.plnkr.co/440NwRHTEJUjsD0P0c8q/
com.example.bungieauth:oauth2redirect true myapp:/oauth true
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bungie-net/api/issues/5#issuecomment-325015570, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFqDGTi0lllrFlwHi7j9bLe4FNY2Npsks5sbyDngaJpZM4PCptH .
Sorry I am late to this party. You need ://...oh I see you figured that out.
@Tetron-bng :// does not resolve it. It's a bug in the javascript like @vpzed pointed out it needs to allow for '.'s before the ':'
/^[a-z\-]+:\/?\/?([\w\-_\.]+)+([\w\-\.\(\)\!,@?^=%&:\/~\+#]*[\w\-\@?^\!=%&\/~\+#])?$/i
Hi @Tetron-bng! You want this:
https://github.com/Bungie-net/api/issues/5#issuecomment-324970691
Sorry, missed that. Currently, the redirect URL validation is using .NET's System.Uri class for validation. Looks like it is not allowing . or - in the scheme. Thankfully, this is not a regression in the new website, I have enough of those to worry about. This is a long thread (TLDR) Is this blocking anyone?
There is a hack to work around by manually bypassing the javascript.
Its not a javascript check. It is server side. Let me know if you work around it, there is a problem if you can.
The sever side check is fine.
Review post: issue comment-324958360
And when data-anyurl="1" is removed from the input inside the #redirectUrl div it allows you to save. Just as @vpzed pointed out in issue comment-324970427
Ahhh, that explains it - good deal! Thanks for the clarification!
It's a long post but the TLDR; is the regex (shown below) in the validateAnyUrl function used in the Javascript on the Application page needs to be updated to conform with RFC standards.
/^[a-z\-]+:\/?\/?([\w\-_\.]+)+([\w\-\.\(\)\!,@?^=%&:\/~\+#]*[\w\-\@?^\!=%&\/~\+#])?$/i
As suggested revision for the scheme section at the beginning is:
^[a-z][a-z0-9\-.+]*:
NOTE: I'm just trying to summarize this long thread. I have not looked at the entire regex to see if anything other than the scheme section requires review.
Finally got around to looking at this - yeah, we'll have this fixed up for the deployment next week!
The JavaScript validation on the page blocks redirect urls such as, myapp:/oauth. Bypassing the JavaScript to submit the form data persist correctly.