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

SPSH-1296 #779

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
72 changes: 69 additions & 3 deletions src/modules/authentication/api/login.guard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const logInSpy: jest.SpyInstance = jest.spyOn(AuthGuard(['jwt', 'oidc']).prototy
describe('LoginGuard', () => {
let module: TestingModule;
let sut: LoginGuard;
let logger: DeepMocked<ClassLogger>;

beforeAll(async () => {
module = await Test.createTestingModule({
Expand All @@ -37,6 +38,7 @@ describe('LoginGuard', () => {
}).compile();

sut = module.get(LoginGuard);
logger = module.get(ClassLogger);
}, 30 * 1_000);

afterAll(async () => {
Expand All @@ -56,7 +58,21 @@ describe('LoginGuard', () => {
canActivateSpy.mockResolvedValueOnce(true);
logInSpy.mockResolvedValueOnce(undefined);
const contextMock: DeepMocked<ExecutionContext> = createMock();
contextMock.switchToHttp().getRequest<DeepMocked<Request>>().isAuthenticated.mockReturnValue(false);
contextMock.switchToHttp().getRequest.mockReturnValue({
query: {
requiredStepUpLevel: StepUpLevel.GOLD,
},
isAuthenticated: jest.fn().mockReturnValue(false),
passportUser: {
userinfo: {
preferred_username: 'test',
},
},
session: {
requiredStepUpLevel: StepUpLevel.GOLD,
},
});
contextMock.switchToHttp().getResponse.mockReturnValue({});

await sut.canActivate(contextMock);

Expand Down Expand Up @@ -86,7 +102,20 @@ describe('LoginGuard', () => {
canActivateSpy.mockResolvedValueOnce(true);
logInSpy.mockResolvedValueOnce(undefined);
const contextMock: DeepMocked<ExecutionContext> = createMock();
contextMock.switchToHttp().getRequest<DeepMocked<Request>>().isAuthenticated.mockReturnValue(false);
contextMock.switchToHttp().getRequest.mockReturnValue({
query: {
requiredStepUpLevel: StepUpLevel.GOLD,
},
isAuthenticated: jest.fn().mockReturnValue(false),
passportUser: {
userinfo: {
preferred_username: 'test',
},
},
session: {
requiredStepUpLevel: StepUpLevel.GOLD,
},
});

await sut.canActivate(contextMock);

Expand All @@ -98,7 +127,20 @@ describe('LoginGuard', () => {
logInSpy.mockResolvedValueOnce(undefined);
const contextMock: DeepMocked<ExecutionContext> = createMock();
const redirectUrl: string = faker.internet.url();
contextMock.switchToHttp().getRequest<Request>().query = { redirectUrl };
contextMock.switchToHttp().getRequest.mockReturnValue({
query: {
redirectUrl,
},
isAuthenticated: jest.fn().mockReturnValue(false),
passportUser: {
userinfo: {
preferred_username: 'test',
},
},
session: {
redirectUrl: redirectUrl,
},
});

await sut.canActivate(contextMock);

Expand Down Expand Up @@ -155,5 +197,29 @@ describe('LoginGuard', () => {
}),
);
});

it('should log successful login', async () => {
canActivateSpy.mockResolvedValueOnce(true);
logInSpy.mockResolvedValueOnce(undefined);
const contextMock: DeepMocked<ExecutionContext> = createMock();
contextMock.switchToHttp().getRequest.mockReturnValue({
query: {
requiredStepUpLevel: 'gold',
},
isAuthenticated: jest.fn().mockReturnValue(true),
passportUser: {
userinfo: {
preferred_username: 'test',
},
},
session: {
requiredStepUpLevel: StepUpLevel.GOLD,
},
});

await sut.canActivate(contextMock);

expect(logger.info).toHaveBeenCalledWith('Benutzer test hat sich im Schulportal angemeldet.');
});
});
});
13 changes: 11 additions & 2 deletions src/modules/authentication/api/login.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,16 @@ export class LoginGuard extends AuthGuard(['jwt', 'oidc']) {

return false;
}

return request.isAuthenticated();
const isAuthenticated: boolean = request.isAuthenticated();
if (isAuthenticated) {
this.logger.info(
`Benutzer ${request.passportUser?.userinfo.preferred_username} hat sich im Schulportal angemeldet.`,
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
);
} else {
this.logger.error(
`Fehlergeschlagener Login mit Benutzer ${request.passportUser?.userinfo.preferred_username}`,
);
}
return isAuthenticated;
}
}
8 changes: 6 additions & 2 deletions src/modules/import/api/import.controller.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { faker } from '@faker-js/faker';
import { APP_PIPE } from '@nestjs/core';
import { Test, TestingModule } from '@nestjs/testing';
import { DEFAULT_TIMEOUT_FOR_TESTCONTAINERS, MapperTestModule } from '../../../../test/utils/index.js';
import {
DEFAULT_TIMEOUT_FOR_TESTCONTAINERS,
LoggingTestModule,
MapperTestModule,
} from '../../../../test/utils/index.js';
import { GlobalValidationPipe } from '../../../shared/validation/global-validation.pipe.js';
import { createMock, DeepMocked } from '@golevelup/ts-jest';

Expand All @@ -23,7 +27,7 @@ describe('Import API with mocked ImportWorkflow', () => {

beforeAll(async () => {
const module: TestingModule = await Test.createTestingModule({
imports: [MapperTestModule],
imports: [MapperTestModule, LoggingTestModule],
providers: [
{
provide: APP_PIPE,
Expand Down
19 changes: 17 additions & 2 deletions src/modules/import/api/import.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Controller,
Delete,
HttpCode,
HttpException,
HttpStatus,
Param,
ParseFilePipeBuilder,
Expand Down Expand Up @@ -45,14 +46,18 @@ import { ImportDomainError } from '../domain/import-domain.error.js';
import { SchulConnexErrorMapper } from '../../../shared/error/schul-connex-error.mapper.js';
import { ImportExceptionFilter } from './import-exception-filter.js';
import { ImportvorgangByIdParams } from './importvorgang-by-id.params.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';

@UseFilters(SchulConnexValidationErrorFilter, new AuthenticationExceptionFilter(), new ImportExceptionFilter())
@ApiTags('import')
@ApiBearerAuth()
@ApiOAuth2(['openid'])
@Controller({ path: 'import' })
export class ImportController {
public constructor(private readonly importWorkflowFactory: ImportWorkflowFactory) {}
public constructor(
private readonly importWorkflowFactory: ImportWorkflowFactory,
private readonly logger: ClassLogger,
) {}

@Post('upload')
@ApiConsumes('multipart/form-data')
Expand Down Expand Up @@ -129,13 +134,23 @@ export class ImportController {

if (!result.ok) {
if (result.error instanceof ImportDomainError) {
this.logger.error(
`Admin ${permissions.personFields.username} (AdminId: ${permissions.personFields.id}) hat versucht für Schule: ${body.organisationId} einen CSV Import durchzuführen. Fehler: ${result.error.message}`,
);
throw result.error;
}

throw SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(
const schulConnexError: HttpException = SchulConnexErrorMapper.mapSchulConnexErrorToHttpException(
SchulConnexErrorMapper.mapDomainErrorToSchulConnexError(result.error as DomainError),
);
this.logger.error(
`Admin ${permissions.personFields.username} (AdminId: ${permissions.personFields.id}) hat versucht für Schule: ${body.organisationId} einen CSV Import durchzuführen. Fehler: ${schulConnexError.message}`,
);
throw schulConnexError;
} else {
this.logger.info(
`Admin ${permissions.personFields.username} (AdminId: ${permissions.personFields.id}) hat für Schule: ${body.organisationId} einen CSV Import durchgeführt.`,
);
const fileName: string = importWorkflow.getFileName(body.importvorgangId);
const contentDisposition: string = `attachment; filename="${fileName}"`;
res.set({
Expand Down
3 changes: 3 additions & 0 deletions src/modules/import/domain/import-workflow.factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { OrganisationRepository } from '../../organisation/persistence/organisat
import { ImportWorkflow } from './import-workflow.js';
import { ImportDataRepository } from '../persistence/import-data.repository.js';
import { PersonenkontextCreationService } from '../../personenkontext/domain/personenkontext-creation.service.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';

@Injectable()
export class ImportWorkflowFactory {
Expand All @@ -12,6 +13,7 @@ export class ImportWorkflowFactory {
private readonly organisationRepository: OrganisationRepository,
private readonly importDataRepository: ImportDataRepository,
private readonly personenkontextCreationService: PersonenkontextCreationService,
private readonly logger: ClassLogger,
) {}

public createNew(): ImportWorkflow {
Expand All @@ -20,6 +22,7 @@ export class ImportWorkflowFactory {
this.organisationRepository,
this.importDataRepository,
this.personenkontextCreationService,
this.logger,
);
}
}
17 changes: 15 additions & 2 deletions src/modules/import/domain/import-workflow.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import { ImportCSVFileEmptyError } from './import-csv-file-empty.error.js';
import { ImportNurLernAnSchuleUndKlasseError } from './import-nur-lern-an-schule-und-klasse.error.js';
import { ImportCSVFileParsingError } from './import-csv-file-parsing.error.js';
import { ImportCSVFileInvalidHeaderError } from './import-csv-file-invalid-header.error.js';
import { VornameForPersonWithTrailingSpaceError } from '../../person/domain/vorname-with-trailing-space.error.js';
import { LoggingTestModule } from '../../../../test/utils/logging-test.module.js';

describe('ImportWorkflow', () => {
let module: TestingModule;
Expand All @@ -46,6 +48,7 @@ describe('ImportWorkflow', () => {

beforeAll(async () => {
module = await Test.createTestingModule({
imports: [LoggingTestModule],
providers: [
ImportWorkflowFactory,
{
Expand Down Expand Up @@ -455,7 +458,11 @@ describe('ImportWorkflow', () => {
organisationRepoMock.findChildOrgasForIds.mockResolvedValueOnce([klasse]);

const importvorgangId: string = faker.string.uuid();
const importDataItem: ImportDataItem<true> = DoFactory.createImportDataItem(true, {
const importDataItem1: ImportDataItem<true> = DoFactory.createImportDataItem(true, {
importvorgangId,
klasse: '1A',
});
const importDataItem2: ImportDataItem<true> = DoFactory.createImportDataItem(true, {
importvorgangId,
klasse: '1A',
});
Expand All @@ -466,8 +473,14 @@ describe('ImportWorkflow', () => {
DoFactory.createPersonenkontext(true, { organisationId: klasse.id }),
],
};
importDataRepositoryMock.findByImportVorgangId.mockResolvedValueOnce([[importDataItem], 1]);
importDataRepositoryMock.findByImportVorgangId.mockResolvedValueOnce([
[importDataItem1, importDataItem2],
2,
]);
personenkontextCreationServiceMock.createPersonWithPersonenkontexte.mockResolvedValueOnce(pks);
personenkontextCreationServiceMock.createPersonWithPersonenkontexte.mockResolvedValueOnce(
new VornameForPersonWithTrailingSpaceError(),
);
organisationRepoMock.findById.mockResolvedValueOnce(DoFactory.createOrganisation(true));
rolleRepoMock.findById.mockResolvedValueOnce(DoFactory.createRolle(true));
jest.spyOn(Buffer, 'from').mockImplementationOnce(() => {
Expand Down
14 changes: 13 additions & 1 deletion src/modules/import/domain/import-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { ImportDomainErrorI18nTypes } from './import-i18n-errors.js';
import { validateSync } from 'class-validator';
import { plainToInstance } from 'class-transformer';
import { ImportCSVFileInvalidHeaderError } from './import-csv-file-invalid-header.error.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';

export type ImportUploadResultFields = {
importVorgangId: string;
Expand Down Expand Up @@ -60,19 +61,22 @@ export class ImportWorkflow {
private readonly organisationRepository: OrganisationRepository,
private readonly importDataRepository: ImportDataRepository,
private readonly personenkontextCreationService: PersonenkontextCreationService,
private readonly logger: ClassLogger,
) {}

public static createNew(
rolleRepo: RolleRepo,
organisationRepository: OrganisationRepository,
importDataRepository: ImportDataRepository,
personenkontextCreationService: PersonenkontextCreationService,
logger: ClassLogger,
): ImportWorkflow {
return new ImportWorkflow(
rolleRepo,
organisationRepository,
importDataRepository,
personenkontextCreationService,
logger,
);
}

Expand Down Expand Up @@ -260,7 +264,15 @@ export class ImportWorkflow {
importDataItem.nachname,
createPersonenkontexte,
);

if (!(savedPersonWithPersonenkontext instanceof DomainError)) {
this.logger.info(
`System hat einen neuen Benutzer ${savedPersonWithPersonenkontext.person.referrer} (${savedPersonWithPersonenkontext.person.id}) angelegt.`,
);
} else {
this.logger.info(
`System hat versucht einen neuen Benutzer für ${importDataItem.vorname} ${importDataItem.nachname} anzulegen. Fehler: ${savedPersonWithPersonenkontext.message}`,
);
}
savedPersonenWithPersonenkontext.push(savedPersonWithPersonenkontext);
}
/* eslint-disable no-await-in-loop */
Expand Down
2 changes: 2 additions & 0 deletions src/modules/import/import-api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import { ImportModule } from './import.module.js';
import { MulterModule } from '@nestjs/platform-express';
import { ConfigModule, ConfigService } from '@nestjs/config';
import { ImportConfig } from '../../shared/config/import.config.js';
import { LoggerModule } from '../../core/logging/logger.module.js';

@Module({
imports: [
ImportModule,
LoggerModule.register(ImportApiModule.name),
MulterModule.registerAsync({
imports: [ConfigModule],
useFactory: (configService: ConfigService) => ({
Expand Down
3 changes: 2 additions & 1 deletion src/modules/import/import.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import { RolleModule } from '../rolle/rolle.module.js';
import { OrganisationModule } from '../organisation/organisation.module.js';
import { ImportDataRepository } from './persistence/import-data.repository.js';
import { PersonenKontextModule } from '../personenkontext/personenkontext.module.js';
import { LoggerModule } from '../../core/logging/logger.module.js';

@Module({
imports: [RolleModule, OrganisationModule, PersonenKontextModule],
imports: [RolleModule, OrganisationModule, PersonenKontextModule, LoggerModule.register(ImportModule.name)],
providers: [ImportWorkflowFactory, ImportDataRepository],
exports: [ImportWorkflowFactory],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { TestingModule, Test } from '@nestjs/testing';
import { ConfigTestModule, LoggingTestModule } from '../../../test/utils/index.js';
import { PersonDeletedEvent } from '../../shared/events/person-deleted.event.js';
import { ResetTokenResponse, PrivacyIdeaToken } from './privacy-idea-api.types.js';
import { TokenResetError } from './api/error/token-reset.error.js';

export const mockPrivacyIdeaToken: PrivacyIdeaToken = {
active: true,
Expand Down Expand Up @@ -101,6 +102,13 @@ describe('PrivacyIdeaAdministration Event Handler', () => {
expect(privacyIdeaAdministrationServiceMock.resetToken).toHaveBeenCalledTimes(1);
expect(privacyIdeaAdministrationServiceMock.deleteUserWrapper).toHaveBeenCalledTimes(1);
});
it('when token reset failed throw error', async () => {
privacyIdeaAdministrationServiceMock.getUserTokens.mockResolvedValueOnce([mockPrivacyIdeaToken]);
privacyIdeaAdministrationServiceMock.resetToken.mockRejectedValueOnce(new TokenResetError());

await expect(sut.handlePersonDeletedEvent(event)).rejects.toThrow(new TokenResetError());
expect(privacyIdeaAdministrationServiceMock.resetToken).toHaveBeenCalledTimes(1);
});
});

describe('when person has no privacyIDEA tokens', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,18 @@ export class PrivacyIdeaAdministrationEventHandler {
event.referrer,
);
if (userTokens.length > 0) {
await this.privacyIdeaAdministrationService.resetToken(event.referrer);
try {
await this.privacyIdeaAdministrationService.resetToken(event.referrer);
this.logger.info(
`System hat für Benutzer ${event.referrer} (BenutzerId: ${event.personId}) den 2FA Token zurückgesetzt.`,
);
} catch (error) {
this.logger.error(
`System hat versucht den 2FA Token von Benutzer ${event.referrer} (BenutzerId: ${event.personId}) zurückzusetzen.`,
error,
);
throw error;
}
}
await this.privacyIdeaAdministrationService.deleteUserWrapper(event.referrer);
}
Expand Down
Loading
Loading