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

Endpoint providing information what actions a user has access to #205

Closed
2 tasks
Tracked by #170 ...
RonnyB71 opened this issue Mar 13, 2023 · 3 comments · Fixed by #237
Closed
2 tasks
Tracked by #170 ...

Endpoint providing information what actions a user has access to #205

RonnyB71 opened this issue Mar 13, 2023 · 3 comments · Fixed by #237
Assignees
Labels
area/signing feature-complete kind/user-story Used for issues that describes functionality for our users.
Milestone

Comments

@RonnyB71
Copy link
Member

RonnyB71 commented Mar 13, 2023

Description

The app backend needs to provide an endpoint providing information about the requesting users access rights on the current step.

This should cover:

  • read
  • write
  • actions
    • sign
    • reject
    • confirm
    • instantiate

confirm and instantiate are not for this issue, but is included to show that we need to support multiple actions related to a step in the process and that we can control access to these actions.

The same endpoint should also list the roles that actually has access to the current step and actions.

Additional Information

Related epic: Altinn/app-template-dotnet#170
Related frontend issue: Altinn/app-frontend-react#991

Tasks

No response

Proposed change in process file and process engine

Given this fairly simple, and not uncommon flow:
image

Implicit actions that happen when an instance leave a Data task is:

  1. Lock the data associated with the task
  2. Generate PDF for the task

Moving "back" in the process we need to delete the existing PDF to ensure it has no outdated data and unlock the data to make changes possible.

To handle the unlock and delete actions we need some way of knowing if the process is moving forwards or backwards

Today

Todays solution of informing the process engine if the process is moving forward or backwards is done by adding custom attributes to the flows out of the gateway. Example BPMN describing the image above (note the altinn:flowtype on SequenceFlow 4 and 5):

<?xml version="1.0" encoding="UTF-8"?>
<bpmn2:definitions id="Altinn_Data_Confirmation_Process_Definition"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xmlns:bpmn2="http://www.omg.org/spec/BPMN/20100524/MODEL"
  xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI"
  xmlns:dc="http://www.omg.org/spec/DD/20100524/DC"
  xmlns:di="http://www.omg.org/spec/DD/20100524/DI"
  xmlns:altinn="http://altinn.no"
  xsi:schemaLocation="http://www.omg.org/spec/BPMN/20100524/MODEL BPMN20.xsd"
  targetNamespace="http://bpmn.io/schema/bpmn" >
  <bpmn2:process id="Altinn_Data_Confirmation_Process" isExecutable="false">
    <bpmn2:startEvent id="Start">
      <bpmn2:outgoing>SequenceFlow_1</bpmn2:outgoing>
    </bpmn2:startEvent>
    <bpmn2:task id="Task1" name="Task1:Data" altinn:tasktype="data">
      <bpmn2:incoming>SequenceFlow_1</bpmn2:incoming>
      <bpmn2:incoming>SequenceFlow_5</bpmn2:incoming>
      <bpmn2:outgoing>SequenceFlow_2</bpmn2:outgoing>
    </bpmn2:task>
    <bpmn2:task id="Task2" name="Task2:Confirm" altinn:tasktype="confirmation">
      <bpmn2:incoming>SequenceFlow_2</bpmn2:incoming>
      <bpmn2:outgoing>SequenceFlow_3</bpmn2:outgoing>
    </bpmn2:task>
	<bpmn2:exclusiveGateway id="Gateway_1">
	    <bpmn2:incoming>SequenceFlow_3</bpmn2:incoming>
	    <bpmn2:outgoing>SequenceFlow_4</bpmn2:outgoing>
	    <bpmn2:outgoing>SequenceFlow_5</bpmn2:outgoing>
	</bpmn2:exclusiveGateway>
    <bpmn2:endEvent id="End">
      <bpmn2:incoming>SequenceFlow_4</bpmn2:incoming>
    </bpmn2:endEvent>
    <bpmn2:sequenceFlow id="SequenceFlow_1" sourceRef="Start" targetRef="Task1" />
    <bpmn2:sequenceFlow id="SequenceFlow_2" sourceRef="Task1" targetRef="Task2" />
    <bpmn2:sequenceFlow id="SequenceFlow_3" sourceRef="Task2" targetRef="Gateway_1" />
	<bpmn2:sequenceFlow id="SequenceFlow_4" sourceRef="Gateway_1" targetRef="End" altinn:flowtype="CompleteCurrentMoveToNext" />
	<bpmn2:sequenceFlow id="SequenceFlow_5" sourceRef="Gateway_1" targetRef="Task1" altinn:flowtype="AbandonCurrentReturnToNext" />
  </bpmn2:process>
</bpmn2:definitions>

To make present the user with the available choices, lets call them "actions", that is possible from this task the process engine needs to do a simulated play of the process and present the user with all the tasks that could be reached by flows from the current task, gateways are evaluated and if not decision can be made all flows are followed and resulting tasks presented as possible "actions" for the user.

In the example above, if the process is in Task2 the user would be presented with a list of possible actions: [Task1, End].
If the user chooses to take the "action" Task1 the process engine would follow the flows from Task2 to Task1 and discover that SequenceFlow_5 has flowtype AbandonCurrentReturnToNext and execute logic connected to this flowtype.

In my opinion this is a flawed, if not wrong, when the process is on a given task I think this Task should contain enough information to present the user with what "actions" are possible to do without reading the rest of the process definition.

Proposal

TL;DR;

  • Remove altinn:flowtype attribute and introduce Extensions properties on Tasks that defines available actions.
  • Remove altinn:tasktype attribute and move it to the extension properties to adhere to the bpmn specification.

Frontend would then get the possible actions from the instance current position(task) in the process. No need to follow flows or evaluating gateways before the user actually takes a action.
Frontend still receives the altinn:tasktype but it is derived from the extension properties instead
Unlocking the data and deleting outdated PDFs would then be performed before the process leaves Task2 in the example process.

Example process definition:

<?xml version="1.0" encoding="UTF-8"?>
<bpmn:definitions xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL"
  xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI"
  xmlns:dc="http://www.omg.org/spec/DD/20100524/DC"
  xmlns:altinn="http://altinncdn.no/schemas/1.0/bpmn"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xmlns:di="http://www.omg.org/spec/DD/20100524/DI"
  xmlns:modeler="http://camunda.org/schema/modeler/1.0" id="Definitions_1exv6jf"
  targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="5.5.0"
  modeler:executionPlatform="Camunda Platform" modeler:executionPlatformVersion="7.18.0">
  <bpmn:process id="Process_1om6n79" isExecutable="true">
    <bpmn:startEvent id="Start">
      <bpmn:outgoing>SequenceFlow_1</bpmn:outgoing>
    </bpmn:startEvent>
    <bpmn:task id="Task1" name="Task1:Data">
      <bpmn:extensionElements>
        <altinn:properties>
          <altinn:actions>
            <altinn:action id="submit" name="Send inn" />
          </altinn:actions>
          <altinn:tasktype id="data" />
        </altinn:properties>
      </bpmn:extensionElements>
      <bpmn:incoming>SequenceFlow_1</bpmn:incoming>
      <bpmn:incoming>SequenceFlow_5</bpmn:incoming>
      <bpmn:outgoing>SequenceFlow_2</bpmn:outgoing>
    </bpmn:task>
    <bpmn:task id="Task2" name="Task2:Confirm">
      <bpmn:extensionElements>
        <altinn:properties>
          <altinn:actions>
            <altinn:action id="confirm" name="Bekreft" />
            <altinn:action id="reject" name="Avslå" />
          </altinn:actions>
          <altinn:tasktype id="confirm" />
        </altinn:properties>
      </bpmn:extensionElements>
      <bpmn:incoming>SequenceFlow_2</bpmn:incoming>
      <bpmn:outgoing>SequenceFlow_3</bpmn:outgoing>
    </bpmn:task>
    <bpmn:exclusiveGateway id="Gateway_1">
      <bpmn:incoming>SequenceFlow_3</bpmn:incoming>
      <bpmn:outgoing>SequenceFlow_4</bpmn:outgoing>
      <bpmn:outgoing>SequenceFlow_5</bpmn:outgoing>
    </bpmn:exclusiveGateway>
    <bpmn:endEvent id="End">
      <bpmn:incoming>SequenceFlow_4</bpmn:incoming>
    </bpmn:endEvent>
    <bpmn:sequenceFlow id="SequenceFlow_1" sourceRef="Start" targetRef="Task1" />
    <bpmn:sequenceFlow id="SequenceFlow_2" sourceRef="Task1" targetRef="Task2" />
    <bpmn:sequenceFlow id="SequenceFlow_3" sourceRef="Task2" targetRef="Gateway_1" />
    <bpmn:sequenceFlow id="SequenceFlow_4" sourceRef="Gateway_1" targetRef="End" />
    <bpmn:sequenceFlow id="SequenceFlow_5" sourceRef="Gateway_1" targetRef="Task1" />
  </bpmn:process>
</bpmn:definitions>

API response proposed by frontend

  • PUT */process/next (response)
  • GET */process (response)
{
  ...
  "currentTask": {
    ...
    "read": boolean,
    "write": boolean,
    "actions": {
      "instantiate": boolean,
      "confirm": boolean,
      "sign": boolean,
      "reject": boolean,
    },
  },
}

Even more verbose process definition OUT OF SCOPE

One could argue that we are implicit doing some actions that are hidden for the developer today.
If we go fullblown BPMN one could introduce UserTasks and ServiceTasks and make every implicit action we do defined in the process. This is not in scope for this issue, but should be considered for future work

Example image of the process described abov:
image

Acceptance Criterias

  • If a user don't have access at all (ie. no read or write access) the endpoint should return 403 Forbidden allowing the frontend to show a generic "you don't have access" layout.
  • Backend returns information about the current logged on users access rights to the current step
@RonnyB71 RonnyB71 added kind/user-story Used for issues that describes functionality for our users. area/signing labels Mar 13, 2023
@bjosttveit
Copy link
Member

I believe that confirm and instantiate are also user actions. I dont know if knowing about instantiate is useful, but in a stateless app it is possible to call instantiate from app-frontend using an InstantiationButton. confirm should at least also be available.

The action write includes both saving data and submitting a data step right?

@RonnyB71
Copy link
Member Author

Agree. The above list was not intended to be complete, only to indicate what we need related to signing. Those should definitely be on the list.

@tjololo
Copy link
Member

tjololo commented Apr 19, 2023

Added a proposal to how we can change the process definition (bpmn) file to support the new "actions" and adhere more to the bpmn standard with the use of extensionElements in the definition.

@RonnyB71 RonnyB71 moved this to 📈 Todo in Team Apps Apr 24, 2023
@tjololo tjololo self-assigned this Apr 26, 2023
@tjololo tjololo moved this from 📈 Todo to 👷 In Progress in Team Apps Apr 26, 2023
@tjololo tjololo added this to the 8.0.0 milestone May 4, 2023
@tjololo tjololo moved this from 👷 In Progress to 🔎 Review in Team Apps May 10, 2023
tjololo added a commit that referenced this issue May 15, 2023
#207 (#237)

* New process engine seems to work.
Needs more tests and verification

* refactored to make class more testable

* added tests for ProcessEngine

* Refactor and delete old and unused code

* added tests for ProcessEventDispatcher

* Add tests and fix ProcessNavigator

* add available actions to currentTask and perform authcheck

* action passed along from PUT process/next to gateway filters

* fix bug in AppProcessState ctor

* add fields for read/write and check users permissions

* Fix test stub implementation of IProcessExclusiveGateway

* Fixing some reported code smells

* Some code smell fixes.
Added logic to dispatch abandon event if action is reject

* remove unfinished test file

* add tests for method in ProcessClient

* add test for classes extending storage classes

* add test for null values in extensions

* revert code changes due to test

* add frontend feature and parse request body on process/next if present

* add frontend feature and parse request body on process/next if present

* fix codeql warning

* add v8 as target of github workflows in addition to main

* Fix return type of all methods in ProcessController returning ProcessState

* Authorize action moved to AuthorizationClient
TaskType is substituted with corresponding action earlier
Resolvs #207

* Fixed some issues after review and added some more tests

* fix codeQL warning and improve test

* Fix some code smells

* Update src/Altinn.App.Api/Controllers/InstancesController.cs

Co-authored-by: Ronny Birkeli <ronny.birkeli@gmail.com>

* fix build error

---------

Co-authored-by: Ronny Birkeli <ronny.birkeli@gmail.com>
@tjololo tjololo moved this from 🔎 Review to 🧪 Test in Team Apps May 15, 2023
@RonnyB71 RonnyB71 moved this from 🧪 Test to ✅ Done in Team Apps May 22, 2023
@tjololo tjololo closed this as completed May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/signing feature-complete kind/user-story Used for issues that describes functionality for our users.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants