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

Authorization V2: Groups of users #4503

Merged
merged 5 commits into from
May 16, 2021
Merged

Conversation

StefanFl
Copy link
Member

After the discussion on Slack, this PR invents groups of users. Groups can get roles for products and product types and so make managing authorization for a large amount of users easier.

This PR contains the class definitions, the authorization check for roles and permissions and queries for authorized product types and products. Subsequent PR's will include the remaining queries, user interface to manage groups and role definitions, API and test cases.

The changes have no effects on already existing functionality for the new authorization, they are just add-ons.

The bold elements in the class diagram are the new classes and relationships:

DefectDojo_Authorisation (2)

@valentijnscholten
Copy link
Member

Hard to get the complete overview from a diff, but two things come to mind:

  • Does something extra need to be cached during the request?
  • Does the test suite need additional tests for these groups?

@StefanFl
Copy link
Member Author

Caching: Yes, the authorization needs to read the groups for a user and Product/Product_Type. This is be cached per request.

Tests: Yes as well. Currently I did manual tests to see if groups work. When the maintainers accept the solution for groups, I will extend the unit tests for the authorization. However, the current unit tests ensure that the functionality has not changed as long as no groups are used.

@valentijnscholten
Copy link
Member

ok, great. To me it looks good, so with some unit tests we could merge it soon.

@StefanFl StefanFl marked this pull request as draft May 14, 2021 07:04
@StefanFl
Copy link
Member Author

Converted it to draft until I have written unit_tests.

@StefanFl
Copy link
Member Author

Added unit tests for authorization

@StefanFl StefanFl marked this pull request as ready for review May 15, 2021 07:47
Copy link
Member

@valentijnscholten valentijnscholten left a comment

Choose a reason for hiding this comment

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

Good addition, let's see the UI and API in next PRs

@valentijnscholten
Copy link
Member

Merging this for dogfooding and to allow Nick Cleary to build on this from dev.

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.

None yet

2 participants