-
Notifications
You must be signed in to change notification settings - Fork 81
32105: DBC undo specific AA/AC DBC access check #4016
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request reverts the account-based access control functionality for digital credentials that was previously added in PRs #3820 and #3860. The changes restore the original access control logic that relied solely on filing party roles and business party roles, removing the additional check for ADMIN/COORDINATOR account roles.
Changes:
- Removed
user_has_account_rolemethod and associated feature flagdbc-enable-account-based-access - Reverted
_has_specific_accessmethod to only check filing and business party roles - Removed all tests related to account-based access functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| legal-api/tests/unit/services/test_digital_credentials_rules.py | Removed account role mock patches from test signatures and deleted all account-based access test cases |
| legal-api/tests/unit/services/test_digital_credentials_helpers_and_utils.py | Removed tests for is_account_based_access_enabled function and updated import statement |
| legal-api/src/legal_api/services/digital_credentials_utils.py | Removed DBC_ENABLE_ACCOUNT_BASED_ACCESS_FLAG constant and is_account_based_access_enabled function |
| legal-api/src/legal_api/services/digital_credentials_rules.py | Removed user_has_account_role method, removed HTTP/requests/jwt imports, and reverted access check logic |
| legal-api/flags.json | Removed dbc-enable-account-based-access feature flag |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
78a29ba to
ce89e6e
Compare
Signed-off-by: Lucas <lucasoneil@gmail.com>
ce89e6e to
11d0790
Compare
| "dbc-enabled-business-types": { | ||
| "types": ["SP"] | ||
| }, | ||
| "dbc-enable-account-based-access": false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment in the release ticket to archive this FF once this commit is released to Prod.
severinbeauvais
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, that was a surprising amount of stuff deleted. LGTM.
Issue #: /bcgov/entity#32105
Description of changes:
This is undoing the changes added in #3820 and the feature flag for it in #3860
There is a FF introduced for this that's never been needed so will coordinate to remove that from LD once this code that removes it is in production
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).