From 4a9eb7b22674ab87513c6e92012bf22a268847d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Sun, 10 Oct 2021 17:57:10 +0200 Subject: [PATCH 1/9] docs: added contribution guide draft --- CONTRIBUTING.md | 119 ++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 12 ++--- 2 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..2a87c47 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,119 @@ +# Contributing to @node-oauth/oauth2-server + +Thank you for your interest in this project and your aims to improving it. +This guide will give you the most important info on how to contribute properly +in order to get your pull requests accepted. + +## Disclose security vulnerabilities + +First things first: +This project has strong security implications and we appreciate every help to +improve security. + +**However, please read our [security policy](./SECURITY.md), before taking +actions.** + +## Development + +If you want to fix bugs or add new features, please clone the source via + +```bash +$ npm run test +``` + +### No PR without issue + +Please make sure your commitment will be appreciated by first opening an issue +and discuss, whether this is a useful addition to the project. + + +### Run the tests + +Please always make sure your code is passing linter and tests **before** +committing. By doing so you help to make reviews much easier and don't pollute +the history with commits, that are solely targeting lint fixes. + +You can run the tests via + +```bash +$ npm run test +``` + +or + +```bash +$ npm run test:coverage +``` + +to see your coverage. + +### Open a pull request (PR) + +Once you have implemented your changes and tested them locally, please open +a [pull request](https://docs.github.com/en/github/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request). + +Note: sometimes a pull request (PR) is also referred to as merge request (MR). + +#### Fundamental PR requirements + +There are a few basic requirements for your pull request to become accepted: + +- Make sure to open your pull request to target the `development` branch and not +`master` +- Make sure you are working on a branch, other than `development`; usually you + can name the branch after the feature or fix you want to provide +- Resolve any merge conflicts (usually by keeping your branch updated with + `development`) +- Have a clear description on what the PR does, including any steps necessary + for testing, reviewing, reproduction etc. +- Link to the existing issue +- Added functions or changed functions need to get documented in compliance with + JSDoc +- Make sure all CI Tests are passing + +Also make sure, to comply with the following list: + +- Do not work on `development` directly +- Do not implement multiple features in one pull request (this includes bumping + versions of dependencies that are not related to the PR/issue) +- Do not bump the release version (unless you are a maintainer) +- Do not edit the Changelog as this will be done after your PR is merged +- Do not introduce tight dependencies to a certain package that has not been + approved during the discussion in the issue + +#### Review process + +Finally your PR needs to pass the review process: + +- A certain amount of maintainers needs to review and accept your PR +- Please **expect change requests**! They will occur and are intended to improve + the overall code quality. +- If your changes have been updated please re-assign the reviewer who asked for + the changes +- Once all reviewers have approved your PR it will be merged by one of the + maintainers :tada: + +## For maintainers + +### When to release a new version? + +- on fixed vulnerabilities +- on fixed dependency-vulnerabilites +- on new added features +- what else? + +### When to decide between major, minor and path release? + +- major = breaking +- minor = features and security fixes +- patch = general fixes and small improvements + +### How to release a new version? + +What's required to publish to npm, which branches are involved, what should not +be done etc. + +## Become a maintainer + +What is required to become a maintainer? + \ No newline at end of file diff --git a/README.md b/README.md index 3862c3e..1dd27cd 100644 --- a/README.md +++ b/README.md @@ -40,12 +40,8 @@ This module has been rewritten using a promise-based approach, introducing chang Please refer to our [3.0 migration guide](https://oauth2-server.readthedocs.io/en/latest/misc/migrating-v2-to-v3.html) for more information. +## Contributing to this project -## Tests - -To run the test suite, install dependencies, then run `npm test`: - -```bash -npm install -npm test -``` +Please read our [contribution guide](./CONTRIBUTING.md) before taking actions. +In any case, please open an issue before opening a pull request to find out, +whether your intend to contribute will actually have a chance to be merged. From 172203985021418b87f416786983a9f4d2556a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Mon, 11 Oct 2021 09:48:56 +0200 Subject: [PATCH 2/9] updated development guidelines --- CONTRIBUTING.md | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2a87c47..13ce4a8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,22 +15,44 @@ actions.** ## Development -If you want to fix bugs or add new features, please clone the source via - -```bash -$ npm run test -``` +If you want to fix bugs or add new features, **please read this chapter and it's +sections carefully!** ### No PR without issue Please make sure your commitment will be appreciated by first opening an issue and discuss, whether this is a useful addition to the project. +### Work on a bug or a new feature -### Run the tests +First, clone and install this project from source via -Please always make sure your code is passing linter and tests **before** -committing. By doing so you help to make reviews much easier and don't pollute +```bash +$ git clone git@github.com:node-oauth/node-oauth2-server.git +$ cd node-oauth2-server +$ git checkout developmemt # important! do not work on master! +$ npm install +``` + +From here you can run several scripts for development purposes: + +```bash +$ npm run lint # runs the linter +$ npm run tets # runs the tests once +$ npm run tests:coverage # runs the tests including coverage +$ npm run docs # generates the API docs +``` + +To work on a new feature or a fix please create a new branch: + +```bash +$ git checkout -b feature-xyz # or fix-xyz +``` + +### Run the tests before committing + +Please always make sure your code is passing linter and tests **before +committing**. By doing so you help to make reviews much easier and don't pollute the history with commits, that are solely targeting lint fixes. You can run the tests via @@ -95,6 +117,9 @@ Finally your PR needs to pass the review process: ## For maintainers +Maintainers of this repository have an extended responsibility for security and +integrity. Therefore you have to take extra care on preparing publishing. + ### When to release a new version? - on fixed vulnerabilities From 6a0c93c06af8afbfae8e1d059c7a6d5bfd32182f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Mon, 11 Oct 2021 10:26:01 +0200 Subject: [PATCH 3/9] contribution guidelines added guiding principles --- CONTRIBUTING.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 13ce4a8..e81dd21 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,26 @@ improve security. **However, please read our [security policy](./SECURITY.md), before taking actions.** + + +## Guiding principles + +Before contributing to this project it is important to understand how this +project and it's collaborators views itself regarding it's scope and purpose. + +### OAuth2 standard compliance + +This project aims full standard compliance. All improvements on functionality, +as well as security implications, are done in a way that the standard remains +as the highest reference of choice. + +### Framework agnostic + +Design decisions and implementations are always done with keeping in mind, that +there are multiple frameworks out there that use this project. + + + ## Development If you want to fix bugs or add new features, **please read this chapter and it's @@ -115,6 +135,8 @@ Finally your PR needs to pass the review process: - Once all reviewers have approved your PR it will be merged by one of the maintainers :tada: + + ## For maintainers Maintainers of this repository have an extended responsibility for security and From c833a373854e4376ea2300c1664d3e772fe6e61c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Mon, 11 Oct 2021 10:28:14 +0200 Subject: [PATCH 4/9] pull request template added --- .github/PULL_REQUEST_TEMPLATE.md | 65 ++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..987e909 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,65 @@ + + +## Summary + + + + +## Linked issue(s) + + + + +## Involved parts of the project + + + + +## Added tests? + + + + +## OAuth2 standard + + + + +## Reproduction + + From de41dc7c5fe8e5a3cd1d78ae65e3fd78bfce09c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Mon, 11 Oct 2021 15:08:25 +0200 Subject: [PATCH 5/9] docs removed maintainers section from contribution guide --- CONTRIBUTING.md | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e81dd21..6b3471c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -26,6 +26,18 @@ This project aims full standard compliance. All improvements on functionality, as well as security implications, are done in a way that the standard remains as the highest reference of choice. +If you are not familiar with the OAuth2 standards, please consult at least the +following documents: + +- [RFC 6749 - The OAuth 2.0 Authorization Framework](https://datatracker.ietf.org/doc/html/rfc6749) +- [RFC 8252 - OAuth 2.0 for Native Apps](https://datatracker.ietf.org/doc/html/rfc8252) + +Extended readings: + +- [RFC 6819 - OAuth 2.0 Threat Model and Security Considerations](https://datatracker.ietf.org/doc/html/rfc6819) +- [RFC 7636 - Proof Key for Code Exchange by OAuth Public Clients](https://datatracker.ietf.org/doc/html/rfc7636) +- [RFC 7591 - OAuth 2.0 Dynamic Client Registration Protocol](https://datatracker.ietf.org/doc/html/rfc7591) + ### Framework agnostic Design decisions and implementations are always done with keeping in mind, that @@ -134,33 +146,4 @@ Finally your PR needs to pass the review process: the changes - Once all reviewers have approved your PR it will be merged by one of the maintainers :tada: - - - -## For maintainers - -Maintainers of this repository have an extended responsibility for security and -integrity. Therefore you have to take extra care on preparing publishing. - -### When to release a new version? - -- on fixed vulnerabilities -- on fixed dependency-vulnerabilites -- on new added features -- what else? - -### When to decide between major, minor and path release? - -- major = breaking -- minor = features and security fixes -- patch = general fixes and small improvements - -### How to release a new version? - -What's required to publish to npm, which branches are involved, what should not -be done etc. - -## Become a maintainer - -What is required to become a maintainer? \ No newline at end of file From ca47721071129fff3e0837665ccd585dca127128 Mon Sep 17 00:00:00 2001 From: Serguei Okladnikov Date: Tue, 12 Oct 2021 10:40:48 +0300 Subject: [PATCH 6/9] docs: add commit message convention and coding rules --- CONTRIBUTING.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6b3471c..41be0a5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,6 +81,22 @@ To work on a new feature or a fix please create a new branch: $ git checkout -b feature-xyz # or fix-xyz ``` +### Coding rules + +- Unit-testing: all features or bug fixes must be tested by specs +- Documentation: all public API methods must be documented + +### Commit message convention + +Commit title must meet [angular commit message format](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-format) +with ticket number at the end of summary: + +``` +(): # +``` +Summary in present tense. Not capitalized. No period at the end. +The and fields are mandatory, the () and # field is optional. + ### Run the tests before committing Please always make sure your code is passing linter and tests **before From 60bad87693b2f53e279f66b97329e8e5494ab4e2 Mon Sep 17 00:00:00 2001 From: Serguei Okladnikov Date: Tue, 12 Oct 2021 13:04:16 +0300 Subject: [PATCH 7/9] docs: lightly softify commit convention requirements --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 41be0a5..3106d60 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -88,7 +88,7 @@ $ git checkout -b feature-xyz # or fix-xyz ### Commit message convention -Commit title must meet [angular commit message format](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-format) +We use a commit convention, inspired by [angular commit message format](https://github.com/angular/angular/blob/master/CONTRIBUTING.md#-commit-message-format) with ticket number at the end of summary: ``` From 4985ca7d38def95d3e58092be87daa84d369a7da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Wed, 13 Oct 2021 17:57:40 +0200 Subject: [PATCH 8/9] docs: correct types and available scripts and add info about removing branch --- CONTRIBUTING.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3106d60..ebd5f9d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -69,9 +69,8 @@ $ npm install From here you can run several scripts for development purposes: ```bash -$ npm run lint # runs the linter -$ npm run tets # runs the tests once -$ npm run tests:coverage # runs the tests including coverage +$ npm run test # runs the tests once +$ npm run test:coverage # runs the tests including coverage $ npm run docs # generates the API docs ``` @@ -162,4 +161,7 @@ Finally your PR needs to pass the review process: the changes - Once all reviewers have approved your PR it will be merged by one of the maintainers :tada: - \ No newline at end of file + +#### After merge + +Please delete your branch after merge. From fcec2769de44ac7f1783cf726811487c76ba8568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=BCster?= Date: Wed, 13 Oct 2021 17:57:59 +0200 Subject: [PATCH 9/9] docs: add pull request template --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 987e909..7c49376 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -7,8 +7,8 @@ We highly appreciate your time and effort to this project! âš  PLEASE READ THIS FIRST âš  -1. If this is a fix for a security vulnerability you discorvered please don't -just open this PR until we have privatey discussed the vulnerability. Disclosing +1. If this is a fix for a security vulnerability you discovered please don't +just open this PR until we have privately discussed the vulnerability. Disclosing it without contacting us can lead to severe implications for many applications that run on this project.