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

Use base64urlencode as optional for Authorization Code Grant #14

Closed
klerick opened this issue Apr 26, 2021 · 8 comments
Closed

Use base64urlencode as optional for Authorization Code Grant #14

klerick opened this issue Apr 26, 2021 · 8 comments

Comments

@klerick
Copy link

klerick commented Apr 26, 2021

Impossible use postman, passport-oauth2, etc with typescript-oauth2-server.
Because these tools do not use base64urlencode for Authorization Code Grant.
Maybe use base64urlencode as optional?

@jasonraimondi
Copy link
Owner

Great point, I can definitely look into making this optional, will take a few days but I will absolutely see what I can do. Also totally willing to take a PR if you want to contribute to the project.

@klerick
Copy link
Author

klerick commented Apr 26, 2021

I have created PR
#15

@jasonraimondi
Copy link
Owner

I have one comment on the PR, if you can get that addressed (or I will when I get a moment) I can get this merged into the next version asap.

@jasonraimondi
Copy link
Owner

jasonraimondi commented Apr 26, 2021

@klerick I have merged this and built your PR into version 1.0.3, basically this is "opting to skip the url encode". I am really thinking now that it should be skipped by default and you can opt into base64 encoding. I might follow up with 1.1.0 that changes this default value to always skip url encoding, and allow users to opt-in. What do you think?

@klerick
Copy link
Author

klerick commented Apr 26, 2021

I think it's better to keep it as is by default for backward compatibility. Otherwise, after updating the version existing code in some projects may be broken. For example, a have mask ^0.0.0 for npm module in 'package.json'

@klerick
Copy link
Author

klerick commented Apr 26, 2021

The base64 encoding is a good feature, but other tools do not use it(

@jasonraimondi
Copy link
Owner

@klerick Thank you for the feedback and talking me down 😄 . You are correct, it is better to leave it and allow the opt-out.

I have created a branch to do a minor amount of cleanup for the authorization server optional configs, as well as adding some documentation for both config options here if you wanted to take a peek: https://github.com/jasonraimondi/typescript-oauth2-server/pull/16/files

I am going to put this refactor work into a release tagged 1.0.4

Closing this issue as it seems to be resolved now.

@jasonraimondi
Copy link
Owner

jasonraimondi commented Apr 26, 2021

released in v1.0.4

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

No branches or pull requests

2 participants