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

Merged
merged 14 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}
5 changes: 5 additions & 0 deletions src/modules/import/api/import.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { ImportvorgangByIdBodyParams } from './importvorgang-by-id.body.params.j
import { HttpException } from '@nestjs/common';
import { MissingPermissionsError } from '../../../shared/error/missing-permissions.error.js';
import { ImportvorgangByIdParams } from './importvorgang-by-id.params.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';

describe('Import API with mocked ImportWorkflow', () => {
let sut: ImportController;
Expand All @@ -37,6 +38,10 @@ describe('Import API with mocked ImportWorkflow', () => {
provide: ImportWorkflow,
useValue: createMock<ImportWorkflow>(),
},
{
provide: ClassLogger,
useValue: createMock<ClassLogger>(),
},
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
ImportController,
],
}).compile();
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: <error>`,
);
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
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: <error>`,
);
throw schulConnexError;
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
} 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,
);
}
}
20 changes: 18 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 { ClassLogger } from '../../../core/logging/class-logger.js';
import { VornameForPersonWithTrailingSpaceError } from '../../person/domain/vorname-with-trailing-space.error.js';

describe('ImportWorkflow', () => {
let module: TestingModule;
Expand Down Expand Up @@ -68,6 +70,10 @@ describe('ImportWorkflow', () => {
provide: PersonPermissions,
useValue: createMock<PersonPermissions>(),
},
{
provide: ClassLogger,
useValue: createMock<ClassLogger>(),
},
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
],
}).compile();
rolleRepoMock = module.get(RolleRepo);
Expand Down Expand Up @@ -455,7 +461,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 +476,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 neuen Benutzer ${savedPersonWithPersonenkontext.person.referrer} (${savedPersonWithPersonenkontext.person.id}) angelegt.`,
DeFi3298 marked this conversation as resolved.
Show resolved Hide resolved
);
} 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
Loading
Loading