Skip to content

Insufficient integration tests #219

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

Closed
jankapunkt opened this issue Aug 15, 2023 · 3 comments · Fixed by #224
Closed

Insufficient integration tests #219

jankapunkt opened this issue Aug 15, 2023 · 3 comments · Fixed by #224
Assignees
Labels
bug 🐛 Something isn't working in progress 👨‍💻 Currently in progress and is being worked on. security ❗ Address a security issue tests 🧪 Relates to tests

Comments

@jankapunkt
Copy link
Member

Specify your setup

  • version of @node-oauth/oauth2-server: 5.0.0-rc.1

Describe the bug

The integration tests do not properly cover the regressions, introduced by the switch to native async/await. This is due to the original integration tests do not really test, whether the correct values are passed to the model as most model functions are not covering parameters or return nearly nothing.

This already resulted in:

Proposal:

All integration tests should also cover proper model testing.

We should target this for 5.0.0, because there may be more parts affected as may have been revealed yet.

@jankapunkt jankapunkt added bug 🐛 Something isn't working security ❗ Address a security issue tests 🧪 Relates to tests labels Aug 15, 2023
@jankapunkt jankapunkt added this to the v5 milestone Aug 15, 2023
@jankapunkt jankapunkt self-assigned this Aug 15, 2023
@jankapunkt jankapunkt added the in progress 👨‍💻 Currently in progress and is being worked on. label Aug 17, 2023
@jankapunkt jankapunkt linked a pull request Aug 17, 2023 that will close this issue
@jankapunkt
Copy link
Member Author

This is partially implemented via #224 but we will add another PR with the rest

@jankapunkt
Copy link
Member Author

Whoever wants to go to implement this, we basically need in the folder /test/compliance another test implementation for authorization_code_grant_type (acgt) workflow.

Fortunately we got the other types already covered so you can take a look at the implementations in these folders and use the techniques you find in there.

Beware, that the acgt is the most complex type of all workflows and requires very thorough implementation for different scenarios (early rejections, full success, refresh etc.).

We will carefully review this one and may even add a commit here and there to support this.

@jankapunkt
Copy link
Member Author

Another compliance test would be, on top of authorization_code_grant_type, to have a fully compliant pkce test setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working in progress 👨‍💻 Currently in progress and is being worked on. security ❗ Address a security issue tests 🧪 Relates to tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant