Skip to content

[bug-fix] Permission revoked approver can still perform workflow approval tasks #27

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.

Sign up for GitHub

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

Jump to bottom
Merged
sadilchamishka merged 13 commits into from
Sep 2, 2025
Merged

[bug-fix] Permission revoked approver can still perform workflow approval tasks #27

sadilchamishka merged 13 commits into from
Sep 2, 2025

Conversation

@NutharaNR
Copy link
Contributor

@NutharaNR NutharaNR commented Sep 1, 2025

Purpose

Update pending approval task corresponding to the workflow, when the flow is updated.

Approach

For each pending state request under the changed workflow, check whether the workflow changes have affected the current state of the request. If affected:

  • READY state requests are removed and recreated according to new workflow configs.
  • RESERVED / BLOCKED state requests are checked for the eligibility to have the same state under the new configs. And if yes; a new set of tasks with RESERVED / BLOCKED is created or else a new READY state is created.

Related Issues

Related PRs

User stories

Summary of user stories addressed by this change>

Release note

Brief description of the new feature or bug fix as it will appear in the release notes

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Training

Link to the PR for changes to the training content in https://github.com/wso2/WSO2-Training, if applicable

Certification

Type “Sent” when you have provided new/updated certification questions, plus four answers for each question (correct answer highlighted in bold), based on this change. Certification questions/answers should be sent to [email protected] and NOT pasted in this PR. If there is no impact on certification exams, type “N/A” and explain why.

Marketing

Link to drafts of marketing content that will describe and promote this feature, including product page changes, technical articles, blog posts, videos, etc., if applicable

Automation tests

  • Unit tests

    Code coverage information

  • Integration tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Migrations (if applicable)

Describe migration steps and platforms on which migration has been tested

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Learning

Describe the research phase and any blog posts, patterns, libraries, or add-ons you used to solve the problem.

@NutharaNR NutharaNR mentioned this pull request Sep 1, 2025
Merged
1 task
Malith-19
Malith-19 reviewed Sep 1, 2025
View reviewed changes
@Malith-19 Malith-19 requested a review from Copilot September 2, 2025 04:01
Copilot AI reviewed Sep 2, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a 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 PR fixes a bug where permission-revoked approvers could still perform workflow approval tasks. The solution implements automatic updates to pending approval tasks when workflows are modified, ensuring that only eligible approvers can continue with approval processes.

  • Updates pending approval task structures when workflow configurations change
  • Validates existing reserved tasks against new approval configurations
  • Adds utility methods for comparing approval steps and parameter changes

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
WorkflowEngineConstants.java Adds new SQL queries and reorganizes error codes with better categorization
Utils.java New utility class with methods for processing approval step parameters and detecting modifications
ApprovalTaskDAOImpl.java Implements new database methods for retrieving pending requests and approval task relations
ApprovalTaskDAO.java Adds interface methods for pending request and approval task relation queries
ApprovalTaskRelationDTO.java New DTO class for representing approval task data from database relations
DefaultTemplateInitializer.java Removes unused role creation logic
ApprovalTaskServiceImpl.java Core logic for updating approval tasks when workflows change
ApprovalTaskService.java Interface method for workflow update handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@jenkins-is-staging
Copy link

jenkins-is-staging commented Sep 2, 2025

PR builder started
Link: https://github.com/wso2/product-is/actions/runs/17393884142

sadilchamishka
sadilchamishka reviewed Sep 2, 2025
View reviewed changes
@jenkins-is-staging
Copy link

jenkins-is-staging commented Sep 2, 2025

PR builder completed
Link: https://github.com/wso2/product-is/actions/runs/17393884142
Status: success

jenkins-is-staging
jenkins-is-staging approved these changes Sep 2, 2025
View reviewed changes
Copy link

@jenkins-is-staging jenkins-is-staging left a comment

Choose a reason for hiding this comment

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

Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/17393884142

@sadilchamishka sadilchamishka merged commit e19d1a2 into wso2-extensions:main Sep 2, 2025
2 checks passed
Malith-19 pushed a commit to Malith-19/identity-workflow that referenced this pull request Sep 4, 2025
@wso2-jenkins-bot
[WSO2 Release] [Jenkins wso2-extensions#27] [Release 1.0.13] prepare …
ddf1122 
…release v1.0.13
Malith-19 pushed a commit to Malith-19/identity-workflow that referenced this pull request Sep 4, 2025
@wso2-jenkins-bot
[WSO2 Release] [Jenkins wso2-extensions#27] [Release 1.0.13] prepare …
7306ccf 
…for next development iteration
BimsaraBodaragama
BimsaraBodaragama reviewed Oct 18, 2025
View reviewed changes
...ain/java/org/wso2/carbon/identity/workflow/engine/internal/dao/impl/ApprovalTaskDAOImpl.java
} catch (DataAccessException e) {
String errorMessage = String.format("Error occurred while retrieving approval tasks for the " +
"workflow id: %s", workflowId);
log.debug(errorMessage, e);
Copy link

@BimsaraBodaragama BimsaraBodaragama Oct 18, 2025

Choose a reason for hiding this comment

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

How about wrapping this with a log.isDebugEnabled() check to avoid unnecessary string formatting when debug logging is disabled?

Copy link
Contributor

@Malith-19 Malith-19 Oct 18, 2025

Choose a reason for hiding this comment

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

In here we are passing the error message when throwing the error anyway. So we can skip the debug check.

BimsaraBodaragama
BimsaraBodaragama reviewed Oct 18, 2025
View reviewed changes
...ain/java/org/wso2/carbon/identity/workflow/engine/internal/dao/impl/ApprovalTaskDAOImpl.java
} catch (DataAccessException e) {
String errorMessage = String.format("Error occurred while retrieving approval task relations for the " +
"request id: %s", requestId);
log.debug(errorMessage, e);
Copy link

@BimsaraBodaragama BimsaraBodaragama Oct 18, 2025

Choose a reason for hiding this comment

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

How about wrapping this with a log.isDebugEnabled() check to avoid unnecessary string formatting when debug logging is disabled?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BimsaraBodaragama BimsaraBodaragama BimsaraBodaragama left review comments

@Malith-19 Malith-19 Malith-19 left review comments

Copilot code review Copilot Copilot left review comments

@sadilchamishka sadilchamishka sadilchamishka approved these changes

+1 more reviewer

@jenkins-is-staging jenkins-is-staging jenkins-is-staging approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@NutharaNR @jenkins-is-staging @sadilchamishka @BimsaraBodaragama @Malith-19