Skip to content

Commit

Permalink
N21-2266 Add error for external users without role (#5387)
Browse files Browse the repository at this point in the history
  • Loading branch information
MarvinOehlerkingCap authored Dec 10, 2024
1 parent a9842f2 commit 4da6d99
Show file tree
Hide file tree
Showing 46 changed files with 287 additions and 167 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Type } from 'class-transformer';
import { IsEnum, IsObject, IsOptional, IsString, ValidateNested } from 'class-validator';
import { SchulconnexGroupType } from './schulconnex-group-type';
import { IsObject, IsOptional, IsString, ValidateNested } from 'class-validator';
import { SchulconnexLaufzeitResponse } from './schulconnex-laufzeit-response';

export class SchulconnexGruppeResponse {
Expand All @@ -10,8 +9,8 @@ export class SchulconnexGruppeResponse {
@IsString()
bezeichnung!: string;

@IsEnum(SchulconnexGroupType)
typ!: SchulconnexGroupType;
@IsString()
typ!: string;

@IsOptional()
@IsObject()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { Type } from 'class-transformer';
import { IsArray, IsEnum, IsObject, IsOptional, IsString, ValidateNested } from 'class-validator';
import { IsArray, IsObject, IsOptional, IsString, ValidateNested } from 'class-validator';
import { SchulconnexErreichbarkeitenResponse } from './schulconnex-erreichbarkeiten-response';
import { SchulconnexGruppenResponse } from './schulconnex-gruppen-response';
import { SchulconnexOrganisationResponse } from './schulconnex-organisation-response';
import { SchulconnexResponseValidationGroups } from './schulconnex-response-validation-groups';
import { SchulconnexRole } from './schulconnex-role';

export class SchulconnexPersonenkontextResponse {
@IsString({ groups: [SchulconnexResponseValidationGroups.USER, SchulconnexResponseValidationGroups.GROUPS] })
id!: string;

@IsEnum(SchulconnexRole, { groups: [SchulconnexResponseValidationGroups.USER] })
rolle!: SchulconnexRole;
@IsString({ groups: [SchulconnexResponseValidationGroups.USER] })
rolle!: string;

@IsObject({ groups: [SchulconnexResponseValidationGroups.SCHOOL] })
@ValidateNested({ groups: [SchulconnexResponseValidationGroups.SCHOOL] })
Expand Down
6 changes: 3 additions & 3 deletions apps/server/src/infra/sync/tsp/tsp-oauth-data.mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export class TspOauthDataMapper {
});

const externalSchools = new Map<string, ExternalSchoolDto>();
const externalClasses = new Map<string, ExternalUserDto>();
const externalClasses = new Map<string, ExternalClassDto>();
const teacherForClasses = new Map<string, Array<string>>();
const oauthDataDtos: OauthDataDto[] = [];

Expand Down Expand Up @@ -85,9 +85,9 @@ export class TspOauthDataMapper {
});

const classIds = teacherForClasses.get(tspTeacher.lehrerUid) ?? [];
const classes = classIds
const classes: ExternalClassDto[] = classIds
.map((classId) => externalClasses.get(classId))
.filter((externalClass) => !!externalClass);
.filter((externalClass: ExternalClassDto | undefined): externalClass is ExternalClassDto => !!externalClass);

const externalSchool = tspTeacher.schuleNummer == null ? undefined : externalSchools.get(tspTeacher.schuleNummer);

Expand Down
3 changes: 2 additions & 1 deletion apps/server/src/infra/sync/tsp/tsp-sync.strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import { schoolFactory } from '@src/modules/school/testing';
import { System } from '@src/modules/system';
import { systemFactory } from '@src/modules/system/testing';
import { SyncStrategyTarget } from '../sync-strategy.types';
import { TspLegacyMigrationService } from './tsp-legacy-migration.service';
import { TspFetchService } from './tsp-fetch.service';
import { TspLegacyMigrationService } from './tsp-legacy-migration.service';
import { TspOauthDataMapper } from './tsp-oauth-data.mapper';
import { TspSyncConfig } from './tsp-sync.config';
import { TspSyncService } from './tsp-sync.service';
Expand Down Expand Up @@ -171,6 +171,7 @@ describe(TspSyncStrategy.name, () => {
}),
externalUser: new ExternalUserDto({
externalId: faker.string.alpha(),
roles: [],
}),
});
const tspTeacher: RobjExportLehrerMigration = {
Expand Down
33 changes: 17 additions & 16 deletions apps/server/src/modules/oauth/service/oauth.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { LegacyLogger } from '@src/core/logger';
import { OauthDataDto } from '@src/modules/provisioning/dto';
import { System } from '@src/modules/system';
import jwt, { JwtPayload } from 'jsonwebtoken';
import { externalUserDtoFactory } from '../../provisioning/testing';
import { OAuthTokenDto } from '../interface';
import {
OauthConfigMissingLoggableException,
Expand Down Expand Up @@ -378,9 +379,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: 'externalSchoolId',
name: 'External School',
Expand Down Expand Up @@ -429,9 +430,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: 'externalSchoolId',
name: 'External School',
Expand Down Expand Up @@ -476,9 +477,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: 'externalSchoolId',
name: 'External School',
Expand Down Expand Up @@ -544,9 +545,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: externalSchoolId,
name: school.name,
Expand Down Expand Up @@ -612,9 +613,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: externalSchoolId,
name: school.name,
Expand Down Expand Up @@ -675,9 +676,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: externalSchoolId,
name: school.name,
Expand Down Expand Up @@ -737,9 +738,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: externalSchoolId,
name: school.name,
Expand Down Expand Up @@ -804,9 +805,9 @@ describe('OAuthService', () => {
provisioningStrategy: SystemProvisioningStrategy.SANIS,
provisioningUrl: 'https://mock.person-info.de/',
},
externalUser: {
externalUser: externalUserDtoFactory.build({
externalId: externalUserId,
},
}),
externalSchool: {
externalId: externalSchoolId,
name: school.name,
Expand Down
12 changes: 6 additions & 6 deletions apps/server/src/modules/provisioning/dto/external-user.dto.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import { RoleName } from '@shared/domain/interface';

export class ExternalUserDto {
externalId: string;
public externalId: string;

firstName?: string;
public firstName?: string;

lastName?: string;
public lastName?: string;

email?: string;
public email?: string;

roles?: RoleName[];
public roles: RoleName[];

birthday?: Date;
public birthday?: Date;

constructor(props: ExternalUserDto) {
this.externalId = props.externalId;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { ExternalUserDto } from '../dto';
import { externalUserDtoFactory } from '../testing';
import { FetchingPoliciesInfoFailedLoggable } from './fetching-policies-info-failed.loggable';

describe(FetchingPoliciesInfoFailedLoggable.name, () => {
describe('getLogMessage', () => {
const setup = () => {
const externalUserDto: ExternalUserDto = {
externalId: 'someId',
};
const externalUserDto = externalUserDtoFactory.build();
const policiesInfoEndpoint = 'someEndpoint';

const loggable = new FetchingPoliciesInfoFailedLoggable(externalUserDto, policiesInfoEndpoint);
Expand Down
2 changes: 2 additions & 0 deletions apps/server/src/modules/provisioning/loggable/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,5 @@ export * from './group-role-unknown.loggable';
export { SchoolExternalToolCreatedLoggable } from './school-external-tool-created.loggable';
export { FetchingPoliciesInfoFailedLoggable } from './fetching-policies-info-failed.loggable';
export { PoliciesInfoErrorResponseLoggable } from './policies-info-error-response-loggable';
export { UserRoleUnknownLoggableException } from './user-role-unknown.loggable-exception';
export { SchoolMissingLoggableException } from './school-missing.loggable-exception';
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { externalUserDtoFactory } from '../testing';
import { SchoolMissingLoggableException } from './school-missing.loggable-exception';

describe(SchoolMissingLoggableException.name, () => {
describe('getLogMessage', () => {
const setup = () => {
const externalUser = externalUserDtoFactory.build();

const loggable = new SchoolMissingLoggableException(externalUser);

return {
loggable,
externalUser,
};
};

it('should return a loggable message', () => {
const { loggable, externalUser } = setup();

const message = loggable.getLogMessage();

expect(message).toEqual({
type: 'SCHOOL_MISSING',
stack: expect.any(String),
message: 'Unable to create new external user without a school',
data: {
externalUserId: externalUser.externalId,
},
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { HttpStatus } from '@nestjs/common';
import { BusinessError } from '@shared/common';
import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
import { ExternalUserDto } from '../dto';

export class SchoolMissingLoggableException extends BusinessError implements Loggable {
constructor(private readonly externalUser: ExternalUserDto) {
super(
{
type: 'SCHOOL_MISSING',
title: 'Invalid school data',
defaultMessage: 'Unable to create new external user without a school',
},
HttpStatus.UNPROCESSABLE_ENTITY
);
}

public getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
return {
type: this.type,
message: this.message,
stack: this.stack,
data: {
externalUserId: this.externalUser.externalId,
},
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { externalUserDtoFactory } from '../testing';
import { UserRoleUnknownLoggableException } from './user-role-unknown.loggable-exception';

describe(UserRoleUnknownLoggableException.name, () => {
describe('getLogMessage', () => {
const setup = () => {
const externalUser = externalUserDtoFactory.build();

const loggable = new UserRoleUnknownLoggableException(externalUser);

return {
loggable,
externalUser,
};
};

it('should return a loggable message', () => {
const { loggable, externalUser } = setup();

const message = loggable.getLogMessage();

expect(message).toEqual({
type: 'EXTERNAL_USER_ROLE_UNKNOWN',
stack: expect.any(String),
message: 'External user has no or no known role assigned to them',
data: {
externalUserId: externalUser.externalId,
},
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { HttpStatus } from '@nestjs/common';
import { BusinessError } from '@shared/common';
import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
import { ExternalUserDto } from '../dto';

export class UserRoleUnknownLoggableException extends BusinessError implements Loggable {
constructor(private readonly externalUser: ExternalUserDto) {
super(
{
type: 'EXTERNAL_USER_ROLE_UNKNOWN',
title: 'Invalid user role',
defaultMessage: 'External user has no or no known role assigned to them',
},
HttpStatus.UNPROCESSABLE_ENTITY
);
}

public getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
return {
type: this.type,
message: this.message,
stack: this.stack,
data: {
externalUserId: this.externalUser.externalId,
},
};
}
}
8 changes: 4 additions & 4 deletions apps/server/src/modules/provisioning/provisioning.module.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { AccountModule } from '@modules/account';
import { ClassModule } from '@modules/class';
import { GroupModule } from '@modules/group';
import { LearnroomModule } from '@modules/learnroom';
import { LegacySchoolModule } from '@modules/legacy-school';
Expand All @@ -8,11 +9,10 @@ import { SystemModule } from '@modules/system/system.module';
import { ExternalToolModule } from '@modules/tool';
import { SchoolExternalToolModule } from '@modules/tool/school-external-tool';
import { UserModule } from '@modules/user';
import { UserLicenseModule } from '@modules/user-license';
import { Module } from '@nestjs/common';
import { LoggerModule } from '@src/core/logger';
import { SchulconnexClientModule } from '@src/infra/schulconnex-client/schulconnex-client.module';
import { ClassModule } from '../class';
import { UserLicenseModule } from '../user-license';
import { ProvisioningService } from './service/provisioning.service';
import { TspProvisioningService } from './service/tsp-provisioning.service';
import {
Expand All @@ -28,8 +28,8 @@ import {
SchulconnexSchoolProvisioningService,
SchulconnexToolProvisioningService,
SchulconnexUserProvisioningService,
} from './strategy/oidc/service';
import { TspProvisioningStrategy } from './strategy/tsp/tsp.strategy';
} from './strategy/schulconnex/service';
import { TspProvisioningStrategy } from './strategy/tsp';

@Module({
imports: [
Expand Down
Loading

0 comments on commit 4da6d99

Please sign in to comment.