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

Consolidate "seenBy", "performedBy" and "sender" properties to common model #851

Closed
6 tasks done
Tracked by #836 ...
elsand opened this issue Jun 13, 2024 · 2 comments
Closed
6 tasks done
Tracked by #836 ...
Assignees

Comments

@elsand
Copy link
Member

elsand commented Jun 13, 2024

Introduction

There is a need for the serviceOwner to supply information about who did what (perform an activity, send a transmission), which matches that of seen log.

Implementation

We create a common entity for this that includes

  • The type of actor associated with this activity/transmission. This is either 1) someone representing the party (ie. a person, system user, an accountant organization etc) or 2) the service owner. At some point in the future other actor types might be introduced (for instance if there are other government actors involved).
    • Enum with the values
      • partyRepresentative
      • serviceOwner
  • The natural id for the actor (fnr/dnr/orgnr/system-id etc)
  • The resolved name for the actor (Ola Nordmann, Foretak AS)

We need different DTOs for this

  • We probably cannot expose PIDs in the enduser-DTOs, but organization numbers are ok
  • We instead rewrite PIDs to a hash , generated on the fly for end-user output DTOs

The input validation should:

  • Always require actorType
  • Require either actorId or actorName, or none of them.
    • If actorId is supplied, actorName should be looked up and stored. If this fails, the entire POST/PUT/PATCH request should fail.
    • If actorName is supplied actorId should be stored as NULL,
    • If neither is supplied:
      • If "actorType" is "serviceOwner", then both id and name is then stored as null. Consumers of the output-DTOs should then assume the "org" of the dialog to be the actorName
      • If "actorType" is "partyRepresentative", then the actorId and resolved name for the dialog party is to be used.

Examples:

In these examples, we see how different inputs DTO from service owners end up as output DTOs.

Inputs

A: ServiceOwner-DTO (input, person representing party):

{
    "actorType": "partyRepresentative", 
    "actorId": "urn:altinn:person:identifier-no:12345678901" 
}

B: ServiceOwner-DTO (input, organization representing party):

{
    "actorType": "partyRepresentative", 
    "actorId": "urn:altinn:organization:identifier-no:912345679"
}

C: ServiceOwner-DTO (input, custom name):

{
    "actorType": "partyRepresentative", 
    "actorName": "Some custom name"
}

D: ServiceOwner-DTO (input, service owner):

{
    "actorType": "serviceOwner" // actorName and actorId must be omitted
}

E: ServiceOwner-DTO (input, for the dialog party):

{
    "actorType": "partyRepresentative" // This should be interpreted as dialog.party
}

X: ServiceOwner-DTO (input, trying to provide both actorId and actorName):

// Below is invalid - cannot supply both actorId and actorName as they might conflict
// This should cause a 400 error
{
    "actorType": "partyRepresentative", 
    "actorId": "urn:altinn:organization:identifier-no:912345679"
    "actorName": "Some custom name"
}

Serviceowner-DTO outputs

A: ServiceOwner-DTO (output, person representing party):

{
    "actorType": "partyRepresentative",
    "actorId": "urn:altinn:person:identifier-no:12345678901", // service owners always gets access to clear text version
    "actorName": "Ola Nordmann" // the resolved name that Dialogporten looked 
}

B: ServiceOwner-DTO (output, organization representing party):

{
    "actorType": "partyRepresentative",
    "actorId": "urn:altinn:organization:identifier-no:912345678",
    "actorName": "Foretak AS" // the resolved name 
}

C: ServiceOwner-DTO (output, custom name):

{
    "actorType": "partyRepresentative", 
    "actorName": "Some custom name"
}

D: ServiceOwner-DTO (output, service owner):

{
    "actorType": "serviceOwner"
}

E: ServiceOwner-DTO (out, for the dialog party):

{
    "actorType": "partyRepresentative" 
}

Enduser-DTOs

A: Enduser-DTO (output, person representing party)

{
    "actorType": "user",
    "actorName": "Ola Nordmann",
    "actorId": "urn:altinn:person:identifier-ephemeral:0676ad6bdf" // hashed version
}

B: Enduser-DTO (output, organization representing party)

{
    "actorType": "partyRepresentative",
    "actorName": "Foretak AS",
    "actorId": "urn:altinn:organization:identifier-no:912345678"
}

C: Enduser-DTO (output, custom name):

{
    "actorType": "partyRepresentative", 
    "actorName": "Some custom name"
}

D: Enduser-DTO (output, service owner):

{
    "actorType": "serviceOwner"
}

E: Enduser-DTO (out, for the dialog party):

{
    "actorType": "partyRepresentative" 
}

Summarized:

  • C, D, and E are identical across all DTOs.
  • A and B have different input and output DTOs - in the output DTOs both actorName and actorId (if available) should be sent.
  • For the output DTOs, only A differs between service owner and end-user

Tasks

Preview Give feedback

Threat modelling

Preview Give feedback

Acceptance criteria

GIVEN ...
WHEN ....
THEN ...

GIVEN ...
WHEN ....
THEN ...

@oskogstad
Copy link
Collaborator

@elsand IsCurrentEndUser is missing from the new model, I assume it is still needed?

@elsand
Copy link
Member Author

elsand commented Jul 20, 2024

@elsand IsCurrentEndUser is missing from the new model, I assume it is still needed?

No, that belongs to the Seen class, not the Actor class. The same goes for the "via serviceowner" part, which initially was part of the actorType-enum - also confusing things. I have now simplified the actorType enum to have two values partyRepresentative and serviceOwner, which are the two principal actor types in a dialog. This is not to be conflated with user types.

oskogstad added a commit that referenced this issue Jul 22, 2024
This is a rewrite of #901, taking into account the clarified
requirements in #851

## Description

* Introduces the DialogActor entity, with DB migrations (breaking)
* Replaces the strings for "performedBy" and "seenBy" with instances of
DialogActor. This should also be used for "sender" when "transmission"
is introduced.
* Adds DTOs, mappings and validations for all enduser and service owner
endpoints
* Hash service replaced with static helper, used directly in mappers
* Refactored PersonNameRegistry to PartyNameRegistry
* Implement actor name lookup if actor id is supplied
* Replaced "via serviceowner" with a simple boolean value (should be
used in #386)
* Updated seen tests (pid masking)
* Added activity log tests (pid masking)

Missing
* Updated e2e tests

## Related Issue(s)

- #851

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
- [ ] Relevant automated test added (if you find this hard, leave it and
we'll help out)

## Documentation

- [ ] Documentation is updated (either in `docs`-directory, Altinnpedia
or a separate linked PR in
[altinn-studio-docs.](https://github.com/Altinn/altinn-studio-docs), if
applicable)

---------

Co-authored-by: Ole Jørgen Skogstad <skogstad@softis.net>
oskogstad pushed a commit that referenced this issue Jul 24, 2024
## Description
This updates the e2e tests to support new actor model. This also
disables the org-filter check as there is currently no appropiate way to
implement it.

## Related Issue(s)

- #851 

## Verification

- [x] **Your** code builds clean without any errors or warnings
- [x] Manual testing done (required)
@elsand elsand closed this as completed Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants