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

url scheme validation in oauth plugin is limited by RFC3986 #436

Closed
1 task
ahmadnassri opened this issue Jul 30, 2015 · 7 comments
Closed
1 task

url scheme validation in oauth plugin is limited by RFC3986 #436

ahmadnassri opened this issue Jul 30, 2015 · 7 comments
Assignees

Comments

@ahmadnassri
Copy link
Contributor

when assigning a redirect_uri in oauth2 plugin, some values are rejected (e.g. anything with underscore) while others are accepted (e.g. with dashes)

  • documentation should list accepted charachter list for all parts of the URI (as defined in RFC3986 & RFC7230)
  • schema should support underscore values.
@ahmadnassri
Copy link
Contributor Author

cc: @lucamaraschi

@subnetmarco
Copy link
Member

@ahmadnassri Can you post an example of an URL that doesn't work?

@ahmadnassri
Copy link
Contributor Author

any_thing_with_underscore://my_custom_dns_name/oauth

@ahmadnassri
Copy link
Contributor Author

doing a deep dive:

the url type validation is defined here:
dao/schemas_validation.lua#L20 which uses luasocket

which in turn is checking for RFC3986 validity in: src/url.lua#L140

specifically Section 3.1 where the scheme is defined as:

scheme      = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

therefore underscores are in fact invalid and kong is properly propagating the error message.

that said, we should also document the reference to RFC3986 as it is made part of RFC7230

@ahmadnassri ahmadnassri changed the title url scheme validation in oauth plugin is limited url scheme validation in oauth plugin is limited by RFC3986 Jul 31, 2015
@subnetmarco
Copy link
Member

That said, I guess we can close this ticket?

@ahmadnassri
Copy link
Contributor Author

let's keep open for the documentation part

@subnetmarco
Copy link
Member

The documentation already references the appropriate specification:

The URL in your app where users will be sent after authorization (RFC 6742 Section 3.1.2)

https://getkong.org/plugins/oauth2-authentication/#create-an-oauth-2.0-authentication-credential/application

Specifically it references the "Redirection Endpoint" of the OAuth 2.0 specification, which in turn references RFC3986.

bungle added a commit that referenced this issue Aug 8, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
bungle added a commit that referenced this issue Aug 8, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
StarlightIbuki pushed a commit that referenced this issue Aug 9, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants