Skip to content

Commit bc0a041

Browse files
gkrajniakgkrajniaksap
gkrajniak
and
gkrajniaksap
authored
feat: change the returned domain (#257)
- **Enhance redirect_uri error handling in auth token service** - **Refactor tests in env service for better structure and coverage** - **Introduce logic to handle subdomain-based base domains in env service** This commit enhances error verbosity and ensures that subdomain configurations and related logic are properly tested and handled in the service modules. Co-authored-by: Grzegorz Krajniak <[email protected]>
1 parent 4eb48e0 commit bc0a041

File tree

3 files changed

+139
-116
lines changed

3 files changed

+139
-116
lines changed

‎src/auth/auth-token.service.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,10 @@ export class AuthTokenService {
7777
`${currentAuthEnv.clientId}:${currentAuthEnv.clientSecret}`,
7878
).toString('base64')}`;
7979

80-
const redirectUri = body.get('redirect_uri');
80+
const redirectUriMsg =
81+
body.get('grant_type') === 'authorization_code'
82+
? `Redirect URI: ${body.get('redirect_uri')}`
83+
: '';
8184
const tokenFetchResult = await firstValueFrom(
8285
this.httpService
8386
.post<AuthTokenData>(currentAuthEnv.oauthTokenUrl, body, {
@@ -92,7 +95,7 @@ export class AuthTokenService {
9295
throw new Error(
9396
`Error response from auth token server: ${e.toString()}.
9497
${e.response.data['error']}: ${e.response.data['error_description']}.
95-
params.redirect_uri: ${redirectUri}`,
98+
${redirectUriMsg}`,
9699
);
97100
}),
98101
),

‎src/env/env.service.spec.ts

Lines changed: 129 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -160,156 +160,171 @@ describe('EnvService', () => {
160160
discoveryServiceMock = mock<DiscoveryService>();
161161
});
162162

163-
it('should get oauthServerUrl and oauthTokenUrl form DISCOVERY_ENDPOINT when discoveryService returns proper values', async () => {
164-
const request = mock<Request>();
165-
request.hostname = 'app.hyper.space';
166-
167-
const discoveryServiceMockGetOIDC = jest.spyOn(
168-
discoveryServiceMock,
169-
'getOIDC',
170-
);
171-
discoveryServiceMockGetOIDC.mockResolvedValue({
172-
authorization_endpoint: 'example.com/authorization_endpoint',
173-
token_endpoint: 'example.com/token_endpoint',
163+
describe('getEnvWithAuth', () => {
164+
it('should get oauthServerUrl and oauthTokenUrl form DISCOVERY_ENDPOINT when discoveryService returns proper values', async () => {
165+
const request = mock<Request>();
166+
request.hostname = 'app.hyper.space';
167+
168+
const discoveryServiceMockGetOIDC = jest.spyOn(
169+
discoveryServiceMock,
170+
'getOIDC',
171+
);
172+
discoveryServiceMockGetOIDC.mockResolvedValue({
173+
authorization_endpoint: 'example.com/authorization_endpoint',
174+
token_endpoint: 'example.com/token_endpoint',
175+
});
176+
177+
process.env['DISCOVERY_ENDPOINT_APP'] = 'example.com';
178+
const envWithAuth = await service.getCurrentAuthEnv(request);
179+
180+
expect(envWithAuth.oauthServerUrl).toEqual(
181+
'example.com/authorization_endpoint',
182+
);
183+
expect(envWithAuth.oauthTokenUrl).toEqual('example.com/token_endpoint');
174184
});
175185

176-
process.env['DISCOVERY_ENDPOINT_APP'] = 'example.com';
177-
const envWithAuth = await service.getCurrentAuthEnv(request);
186+
it('should get oauthServerUrl and oauthTokenUrl from env when discoveryService returns null ', async () => {
187+
const request = mock<Request>();
188+
request.hostname = 'app.hyper.space';
178189

179-
expect(envWithAuth.oauthServerUrl).toEqual(
180-
'example.com/authorization_endpoint',
181-
);
182-
expect(envWithAuth.oauthTokenUrl).toEqual('example.com/token_endpoint');
183-
});
190+
const discoveryServiceMockGetOIDC = jest.spyOn(
191+
discoveryServiceMock,
192+
'getOIDC',
193+
);
194+
discoveryServiceMockGetOIDC.mockResolvedValue(null);
184195

185-
it('should get oauthServerUrl and oauthTokenUrl from env when discoveryService returns null ', async () => {
186-
const request = mock<Request>();
187-
request.hostname = 'app.hyper.space';
196+
process.env['DISCOVERY_ENDPOINT_APP'] = 'example.com';
197+
const envWithAuth = await service.getCurrentAuthEnv(request);
188198

189-
const discoveryServiceMockGetOIDC = jest.spyOn(
190-
discoveryServiceMock,
191-
'getOIDC',
192-
);
193-
discoveryServiceMockGetOIDC.mockResolvedValue(null);
199+
expect(envWithAuth.oauthServerUrl).toEqual('www.app.com');
200+
expect(envWithAuth.oauthTokenUrl).toEqual('www.app.token.com');
201+
});
194202

195-
process.env['DISCOVERY_ENDPOINT_APP'] = 'example.com';
196-
const envWithAuth = await service.getCurrentAuthEnv(request);
203+
it('should map the idp to the url for foo', async () => {
204+
const request = mock<Request>();
205+
request.hostname = 'foo.hyper.space';
197206

198-
expect(envWithAuth.oauthServerUrl).toEqual('www.app.com');
199-
expect(envWithAuth.oauthTokenUrl).toEqual('www.app.token.com');
200-
});
207+
const envWithAuth = await service.getCurrentAuthEnv(request);
201208

202-
it('should map the idp to the url for foo', async () => {
203-
const request = mock<Request>();
204-
request.hostname = 'foo.hyper.space';
209+
expect(envWithAuth.clientId).toBe(clientIdFoo);
210+
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlFoo);
211+
});
205212

206-
const envWithAuth = await service.getCurrentAuthEnv(request);
213+
it('should map the idp to the token url for foo', async () => {
214+
const request = mock<Request>();
215+
request.hostname = 'foo.hyper.space';
207216

208-
expect(envWithAuth.clientId).toBe(clientIdFoo);
209-
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlFoo);
210-
});
217+
const envWithAuth = await service.getCurrentAuthEnv(request);
211218

212-
it('should map the idp to the token url for foo', async () => {
213-
const request = mock<Request>();
214-
request.hostname = 'foo.hyper.space';
219+
expect(envWithAuth.clientId).toBe(clientIdFoo);
220+
expect(envWithAuth.oauthTokenUrl).toBe(oauthTokenUrlFoo);
221+
});
215222

216-
const envWithAuth = await service.getCurrentAuthEnv(request);
223+
it('should map the idp to the token url for app', async () => {
224+
const request = mock<Request>();
225+
request.hostname = 'app.k8s.ondemand.com';
217226

218-
expect(envWithAuth.clientId).toBe(clientIdFoo);
219-
expect(envWithAuth.oauthTokenUrl).toBe(oauthTokenUrlFoo);
220-
});
227+
const envWithAuth = await service.getCurrentAuthEnv(request);
221228

222-
it('should map the idp to the token url for app', async () => {
223-
const request = mock<Request>();
224-
request.hostname = 'app.k8s.ondemand.com';
229+
expect(envWithAuth.clientId).toBe(clientIdApp);
230+
expect(envWithAuth.oauthTokenUrl).toBe(oauthTokenUrlAPP);
231+
expect(envWithAuth.idpName).toBe('app');
232+
expect(envWithAuth.organization).toBe('app');
233+
expect(envWithAuth.baseDomain).toBe('app.k8s.ondemand.com');
234+
});
225235

226-
const envWithAuth = await service.getCurrentAuthEnv(request);
236+
it('should map the idp of foo to its domain, the foo is configured', async () => {
237+
const request = mock<Request>();
238+
request.hostname = 'foo.app.k8s.ondemand.com';
227239

228-
expect(envWithAuth.clientId).toBe(clientIdApp);
229-
expect(envWithAuth.oauthTokenUrl).toBe(oauthTokenUrlAPP);
230-
expect(envWithAuth.idpName).toBe('app');
231-
expect(envWithAuth.organization).toBe('app');
232-
expect(envWithAuth.baseDomain).toBe('app.k8s.ondemand.com');
233-
});
240+
const envWithAuth = await service.getCurrentAuthEnv(request);
234241

235-
it('should map the idp of foo to a different base url', async () => {
236-
const request = mock<Request>();
237-
request.hostname = 'foo.app.k8s.ondemand.com';
242+
expect(envWithAuth.clientId).toBe(clientIdFoo);
243+
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlFoo);
244+
expect(envWithAuth.clientSecret).toBe(clientSecretFoo);
245+
expect(envWithAuth.baseDomain).toBe('foo.app.k8s.ondemand.com');
246+
});
238247

239-
const envWithAuth = await service.getCurrentAuthEnv(request);
248+
it('should map the idp of test to an existing its base domain', async () => {
249+
const request = mock<Request>();
250+
request.hostname = 'test.app.k8s.ondemand.com';
240251

241-
expect(envWithAuth.clientId).toBe(clientIdFoo);
242-
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlFoo);
243-
expect(envWithAuth.clientSecret).toBe(clientSecretFoo);
244-
});
252+
const envWithAuth = await service.getCurrentAuthEnv(request);
245253

246-
it('should return the default tenant, if a host name is directly matched', async () => {
247-
const request = mock<Request>();
248-
request.hostname = 'app.k8s.ondemand.com';
254+
expect(envWithAuth.clientId).toBe(clientIdApp);
255+
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlAPP);
256+
expect(envWithAuth.clientSecret).toBe(clientSecretApp);
257+
expect(envWithAuth.baseDomain).toBe('app.k8s.ondemand.com');
258+
});
249259

250-
const envWithAuth = await service.getCurrentAuthEnv(request);
260+
it('should return the default tenant, if a host name is directly matched', async () => {
261+
const request = mock<Request>();
262+
request.hostname = 'app.k8s.ondemand.com';
251263

252-
expect(envWithAuth.clientId).toBe(clientIdApp);
253-
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlAPP);
254-
expect(envWithAuth.clientSecret).toBe(clientSecretApp);
255-
});
264+
const envWithAuth = await service.getCurrentAuthEnv(request);
256265

257-
it('should return the default tenant, for a different host with multiple names is directly matched', async () => {
258-
const request = mock<Request>();
259-
request.hostname = 'hyper.space';
266+
expect(envWithAuth.clientId).toBe(clientIdApp);
267+
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlAPP);
268+
expect(envWithAuth.clientSecret).toBe(clientSecretApp);
269+
});
260270

261-
const envWithAuth = await service.getCurrentAuthEnv(request);
271+
it('should return the default tenant, for a different host with multiple names is directly matched', async () => {
272+
const request = mock<Request>();
273+
request.hostname = 'hyper.space';
262274

263-
expect(envWithAuth.clientId).toBe(clientIdHyperspace);
264-
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlHyperspace);
265-
expect(envWithAuth.clientSecret).toBe(clientSecretHyperspace);
266-
});
275+
const envWithAuth = await service.getCurrentAuthEnv(request);
267276

268-
it('should throw when the idp is not existing, neither base domain is present', async () => {
269-
const request = mock<Request>();
270-
request.hostname = 'not-existing.app.k8s.ondemand2.com';
277+
expect(envWithAuth.clientId).toBe(clientIdHyperspace);
278+
expect(envWithAuth.oauthServerUrl).toBe(oauthServerUrlHyperspace);
279+
expect(envWithAuth.clientSecret).toBe(clientSecretHyperspace);
280+
});
271281

272-
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
273-
"not-existing.app.k8s.ondemand2.com is not listed in the portal's base urls: 'app.k8s.ondemand.com,hyper.space,localhost'",
274-
);
275-
});
282+
it('should throw when the idp is not existing, neither base domain is present', async () => {
283+
const request = mock<Request>();
284+
request.hostname = 'not-existing.app.k8s.ondemand2.com';
276285

277-
it('should return the base domain in case the idp is not existing in the env variables, and the organization indicated by the sub domain', async () => {
278-
const request = mock<Request>();
279-
request.hostname = 'not-existing.app.k8s.ondemand.com';
286+
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
287+
"not-existing.app.k8s.ondemand2.com is not listed in the portal's base urls: 'app.k8s.ondemand.com,hyper.space,localhost'",
288+
);
289+
});
280290

281-
const envWithAuth = await service.getCurrentAuthEnv(request);
291+
it('should return the base domain in case the idp is not existing in the env variables, and the organization indicated by the sub domain', async () => {
292+
const request = mock<Request>();
293+
request.hostname = 'not-existing.app.k8s.ondemand.com';
282294

283-
expect(envWithAuth.baseDomain).toBe('app.k8s.ondemand.com');
284-
// the idp value here is used to retrieve the auth configuration from env variables, for the idp not-existing there are no env variables
285-
expect(envWithAuth.idpName).toBe('app');
286-
expect(envWithAuth.organization).toBe('not-existing');
287-
});
295+
const envWithAuth = await service.getCurrentAuthEnv(request);
288296

289-
it('should throw when the token url is not configured is not existing', async () => {
290-
const request = mock<Request>();
291-
request.hostname = 'app.k8s.ondemand.com';
292-
delete process.env[`TOKEN_URL_APP`];
297+
expect(envWithAuth.baseDomain).toBe('app.k8s.ondemand.com');
298+
// the idp value here is used to retrieve the auth configuration from env variables, for the idp not-existing there are no env variables
299+
expect(envWithAuth.idpName).toBe('app');
300+
expect(envWithAuth.organization).toBe('not-existing');
301+
});
293302

294-
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(Error);
295-
});
303+
it('should throw when the token url is not configured is not existing', async () => {
304+
const request = mock<Request>();
305+
request.hostname = 'app.k8s.ondemand.com';
306+
delete process.env[`TOKEN_URL_APP`];
296307

297-
it('should throw when the domain is not existing', async () => {
298-
const request = mock<Request>();
299-
request.hostname = 'app-too.foo.com';
308+
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(Error);
309+
});
300310

301-
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
302-
"app-too.foo.com is not listed in the portal's base urls: 'app.k8s.ondemand.com,hyper.space,localhost'",
303-
);
304-
});
311+
it('should throw when the domain is not existing', async () => {
312+
const request = mock<Request>();
313+
request.hostname = 'app-too.foo.com';
305314

306-
it('should throw when the idp is not properly configured', async () => {
307-
const request = mock<Request>();
308-
request.hostname = 'not-configured.app.k8s.ondemand.com';
315+
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
316+
"app-too.foo.com is not listed in the portal's base urls: 'app.k8s.ondemand.com,hyper.space,localhost'",
317+
);
318+
});
309319

310-
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
311-
"the idp not-configured is not properly configured. oauthServerUrl: 'undefined' oauthTokenUrl: 'undefined' clientId: 'undefined', has client secret (OIDC_CLIENT_SECRET_NOT_CONFIGURED): false",
312-
);
320+
it('should throw when the idp is not properly configured', async () => {
321+
const request = mock<Request>();
322+
request.hostname = 'not-configured.app.k8s.ondemand.com';
323+
324+
await expect(service.getCurrentAuthEnv(request)).rejects.toThrow(
325+
"the idp not-configured is not properly configured. oauthServerUrl: 'undefined' oauthTokenUrl: 'undefined' clientId: 'undefined', has client secret (OIDC_CLIENT_SECRET_NOT_CONFIGURED): false",
326+
);
327+
});
313328
});
314329
});
315330

‎src/env/env.service.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,11 @@ export class EnvService {
172172
);
173173
}
174174

175+
// if we have configured the subdomain idp name in the environment, we use it as a baseDomain
176+
if (idpName === subDomainIdpName) {
177+
baseDomain = `${subDomainIdpName}.${baseDomain}`;
178+
}
179+
175180
return {
176181
idpName,
177182
baseDomain,

0 commit comments

Comments
 (0)