Pin 5687: Verify tenant certified attributes for agreement creation #1224
Conversation
…7_agreementsVerify
AsterITA
requested review from
beetlecrunch,
galales,
Carminepo2 and
taglioni-r
as code owners
AsterITA
requested review from
ecamellini,
MalpenZibo and
paologaleotti
and removed request for
beetlecrunch,
galales,
Carminepo2 and
taglioni-r
There was a problem hiding this comment.
Do we need to implement this logic in the agreement process?
I think we have everything we need already in the BFF, the call is named /agreements/verify but it doesn't seem to be related to agreements since an agreement does not yet exist (unless I am missing something).
I think we can do everything in the BFF, doing something similar to what @MalpenZibo did in #1218, but simpler:
- We GET the delegation
- We GET the delegator from the delegation
- We use the hasCertifiedAttributes utility (already implemented in BFF) and return true if it's true for at least one of these tenants
Let me know what you think. I would also consider renaming the call since it is not related to agreements but just to the verification of attributes given a descriptor and a delegation
packages/agreement-process/src/model/domain/agreement-validators.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Eric Camellini <[email protected]>
…/github.com/pagopa/interop-be-monorepo into PIN-5607_DelegatedConsumerAgreementCreation
| .with( | ||
| "eServiceNotFound", | ||
| "descriptorNotFound", | ||
| () => HTTP_STATUS_BAD_REQUEST |
There was a problem hiding this comment.
shouldn't be a not found?
There was a problem hiding this comment.
shouldn't be a not found?
In these cases we usually decide which is the main "subject" of the route, on which we return 404 if not found (in this case the tenant), meanwhile the other objects if not found we return a 400 since it's an incorrect input.
We can rethink this behaviour if we want, but for now it's of the same pattern in various mappers
There was a problem hiding this comment.
Ok to leave it like this also for me, when this route is called we shall have eservice and descriptor defined and found, tenant is the main subject (the potential consumer, it can be any tenant) - the case where they are not should not be possible, so I think it's ok to return a bad request and not 404
…rs.ts Co-authored-by: Eric Camellini <[email protected]>
…687_agreementsVerify
This reverts commit 0bef0f7.
Closes PIN-5607
This route will check the tenant attributes against the selected descriptor when creating an agreement for itself or for a delegator
Creating agreement without a delegation (Creating for itself)
Creating agreement with a delegation by the delegate
Creating agreement when not the delegate nor for himself
Creating agreement for itself when there's a valid delegation
