tg icon indicating copy to clipboard operation
tg copied to clipboard

Issue #2444 - Security Matrix: security tokens + auditability

Open oleh-maikovych opened this issue 2 months ago • 0 comments

Resolve #2444

To be completed by the pull request creator

This section should be completed with reference to section Preparing PR of the Code and PR reviews wiki page.

  • [x] Create the pull request as a draft by tapping the dropdown arrow on the 'Create pull request' button under the pull request description (below the text box where this description is being edited) and changing the default Create pull request to Draft pull request. Or, if the pull request has already been created, convert it to draft by tapping the "Convert to draft" link beneath the "Reviewers" section.

  • [x] A self-review of all changes has been completed, and the changes are in sync with the issue requirements.

  • [x] Changes to the requirements have been reflected in the issue description.

  • [x] Any "leftovers" such as sysouts, printing of stack traces, and any other "temporary" code, have been removed.

  • [x] Minor refactorings, such as renamings, extraction of constants, etc., have been addressed.

  • [x] Developer documentation (e.g., comments, Javadoc), have been provided where required.

  • [x] New Java tests have been written or existing tests adjusted, if required, to cover the new functionality.

  • [x] All existing and new Java tests pass successfully by running them with Maven.

  • [x] New Web tests have been written or existing tests adjusted, if required, to cover the new functionality.

  • [x] All existing and new Web tests pass successfully.

  • [x] Established security practices have been followed, including the existence and attribution of security tokens.

  • [x] Changes have been inspected for possible NPE situations, and the changes are sufficiently defensive.

  • [x] Correct transaction demarcation is in place, which is especially important in cases with nested transactional code (e.g., iterative data process), data streams, code shared between interactive actions and calls as part of some other execution workflows, such as background jobs.

  • [x] The correct base branch has been selected for these changes to be merged into.

  • [x] The latest changes from the base branch have already been merged into this feature branch (and tested).

  • [x] Added a change overview to the issue description or as a wiki page, referenced in the issue description. Some issues might be very descriptive and serve in place of a wiki page. In such cases consider adding label Wiki like to the issue.

  • [ ] Changes subject to performance considerations have been evaluated, and tested against production-size data if applicable.

  • [x] This pull request does contain significant changes, the section "Significant changes" below is completed and at least one Senior Software Engineer with the relevant area of expertise has been selected as reviewer.

  • [ ] The In progress label has been removed from the issue.

  • [ ] The Pull request label has been added to the issue.

  • [ ] The pull request has been made ready for review by tapping the "Ready for review" button below the list of commits on the pull request page.

Additional details

Pay attention to the following details during the PR review:

  • A new SecurityRoleAssociation becomes active automatically when it is created using the SecurityRoleAssociationDao.new_ method. This ensures that the previous behavior of creating and saving a new association is preserved.
  • The @Dependent("active") annotation was added to the role property of the SecurityRoleAssociation entity, which prevents creating a new association with an inactive UserRole entity.
  • Review how the save method was overridden in the SecurityRoleAssociationDao class.
  • The @DeactivatableDependencies({SecurityRoleAssociation.class}) annotation was added to the UserRole entity class.
  • Unit tests were updated so that new SecurityRoleAssociation instances are created using the new_ method from SecurityRoleAssociationDao.
  • Verify the usage of the @Authorise(SecurityRoleAssociation_CanRead_Token.class) and @Authorise(SecurityRoleAssociation_CanSave_Token.class) annotations.
  • SecurityRoleAssociationBatchAction and related classes were removed because all actions related to saving and removing associations were implemented in the SecurityRoleAssociationDao class.

Significant changes

This pull request contains significant changes as defined in the wiki page.

Significant changes are related to Authentication or authorisation mechanisms.

Details are as follows:

This issue changes the way SecurityRoleAssociations are managed. Removing a security role association now deactivates it instead of physically removing it from the database. Adding a new association between a security token and user roles activates it, or creates a new one if it did not previously exist. Developers can add (activate) or remove (deactivate) such associations using the SecurityRoleAssociation companion object with the addAssociations and removeAssociations methods, respectively.

To be completed by the pull request reviewer

This section should be completed with reference to section Performing PR review of the Code and PR reviews wiki page.

  • [ ] The In progress label has been added to the pull request in GitHub.

  • [ ] The issue requirements have been read and understood (along with any relevant emails and/or Slack messages).

  • [ ] The correct base branch is specified, and that base branch is up-to-date in the local source.

  • [ ] The issue branch has been checked out locally, and had the base branch merged into it.

  • [ ] All automated tests pass successfully.

  • [ ] Ensure the implementation satisfies the functional requirements.

  • [ ] Ensure that code changes are secure and align with the established coding practices, including code formatting and naming conventions.

  • [ ] Ensure that code changes are documented and covered with automated tests as applicable.

  • [ ] Ensure that code changes are well-suited for informal reasoning.

  • [ ] Ensure that changes are documented for the end-user (a software engineer in the case of TG, or an application user in the case of TG-based applications).

  • [ ] If there are significant changes (described above), special attention has been paid to them. Marked the task items in section "Significant changes" as completed to indicate that corresponding changes have been reviewed, improved if necessary, and approved.

  • [ ] The issue or issues addressed by the pull request are associated with the relevant release milestone.

To be completed by the pull request reviewer once the changes have been reviewed and accepted

  • [ ] The changes have been merged into the base branch (unless there is a specific request not to do so, e.g., they are to be released to SIT).

  • [ ] The issue branch has been deleted (unless the changes have not been merged - see above, or there is a specific request not to do so).

  • [ ] The In progress label has been removed from the pull request.

  • [ ] The Pull request label has been removed from the issue.

oleh-maikovych avatar Nov 13 '25 19:11 oleh-maikovych