Skip to content

Commit

Permalink
[#ENTE-82] Limit API access for incomplete services (#116)
Browse files Browse the repository at this point in the history
* [#ENTE-82] Disable GetLimitedProfile for incomplete services

* [#ENTE-82] Disable GetSubscriptionFeed for incomplete services

* [#ENTE-82] Disable CreateMessage for incomplete services

* [#ENTE-82] Fix unit test (jest mock timer bugs)

* [#ENTE-82] Add feature flag for enabling quality service checks

* [#ENTE-82] Add Service whitelist for incomplete service filter

* [#ENTE-82] Add unit tests for Service withelist

* [#ENTE-82] Use bimap instead map/mapLeft

* [#ENTE-82] Remove useless library from yarn.lock
  • Loading branch information
BurnedMarshal authored May 14, 2021
1 parent e3e4556 commit 7c1e65a
Show file tree
Hide file tree
Showing 20 changed files with 1,002 additions and 365 deletions.
72 changes: 69 additions & 3 deletions CreateMessage/__tests__/handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@
import * as fc from "fast-check";

import { MessageModel } from "@pagopa/io-functions-commons/dist/src/models/message";
import { UserGroup } from "@pagopa/io-functions-commons/dist/src/utils/middlewares/azure_api_auth";
import {
IAzureApiAuthorization,
UserGroup
} from "@pagopa/io-functions-commons/dist/src/utils/middlewares/azure_api_auth";

import { none, some } from "fp-ts/lib/Option";

import {
ApiNewMessageWithDefaults,
canDefaultAddresses,
canPaymentAmount,
canWriteMessage,
Expand All @@ -31,6 +35,15 @@ import {
newMessageWithPaymentDataArb,
versionedServiceArb
} from "../../utils/__tests__/arbitraries";
import { EmailString, NonEmptyString } from "@pagopa/ts-commons/lib/strings";
import { IAzureUserAttributes } from "@pagopa/io-functions-commons/dist/src/utils/middlewares/azure_user_attributes";
import {
aFiscalCode,
anIncompleteService,
anotherFiscalCode
} from "../../__mocks__/mocks";
import { initAppInsights } from "italia-ts-commons/lib/appinsights";
import { mockOrchestratorContext } from "../../__mocks__/durable-functions";

//
// tests
Expand Down Expand Up @@ -256,7 +269,9 @@ describe("CreateMessageHandler", () => {
const createMessageHandler = CreateMessageHandler(
undefined as any,
undefined as any,
undefined as any
undefined as any,
true,
[]
);

const response = await createMessageHandler(
Expand All @@ -279,7 +294,9 @@ describe("CreateMessageHandler", () => {
const createMessageHandler = CreateMessageHandler(
undefined as any,
undefined as any,
undefined as any
undefined as any,
true,
[]
);

const response = await createMessageHandler(
Expand All @@ -293,4 +310,53 @@ describe("CreateMessageHandler", () => {

expect(response.kind).toBe("IResponseErrorValidation");
});

it("should return IResponseErrorForbiddenNotAuthorizedForProduction if the service hasn't quality field", async () => {
const mockAzureApiAuthorization: IAzureApiAuthorization = {
groups: new Set([UserGroup.ApiMessageWrite]),
kind: "IAzureApiAuthorization",
subscriptionId: "" as NonEmptyString,
userId: "" as NonEmptyString
};

const mockAzureUserAttributes: IAzureUserAttributes = {
email: "" as EmailString,
kind: "IAzureUserAttributes",
service: {
...anIncompleteService,
authorizedRecipients: new Set([aFiscalCode])
} as IAzureUserAttributes["service"]
};
const mockGenerateObjId = jest
.fn()
.mockImplementationOnce(() => "mocked-message-id");
const mockTelemetryClient = ({
trackEvent: jest.fn()
} as unknown) as ReturnType<typeof initAppInsights>;
const createMessageHandler = CreateMessageHandler(
mockTelemetryClient,
undefined as any,
mockGenerateObjId,
true,
[]
);

const response = await createMessageHandler(
mockOrchestratorContext,
mockAzureApiAuthorization,
undefined as any,
mockAzureUserAttributes,
{
content: {
markdown: "md",
subject: "subject"
}
} as ApiNewMessageWithDefaults,
some(anotherFiscalCode)
);

expect(response.kind).toBe(
"IResponseErrorForbiddenNotAuthorizedForRecipient"
);
});
});
37 changes: 30 additions & 7 deletions CreateMessage/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ import {
MessageModel,
NewMessageWithoutContent
} from "@pagopa/io-functions-commons/dist/src/models/message";
import { ServiceModel } from "@pagopa/io-functions-commons/dist/src/models/service";
import {
ServiceModel,
ValidService
} from "@pagopa/io-functions-commons/dist/src/models/service";
import {
AzureApiAuthMiddleware,
IAzureApiAuthorization,
Expand Down Expand Up @@ -77,6 +80,7 @@ import {
} from "italia-ts-commons/lib/responses";
import { NonEmptyString } from "italia-ts-commons/lib/strings";
import { PromiseType } from "italia-ts-commons/lib/types";
import { ServiceId } from "@pagopa/io-functions-commons/dist/generated/definitions/ServiceId";

const ApiNewMessageWithDefaults = t.intersection([
ApiNewMessage,
Expand Down Expand Up @@ -334,7 +338,9 @@ const redirectToNewMessage = (
export function CreateMessageHandler(
telemetryClient: ReturnType<typeof initAppInsights>,
messageModel: MessageModel,
generateObjectId: ObjectIdGenerator
generateObjectId: ObjectIdGenerator,
disableIncompleteServices: boolean,
incompleteServiceWhitelist: ReadonlyArray<ServiceId>
): ICreateMessageHandler {
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
return async (
Expand Down Expand Up @@ -418,11 +424,24 @@ export function CreateMessageHandler(
canWriteMessage(auth.groups, authorizedRecipients, fiscalCode)
)
)
.chainSecond(
// Verify if the Service has the required quality to sent message
.chain(_ =>
disableIncompleteServices &&
!incompleteServiceWhitelist.includes(serviceId) &&
!authorizedRecipients.has(fiscalCode)
? fromEither(
ValidService.decode(userAttributes.service).bimap(
_1 => ResponseErrorForbiddenNotAuthorizedForRecipient,
_1 => true
)
)
: fromEither(right(true))
)
.chain(_ =>
// check whether the client can provide default addresses
fromEither(canDefaultAddresses(messagePayload))
)
.chainSecond(
.chain(_ =>
// check whether the client can ask for payment
fromEither(
canPaymentAmount(
Expand All @@ -431,7 +450,7 @@ export function CreateMessageHandler(
)
)
)
.chainSecond(
.chain(_ =>
// create a Message document in the database
createMessageDocument(
messageId,
Expand Down Expand Up @@ -475,12 +494,16 @@ export function CreateMessageHandler(
export function CreateMessage(
telemetryClient: ReturnType<typeof initAppInsights>,
serviceModel: ServiceModel,
messageModel: MessageModel
messageModel: MessageModel,
disableIncompleteServices: boolean,
incompleteServiceWhitelist: ReadonlyArray<ServiceId>
): express.RequestHandler {
const handler = CreateMessageHandler(
telemetryClient,
messageModel,
ulidGenerator
ulidGenerator,
disableIncompleteServices,
incompleteServiceWhitelist
);
const middlewaresWrap = withRequestMiddlewares(
// extract Azure Functions bindings
Expand Down
8 changes: 7 additions & 1 deletion CreateMessage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,13 @@ const telemetryClient = initTelemetryClient(

app.post(
"/api/v1/messages/:fiscalcode?",
CreateMessage(telemetryClient, serviceModel, messageModel)
CreateMessage(
telemetryClient,
serviceModel,
messageModel,
config.FF_DISABLE_INCOMPLETE_SERVICES,
config.FF_INCOMPLETE_SERVICE_WHITELIST
)
);

const azureFunctionHandler = createAzureFunctionHandler(app);
Expand Down
Loading

0 comments on commit 7c1e65a

Please sign in to comment.