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

Implement PKCE #76

Closed
Uzlopak opened this issue Nov 17, 2021 · 9 comments · Fixed by #86
Closed

Implement PKCE #76

Uzlopak opened this issue Nov 17, 2021 · 9 comments · Fixed by #86
Assignees
Labels
compliance 📜 OAuth 2.0 standard compliance high priority 💥 This issue should be fixed before others security ❗ Address a security issue
Milestone

Comments

@Uzlopak
Copy link
Collaborator

Uzlopak commented Nov 17, 2021

See https://snyk.io/vuln/npm:oauth2-server
oauthjs/node-oauth2-server#452

@jankapunkt jankapunkt added compliance 📜 OAuth 2.0 standard compliance high priority 💥 This issue should be fixed before others security ❗ Address a security issue labels Nov 17, 2021
@HappyZombies
Copy link
Member

There are some PRs in the original repo that contains PKCE implementation -- not saying we should copy and paste these but they can definitely assists us in the implementation.

oauthjs/node-oauth2-server#658 <- example

@jankapunkt
Copy link
Member

Related: rfc7636

@jankapunkt jankapunkt pinned this issue Nov 19, 2021
@jankapunkt jankapunkt linked a pull request Nov 25, 2021 that will close this issue
@jankapunkt
Copy link
Member

I started this here: #86

@FStefanni
Copy link
Contributor

Hi,

if can be of any help, this is the full list of pr related to PKCE, in the original project:

Regards.

@jankapunkt
Copy link
Member

@FStefanni @Uzlopak I oriented on oauthjs/node-oauth2-server#658 because the others were targeting a much different state of the code, this was by far the most easy to integrate. Please check out #86 and clone this branch to test locally if you need PKCE as I have currently not the resources to set this up on our systems.

@nekman
Copy link

nekman commented Sep 28, 2022

This issue is labeled with high priority. However, it have been open for 10 months. When do you plan to release this? Or must all issues in the v4.3 milestone be completed first? I don't want to stress anything, just would like to have this feature in the library 👍

@jankapunkt
Copy link
Member

jankapunkt commented Sep 28, 2022

Hi @nekman as I stated a few times, we need more people to test this with a real world setup. Just a few days ago someone did and revealed a missing feature which is now added as another PR.

If you want to get this feature faster merged then please help out with testing.

I have limited resources as I have only the authorization code workflow as setup to test. The other workflows can only be tested by me locally in an artificial environment.

You can also help out by reviewing existing pull requests regarding PKCE.

I will also try to get this repo into Hacktoberfest (never did this but I see some potential for getting some support).

@nekman
Copy link

nekman commented Sep 28, 2022

Thanks for the information @jankapunkt. Of course we should not stress anything. Important to get it right and well tested. I will see what I can do!

@jankapunkt
Copy link
Member

implemented in 4.3.0 by PR #86

@jankapunkt jankapunkt unpinned this issue Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compliance 📜 OAuth 2.0 standard compliance high priority 💥 This issue should be fixed before others security ❗ Address a security issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants