Pentest - #3308
Draft
mattjreynolds wants to merge 35 commits into
Draft
Pentest#3308mattjreynolds wants to merge 35 commits into
mattjreynolds wants to merge 35 commits into
Conversation
Topic and metric documnetation child pages are non-public capable pages which therefore must have the non-public tick box, page classification, theme, sub-theme, topic, and metric associated with them to allow us to filter based on a user's permissions if the page is set to non-public. In the original implementation of this, there was a bug where the metric field on the documentation child page model was being reused as it already existed. Conceptually this is incorrect as it should have been a separate field, but also it broke the metric child pages as they were expecting the metric to be stored as the full name of the metric whereas the non-public logic was expecting an ID. To solve this, the metric field has been removed because it's not currently in scope for page permissions. However, to future proof this and also reduce repeated code (the topic and metric documentation child models have the same code copied in both) a new non-public abstract page model has been created which carries the core fields required for a non-public page. The topic and metric documentation child page models are no subclasses of this page. As mentioned, the metric field for non-pub page permissions has been removed rather than left half implemented. Additionally, the theme, sub-theme, and topic fields have been renamed with the prefix `page_` to avoid clashing with possible fields on subclasses. This aligns with the `page_classification` that already existed. On top of this, the admin form for non-public fields has been centralised into a class as well to again avoid copied code. This presents the fields slightly differently to try and make the DPD editor user experience better - the public checkbox still shows as before but the classification, theme, sub-theme, and topic are grouped together and collapsed by default to reduce clutter when they are likely only to be set once and not looked at again. These could be moved into a tab potentially if we wanted to but for the moment this change is better than before and makes it clearer for users. There is also a performance fix within this commit which I tried to split out but it didn't work as it's related to the changes described above. Specifically, the populating of choices on the metric documentation child model is moved from the `__init__` method to the editor's `__init__`. This means we don't pull metric names from the db on every instantiation of a new metric documentation object (which for example could happen 80+ times (depends how many metric documentation child pages you have) just through the get pages endpoint (because we use `.specific()`). Finally, to facilitate the field changes on the topic and metric documentation child pages, migrations have been created. These are written as renames to try and avoid data loss. However, any metric documentation child pages which were created with the old code will need to fixed as their database entries will be incorrect (they'll be using an ID in the `metric` column instead of a string name). This has been fixed in the bootstrap code as well as the issue was present there too.
If this class is inheritied from that doesn't mean the page is NonPublic, it means it can be, so this is a better name.
…ild page unit test
…opying the most recent wagtail.fields.StreamField and inserting theme and sub_theme into it)
…e_topic_and_metric_id_by_name() and get_geography_type_id_and_code_by_name() so that any inconsistent combination is denied access
* Update theme functionality to pull available themes from the db via the metrics interface. * CDD-3175: added endpoints for retrieving subtheme/topics/metrics and wired up javascript to call endpoints. * Update the model and the permission_set javascript when handling wildcard selection * Update to add serializer to handle request and response for subthemes and update to subthemes to handle querying db * CDD-3175: updated the JS to add wildcard and empty object options * CDD-3175: Updated the topics and metrics endpoints to retrieve data from the DB * CDD-3175: wired up the logic for selecting geography types * CDD-3175: update permission set for geographies * CDD-3175: updates for limiting the creation of duplicate permission sets * CDD-3175: updates for handling the naming of permission sets * CDD-3085: updated validations and wildcard functionality * CDD-3175: update migrations and add tidy up javascript and validation * CDD-3175: remove console logs from javascript * CDD-3175: Update PermissionSet model * CDD-3175: Update wagtail hooks * CDD-3175: remove print statements and tidy up field_choice_callables * CDD-3175: Update method descriptions * CDD-3175: tidied up the geography serializer * CDD-3175: formatting * Update documentation * CDD-3175: linting * CDD-3175: tests * CDD-3176: Add initial model * CDD-3175: Initial Commit * Create initial permission set * Add conditional sub_theme dropdown * Update migration file and tidy up child_theme.js * pip: (deps): bump python-dotenv from 1.2.1 to 1.2.2 Bumps [python-dotenv](https://github.com/theskumar/python-dotenv) from 1.2.1 to 1.2.2. - [Release notes](https://github.com/theskumar/python-dotenv/releases) - [Changelog](https://github.com/theskumar/python-dotenv/blob/main/CHANGELOG.md) - [Commits](theskumar/python-dotenv@v1.2.1...v1.2.2) --- updated-dependencies: - dependency-name: python-dotenv dependency-version: 1.2.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * Testing dummy secret with gitleaks * Testing dummy secret with gitleaks * Added secret scan to the existing action.yaml * Reverted to script installation of gitleaks * Changed job name * Changed ubuntu version * Using official gitleaks action * Updated ubuntu version * Gitleaks arg removed * CDD-3175: populate the Topic dropdown * Update theme functionality to pull available themes from the db via the metrics interface. * Update to add serializer to handle request and response for subthemes and update to subthemes to handle querying db * CDD-3175: updated the JS to add wildcard and empty object options * CDD-3175: Updated the topics and metrics endpoints to retrieve data from the DB * CDD-3085: updated validations and wildcard functionality * WIP: Separate model files and create permission set block * add name back in * working draft * Split models into two files * CDD-3175: removed duplicate functionality * CDD-3175: refactored naming of endpoints * Add unit testing * CDD-3175: update to fix wildcard selection * Simplified version * remove old code * Remove old code * CDD-3175: update for PR comments * CDD-3175: update for PR comments * remove old file * CDD-3175: Update method annotation * CDD-3175: Update method annotation * Update checkboxes * Linting fixes * remove merge issue * linting things * CDD-3175: Update urls for permission set endpoints * CDD-3176: remove duplicated tests * CDD-3172: Update to add the functionality for retrieving user permission sets * CDD-3172: linting * CDD-3175: Update to add ability to get by id and to create initial permission set hierarchy * CDD-3175: add group by functionality * CDD-3172: small refactor of permission_hierarchy and users and topics * Remove testing changes to truncated_dataset * Remove group by geography * refactor permission grouping to group by id rather than name * CDD-3172: linting * CDD-3172: tests * CDD-3172: tests * CDD-3172: tests and refactoring * CDD-3172: Update response format * sonar feedback: update based on sonarqube output * sonar feedback: update based on sonarqube output * Linting * CDD-3171: Update permission set form now it's a page not a snippet * Update topic page to include theme/subtheme/topic fields * WIP: filter getPages based on is_public field * Move auth content underneath CMS * Expose themes/subthemes/topics on topic and metric doc child pages * CDD-3172: move class for blocks * WIP: Add theme/subtheme/topic to pages * linting and permission set url changes * CDD-3172: refactored naming of geography method and updated the tests based on feedback. * CDD-3172: updated test to better name test and updated permission hierarchy error * CDD-2172: Add examples for each of the potential responses for get permissions sets hierarchy requests * CDD-2172: linting * CDD-3147: Update Cognito User for permission sets Add permission sets to request.user object Make cognito user ephemeral for speed - DB access is not needed Move auth header name to settings for flexibility * CDD-3147: Improve logging of JWT * CDD-3147: Update readme for using JWT locally * CDD-3147: Update readme for using JWT locally * CDD-3119 Add a new SimpleMenu model. This is a simplified version of the current Menu model where menus are now just simple links with a title. * CDD-3119 Add panels attribute to SimpleMenu model. * CDD-3119 Beef up the SimpleMenu serializer tests. * CDD-3232 Update chart response styles. Bar and line charts now have borders and grid lines on both axis. * pip dev: (deps-dev): bump pre-commit from 4.5.1 to 4.6.0 Bumps [pre-commit](https://github.com/pre-commit/pre-commit) from 4.5.1 to 4.6.0. - [Release notes](https://github.com/pre-commit/pre-commit/releases) - [Changelog](https://github.com/pre-commit/pre-commit/blob/main/CHANGELOG.md) - [Commits](pre-commit/pre-commit@v4.5.1...v4.6.0) --- updated-dependencies: - dependency-name: pre-commit dependency-version: 4.6.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * pip: (deps): bump idna from 3.11 to 3.12 Bumps [idna](https://github.com/kjd/idna) from 3.11 to 3.12. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](kjd/idna@v3.11...v3.12) --- updated-dependencies: - dependency-name: idna dependency-version: '3.12' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * pip dev: (deps-dev): bump gitpython from 3.1.46 to 3.1.47 Bumps [gitpython](https://github.com/gitpython-developers/GitPython) from 3.1.46 to 3.1.47. - [Release notes](https://github.com/gitpython-developers/GitPython/releases) - [Changelog](https://github.com/gitpython-developers/GitPython/blob/main/CHANGES) - [Commits](gitpython-developers/GitPython@3.1.46...3.1.47) --- updated-dependencies: - dependency-name: gitpython dependency-version: 3.1.47 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com> * pip: (deps): bump click from 8.3.2 to 8.3.3 Bumps [click](https://github.com/pallets/click) from 8.3.2 to 8.3.3. - [Release notes](https://github.com/pallets/click/releases) - [Changelog](https://github.com/pallets/click/blob/main/CHANGES.rst) - [Commits](pallets/click@8.3.2...8.3.3) --- updated-dependencies: - dependency-name: click dependency-version: 8.3.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * pip: (deps): bump psycopg2-binary from 2.9.10 to 2.9.12 Bumps [psycopg2-binary](https://github.com/psycopg/psycopg2) from 2.9.10 to 2.9.12. - [Changelog](https://github.com/psycopg/psycopg2/blob/master/NEWS) - [Commits](psycopg/psycopg2@2.9.10...2.9.12) --- updated-dependencies: - dependency-name: psycopg2-binary dependency-version: 2.9.12 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * pip: (deps): bump pydantic from 2.13.2 to 2.13.3 Bumps [pydantic](https://github.com/pydantic/pydantic) from 2.13.2 to 2.13.3. - [Release notes](https://github.com/pydantic/pydantic/releases) - [Changelog](https://github.com/pydantic/pydantic/blob/main/HISTORY.md) - [Commits](pydantic/pydantic@v2.13.2...v2.13.3) --- updated-dependencies: - dependency-name: pydantic dependency-version: 2.13.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> * build: remove simplejson dependency Saw a dependabot update for this package and in investigating if it was safe to update realised we're not actually using it... * CDD-3313: Add topic page link to headline metrics card (#3151) * pip: (deps): bump idna from 3.12 to 3.13 Bumps [idna](https://github.com/kjd/idna) from 3.12 to 3.13. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](kjd/idna@v3.12...v3.13) --- updated-dependencies: - dependency-name: idna dependency-version: '3.13' dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * topics: add HIV topic * CDD-3087: new CMS page for logged-out functionality (#3163) * pip: (deps): bump filelock from 3.28.0 to 3.29.0 Bumps [filelock](https://github.com/tox-dev/py-filelock) from 3.28.0 to 3.29.0. - [Release notes](https://github.com/tox-dev/py-filelock/releases) - [Changelog](https://github.com/tox-dev/filelock/blob/main/docs/changelog.rst) - [Commits](tox-dev/filelock@3.28.0...3.29.0) --- updated-dependencies: - dependency-name: filelock dependency-version: 3.29.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * WIP: pseudo code / note form of solution * WIP: filter pages on permission sets * Permission check updates and form handling * WIP: Fix comparison function * Finish getPages endpoint * CDD-3172: Remove permission_sets from CMS API * CDD-3172: Update docstring * remove redundant code * fixes for existing unit tests * Update imports * add endpoint back in for testing * fix import * New tests, and fixes and updates to existing tests * Naming fixes * test coverage * CDD-3171: Tweaks * CDD-3171: Add display name to permission sets * remove log file * Fix js file * Linting * Update migration * refactor for sonarqube checks * Fix constraints on permission sets * update unit tests * fix allowed_pages overwrite * Fix unit test * Fix test * linting * CDD-3171: Move permission_set.js insert to Media class * CDD-3171: Add ignores for importlint This could do with some more time spent on it to simplify the imports being ignored * Update architectural constraints * linting * Update wildcard value in viewsets * Update test * remove comment * Update import * fix test name * linting * combine imports * refactor for simplicity * linting * CDD-3173: prototype authorization curl call on /api/downloads/v2 * CDD-3173: get rid of check_permissions_by_name() and make /api/downloads/v2 use check_permissions() instead * CDD-3173: let cms/dashboard/viewsets.py from CDD-3171 use my fully equivalent check_permissions() function instead * CDD-3173: add debugging code to user_manager.py to be able to test this JIRA ticket in isolation * CDD-3173: evaluate metric- and geography-related permissions separately * CDD-3173: lint * CDD-3173: lint * CDD-3173: convert permission function arguments into named arguments (safer) * CDD-3173: move things around for architectural constraints * CDD-3173: reduce noisy comments * CDD-3173: remove rbac_permissions parameter (easy to do, cos doesn't cause other failures) * Pass permission set array to check permissions * CDD-3174: re-added noisy comment * CDD-3174: formalize comment * CDD-3174: revert rbac_permissions warning fix * CDD-3174: revert every logger.info("Entered function ...) * Add logs for permission sets * CDD-3174: revert SQL printing * CDD-3174: revert permission set debugging * CDD-3174: add type hints to permission functions & vars * CDD-3174: make permissions disallow empty "" requests * Update test mock and linting * CDD-3174: centralize WILDCARD_ID_VALUE * CDD-3174: use MetricsAPIInterface to access data mappers from common folder * CDD-3174: remove duplicate check_metric_related_permissions() call * CDD-3174: add BaseRequestParams() class that all other request classes inherit from * Update log * Update log line * CDD-3174: add named arguments to check_permissions() * CDD-3174: fix viewsets.py error since merge * CDD-3174: simplify convoluted WHERE clauses in SQL * CDD-3174: add _get_id_string_or_none() to normalize strings to prevent unintentional "None" == "None" string comparisons * CDD-3174: separate check_page_permissions() and check_chart_permissions() which allows all arguments to be mandatory (makes behaviour more predictable) * CDD-3174: add tests for permission filtering functionality * CDD-3174: move TestCheckPagePermissions() from test_viewsets.py to test_permissions.py * CDD-3174: log user permissions for every API call that comes with a JWT * CDD-3174: make comment more prominent (so it can't be missed) * CDD-3174: simplify permission logging * CDD-3174: avoid illogical ChartRequestParams(ChartRequestParams) class name * CDD-3174: simplify to "if permission_sets and check_chart_permissions_by_name()" * CDD-3174: remove 2 redundant check_chart_permissions() tests * CDD-3174: add docstrings to functions * CDD-3174: remove duplicate sanity check * CDD-3174: build permission set from the non-public instead of the public recordset * CDD-3174: fix type hints * CDD-3174: call log_user_permission_summary() not on every API call anymore * CDD-3174: fix lint * CDD-3174: protect against geographies with the same name across geography types * CDD-3174: fix geography code bug * CDD-3174: remove old RBAC @require_authorisation decorator from DownloadsView.post --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Luke Towell <luke.towell@burendo.com> Co-authored-by: Luke Towell <luke.towell@ukhsa.gov.uk> Co-authored-by: Kathryn Dale <kathryn.dale@burendo.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: abdihakim92x1 <AbdiHakim.Mohamed@ukhsa.gov.uk> Co-authored-by: Matt Reynolds <18287679+mattjreynolds@users.noreply.github.com> Co-authored-by: David Logie <david.logie@ukha.gov.uk> Co-authored-by: Josh Humphries <josh.humphries@ukhsa.gov.uk> Co-authored-by: Taiwo Kareem <13158672+tushortz@users.noreply.github.com> Co-authored-by: Aidan Skinner <aidan@skinner.me.uk> Co-authored-by: itsthatianguy <ian.j.rufus@gmail.com> Co-authored-by: kathryn-dale <161315272+kathryn-dale@users.noreply.github.com> Co-authored-by: sahmed06 <58435820+sahmed06@users.noreply.github.com>
…fields & the new strict permission name-to-id requirements
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not to be merged, just a placeholder for the code going into pentest.