diff --git a/ValidateProfileEmail/__tests__/handler.test.ts b/ValidateProfileEmail/__tests__/handler.test.ts index 47633ab..19a7d7c 100644 --- a/ValidateProfileEmail/__tests__/handler.test.ts +++ b/ValidateProfileEmail/__tests__/handler.test.ts @@ -1,10 +1,6 @@ import { ResourceNotFoundCode } from "@pagopa/io-functions-commons/dist/src/utils/azure_storage"; -import { - TokenQueryParam, - ValidateProfileEmailHandler, - ValidationErrors -} from "../handler"; +import { ValidateProfileEmailHandler } from "../handler"; import { EmailString, FiscalCode } from "@pagopa/ts-commons/lib/strings"; import { IProfileEmailReader } from "@pagopa/io-functions-commons/dist/src/utils/unique_email_enforcement"; import { constTrue } from "fp-ts/lib/function"; @@ -12,6 +8,13 @@ import * as TE from "fp-ts/TaskEither"; import * as O from "fp-ts/Option"; import { ProfileModel } from "@pagopa/io-functions-commons/dist/src/models/profile"; import { aFiscalCode, aRetrievedProfile, anEmail } from "../__mocks__/profile"; +import { ValidUrl } from "@pagopa/ts-commons/lib/url"; +import { FlowTypeEnum, TokenQueryParam } from "../../utils/middleware"; +import { + confirmChoicePageUrl, + validationFailureUrl, + validationSuccessUrl +} from "../../utils/redirect_url"; const VALIDATION_TOKEN = "01DPT9QAZ6N0FJX21A86FRCWB3:8c652f8566ba53bd8cf0b1b9" as TokenQueryParam; @@ -36,20 +39,14 @@ const contextMock = { }; const validationCallbackUrl = { - href: "" -}; -const timestampGeneratorMock = () => 1234567890; + href: "localhost/validation" +} as ValidUrl; -const errorUrl = ( - error: keyof typeof ValidationErrors, - timestampGenerator: () => number -) => { - return `?result=failure&error=${error}&time=${timestampGenerator()}`; -}; +const confirmValidationUrl = { + href: "localhost/confirm-choice" +} as ValidUrl; -const successUrl = (timestampGenerator: () => number) => { - return `?result=success&time=${timestampGenerator()}`; -}; +const emailValidationUrls = { confirmValidationUrl, validationCallbackUrl }; const mockRetrieveEntity = jest .fn() @@ -94,7 +91,18 @@ const expiredTokenEntity = { RowKey: "026c47ead971b9af13353f5d5e563982ebca542f8df3246bdaf1f86e16075072" }; -describe("ValidateProfileEmailHandler", () => { +// Flow types: +// CONFIRM -> verify token and on success redirect to confirm page +// VALIDATE -> verify token and on success update the user data and redirect to result page +describe.each` + isConfirmFlow + ${true} + ${false} +`("ValidateProfileEmailHandler#Errors", ({ isConfirmFlow }) => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it.each` scenario | expectedError | callbackInputs ${"GENERIC_ERROR in case the query versus the table storage fails"} | ${"GENERIC_ERROR"} | ${[new Error()]} @@ -111,20 +119,20 @@ describe("ValidateProfileEmailHandler", () => { tableServiceMock as any, "", mockProfileModel, - validationCallbackUrl as any, - timestampGeneratorMock, + emailValidationUrls, profileEmailReader, constTrue ); const response = await verifyProfileEmailHandler( contextMock as any, - VALIDATION_TOKEN + VALIDATION_TOKEN, + isConfirmFlow ? FlowTypeEnum.CONFIRM : FlowTypeEnum.VALIDATE ); expect(response.kind).toBe("IResponseSeeOtherRedirect"); expect(response.detail).toBe( - errorUrl(expectedError, timestampGeneratorMock) + validationFailureUrl(validationCallbackUrl, expectedError).href ); expect(mockFindLastVersionByModelId).not.toBeCalled(); expect(mockUpdate).not.toBeCalled(); @@ -135,39 +143,75 @@ describe("ValidateProfileEmailHandler", () => { scenario | expectedError | isThrowing ${"should return IResponseSeeOtherRedirect if the e-mail is already taken (unique email enforcement = %uee) WHEN a citizen changes e-mail"} | ${"EMAIL_ALREADY_TAKEN"} | ${undefined} ${"return 500 WHEN the unique e-mail enforcement check fails"} | ${"GENERIC_ERROR"} | ${true} - `("should $scenario", async ({ expectedError, isThrowing }) => { + `( + "should $scenario", + async ({ expectedError, isThrowing, isConfirmFlow }) => { + const verifyProfileEmailHandler = ValidateProfileEmailHandler( + tableServiceMock as any, + "", + mockProfileModel, + emailValidationUrls, + { + list: generateProfileEmails(1, isThrowing) + }, + constTrue + ); + + const response = await verifyProfileEmailHandler( + contextMock as any, + VALIDATION_TOKEN, + isConfirmFlow ? FlowTypeEnum.CONFIRM : FlowTypeEnum.VALIDATE + ); + + expect(response.kind).toBe("IResponseSeeOtherRedirect"); + expect(response.detail).toBe( + validationFailureUrl(validationCallbackUrl, expectedError).href + ); + expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]); + expect(mockUpdate).not.toBeCalled(); + } + ); +}); + +describe("ValidateProfileEmailHandler#Happy path", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("should validate the email in profile if all the condition are verified during VALIDATE flow", async () => { const verifyProfileEmailHandler = ValidateProfileEmailHandler( tableServiceMock as any, "", mockProfileModel, - validationCallbackUrl as any, - timestampGeneratorMock, + emailValidationUrls, { - list: generateProfileEmails(1, isThrowing) + list: generateProfileEmails(0) }, constTrue ); const response = await verifyProfileEmailHandler( contextMock as any, - VALIDATION_TOKEN + VALIDATION_TOKEN, + FlowTypeEnum.VALIDATE ); expect(response.kind).toBe("IResponseSeeOtherRedirect"); expect(response.detail).toBe( - errorUrl(expectedError, timestampGeneratorMock) + validationSuccessUrl(validationCallbackUrl).href ); expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]); - expect(mockUpdate).not.toBeCalled(); + expect(mockUpdate).toBeCalledWith( + expect.objectContaining({ isEmailValidated: true }) + ); }); - it("should validate the email in profile if all the condition are verified", async () => { + it("should NOT validate the email in profile if we are in the CONFIRM flow", async () => { const verifyProfileEmailHandler = ValidateProfileEmailHandler( tableServiceMock as any, "", mockProfileModel, - validationCallbackUrl as any, - timestampGeneratorMock, + emailValidationUrls, { list: generateProfileEmails(0) }, @@ -176,14 +220,15 @@ describe("ValidateProfileEmailHandler", () => { const response = await verifyProfileEmailHandler( contextMock as any, - VALIDATION_TOKEN + VALIDATION_TOKEN, + FlowTypeEnum.CONFIRM ); expect(response.kind).toBe("IResponseSeeOtherRedirect"); - expect(response.detail).toBe(successUrl(timestampGeneratorMock)); - expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]); - expect(mockUpdate).toBeCalledWith( - expect.objectContaining({ isEmailValidated: true }) + expect(response.detail).toBe( + confirmChoicePageUrl(confirmValidationUrl, VALIDATION_TOKEN, anEmail).href ); + expect(mockFindLastVersionByModelId).toBeCalledWith([aFiscalCode]); + expect(mockUpdate).not.toHaveBeenCalled(); }); }); diff --git a/ValidateProfileEmail/handler.ts b/ValidateProfileEmail/handler.ts index 65ae7b3..03c7d9d 100644 --- a/ValidateProfileEmail/handler.ts +++ b/ValidateProfileEmail/handler.ts @@ -5,7 +5,8 @@ import * as express from "express"; import { isLeft } from "fp-ts/lib/Either"; import { isNone } from "fp-ts/lib/Option"; -import * as t from "io-ts"; +import * as TE from "fp-ts/lib/TaskEither"; +import { pipe } from "fp-ts/function"; import { Context } from "@azure/functions"; import { TableService } from "azure-storage"; @@ -16,11 +17,8 @@ import { IResponseSeeOtherRedirect, ResponseSeeOtherRedirect } from "@pagopa/ts-commons/lib/responses"; -import { FiscalCode, PatternString } from "@pagopa/ts-commons/lib/strings"; +import { FiscalCode } from "@pagopa/ts-commons/lib/strings"; import { hashFiscalCode } from "@pagopa/ts-commons/lib/hash"; - -import { RequiredQueryParamMiddleware } from "@pagopa/io-functions-commons/dist/src/utils/middlewares/required_query_param"; - import { ValidationTokenEntity } from "@pagopa/io-functions-commons/dist/src/entities/validation_token"; import { ProfileModel } from "@pagopa/io-functions-commons/dist/src/models/profile"; import { retrieveTableEntity } from "@pagopa/io-functions-commons/dist/src/utils/azure_storage"; @@ -35,76 +33,45 @@ import { isEmailAlreadyTaken } from "@pagopa/io-functions-commons/dist/src/utils/unique_email_enforcement"; import { trackEvent } from "../utils/appinsights"; - -// Tokens are generated by CreateValidationTokenActivity function inside the -// io-functions-app project (https://github.com/pagopa/io-functions-app) -// A token is in the following format: -// [tokenId ULID] + ":" + [validatorHash crypto.randomBytes(12)] -export const TokenQueryParam = PatternString( - "^[A-Za-z0-9]{26}:[A-Fa-f0-9]{24}$" -); -export type TokenQueryParam = t.TypeOf; +import { + ConfirmEmailFlowQueryParamMiddleware, + FlowType, + FlowTypeEnum, + TokenQueryParam, + TokenQueryParamMiddleware +} from "../utils/middleware"; +import { + confirmChoicePageUrl, + validationFailureUrl, + validationSuccessUrl +} from "../utils/redirect_url"; +import { ValidationErrors } from "../utils/validation_errors"; type IValidateProfileEmailHandler = ( context: Context, - token: TokenQueryParam + token: TokenQueryParam, + flowChoice: FlowType ) => Promise; -// Used in the callback -export enum ValidationErrors { - GENERIC_ERROR = "GENERIC_ERROR", - INVALID_TOKEN = "INVALID_TOKEN", - TOKEN_EXPIRED = "TOKEN_EXPIRED", - EMAIL_ALREADY_TAKEN = "EMAIL_ALREADY_TAKEN" -} - -/** - * Returns a ValidUrl that represents a successful validation - */ -const validationSuccessUrl = ( - validationCallbackUrl: ValidUrl, - timeStampGenerator: () => number -): ValidUrl => - ({ - href: `${ - validationCallbackUrl.href - }?result=success&time=${timeStampGenerator()}` - } as ValidUrl); - -/** - * Returns a ValidUrl that represents a failed validation - */ -const validationFailureUrl = ( - validationCallbackUrl: ValidUrl, - error: keyof typeof ValidationErrors, - timeStampGenerator: () => number -): ValidUrl => - ({ - href: `${ - validationCallbackUrl.href - }?result=failure&error=${error}&time=${timeStampGenerator()}` - } as ValidUrl); - -const TokenQueryParamMiddleware = RequiredQueryParamMiddleware( - "token", - TokenQueryParam -); - export const ValidateProfileEmailHandler = ( tableService: TableService, validationTokensTableName: string, profileModel: ProfileModel, - validationCallbackUrl: ValidUrl, - timeStampGenerator: () => number, + emailValidationUrls: { + readonly confirmValidationUrl: ValidUrl; + readonly validationCallbackUrl: ValidUrl; + }, profileEmails: IProfileEmailReader, FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean ): IValidateProfileEmailHandler => async ( context, - token + token, + flowChoice ): Promise => { const logPrefix = `ValidateProfileEmail|TOKEN=${token}`; + const { validationCallbackUrl, confirmValidationUrl } = emailValidationUrls; const vFailureUrl = (error: keyof typeof ValidationErrors): ValidUrl => - validationFailureUrl(validationCallbackUrl, error, timeStampGenerator); + validationFailureUrl(validationCallbackUrl, error); // STEP 1: Find and verify validation token @@ -225,33 +192,49 @@ export const ValidateProfileEmailHandler = ( } } - // Update the profile and set isEmailValidated to `true` - const errorOrUpdatedProfile = await profileModel.update({ - ...existingProfile, - isEmailValidated: true - })(); - - if (isLeft(errorOrUpdatedProfile)) { - context.log.error( - `${logPrefix}|Error updating profile|ERROR=${errorOrUpdatedProfile.left}` - ); - return ResponseSeeOtherRedirect( - vFailureUrl(ValidationErrors.GENERIC_ERROR) - ); - } - - trackEvent({ - name: "io.citizen-auth.validate_email", - tagOverrides: { - "ai.user.id": hashFiscalCode(existingProfile.fiscalCode), - samplingEnabled: "false" - } - }); - - context.log.verbose(`${logPrefix}|The profile has been updated`); - return ResponseSeeOtherRedirect( - validationSuccessUrl(validationCallbackUrl, timeStampGenerator) - ); + // Update the profile and set isEmailValidated to `true` ONLY if the flowChoice equals to VALIDATE + // otherwise just redirect to confirm page with token and email(base64url encoded) in query param + return await pipe( + flowChoice, + TE.fromPredicate( + flow => flow === FlowTypeEnum.VALIDATE, + () => + ResponseSeeOtherRedirect( + confirmChoicePageUrl(confirmValidationUrl, token, email) + ) + ), + TE.chain(() => + pipe( + profileModel.update({ + ...existingProfile, + isEmailValidated: true + }), + TE.mapLeft(error => { + context.log.error( + `${logPrefix}|Error updating profile|ERROR=${error}` + ); + return ResponseSeeOtherRedirect( + vFailureUrl(ValidationErrors.GENERIC_ERROR) + ); + }), + TE.map(() => { + trackEvent({ + name: "io.citizen-auth.validate_email", + tagOverrides: { + "ai.user.id": hashFiscalCode(existingProfile.fiscalCode), + samplingEnabled: "false" + } + }); + + context.log.verbose(`${logPrefix}|The profile has been updated`); + return ResponseSeeOtherRedirect( + validationSuccessUrl(validationCallbackUrl) + ); + }) + ) + ), + TE.toUnion + )(); }; /** @@ -262,8 +245,10 @@ export const ValidateProfileEmail = ( tableService: TableService, validationTokensTableName: string, profileModel: ProfileModel, - validationCallbackUrl: ValidUrl, - timeStampGenerator: () => number, + emailValidationUrls: { + readonly confirmValidationUrl: ValidUrl; + readonly validationCallbackUrl: ValidUrl; + }, profileEmails: IProfileEmailReader, FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED: (fiscalCode: FiscalCode) => boolean ): express.RequestHandler => { @@ -271,15 +256,15 @@ export const ValidateProfileEmail = ( tableService, validationTokensTableName, profileModel, - validationCallbackUrl, - timeStampGenerator, + emailValidationUrls, profileEmails, FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED ); const middlewaresWrap = withRequestMiddlewares( ContextMiddleware(), - TokenQueryParamMiddleware + TokenQueryParamMiddleware, + ConfirmEmailFlowQueryParamMiddleware ); return wrapRequestHandler(middlewaresWrap(handler)); }; diff --git a/ValidateProfileEmail/index.ts b/ValidateProfileEmail/index.ts index 1b85c79..ce78a04 100644 --- a/ValidateProfileEmail/index.ts +++ b/ValidateProfileEmail/index.ts @@ -65,8 +65,10 @@ app.get( tableService, VALIDATION_TOKEN_TABLE_NAME, profileModel, - validationCallbackValidUrl, - Date.now, + { + confirmValidationUrl: config.CONFIRM_CHOICE_PAGE_URL, + validationCallbackUrl: validationCallbackValidUrl + }, profileEmailsReader, FF_UNIQUE_EMAIL_ENFORCEMENT_ENABLED ) diff --git a/env.example b/env.example index 73a1c4c..f73aeb7 100644 --- a/env.example +++ b/env.example @@ -4,6 +4,7 @@ WEBSITE_NODE_DEFAULT_VERSION=10.14.1 AzureWebJobsStorage=${STORAGE_CONNECTION_STRING} StorageConnection=${STORAGE_CONNECTION_STRING} VALIDATION_CALLBACK_URL=localhost +CONFIRM_CHOICE_PAGE_URL=localhost SLOT_TASK_HUBNAME=FNPUBLICTESTMOCK FF_UNIQUE_EMAIL_ENFORCEMENT=NONE @@ -12,4 +13,4 @@ UNIQUE_EMAIL_ENFORCEMENT_USERS=[] PROFILE_EMAIL_STORAGE_CONNECTION_STRING=${STORAGE_CONNECTION_STRING} PROFILE_EMAIL_STORAGE_TABLE_NAME=profileEmails -APPINSIGHTS_INSTRUMENTATIONKEY=InstrumentationKeyHere \ No newline at end of file +APPINSIGHTS_INSTRUMENTATIONKEY=InstrumentationKeyHere diff --git a/openapi/external.yaml b/openapi/external.yaml index d93dd20..b80b45c 100644 --- a/openapi/external.yaml +++ b/openapi/external.yaml @@ -17,8 +17,19 @@ paths: name: token required: true schema: - type: integer + type: string minLength: 1 + - in: query + name: flow + description: | + CONFIRM -> verify token and on success redirect to confirm page + VALIDATE -> verify token and on success redirect to result page + schema: + type: string + enum: + - "CONFIRM" + - "VALIDATE" + default: "CONFIRM" responses: "303": description: See Others diff --git a/package.json b/package.json index c7f2365..6ec9e9a 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "@pagopa/ts-commons": "^12.5.0", "applicationinsights": "^2.9.2", "azure-storage": "^2.10.7", + "base64url": "^3.0.1", "express": "^4.15.3", "fp-ts": "^2.16.1", "io-ts": "^2.2.21", diff --git a/tsconfig.json b/tsconfig.json index eadb49e..72cbb70 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,6 +9,8 @@ "resolveJsonModule": true }, "exclude": [ - "__mocks__" + "**/__mocks__", + "**/__tests__", + "node_modules" ] -} \ No newline at end of file +} diff --git a/utils/config.ts b/utils/config.ts index e67b42e..cd24a32 100644 --- a/utils/config.ts +++ b/utils/config.ts @@ -17,6 +17,7 @@ import { import { pipe } from "fp-ts/lib/function"; import * as t from "io-ts"; import * as E from "fp-ts/lib/Either"; +import { UrlFromString } from "@pagopa/ts-commons/lib/url"; export const BetaUsers = t.readonlyArray(FiscalCode); export type BetaUsers = t.TypeOf; @@ -34,6 +35,7 @@ export const FeatureFlagFromString = withFallback( export const IConfig = t.type({ APPINSIGHTS_INSTRUMENTATIONKEY: NonEmptyString, + CONFIRM_CHOICE_PAGE_URL: UrlFromString, COSMOSDB_KEY: NonEmptyString, COSMOSDB_NAME: NonEmptyString, COSMOSDB_URI: NonEmptyString, diff --git a/utils/middleware.ts b/utils/middleware.ts new file mode 100644 index 0000000..8745f9e --- /dev/null +++ b/utils/middleware.ts @@ -0,0 +1,34 @@ +import * as t from "io-ts"; +import { PatternString } from "@pagopa/ts-commons/lib/strings"; +import { RequiredQueryParamMiddleware } from "@pagopa/io-functions-commons/dist/src/utils/middlewares/required_query_param"; +import { enumType, withDefault } from "@pagopa/ts-commons/lib/types"; + +// Tokens are generated by CreateValidationTokenActivity function inside the +// io-functions-app project (https://github.com/pagopa/io-functions-app) +// A token is in the following format: +// [tokenId ULID] + ":" + [validatorHash crypto.randomBytes(12)] +export const TokenQueryParam = PatternString( + "^[A-Za-z0-9]{26}:[A-Fa-f0-9]{24}$" +); +export type TokenQueryParam = t.TypeOf; + +export const TokenQueryParamMiddleware = RequiredQueryParamMiddleware( + "token", + TokenQueryParam +); + +// CONFIRM -> verify token and on success redirect to confirm page +// VALIDATE -> verify token and on success update the user data and redirect to result page +export enum FlowTypeEnum { + "CONFIRM" = "CONFIRM", + "VALIDATE" = "VALIDATE" +} +export const FlowType = enumType(FlowTypeEnum, "FlowChoice"); +export type FlowType = t.TypeOf; + +// even if the query param is optional the withDefault type is covering the absence +// of the value on the URL +export const ConfirmEmailFlowQueryParamMiddleware = RequiredQueryParamMiddleware( + "flow", + withDefault(FlowType, FlowTypeEnum.CONFIRM) +); diff --git a/utils/redirect_url.ts b/utils/redirect_url.ts new file mode 100644 index 0000000..aaff04f --- /dev/null +++ b/utils/redirect_url.ts @@ -0,0 +1,38 @@ +import { EmailString } from "@pagopa/ts-commons/lib/strings"; +import { ValidUrl } from "@pagopa/ts-commons/lib/url"; +import base64url from "base64url"; +import { TokenQueryParam } from "./middleware"; +import { ValidationErrors } from "./validation_errors"; + +/** + * Returns a ValidUrl that represents a successful validation + */ +export const confirmChoicePageUrl = ( + url: ValidUrl, + token: TokenQueryParam, + email: EmailString +): ValidUrl => + ({ + href: `${url.href}?token=${token}&email=${base64url(email)}` + } as ValidUrl); + +/** + * Returns a ValidUrl that represents a successful validation + */ +export const validationSuccessUrl = ( + validationCallbackUrl: ValidUrl +): ValidUrl => + ({ + href: `${validationCallbackUrl.href}?result=success}` + } as ValidUrl); + +/** + * Returns a ValidUrl that represents a failed validation + */ +export const validationFailureUrl = ( + validationCallbackUrl: ValidUrl, + error: keyof typeof ValidationErrors +): ValidUrl => + ({ + href: `${validationCallbackUrl.href}?result=failure&error=${error}}` + } as ValidUrl); diff --git a/utils/validation_errors.ts b/utils/validation_errors.ts new file mode 100644 index 0000000..1628978 --- /dev/null +++ b/utils/validation_errors.ts @@ -0,0 +1,6 @@ +export enum ValidationErrors { + GENERIC_ERROR = "GENERIC_ERROR", + INVALID_TOKEN = "INVALID_TOKEN", + TOKEN_EXPIRED = "TOKEN_EXPIRED", + EMAIL_ALREADY_TAKEN = "EMAIL_ALREADY_TAKEN" +} diff --git a/yarn.lock b/yarn.lock index 30a0f3a..503b780 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2282,6 +2282,11 @@ balanced-match@^1.0.0: resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-1.0.2.tgz#e83e3a7e3f300b34cb9d87f615fa0cbf357690ee" integrity sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw== +base64url@^3.0.1: + version "3.0.1" + resolved "https://registry.yarnpkg.com/base64url/-/base64url-3.0.1.tgz#6399d572e2bc3f90a9a8b22d5dbb0a32d33f788d" + integrity sha512-ir1UPr3dkwexU7FdV8qBBbNDRUhMmIekYMFZfi+C/sLNnRESKPl23nB9b2pltqfOQNnGzsDdId90AEtG5tCx4A== + base@^0.11.1: version "0.11.2" resolved "https://registry.yarnpkg.com/base/-/base-0.11.2.tgz#7bde5ced145b6d551a90db87f83c558b4eb48a8f"