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

[#175227073] Allow updating ADB2C User and introduce Token Name management on Create/Update operations #92

Merged
merged 9 commits into from
Oct 15, 2020
18 changes: 13 additions & 5 deletions CreateUser/__tests__/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { GraphRbacManagementClient } from "@azure/graph";
import * as msRestNodeAuth from "@azure/ms-rest-nodeauth";
import { left } from "fp-ts/lib/Either";
import { fromEither, fromLeft } from "fp-ts/lib/TaskEither";
import { NonEmptyString } from "italia-ts-commons/lib/strings";
import { User } from "../../generated/definitions/User";
import { UserPayload } from "../../generated/definitions/UserPayload";
import { UserStateEnum } from "../../generated/definitions/UserState";
Expand Down Expand Up @@ -64,7 +65,9 @@ mockApiManagementClient.mockImplementation(() => ({
}
}));

const fakeAdb2cExtensionAppClientId = "extension-client-id" as NonEmptyString;
const mockedContext = { log: { error: mockLog } };

describe("CreateUser", () => {
it("should return an internal error response if the ADB2C client can not be got", async () => {
mockLoginWithServicePrincipalSecret.mockImplementationOnce(() =>
Expand All @@ -74,7 +77,8 @@ describe("CreateUser", () => {
const createUserHandler = CreateUserHandler(
fakeServicePrincipalCredentials,
fakeServicePrincipalCredentials,
fakeApimConfig
fakeApimConfig,
fakeAdb2cExtensionAppClientId
);

const response = await createUserHandler(
Expand All @@ -94,7 +98,8 @@ describe("CreateUser", () => {
const createUserHandler = CreateUserHandler(
fakeServicePrincipalCredentials,
fakeServicePrincipalCredentials,
fakeApimConfig
fakeApimConfig,
fakeAdb2cExtensionAppClientId
);

const response = await createUserHandler(
Expand All @@ -119,7 +124,8 @@ describe("CreateUser", () => {
const createUserHandler = CreateUserHandler(
fakeServicePrincipalCredentials,
fakeServicePrincipalCredentials,
fakeApimConfig
fakeApimConfig,
fakeAdb2cExtensionAppClientId
);

const response = await createUserHandler(
Expand All @@ -142,7 +148,8 @@ describe("CreateUser", () => {
const createUserHandler = CreateUserHandler(
fakeServicePrincipalCredentials,
fakeServicePrincipalCredentials,
fakeApimConfig
fakeApimConfig,
fakeAdb2cExtensionAppClientId
);

const response = await createUserHandler(
Expand Down Expand Up @@ -187,7 +194,8 @@ describe("CreateUser", () => {
const createUserHandler = CreateUserHandler(
fakeServicePrincipalCredentials,
fakeServicePrincipalCredentials,
fakeApimConfig
fakeApimConfig,
fakeAdb2cExtensionAppClientId
);

const response = await createUserHandler(
Expand Down
54 changes: 31 additions & 23 deletions CreateUser/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import {
IResponseSuccessJson,
ResponseSuccessJson
} from "italia-ts-commons/lib/responses";
import { NonEmptyString } from "italia-ts-commons/lib/strings";
import { withoutUndefinedValues } from "italia-ts-commons/lib/types";
import * as randomString from "randomstring";
import { ulid } from "ulid";
import { UserCreated } from "../generated/definitions/UserCreated";
Expand All @@ -38,7 +40,8 @@ type ICreateUserHandler = (
export function CreateUserHandler(
adb2cCredentials: IServicePrincipalCreds,
apimCredentials: IServicePrincipalCreds,
azureApimConfig: IAzureApimConfig
azureApimConfig: IAzureApimConfig,
adb2cTokenAttributeName: NonEmptyString
): ICreateUserHandler {
return async (context, _, userPayload) => {
const internalErrorHandler = (errorMessage: string, error: Error) =>
Expand All @@ -55,26 +58,29 @@ export function CreateUserHandler(
.chain(graphRbacManagementClient =>
tryCatch(
() =>
graphRbacManagementClient.users.create({
accountEnabled: true,
creationType: "LocalAccount",
displayName: `${userPayload.first_name} ${userPayload.last_name}`,
givenName: userPayload.first_name,
mailNickname: userPayload.email.split("@")[0],
passwordProfile: {
forceChangePasswordNextLogin: true,
password: randomString.generate({ length: 24 })
},
signInNames: [
{
type: "emailAddress",
value: userPayload.email
}
],
surname: userPayload.last_name,
userPrincipalName: `${ulid()}@${adb2cCredentials.tenantId}`,
userType: "Member"
}),
graphRbacManagementClient.users.create(
withoutUndefinedValues({
accountEnabled: true,
creationType: "LocalAccount",
displayName: `${userPayload.first_name} ${userPayload.last_name}`,
givenName: userPayload.first_name,
mailNickname: userPayload.email.split("@")[0],
passwordProfile: {
forceChangePasswordNextLogin: true,
password: randomString.generate({ length: 24 })
},
signInNames: [
{
type: "emailAddress",
value: userPayload.email
}
],
surname: userPayload.last_name,
userPrincipalName: `${ulid()}@${adb2cCredentials.tenantId}`,
userType: "Member",
[adb2cTokenAttributeName]: userPayload.token_name
})
),
toError
).mapLeft(error =>
internalErrorHandler("Could not create the user on the ADB2C", error)
Expand Down Expand Up @@ -136,12 +142,14 @@ export function CreateUserHandler(
export function CreateUser(
adb2cCreds: IServicePrincipalCreds,
servicePrincipalCreds: IServicePrincipalCreds,
azureApimConfig: IAzureApimConfig
azureApimConfig: IAzureApimConfig,
adb2cTokenAttributeName: NonEmptyString
): express.RequestHandler {
const handler = CreateUserHandler(
adb2cCreds,
servicePrincipalCreds,
azureApimConfig
azureApimConfig,
adb2cTokenAttributeName
);

const middlewaresWrap = withRequestMiddlewares(
Expand Down
11 changes: 10 additions & 1 deletion CreateUser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ const azureApimConfig = {
subscriptionId: config.AZURE_SUBSCRIPTION_ID
};

const adb2cTokenAttributeName = getRequiredStringEnv(
"ADB2C_TOKEN_ATTRIBUTE_NAME"
);

// tslint:disable-next-line: no-let
let logger: Context["log"] | undefined;
const contextTransport = new AzureContextTransport(() => logger, {
Expand All @@ -46,7 +50,12 @@ secureExpressApp(app);
// Add express route
app.post(
"/adm/users",
CreateUser(adb2cCreds, servicePrincipalCreds, azureApimConfig)
CreateUser(
adb2cCreds,
servicePrincipalCreds,
azureApimConfig,
adb2cTokenAttributeName
)
);

const azureFunctionHandler = createAzureFunctionHandler(app);
Expand Down
147 changes: 147 additions & 0 deletions UpdateUser/__tests__/handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// tslint:disable:no-any

import { GraphRbacManagementClient } from "@azure/graph";
import * as msRestNodeAuth from "@azure/ms-rest-nodeauth";
import { NonEmptyString } from "italia-ts-commons/lib/strings";
import { UserCreated } from "../../generated/definitions/UserCreated";
import { UserPayload } from "../../generated/definitions/UserPayload";
import { UserStateEnum } from "../../generated/definitions/UserState";
import { IServicePrincipalCreds } from "../../utils/apim";
import { UpdateUserHandler } from "../handler";

const aTokenName = "ATokenName" as NonEmptyString;
const fakeServicePrincipalCredentials: IServicePrincipalCreds = {
clientId: "client-id",
secret: "secret",
tenantId: "tenant-id"
};

const fakeRequestPayload = {
email: "user@example.com",
first_name: "first-name",
last_name: "family-name",
token_name: aTokenName
} as UserPayload;

const fakeObjectId = "ADB2C-user";

const mockLoginWithServicePrincipalSecret = jest.spyOn(
msRestNodeAuth,
"loginWithServicePrincipalSecret"
);

jest.mock("@azure/graph");
jest.mock("@azure/arm-apimanagement");
const mockGraphRbacManagementClient = GraphRbacManagementClient as jest.Mock;
const mockLog = jest.fn();
const mockGetToken = jest.fn();

mockLoginWithServicePrincipalSecret.mockImplementation(() => {
return Promise.resolve({ getToken: mockGetToken });
});
mockGetToken.mockImplementation(() => {
return Promise.resolve(undefined);
});
const mockUsersCreate = jest.fn();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be this mockUsersUpdate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2948303


mockGraphRbacManagementClient.mockImplementation(() => ({
users: {
list: jest.fn(() =>
Promise.resolve([
{
email: "user@example.com"
}
])
),
update: mockUsersCreate
}
}));

const fakeAdb2cExtensionAppClientId = "extension-client-id" as NonEmptyString;

const mockedContext = { log: { error: mockLog } };

describe("UpdateUser", () => {
it("should return an internal error response if the ADB2C client can not be got", async () => {
mockLoginWithServicePrincipalSecret.mockImplementationOnce(() =>
Promise.reject("Error from ApiManagementClient constructor")
);

const updateUserHandler = UpdateUserHandler(
fakeServicePrincipalCredentials,
fakeAdb2cExtensionAppClientId
);

const response = await updateUserHandler(
mockedContext as any,
undefined as any,
undefined as any
);

expect(response.kind).toEqual("IResponseErrorInternal");
});

it("should return an internal error response if the ADB2C client can not update the user", async () => {
mockUsersCreate.mockImplementationOnce(() =>
Promise.reject("Users update error")
);

const updateUserHandler = UpdateUserHandler(
fakeServicePrincipalCredentials,
fakeAdb2cExtensionAppClientId
);

const response = await updateUserHandler(
mockedContext as any,
undefined as any,
fakeRequestPayload
);

expect(response.kind).toEqual("IResponseErrorInternal");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we relate the behaviour of this test with the test conditions?
For example

Suggested change
expect(response.kind).toEqual("IResponseErrorInternal");
expect(mockUsersCreate).toBeCalledTimes(1);
expect(response).toEqual({
apply: expect.any(Function),
detail: expectedError,
kind: "IResponseErrorInternal"
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2948303

});

it("should return the user updated", async () => {
const fakeApimUser = {
email: fakeRequestPayload.email,
firstName: fakeRequestPayload.first_name,
id: "user-id",
identities: [
{
id: fakeObjectId,
provider: "AadB2C"
}
],
lastName: fakeRequestPayload.last_name,
name: fakeObjectId,
registrationDate: new Date(),
state: UserStateEnum.active,
type: "Microsoft.ApiManagement/service/users"
};
const expectedUpdatedUser: UserCreated = {
email: fakeApimUser.email,
first_name: fakeApimUser.firstName,
id: fakeApimUser.name,
last_name: fakeApimUser.lastName,
token_name: aTokenName
};
mockUsersCreate.mockImplementationOnce(() =>
Promise.resolve({ objectId: fakeObjectId })
);

const updateUserHandler = UpdateUserHandler(
fakeServicePrincipalCredentials,
fakeAdb2cExtensionAppClientId
);

const response = await updateUserHandler(
mockedContext as any,
undefined as any,
fakeRequestPayload
);
expect(response).toEqual({
apply: expect.any(Function),
kind: "IResponseSuccessJson",
value: expectedUpdatedUser
});
});
});
20 changes: 20 additions & 0 deletions UpdateUser/function.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"bindings": [
{
"authLevel": "function",
"type": "httpTrigger",
"direction": "in",
"name": "req",
"route": "adm/users",
"methods": [
"put"
]
},
{
"type": "http",
"direction": "out",
"name": "res"
}
],
"scriptFile": "../dist/UpdateUser/index.js"
}
Loading