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

Scan level inheritance chain #722

Merged
merged 11 commits into from
Apr 14, 2023
Merged

Conversation

Lisser
Copy link
Contributor

@Lisser Lisser commented Apr 12, 2023

Changes

Implements:

  • Octopoes: on-demand calculation of inheritance chain of an OOI
  • Rocky: basic visualization

Issue ticket number and link

#501

Proof

Screenshot 2023-04-12 at 23 14 31

Extra instructions for others

This section may be skipped or omitted. Uncomment and answer the below questions if relevant.

Checklist for author(s):

  • All the commits in this PR are properly PGP-signed and verified;
  • This PR comes from a feature or hotfix branch, in line with our git branching strategy;
  • This PR is "bite-sized" and only focuses on a single issue, problem, or feature;
  • I am not reinventing the wheel: there is no high-quality library that already has this feature;
  • I have changed the example .env files if I added, removed, or changed any config options, and I have informed others that they need to modify their .env files if required;
  • I have performed a self-review of my own code;
  • I have commented my code, particularly in hard-to-understand areas;
  • I have made corresponding changes to the documentation, if necessary;
  • I have written unit, integration, and end-to-end tests for the change that I made;

If a non-trivial PR:

  • This PR is part of a milestone and has appropriate labels;
  • This PR is properly linked to the project board (either directly or via an issue);
  • I have added screenshots or some other proof that my code does what it is supposed to do;
## Checklist for functional reviewer(s):
- [ ] If a non-trivial PR: This PR is properly linked to an issue on the project board;
- [ ] I have checked out this branch, and successfully ran `make kat`;
- [ ] I have ran `make test-rf` and all end-to-end Robot Framework tests pass;
- [ ] I confirmed that the PR's advertised `feature` or `hotfix` works as intended;
- [ ] I confirmed that there are no unintended functional regressions in this branch;

### What works:
* _bullet point + screenshot (if useful) per tested functionality_

### What doesn't work:
* _bullet point + screenshot (if useful) per tested functionality_

### Bug or feature?:
* _bullet point + screenshot (if useful) if it is unclear whether something is a bug or an intended feature._
## Checklist for code reviewer(s):
- [ ] The code passes the CI tests and linters;
- [ ] The code does not bypass authentication or security mechanisms;
- [ ] The code does not introduce any dependency on a library that has not been properly vetted;
- [ ] The code does not violate Model-View-Template and our other architectural principles;
- [ ] The code contains docstrings, comments, and documentation where needed;
- [ ] The code prioritizes readability over performance where appropriate;
- [ ] The code conforms to our agreed coding standards.

@Lisser Lisser requested a review from a team as a code owner April 12, 2023 21:16
@github-actions
Copy link
Contributor

github-actions bot commented Apr 12, 2023

File Coverage
All files 65%
bits/definitions.py 65%
bits/runner.py 56%
bits/https_availability/https_availability.py 93%
bits/oois_in_headers/oois_in_headers.py 57%
bits/spf_discovery/internetnl_spf_parser.py 55%
bits/spf_discovery/spf_discovery.py 72%
octopoes/api/api.py 89%
octopoes/api/models.py 75%
octopoes/api/router.py 52%
octopoes/core/app.py 69%
octopoes/core/service.py 46%
octopoes/events/events.py 96%
octopoes/events/manager.py 65%
octopoes/models/__init__.py 86%
octopoes/models/datetime.py 66%
octopoes/models/exception.py 83%
octopoes/models/origin.py 70%
octopoes/models/path.py 99%
octopoes/models/types.py 95%
octopoes/models/ooi/certificate.py 96%
octopoes/models/ooi/email_security.py 95%
octopoes/models/ooi/findings.py 94%
octopoes/models/ooi/network.py 97%
octopoes/models/ooi/service.py 91%
octopoes/models/ooi/software.py 71%
octopoes/models/ooi/web.py 81%
octopoes/models/ooi/dns/records.py 95%
octopoes/models/ooi/dns/zone.py 77%
octopoes/repositories/ooi_repository.py 40%
octopoes/repositories/origin_parameter_repository.py 52%
octopoes/repositories/origin_repository.py 52%
octopoes/repositories/scan_profile_repository.py 45%
octopoes/xtdb/client.py 39%
octopoes/xtdb/query_builder.py 69%
octopoes/xtdb/related_field_generator.py 73%
tests/conftest.py 91%

Minimum allowed coverage is 75%

Generated by 🐒 cobertura-action against 18a5719

ammar92
ammar92 previously approved these changes Apr 13, 2023
@Darwinkel
Copy link
Contributor

Darwinkel commented Apr 13, 2023

Checklist for functional reviewer(s):

  • If a non-trivial PR: This PR is properly linked to an issue on the project board;
  • I have checked out this branch, and successfully ran make kat;
  • I have ran make test-rf and all end-to-end Robot Framework tests pass;
  • I confirmed that the PR's advertised feature or hotfix works as intended;
  • I confirmed that there are no unintended functional regressions in this branch;

What works:

  • UI/UX for showing the inheritance chain for objects with scan level >0

image

What doesn't work:

  • Clicking Show clearance level inheritance refreshes the page but doesn't appear to render anything when scan level is 0 or if it concerns a Finding. Maybe only show the link for objects with scan level >0?
    image

  • Clicking Show clearance level inheritance when an object has been changed from Inherited to Declared, results in very long loading times in some cases (but not all).

  • Setting clearance level back to inherit after being declared throws a template error (caused by a previously merged PR perhaps?)
    image

Bug or feature?:

  • When the clearance level of an object is manually changed to a higher clearance than the one that was inherited (e.g. declaring level 3 while inheriting 2), the inheritance list is empty. Makes sense I suppose, but this behaviour is not made clear to users.

@underdarknl
Copy link
Contributor

This calculates a path to a declaration, once a declaration has been set on the object itself, it becomes a rather short path indeed. A info message for the user would be logical.

@Lisser
Copy link
Contributor Author

Lisser commented Apr 14, 2023

Clicking Show clearance level inheritance refreshes the page but doesn't appear to render anything when scan level is 0 or if it concerns a Finding. Maybe only show the link for objects with scan level >0?

Fixed it by only showing the explanation block for scan_profile_type == 'inherited'

When the clearance level of an object is manually changed to a higher clearance than the one that was inherited (e.g. declaring level 3 while inheriting 2), the inheritance list is empty. Makes sense I suppose, but this behaviour is not made clear to users.

Fixed with change above

Clicking Show clearance level inheritance when an object has been changed from Inherited to Declared, results in very long loading times in some cases (but not all).

Loading times are indeed unpredictable and acceptable for now

Setting clearance level back to inherit after being declared throws a template error (caused by a previously merged PR perhaps?)

Separate issue, already fixed

# Conflicts:
#	octopoes/octopoes/api/router.py
#	octopoes/octopoes/connector/octopoes.py
#	rocky/rocky/views/ooi_detail.py
@dekkers dekkers merged commit 746ad3b into main Apr 14, 2023
@dekkers dekkers deleted the feature/octopoes-explanation-endpoint branch April 14, 2023 15:41
jpbruinsslot added a commit that referenced this pull request Apr 17, 2023
* main:
  Update `pre-commit` dependencies and enable Ruff autofix (#739)
  Cleanup Rocky requirements (#729)
  Add (I) to ruff and fix imports (#723)
  Add port-common bit and KAT-OPEN-COMMON-PORT FindingType (#734)
  Use setuptools-scm to write correct version to version.py (#737)
  Enable and disable bits (#732)
  Scan level inheritance chain (#722)
  Octopoes origin param endpoint (#731)
  Do not propagate scan level from DNSSPFMechanismHostname (#721)
  Fix: Unnecessary white space within links (#728)
  chore/refactor katalogus settings setup (#542)
jpbruinsslot added a commit that referenced this pull request Apr 17, 2023
* main:
  Update `pre-commit` dependencies and enable Ruff autofix (#739)
  Cleanup Rocky requirements (#729)
  Add (I) to ruff and fix imports (#723)
  Add port-common bit and KAT-OPEN-COMMON-PORT FindingType (#734)
  Use setuptools-scm to write correct version to version.py (#737)
  Enable and disable bits (#732)
  Scan level inheritance chain (#722)
  Octopoes origin param endpoint (#731)
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