Skip to content

Development #260

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

Merged
merged 135 commits into from
Nov 7, 2023
Merged

Development #260

merged 135 commits into from
Nov 7, 2023

Conversation

jankapunkt
Copy link
Member

Merge development into master, making 5.0.0 the new stable

daniel.santos and others added 30 commits December 18, 2021 18:27
# Conflicts:
#	lib/grant-types/refresh-token-grant-type.js
#	package-lock.json
 Merge pull request #109 from dsschiramm/development
Thanks to @dsschiramm
Bumps [sinon](https://github.com/sinonjs/sinon) from 15.1.0 to 15.2.0.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/main/docs/changelog.md)
- [Commits](sinonjs/sinon@v15.1.0...v15.2.0)

---
updated-dependencies:
- dependency-name: sinon
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…isify-any

Merge pull request #190 from node-oauth/refactor-native-promises
Bumps [eslint](https://github.com/eslint/eslint) from 8.42.0 to 8.44.0.
- [Release notes](https://github.com/eslint/eslint/releases)
- [Changelog](https://github.com/eslint/eslint/blob/main/CHANGELOG.md)
- [Commits](eslint/eslint@v8.42.0...v8.44.0)

---
updated-dependencies:
- dependency-name: eslint
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
…n body

Merge pull request #197 from node-oauth/fix-pkce-missing-query
…t-8.44.0

build(deps-dev): bump eslint from 8.42.0 to 8.44.0
Copy link

@eddy-minet-holis eddy-minet-holis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a blocking point for those who want to use pkce since challenge in authorize are never located in request body.

jorenvandeweyer
jorenvandeweyer previously approved these changes Nov 6, 2023
@jankapunkt
Copy link
Member Author

@eddy-minet-holis I think I don't understand, does release 5.0.0 fix the issue you described with the code challenge?

@jankapunkt
Copy link
Member Author

jankapunkt commented Nov 7, 2023

@jorenvandeweyer I added three missing files that were removed by merge-override. Master should not again point to the same HEAD as development. From here we should work keep up working on development and merge into the respective target branches. Edit: would you mind to review and approve again?

@jorenvandeweyer
Copy link
Member

Approved it again.

@jankapunkt I also suggest we stop using release-x.y.z branches and only use releases. I think this is a better way to manage our releases since there will never be any new commits to a released branch.

So to clarify

  • master will be the stable 5.x release
  • development is the development branch and will be merged into master for each release
  • 4.x will be the support branch for security fixes. We can create this from the existing release-4.3.0 branch.
    • new 4.x.x+1 patches will be started from this branch.

I suggest we clean up and delete all our branches

  • stale branches (eg. promisify, v4.3.0-dev)
  • any branches from the original repository (eg. v5-dev, security)
  • feature branches which are merged (eg. remove-util-inherits)

@jankapunkt
Copy link
Member Author

@jorenvandeweyer I fully agree with your proposal.

@jankapunkt jankapunkt merged commit 848a03a into master Nov 7, 2023
@eddy-minet-holis
Copy link

eddy-minet-holis commented Nov 7, 2023

@eddy-minet-holis I think I don't understand, does release 5.0.0 fix the issue you described with the code challenge?

Yes. And I think the fix should be applied for 4.3 users.
Actually the workaround is easy when using the lib, but without it the PKCE flow just can't work :

req.body.code_challenge = req.query.code_challenge;
req.body.code_challenge_method = req.query.code_challenge_method:
const oauthReq = new oauth2.Request(req);
const oauthRes = new oauth2.Response(res);
...

talking about: commit ca43d4a

@jankapunkt
Copy link
Member Author

@eddy-minet-holis I published a fix as 4.3.3, feel free to test and please file an issue if this still not fixes things.

@jorenvandeweyer in concordance with your proposed branch structure I also aligned the NPM tags:

  • next - development and RC releases
  • latest - latest stable releases from the current stable major (currently v5)
  • maintenance - the maintenance-mode releases, currently v4

see: https://www.npmjs.com/package/@node-oauth/oauth2-server?activeTab=versions

@eddy-minet-holis
Copy link

@eddy-minet-holis I published a fix as 4.3.3, feel free to test and please file an issue if this still not fixes things.

Works great in 4.3.3, thanks!

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

Successfully merging this pull request may close these issues.

6 participants