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

EW-965 Fixes for latest Keycloak upgrade #5102

Merged
merged 26 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
21466de
Add logging.
mkreuzkam-cap Jul 8, 2024
6f3f56f
More logs.
mkreuzkam-cap Jul 8, 2024
39d8a1d
EW-965 temporary changes for debugging
psachmann Jul 8, 2024
e1547d7
EW-965 console debugging
psachmann Jul 8, 2024
83c119c
EW-965 console debugging
psachmann Jul 8, 2024
b8d8a75
EW-965 code clean up
psachmann Jul 8, 2024
fe6ed11
EW-965 removing comment
psachmann Jul 8, 2024
a58cab7
EW-965 adding debug logs
psachmann Jul 9, 2024
1a1fcdf
EW-965 adding debug logs
psachmann Jul 9, 2024
c87b3c4
EW-965 disable in memory encryption
psachmann Jul 9, 2024
1307cc5
EW-965 some changes
psachmann Jul 9, 2024
8240d07
EW-965 code cleanup
psachmann Jul 10, 2024
7052d91
Merge branch 'main' into EW-965
SimoneRadtke-Cap Jul 10, 2024
b752891
EW-965 improving logging
psachmann Jul 10, 2024
4ca03c4
EW-965 fixing module dependencies
psachmann Jul 10, 2024
a0c8e58
EW-965 adding env variables
psachmann Jul 10, 2024
44a82ae
Only check unique email for exact matches.
mkreuzkam-cap Jul 11, 2024
c71b257
Merge branch 'main' into EW-965
mkreuzkam-cap Jul 11, 2024
21baadf
Add missing logger in test.
mkreuzkam-cap Jul 11, 2024
c89c66c
Fix import order.
mkreuzkam-cap Jul 11, 2024
d22e9d5
Merge branch 'main' into EW-965
SimoneRadtke-Cap Jul 11, 2024
e60e4e1
No longer log the full axios error.
mkreuzkam-cap Jul 11, 2024
3879c4b
Move loggable to new file.
mkreuzkam-cap Jul 11, 2024
f5f74f0
Merge branch 'main' into EW-965
SimoneRadtke-Cap Jul 11, 2024
4339181
Fix settings in test.
mkreuzkam-cap Jul 11, 2024
0d79473
EW-965 Remove comments
SimoneRadtke-Cap Jul 12, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ data:
FEATURE_IDENTITY_MANAGEMENT_ENABLED: "{{ FEATURE_IDENTITY_MANAGEMENT_ENABLED }}"
FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED: "{{ FEATURE_IDENTITY_MANAGEMENT_STORE_ENABLED }}"
FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED: "{{ FEATURE_IDENTITY_MANAGEMENT_LOGIN_ENABLED }}"
IDENTITY_MANAGEMENT__URI: "{{ IDENTITY_MANAGEMENT__URI }}"
IDENTITY_MANAGEMENT__INTERNAL_URI: "{{ IDENTITY_MANAGEMENT__INTERNAL_URI }}"
IDENTITY_MANAGEMENT__EXTERNAL_URI: "{{ IDENTITY_MANAGEMENT__EXTERNAL_URI }}"
IDENTITY_MANAGEMENT__TENANT: "{{ IDENTITY_MANAGEMENT__TENANT }}"
IDENTITY_MANAGEMENT__CLIENTID: "{{ IDENTITY_MANAGEMENT__CLIENTID }}"
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { HttpModule } from '@nestjs/axios';
import { Module } from '@nestjs/common';
import { LoggerModule } from '@src/core/logger';
import { EncryptionModule } from '../encryption';
import { IdentityManagementOauthService } from './identity-management-oauth.service';
import { IdentityManagementService } from './identity-management.service';
Expand All @@ -9,7 +10,7 @@ import { KeycloakIdentityManagementOauthService } from './keycloak/service/keycl
import { KeycloakIdentityManagementService } from './keycloak/service/keycloak-identity-management.service';

@Module({
imports: [KeycloakModule, KeycloakAdministrationModule, HttpModule, EncryptionModule],
imports: [KeycloakModule, KeycloakAdministrationModule, HttpModule, EncryptionModule, LoggerModule],
providers: [
{ provide: IdentityManagementService, useClass: KeycloakIdentityManagementService },
{ provide: IdentityManagementOauthService, useClass: KeycloakIdentityManagementOauthService },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
export const KeycloakSettings = Symbol('KeycloakSettings');

export interface IKeycloakSettings {
baseUrl: string;
internalBaseUrl: string;
externalBaseUrl: string;
realmName: string;
clientId: string;
credentials: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { IKeycloakSettings } from './interface/keycloak-settings.interface';
export default class KeycloakAdministration {
static keycloakSettings = (Configuration.get('FEATURE_IDENTITY_MANAGEMENT_ENABLED') as boolean)
? ({
baseUrl: Configuration.get('IDENTITY_MANAGEMENT__URI') as string,
internalBaseUrl: Configuration.get('IDENTITY_MANAGEMENT__INTERNAL_URI') as string,
externalBaseUrl: Configuration.get('IDENTITY_MANAGEMENT__EXTERNAL_URI') as string,
realmName: Configuration.get('IDENTITY_MANAGEMENT__TENANT') as string,
clientId: Configuration.get('IDENTITY_MANAGEMENT__CLIENTID') as string,
credentials: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ describe('KeycloakAdministrationService', () => {

const getSettings = (): IKeycloakSettings => {
return {
baseUrl: 'http://localhost:8080',
internalBaseUrl: 'http://localhost:8080',
externalBaseUrl: 'http://localhost:8080',
realmName: 'master',
clientId: 'client',
credentials: {
Expand Down Expand Up @@ -110,7 +111,7 @@ describe('KeycloakAdministrationService', () => {
describe('getWellKnownUrl', () => {
it('should return the well known URL', () => {
const wellKnownUrl = service.getWellKnownUrl();
expect(wellKnownUrl).toContain(settings.baseUrl);
expect(wellKnownUrl).toContain(settings.internalBaseUrl);
expect(wellKnownUrl).toContain(settings.realmName);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class KeycloakAdministrationService {
@Inject(KeycloakSettings) private readonly kcSettings: IKeycloakSettings
) {
this.kcAdminClient.setConfig({
baseUrl: kcSettings.baseUrl,
baseUrl: kcSettings.internalBaseUrl,
realmName: kcSettings.realmName,
});
}
Expand All @@ -33,7 +33,7 @@ export class KeycloakAdministrationService {
}

public getWellKnownUrl(): string {
return `${this.kcSettings.baseUrl}/realms/${this.kcSettings.realmName}/.well-known/openid-configuration`;
return `${this.kcSettings.externalBaseUrl}/realms/${this.kcSettings.realmName}/.well-known/openid-configuration`;
}

public getAdminUser(): string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ describe('KeycloakConfigurationService Unit', () => {

const getSettings = (): IKeycloakSettings => {
return {
baseUrl: 'http://localhost:8080',
internalBaseUrl: 'http://localhost:8080',
externalBaseUrl: 'http://localhost:8080',
realmName: 'master',
clientId: 'dBildungscloud',
credentials: {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { IDMLoginError } from './idm-login-error.loggable';

describe('IDMLoginError', () => {
describe('getLogMessage', () => {
it('should return log message', () => {
const err = new Error();
const loggable = new IDMLoginError(err);

expect(loggable.getLogMessage()).toStrictEqual({
message: 'Error while trying to login via IDM',
stack: err.stack,
type: 'IDM_LOGIN_ERROR',
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { ErrorLogMessage, Loggable, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';

export class IDMLoginError implements Loggable {
constructor(private readonly error: Error) {}

public getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
return {
message: 'Error while trying to login via IDM',
stack: this.error.stack,
type: 'IDM_LOGIN_ERROR',
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { DefaultEncryptionService, EncryptionService, SymetricKeyEncryptionService } from '@infra/encryption';
import KeycloakAdminClient from '@keycloak/keycloak-admin-client';
import { HttpService } from '@nestjs/axios';
import { ConfigService } from '@nestjs/config';
import { Test, TestingModule } from '@nestjs/testing';
import { AxiosResponse } from 'axios';
import { of } from 'rxjs';
import { Logger } from '@src/core/logger';
import { KeycloakAdministrationService } from '../../keycloak-administration/service/keycloak-administration.service';
import { KeycloakIdentityManagementOauthService } from './keycloak-identity-management-oauth.service';

Expand All @@ -32,8 +32,8 @@ describe('KeycloakIdentityManagementService', () => {
useValue: createMock<HttpService>(),
},
{
provide: ConfigService,
useValue: createMock<ConfigService>(),
provide: Logger,
useValue: createMock<Logger>(),
},
{
provide: DefaultEncryptionService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ import { DefaultEncryptionService, EncryptionService } from '@infra/encryption';
import { OauthConfig } from '@modules/system/domain';
import { HttpService } from '@nestjs/axios';
import { Inject, Injectable } from '@nestjs/common';
import { Logger } from '@src/core/logger';
import qs from 'qs';
import { lastValueFrom } from 'rxjs';
import { IdentityManagementOauthService } from '../../identity-management-oauth.service';
import { KeycloakAdministrationService } from '../../keycloak-administration/service/keycloak-administration.service';
import { IDMLoginError } from '../errors/idm-login-error.loggable';

@Injectable()
export class KeycloakIdentityManagementOauthService extends IdentityManagementOauthService {
Expand All @@ -14,7 +16,8 @@ export class KeycloakIdentityManagementOauthService extends IdentityManagementOa
constructor(
private readonly kcAdminService: KeycloakAdministrationService,
private readonly httpService: HttpService,
@Inject(DefaultEncryptionService) private readonly oAuthEncryptionService: EncryptionService
@Inject(DefaultEncryptionService) private readonly oAuthEncryptionService: EncryptionService,
private readonly logger: Logger
) {
super();
}
Expand Down Expand Up @@ -54,15 +57,15 @@ export class KeycloakIdentityManagementOauthService extends IdentityManagementOa
}

async resourceOwnerPasswordGrant(username: string, password: string): Promise<string | undefined> {
const { clientId, clientSecret, tokenEndpoint } = await this.getOauthConfig();
const data = {
username,
password,
grant_type: 'password',
client_id: clientId,
client_secret: this.oAuthEncryptionService.decrypt(clientSecret),
};
try {
const { clientId, clientSecret, tokenEndpoint } = await this.getOauthConfig();
const data = {
username,
password,
grant_type: 'password',
client_id: clientId,
client_secret: this.oAuthEncryptionService.decrypt(clientSecret),
};
const response = await lastValueFrom(
this.httpService.request<{ access_token: string }>({
method: 'post',
Expand All @@ -75,6 +78,8 @@ export class KeycloakIdentityManagementOauthService extends IdentityManagementOa
);
return response.data.access_token;
} catch (err) {
this.logger.warning(new IDMLoginError(err as Error));

return undefined;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ export class AccountServiceIdm extends AbstractAccountService {
}

public async isUniqueEmail(email: string): Promise<boolean> {
const [, count] = await this.identityManager.findAccountsByUsername(email);
const isUniqueEmail = count === 0;
const [accounts] = await this.identityManager.findAccountsByUsername(email, { exact: true });
const isUniqueEmail = accounts.length === 0;

return isUniqueEmail;
}
Expand Down
19 changes: 16 additions & 3 deletions config/default.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -434,12 +434,25 @@
"IDENTITY_MANAGEMENT": {
"type": "object",
"description": "Identity management server properties.",
"required": ["URI", "TENANT", "CLIENTID", "ADMIN_CLIENTID", "ADMIN_USER", "ADMIN_PASSWORD"],
"required": [
"INTERNAL_URI",
"EXTERNAL_URI",
"TENANT",
"CLIENTID",
"ADMIN_CLIENTID",
"ADMIN_USER",
"ADMIN_PASSWORD"
],
"properties": {
"URI": {
"INTERNAL_URI": {
"type": "string",
"default": null,
"description": "The ErWIn IDM base URI for Kubernetes cluster internal use."
},
"EXTERNAL_URI": {
"type": "string",
"default": null,
"description": "The ErWIn IDM base URI."
"description": "The ErWIn IDM base URI for Kubernetes cluster external use."
},
"TENANT": {
"type": "string",
Expand Down
3 changes: 2 additions & 1 deletion config/development.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
},
"FEATURE_IDENTITY_MANAGEMENT_ENABLED": true,
"IDENTITY_MANAGEMENT": {
"URI": "http://localhost:8080",
"INTERNAL_URI": "http://localhost:8080",
"EXTERNAL_URI": "http://localhost:8080",
"TENANT": "dBildungscloud",
"CLIENTID": "dbc",
"ADMIN_CLIENTID": "admin-cli",
Expand Down
3 changes: 2 additions & 1 deletion config/test.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
},
"FEATURE_IDENTITY_MANAGEMENT_ENABLED": true,
"IDENTITY_MANAGEMENT": {
"URI": "http://localhost:8080",
"INTERNAL_URI": "http://localhost:8080",
"EXTERNAL_URI": "http://localhost:8080",
"TENANT": "master",
"CLIENTID": "dbc",
"ADMIN_CLIENTID": "admin-cli",
Expand Down
Loading