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

Query non-reference fields and subclass-specific fields through path queries #2662

Merged
merged 8 commits into from
Mar 28, 2024

Conversation

Donnype
Copy link
Contributor

@Donnype Donnype commented Mar 15, 2024

Changes

This extends the path query functionality by handling cases where the path ends on a non-relation field, such as Hostname.name (non-relation field) versus Hostname.network (relation-field). Furthermore, we were having issues querying subclass-specific fields. Consider for example:

class URL(OOI):
    object_type: Literal["URL"] = "URL"

    network: Reference = ReferenceField(Network)
    raw: AnyUrl

    web_url: Reference | None = ReferenceField(WebURL, max_issue_scan_level=2, default=None)

class WebURL(OOI):
    network: Reference = ReferenceField(Network)

    scheme: WebScheme
    port: int
    path: str

class HostnameHTTPURL(WebURL):
    object_type: Literal["HostnameHTTPURL"] = "HostnameHTTPURL"

    netloc: Reference = ReferenceField(Hostname, max_issue_scan_level=2, max_inherit_scan_level=4)

With the first feature we can now query URL.web_url.scheme and with the second feature we can now query URL.web_url.netloc.

Querying non-reference fields

Before:

>>>> octopoes_api_connector.query("Network.name", valid_time, "Network|test")
*** octopoes.connector.RemoteException: KeyError: 'name'

After:

>>>> octopoes_api_connector.query("Network.name", valid_time, "Network|test")
['test']

Querying subclass-specific fields

Before:

>>>> octopoes_api_connector.query("URL.web_url.netloc", valid_time, "URL|test|https://test.com/test")
*** octopoes.connector.RemoteException: KeyError: 'netloc'

After:

>>>> octopoes_api_connector.query("URL.web_url.netloc", valid_time, "URL|test|https://test.com/security")
[Hostname(object_type='Hostname', scan_profile=None, primary_key='Hostname|test|example.com', network=Reference('Network|test'), name='example.com', dns_zone=None, registered_domain=None)]

Note for QA

This is a bit hard to QA as it is not part of regular flows yet but an internal API feature. One way to check this is trying this configuration from the ticket at http://localhost:8001/docs for both the /{client}/query and /{client}/query-many:
image

Issue link

Closes #2579

Demo

See the new tests.


Code Checklist

  • All the commits in this PR are properly PGP-signed and verified.
  • This PR only contains functionality relevant to the issue; tickets have been created for newly discovered issues.
  • I have written unit tests for the changes or fixes I made.
  • For any non-trivial functionality, I have added integration and/or end-to-end tests.
  • I have performed a self-review of my code and refactored it to the best of my abilities.

Communication

  • I have informed others of any required .env changes files if required and changed the .env-dist accordingly.
  • I have made corresponding changes to the documentation, if necessary.
  • I have included comments in the code to elaborate on what is not self-evident from the code itself, including references to issues and discussions online, or implicit behavior of an interface.

Checklist for code reviewers:

Copy-paste the checklist from the docs/source/templates folder into your comment.


Checklist for QA:

Copy-paste the checklist from the docs/source/templates folder into your comment.

@Donnype Donnype requested a review from a team as a code owner March 15, 2024 14:59
@underdarknl underdarknl added the octopoes Issues related to octopoes label Mar 18, 2024
Copy link
Contributor

@ammar92 ammar92 left a comment

Choose a reason for hiding this comment

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

Great job on this fix 👍 I have only a few remarks/ suggestions for further improvement. I also noticed several new exception throws in the implementation, which seem important, but none of these are tested. Keep up the good work!

octopoes/octopoes/models/types.py Show resolved Hide resolved
octopoes/tests/test_query.py Outdated Show resolved Hide resolved
ammar92
ammar92 previously approved these changes Mar 21, 2024
Copy link
Contributor

@ammar92 ammar92 left a comment

Choose a reason for hiding this comment

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

Looks good 👍 But please keep a note of the exceptions testing for a next iteration

@stephanie0x00
Copy link
Contributor

Checklist for QA:

  • I have checked out this branch, and successfully ran a fresh make reset.
  • I confirmed that there are no unintended functional regressions in this branch:
    • I have managed to pass the onboarding flow
    • Objects and Findings are created properly
    • Tasks are created and completed properly
  • I confirmed that the PR's advertised feature or hotfix works as intended.

What works:

From the UI

  • Onboarding, generating a report, tasks are created, objects can be viewed.

What doesn't work:

  • Opening the Katalogus in the UI gives a stack trace error as shown below.

Bug or feature?:

Based on what I read in the ticket I'd expect that the query below would give results? Could be I'm doing something wrong here.

Request:

image

Response:

image

Stack trace

This change seems to break the Katalogus in the UI:

Katalogus

Environment:


Request Method: GET
Request URL: http://127.0.0.1:8000/en/ee/kat-alogus/

Django Version: 4.2.11
Python Version: 3.11.8
Installed Applications:
['whitenoise.runserver_nostatic',
 'django.contrib.admin',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.forms',
 'django_otp',
 'django_otp.plugins.otp_static',
 'django_otp.plugins.otp_totp',
 'two_factor',
 'account',
 'tools',
 'fmea',
 'crisis_room',
 'onboarding',
 'katalogus',
 'django_password_validators',
 'django_password_validators.password_history',
 'rest_framework',
 'tagulous',
 'compressor',
 'reports',
 'knox',
 'csp']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
 'whitenoise.middleware.WhiteNoiseMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.locale.LocaleMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django_otp.middleware.OTPMiddleware',
 'rocky.middleware.auth_required.AuthRequiredMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'rocky.middleware.onboarding.OnboardingMiddleware',
 'csp.middleware.CSPMiddleware']



Traceback (most recent call last):
  File "/app/rocky/octopoes/models/types.py", line 214, in type_by_name
    return next(t for t in ALL_TYPES if t.__name__ == type_name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

During handling of the above exception (), another exception occurred:
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
    return handler(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/django/views/generic/list.py", line 154, in get
    self.object_list = self.get_queryset()
                       ^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/katalogus/views/katalogus.py", line 74, in get_queryset
    queryset = self.sort_alphabetic_ascending(self.katalogus_client.get_plugins())
                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/katalogus/client.py", line 87, in get_plugins
    return [parse_plugin(plugin) for plugin in response.json()]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/katalogus/client.py", line 87, in <listcomp>
    return [parse_plugin(plugin) for plugin in response.json()]
            ^^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/katalogus/client.py", line 227, in parse_plugin
    return parse_normalizer(plugin)
           ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/katalogus/client.py", line 207, in parse_normalizer
    produces.add(type_by_name(type_name))
                 ^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/rocky/octopoes/models/types.py", line 216, in type_by_name
    raise TypeNotFound
    ^^^^^^^^^^^^^^^^^^

Exception Type: TypeNotFound at /en/ee/kat-alogus/
Exception Value: 

@stephanie0x00 stephanie0x00 added the 😸 Review/QA feedback Review/QA feedback provided label Mar 25, 2024
@Donnype
Copy link
Contributor Author

Donnype commented Mar 25, 2024

@stephanie0x00 Should hopefully be fixed now!

@stephanie0x00
Copy link
Contributor

Yes fixed!

@underdarknl underdarknl merged commit 8fef333 into main Mar 28, 2024
21 checks passed
@underdarknl underdarknl deleted the fix/path-queries-for-subclass-specific-fields branch March 28, 2024 10:37
jpbruinsslot added a commit that referenced this pull request Apr 4, 2024
* main: (51 commits)
  Fix static files for container images/Debian packages when DEBUG is on (#2742)
  OOI selection at Aggregate report does not remember changed selection (#2619)
  fix schema errors on empty / missing schemas (#2744)
  Updated `phonenumbers` and `django-phonenumber-field` (#2757)
  Remove octopoes coverage workflow (#2755)
  Bump actions/configure-pages from 4 to 5 (#2745)
  Add xtdb-cli tool to Octopoes (#2733)
  Dont report vulnerabilities without version info of the software for snyk (#2730)
  Feature/boefjes to oci images (#2709)
  Query non-reference fields and subclass-specific fields through path queries (#2662)
  Fix in System Specific (#2732)
  Plugins overview in appendix not showing any plugins (#2694)
  Feat stepper design v2 (#2704)
  Undo project-directory in Rocky (#2734)
  Remove Docker Compose: "version" (#2718)
  Upgrade `pre-commit` hooks (#2729)
  Fix #1739 (#2705)
  Improve generate report (#2633)
  Fix critical vulnerability counter (#2712)
  Fix pdf alignment (#2674)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
octopoes Issues related to octopoes 😸 Review/QA feedback Review/QA feedback provided
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Path queries from URL to Hostname not supported
4 participants