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

N21-1324-migration-page-buttons-fix #4447

Closed
wants to merge 9 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,100 @@ describe('OAuth SSO Controller (API)', () => {
});
});

describe('when school is in migration during the login process', () => {
const setupMigration = async () => {
const externalUserId = 'externalUserId';
const system: SystemEntity = systemFactory
.withOauthConfig()
.buildWithId({ provisioningStrategy: SystemProvisioningStrategy.SANIS });
const userLoginMigration: UserLoginMigrationEntity = userLoginMigrationFactory.buildWithId({
sourceSystem: system,
targetSystem: system,
startedAt: new Date('2022-12-17T03:24:00'),
});
const school: SchoolEntity = schoolFactory.buildWithId({
systems: [system],
userLoginMigration,
officialSchoolNumber: 'officialSchoolNumber',
});
const user: User = userFactory.buildWithId({ school });
const account: Account = accountFactory.buildWithId({ systemId: system.id, userId: user.id });
await em.persistAndFlush([system, user, school, account, userLoginMigration]);
em.clear();

const query: AuthorizationParams = new AuthorizationParams();
query.code = 'code';
query.state = 'state';

const idToken: string = JwtTestFactory.createJwt({
sub: 'testUser',
iss: system.oauthConfig?.issuer,
aud: system.oauthConfig?.clientId,
// For OIDC provisioning strategy
external_sub: externalUserId,
});

axiosMock
.onPost(system.oauthConfig?.tokenEndpoint)
.reply<OauthTokenResponse>(200, {
id_token: idToken,
refresh_token: 'refreshToken',
access_token: 'accessToken',
})
.onGet(system.provisioningUrl)
.replyOnce<SanisResponse>(200, {
pid: externalUserId,
person: {
name: {
familienname: 'familienName',
vorname: 'vorname',
},
geschlecht: 'weiblich',
lokalisierung: 'not necessary',
vertrauensstufe: 'not necessary',
},
personenkontexte: [
{
id: new UUID('aef1f4fd-c323-466e-962b-a84354c0e713').toString(),
rolle: SanisRole.LEHR,
organisation: {
id: new UUID('aef1f4fd-c323-466e-962b-a84354c0e713').toString(),
kennung: 'officialSchoolNumber',
name: 'schulName',
typ: 'not necessary',
},
personenstatus: 'not necessary',
},
],
});

return {
system,
user,
externalUserId,
school,
query,
userLoginMigration,
};
};

it('should redirect to login page with migration error', async () => {
const { system, query } = await setupMigration();
const { state, cookies } = await setupSessionState(system.id, false);
const baseUrl: string = Configuration.get('HOST') as string;
query.state = state;

await request(app.getHttpServer())
.get(`/sso/oauth`)
.set('Cookie', cookies)
.query(query)
.expect(302)
.expect('Location', `${baseUrl}/login?error=SCHOOL_IN_MIGRATION&provider=mock_type`);
});
});

describe('when a faulty query is passed', () => {
it('should redirect to the login page with an error', async () => {
it('should redirect to the login page with an sso error', async () => {
const { system, query } = await setup();
const { state, cookies } = await setupSessionState(system.id, false);
const clientUrl: string = Configuration.get('HOST') as string;
Expand Down
12 changes: 11 additions & 1 deletion apps/server/src/modules/oauth/controller/oauth-sso.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { HydraOauthUc } from '@src/modules/oauth/uc/hydra-oauth.uc';
import { OAuthMigrationError } from '@src/modules/user-login-migration/error/oauth-migration.error';
import { MigrationDto } from '@src/modules/user-login-migration/service/dto';
import { CookieOptions, Request, Response } from 'express';
import { SchoolInMigrationError } from '@src/modules/authentication/errors/school-in-migration.error';
import { OAuthSSOError } from '../error/oauth-sso.error';
import { OAuthTokenDto } from '../interface';
import { OauthLoginStateMapper } from '../mapper/oauth-login-state.mapper';
Expand All @@ -47,7 +48,16 @@ export class OauthSSOController {

private errorHandler(error: unknown, session: ISession, res: Response, provider?: string) {
this.logger.error(error);
const ssoError: OAuthSSOError = error instanceof OAuthSSOError ? error : new OAuthSSOError();

let ssoError: OAuthSSOError;

if (error instanceof SchoolInMigrationError) {
ssoError = new OAuthSSOError(error.message, error.type);
} else if (error instanceof OAuthSSOError) {
ssoError = error;
} else {
ssoError = new OAuthSSOError();
}

session.destroy((err) => {
this.logger.log(err);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { UserLoginMigrationRepo } from '@shared/repo';
import { legacySchoolDoFactory, userDoFactory } from '@shared/testing';
import { LegacySchoolService } from '@src/modules/legacy-school';
import { UserService } from '@src/modules/user';
import { SchoolInMigrationError } from '@src/modules/authentication/errors/school-in-migration.error';
import { MigrationCheckService } from './migration-check.service';

describe('MigrationCheckService', () => {
Expand Down Expand Up @@ -95,9 +96,9 @@ describe('MigrationCheckService', () => {
it('should return true', async () => {
setup();

const result: boolean = await service.shouldUserMigrate('externalId', 'systemId', 'officialSchoolNumber');

expect(result).toEqual(true);
await expect(service.shouldUserMigrate('externalId', 'systemId', 'officialSchoolNumber')).rejects.toThrowError(
new SchoolInMigrationError()
);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { EntityId, LegacySchoolDo, UserDO, UserLoginMigrationDO } from '@shared/
import { UserLoginMigrationRepo } from '@shared/repo';
import { LegacySchoolService } from '@src/modules/legacy-school';
import { UserService } from '@src/modules/user';
import { SchoolInMigrationError } from '@src/modules/authentication/errors/school-in-migration.error';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really need the whole import?
Until errors should be enough when the error itself will be exported in the barrel file


@Injectable()
export class MigrationCheckService {
Expand All @@ -22,11 +23,19 @@ export class MigrationCheckService {

const user: UserDO | null = await this.userService.findByExternalId(externalUserId, systemId);

if (user?.lastLoginSystemChange && userLoginMigration && !userLoginMigration.closedAt) {
if (
!user &&
userLoginMigration?.startedAt &&
!userLoginMigration.closedAt &&
userLoginMigration.sourceSystemId === userLoginMigration.targetSystemId
) {
throw new SchoolInMigrationError();
} else if (user && user.lastLoginSystemChange && userLoginMigration && !userLoginMigration.closedAt) {
const hasMigrated: boolean = user.lastLoginSystemChange > userLoginMigration.startedAt;
return !hasMigrated;
} else {
return !!userLoginMigration && !userLoginMigration.closedAt;
}
return !!userLoginMigration && !userLoginMigration.closedAt;
}
return false;
}
Expand Down
1 change: 1 addition & 0 deletions backup/setup/user_login_migrations.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[]
Copy link
Contributor

Choose a reason for hiding this comment

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

Thx

Loading