Skip to content

Commit

Permalink
Spsh 929 (#647)
Browse files Browse the repository at this point in the history
* add function to retrieve date of last password update

* missing import + smoke test

* fix test

* fix stale data in test

* fix test

* add tests

* add tolerance for time difference

* refactor test

* refactor timestamps to date/iso-format

* fix types in tests

* fix tests

* lower tolerance
  • Loading branch information
clauyan authored Sep 16, 2024
1 parent 18d19b8 commit b8d5fc6
Show file tree
Hide file tree
Showing 7 changed files with 253 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { ServiceProviderModule } from '../../service-provider/service-provider.m
import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js';
import { KeycloakConfig } from '../../../shared/config/keycloak.config.js';
import { KeycloakUserService } from '../../keycloak-administration/index.js';

describe('AuthenticationController', () => {
let module: TestingModule;
Expand All @@ -35,6 +36,7 @@ describe('AuthenticationController', () => {
let dbiamPersonenkontextRepoMock: DeepMocked<DBiamPersonenkontextRepo>;
let organisationRepoMock: DeepMocked<OrganisationRepository>;
let rolleRepoMock: DeepMocked<RolleRepo>;
const keycloakUserServiceMock: DeepMocked<KeycloakUserService> = createMock<KeycloakUserService>();
let keyCloakConfig: KeycloakConfig;

beforeAll(async () => {
Expand Down Expand Up @@ -70,6 +72,10 @@ describe('AuthenticationController', () => {
provide: OIDC_CLIENT,
useValue: createMock<Client>(),
},
{
provide: KeycloakUserService,
useValue: keycloakUserServiceMock,
},
],
}).compile();

Expand Down Expand Up @@ -250,6 +256,10 @@ describe('AuthenticationController', () => {
},
]),
});
keycloakUserServiceMock.getLastPasswordChange.mockResolvedValueOnce({
ok: true,
value: person.updatedAt,
});
const result: UserinfoResponse = await authController.info(permissions);

expect(result).toBeInstanceOf(UserinfoResponse);
Expand Down
18 changes: 14 additions & 4 deletions src/modules/authentication/api/authentication.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,17 @@ import { FrontendConfig, KeycloakConfig, ServerConfig } from '../../../shared/co
import { OIDC_CLIENT } from '../services/oidc-client.service.js';
import { LoginGuard } from './login.guard.js';
import { RedirectQueryParams } from './redirect.query.params.js';
import { UserinfoResponse } from './userinfo.response.js';
import { UserinfoExtension, UserinfoResponse } from './userinfo.response.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';
import { PersonPermissions } from '../domain/person-permissions.js';
import { PersonPermissions, PersonenkontextRolleFields } from '../domain/person-permissions.js';
import { Permissions } from './permissions.decorator.js';
import { Public } from './public.decorator.js';
import { PersonenkontextRolleFields } from '../domain/person-permissions.js';
import { RolleID } from '../../../shared/types/index.js';
import { PersonenkontextRolleFieldsResponse } from './personen-kontext-rolle-fields.response.js';
import { RollenSystemRechtServiceProviderIDResponse } from './rolle-systemrechte-serviceproviderid.response.js';
import { AuthenticationExceptionFilter } from './authentication-exception-filter.js';
import { KeycloakUserService } from '../../keycloak-administration/index.js';
import { DomainError } from '../../../shared/error/domain.error.js';
@UseFilters(new AuthenticationExceptionFilter())
@ApiTags('auth')
@Controller({ path: 'auth' })
Expand All @@ -43,6 +44,7 @@ export class AuthenticationController {
configService: ConfigService<ServerConfig>,
@Inject(OIDC_CLIENT) private client: Client,
private readonly logger: ClassLogger,
private keycloakUserService: KeycloakUserService,
) {
const frontendConfig: FrontendConfig = configService.getOrThrow<FrontendConfig>('FRONTEND');
const keycloakConfig: KeycloakConfig = configService.getOrThrow<KeycloakConfig>('KEYCLOAK');
Expand Down Expand Up @@ -120,7 +122,15 @@ export class AuthenticationController {
),
),
);
return new UserinfoResponse(permissions, rolleFieldsResponse);
const userinfoExtension: UserinfoExtension = {};
if (permissions.personFields.keycloakUserId) {
const lastPasswordChange: Result<Date, DomainError> = await this.keycloakUserService.getLastPasswordChange(
permissions.personFields.keycloakUserId,
);
if (lastPasswordChange.ok) userinfoExtension.password_updated_at = lastPasswordChange.value;
}

return new UserinfoResponse(permissions, rolleFieldsResponse, userinfoExtension);
}

@Get('reset-password')
Expand Down
36 changes: 36 additions & 0 deletions src/modules/authentication/api/userinfo.response.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { faker } from '@faker-js/faker';
import 'reflect-metadata';
import { UserinfoExtension, UserinfoResponse } from './userinfo.response.js';
import { PersonPermissions } from '../domain/person-permissions.js';
import { DoFactory } from '../../../../test/utils/do-factory.js';
import { DBiamPersonenkontextRepo } from '../../personenkontext/persistence/dbiam-personenkontext.repo.js';
import { OrganisationRepository } from '../../organisation/persistence/organisation.repository.js';
import { RolleRepo } from '../../rolle/repo/rolle.repo.js';
import { PersonenkontextRolleFieldsResponse } from './personen-kontext-rolle-fields.response.js';
import { createMock } from '@golevelup/ts-jest';

describe('UserinfoResponse', () => {
const permissions: PersonPermissions = new PersonPermissions(
createMock<DBiamPersonenkontextRepo>(),
createMock<OrganisationRepository>(),
createMock<RolleRepo>(),
DoFactory.createPerson(true),
);
const pk: PersonenkontextRolleFieldsResponse = {
organisationsId: faker.string.uuid(),
rolle: { systemrechte: [faker.string.alpha()], serviceProviderIds: [faker.string.uuid()] },
};

it('constructs the object without optional extension', () => {
const userinfoResponse: UserinfoResponse = new UserinfoResponse(permissions, [pk]);
expect(userinfoResponse).toBeDefined();
expect(userinfoResponse.password_updated_at).toBeUndefined();
});

it('constructs the object with optional extension', () => {
const extension: UserinfoExtension = { password_updated_at: faker.date.past() };
const userinfoResponse: UserinfoResponse = new UserinfoResponse(permissions, [pk], extension);
expect(userinfoResponse).toBeDefined();
expect(userinfoResponse.password_updated_at).toEqual(extension.password_updated_at?.toISOString());
});
});
18 changes: 15 additions & 3 deletions src/modules/authentication/api/userinfo.response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import { ApiProperty } from '@nestjs/swagger';
import { PersonPermissions } from '../domain/person-permissions.js';
import { PersonenkontextRolleFieldsResponse } from './personen-kontext-rolle-fields.response.js';

export type UserinfoExtension = {
password_updated_at?: Date;
};

export class UserinfoResponse {
@ApiProperty()
public sub: string;
Expand Down Expand Up @@ -58,12 +62,19 @@ export class UserinfoResponse {
public phone_number?: string;

@ApiProperty({ nullable: true })
public updated_at?: number;
public updated_at?: string;

@ApiProperty({ nullable: true })
public password_updated_at?: string;

@ApiProperty({ type: PersonenkontextRolleFieldsResponse, isArray: true })
public personenkontexte: PersonenkontextRolleFieldsResponse[];

public constructor(info: PersonPermissions, personenkontexte: PersonenkontextRolleFieldsResponse[]) {
public constructor(
info: PersonPermissions,
personenkontexte: PersonenkontextRolleFieldsResponse[],
extension?: UserinfoExtension,
) {
this.sub = info.personFields.keycloakUserId!;
this.personId = info.personFields.id;
this.name = `${info.personFields.vorname} ${info.personFields.familienname}`;
Expand All @@ -73,7 +84,8 @@ export class UserinfoResponse {
this.preferred_username = info.personFields.username;
this.gender = info.personFields.geschlecht;
this.birthdate = info.personFields.geburtsdatum?.toISOString();
this.updated_at = info.personFields.updatedAt.getTime() / 1000;
this.updated_at = info.personFields.updatedAt.toISOString();
this.personenkontexte = personenkontexte;
this.password_updated_at = extension?.password_updated_at?.toISOString();
}
}
2 changes: 2 additions & 0 deletions src/modules/authentication/authentication-api.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { PersonenKontextModule } from '../personenkontext/personenkontext.module
import { JwtStrategy } from './passport/jwt.strategy.js';
import { OrganisationModule } from '../organisation/organisation.module.js';
import { RolleModule } from '../rolle/rolle.module.js';
import { KeycloakAdministrationModule } from '../keycloak-administration/keycloak-administration.module.js';

@Module({
imports: [
Expand All @@ -21,6 +22,7 @@ import { RolleModule } from '../rolle/rolle.module.js';
PersonenKontextModule,
OrganisationModule,
RolleModule,
KeycloakAdministrationModule,
],
providers: [
OpenIdConnectStrategy,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { Test, TestingModule } from '@nestjs/testing';
import { GroupRepresentation, KeycloakAdminClient, UserRepresentation } from '@s3pweb/keycloak-admin-client-cjs';
import {
CredentialRepresentation,
GroupRepresentation,
KeycloakAdminClient,
UserRepresentation,
} from '@s3pweb/keycloak-admin-client-cjs';

import { faker } from '@faker-js/faker';
import { ConfigTestModule, DoFactory, LoggingTestModule, MapperTestModule } from '../../../../test/utils/index.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';
import { DomainError, EntityNotFoundError, KeycloakClientError } from '../../../shared/error/index.js';
import { OXContextName, OXUserName } from '../../../shared/types/ox-ids.types.js';
import { PersonService } from '../../person/domain/person.service.js';
import { Rolle } from '../../rolle/domain/rolle.js';
import { KeycloakAdministrationService } from './keycloak-admin-client.service.js';
import { type FindUserFilter, KeycloakUserService } from './keycloak-user.service.js';
import { PersonService } from '../../person/domain/person.service.js';
import { User } from './user.js';
import { Rolle } from '../../rolle/domain/rolle.js';
import { ClassLogger } from '../../../core/logging/class-logger.js';
import { OXContextName, OXUserName } from '../../../shared/types/ox-ids.types.js';

describe('KeycloakUserService', () => {
let module: TestingModule;
Expand Down Expand Up @@ -1381,4 +1386,127 @@ describe('KeycloakUserService', () => {
});
});
});

describe('getLastPasswordChange', () => {
const userCreationTimestamp: number = faker.date.past().valueOf();
const mockUser: UserRepresentation = { id: faker.string.uuid(), createdTimestamp: userCreationTimestamp };
describe('when the password has been updated', () => {
const updatedAt: Date = new Date();
const mockCredentials: Array<CredentialRepresentation> = [
{ type: 'password', createdDate: updatedAt.valueOf() },
{ type: 'other', createdDate: faker.date.past().valueOf() },
];
it('should return the timestamp', async () => {
kcUsersMock.findOne.mockResolvedValueOnce(mockUser);
kcUsersMock.getCredentials.mockResolvedValueOnce(mockCredentials);
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: true,
value: updatedAt,
});
});
});
describe('when the credential request fails', () => {
it('should return an error', async () => {
kcUsersMock.findOne.mockResolvedValueOnce(mockUser);
kcUsersMock.getCredentials.mockRejectedValueOnce({});
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: false,
error: new KeycloakClientError('Keycloak request failed'),
});
});
});

describe('when the password has not been updated', () => {
it('should return an error', async () => {
const updatedAt: number = mockUser.createdTimestamp!;
const mockCredentials: Array<CredentialRepresentation> = [
{ type: 'password', createdDate: updatedAt },
{ type: 'other', createdDate: faker.date.past().valueOf() },
];
kcUsersMock.findOne.mockResolvedValueOnce(mockUser);
kcUsersMock.getCredentials.mockResolvedValueOnce(mockCredentials);
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: false,
error: new KeycloakClientError('Keycloak user password has never been updated'),
});
});
});
describe('when the user does not exist', () => {
const updatedAt: number = mockUser.createdTimestamp!;
const mockCredentials: Array<CredentialRepresentation> = [
{ type: 'password', createdDate: updatedAt },
{ type: 'other', createdDate: faker.date.past().valueOf() },
];
it('should return an error', async () => {
kcUsersMock.findOne.mockRejectedValueOnce(null);
kcUsersMock.getCredentials.mockResolvedValueOnce(mockCredentials);
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: false,
error: new KeycloakClientError('Keycloak request failed'),
});
});
});
describe('when the user does not have a proper timestamp', () => {
const updatedAt: number = mockUser.createdTimestamp!;
const brokenUser: UserRepresentation = { id: faker.string.uuid() };
const mockCredentials: Array<CredentialRepresentation> = [
{ type: 'password', createdDate: updatedAt },
{ type: 'other', createdDate: faker.date.past().valueOf() },
];
it('should return an error', async () => {
kcUsersMock.findOne.mockResolvedValueOnce(brokenUser);
kcUsersMock.getCredentials.mockResolvedValueOnce(mockCredentials);
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: false,
error: new KeycloakClientError('Keycloak user has no createdTimestamp'),
});
});
});
describe('when something is wrong with the credentials', () => {
const data: Array<{ credentials: Array<CredentialRepresentation>; error: string }> = [
{ credentials: [], error: 'Keycloak returned no credentials' },
{ credentials: [{ type: 'other' }], error: 'Keycloak user has no password' },
{ credentials: [{ type: 'password' }], error: 'Keycloak user password has no createdDate' },
{
credentials: [{ type: 'password', createdDate: userCreationTimestamp }],
error: 'Keycloak user password has never been updated',
},
];
beforeEach(() => {
kcUsersMock.findOne.mockReset();
kcUsersMock.getCredentials.mockReset();
});
it.each(data)(
'should return an error',
async ({ credentials, error }: { credentials: Array<CredentialRepresentation>; error: string }) => {
kcUsersMock.findOne.mockResolvedValueOnce(mockUser);
kcUsersMock.getCredentials.mockResolvedValueOnce(credentials);
const result: Result<Date, DomainError> = await service.getLastPasswordChange(mockUser.id!);
expect(result).toStrictEqual({
ok: false,
error: new KeycloakClientError(error),
});
},
);
});

describe('when getAuthedKcAdminClient fails', () => {
it('should pass along error result', async () => {
const error: Result<KeycloakAdminClient, DomainError> = {
ok: false,
error: new KeycloakClientError('Could not authenticate'),
};

adminService.getAuthedKcAdminClient.mockResolvedValueOnce(error);
const res: Result<Date, DomainError> = await service.getLastPasswordChange(faker.string.uuid());

expect(res).toBe(error);
});
});
});
});
Loading

0 comments on commit b8d5fc6

Please sign in to comment.