Skip to content

Commit

Permalink
fix(core): Report missing SAML attributes early with an actionable er…
Browse files Browse the repository at this point in the history
…ror message (#9316)
  • Loading branch information
despairblue committed May 7, 2024
1 parent ff31749 commit 225fdbb
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/cli/src/sso/saml/saml.service.ee.ts
Expand Up @@ -359,7 +359,7 @@ export class SamlService {
if (!attributes) {
throw new AuthError('SAML Authentication failed. Invalid SAML response.');
}
if (!attributes.email && missingAttributes.length > 0) {
if (missingAttributes.length > 0) {
throw new AuthError(
`SAML Authentication failed. Invalid SAML response (missing attributes: ${missingAttributes.join(
', ',
Expand Down
53 changes: 53 additions & 0 deletions packages/cli/test/unit/sso/saml/saml.service.ee.test.ts
@@ -0,0 +1,53 @@
import { mock } from 'jest-mock-extended';
import type express from 'express';
import { SamlService } from '@/sso/saml/saml.service.ee';
import { mockInstance } from '../../../shared/mocking';
import { UrlService } from '@/services/url.service';
import { Logger } from '@/Logger';
import type { IdentityProviderInstance, ServiceProviderInstance } from 'samlify';
import * as samlHelpers from '@/sso/saml/samlHelpers';

describe('SamlService', () => {
const logger = mockInstance(Logger);
const urlService = mockInstance(UrlService);
const samlService = new SamlService(logger, urlService);

describe('getAttributesFromLoginResponse', () => {
test('throws when any attribute is missing', async () => {
//
// ARRANGE
//
jest
.spyOn(samlService, 'getIdentityProviderInstance')
.mockReturnValue(mock<IdentityProviderInstance>());

const serviceProviderInstance = mock<ServiceProviderInstance>();
serviceProviderInstance.parseLoginResponse.mockResolvedValue({
samlContent: '',
extract: {},
});
jest
.spyOn(samlService, 'getServiceProviderInstance')
.mockReturnValue(serviceProviderInstance);

jest.spyOn(samlHelpers, 'getMappedSamlAttributesFromFlowResult').mockReturnValue({
attributes: {} as never,
missingAttributes: [
'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress',
'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/firstname',
'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/lastname',
'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn',
],
});

//
// ACT & ASSERT
//
await expect(
samlService.getAttributesFromLoginResponse(mock<express.Request>(), 'post'),
).rejects.toThrowError(
'SAML Authentication failed. Invalid SAML response (missing attributes: http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/firstname, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/lastname, http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn).',
);
});
});
});

0 comments on commit 225fdbb

Please sign in to comment.