refactor: 💡 refactor test and serializer (#1786)

* refactor: 💡 refactor test and serializer

Refactor `auth-method` mirage serializer to delete secret attributes.
Refactor `auth-method` mirage serializer unit test for LDAP

✅ Closes: ICU-10025

* refactor: 💡 merge main

merge main

* refactor: 💡 rebase

rebase

* refactor: 💡 refactor serializer and test

Refactor serializer normalize method to delete secret attributes field.

* refactor: 💡 refactor test

Refactor failing test due to deleting secrets in mirage

* refactor: 💡 refactor test

changed assertion

* refactor: 💡 refactor test

change assertion expect

* refactor: 💡 refactor test

Refactor `auth-method-test` to include normalize oidc and ldap secrets

* refactor: 💡 rebase

rebase

addons/api/addon/serializers/auth-method.js

@@ -92,15 +92,14 @@ export default class AuthMethodSerializer extends ApplicationSerializer {
    */
   normalize(typeClass, hash, ...rest) {
     let normalizedHash = structuredClone(hash);
-    // switch (normalizedHash.type) {
-    //   case 'oidc':
-    //     normalizedHash = this.normalizeOIDC(normalizedHash);
-    //     break;
-    // }
     const normalized = super.normalize(typeClass, normalizedHash, ...rest);
     if (!normalized.data.attributes.is_primary) {
       normalized.data.attributes.is_primary = false;
     }
+    // Remove secret fields as we don't track them after being created/updated
+    normalized.data.attributes.client_certificate_key = '';
+    normalized.data.attributes.bind_password = '';
+    normalized.data.attributes.client_secret = '';
     return normalized;
   }
 }

addons/api/mirage/serializers/auth-method.js

@@ -18,6 +18,9 @@ export default ApplicationSerializer.extend({
     const isPrimary =
       model.scope && model.scope.primaryAuthMethodId === model.id;
     json.is_primary = isPrimary;
+    delete json.attributes?.client_certificate_key;
+    delete json.attributes?.bind_password;
+    delete json.attributes?.client_secret;
     return json;
   },
 });

addons/api/tests/unit/serializers/auth-method-test.js

@@ -10,6 +10,7 @@ import setupMirage from 'ember-cli-mirage/test-support/setup-mirage';
 import {
   TYPE_AUTH_METHOD_PASSWORD,
   TYPE_AUTH_METHOD_OIDC,
+  TYPE_AUTH_METHOD_LDAP,
 } from 'api/models/auth-method';
 
 module('Unit | Serializer | auth method', function (hooks) {
@@ -89,7 +90,68 @@ module('Unit | Serializer | auth method', function (hooks) {
     });
   });
 
-  test("it doesn't serialize client secrets when not set", function (assert) {
+  test('it serializes LDAP records without state', function (assert) {
+    assert.expect(1);
+    let store = this.owner.lookup('service:store');
+    let record = store.createRecord('auth-method', {
+      type: TYPE_AUTH_METHOD_LDAP,
+      name: 'LDAP Auth Method',
+      state: 'baz',
+      account_attribute_maps: [{ from: 'foo', to: 'bar' }],
+      urls: [{ value: 'ldap://ldap.example.com' }],
+      use_token_groups: false,
+      start_tls: true,
+      insecrure_tls: false,
+      discover_dn: true,
+      anon_group_search: true,
+      bind_dn: 'cn=users,dc=example,dc=com',
+      upn_domain: 'example.com',
+      user_dn: 'cn=users,dc=example,dc=com',
+      user_attr: 'uid',
+      user_filter: '({{.UserAttr}}={{.Username}})',
+      enable_groups: true,
+      group_dn: 'cn=groups,dc=example,dc=com',
+      group_attr: 'cn',
+      group_filter: '(member={{.UserDN}})',
+      certificates: [
+        { value: 'certificate-1234' },
+        { value: 'certificate-5678' },
+      ],
+      client_certificate: 'certificate-1234',
+      client_certificate_key: 'secret456',
+    });
+
+    let serializedRecord = record.serialize();
+
+    assert.deepEqual(serializedRecord, {
+      type: TYPE_AUTH_METHOD_LDAP,
+      name: 'LDAP Auth Method',
+      description: null,
+      attributes: {
+        account_attribute_maps: ['foo=bar'],
+        urls: ['ldap://ldap.example.com'],
+        use_token_groups: false,
+        start_tls: true,
+        insecure_tls: false,
+        discover_dn: true,
+        anon_group_search: true,
+        bind_dn: 'cn=users,dc=example,dc=com',
+        upn_domain: 'example.com',
+        user_dn: 'cn=users,dc=example,dc=com',
+        user_attr: 'uid',
+        user_filter: '({{.UserAttr}}={{.Username}})',
+        enable_groups: true,
+        group_dn: 'cn=groups,dc=example,dc=com',
+        group_attr: 'cn',
+        group_filter: '(member={{.UserDN}})',
+        certificates: ['certificate-1234', 'certificate-5678'],
+        client_certificate: 'certificate-1234',
+        client_certificate_key: 'secret456',
+      },
+    });
+  });
+
+  test("OIDC auth-method doesn't serialize client secrets when not set", function (assert) {
     assert.expect(1);
     let store = this.owner.lookup('service:store');
     let record = store.createRecord('auth-method', {
@@ -138,6 +200,66 @@ module('Unit | Serializer | auth method', function (hooks) {
     });
   });
 
+  test("LDAP auth-method doesn't serialize client secrets when not set", function (assert) {
+    assert.expect(1);
+    let store = this.owner.lookup('service:store');
+    let record = store.createRecord('auth-method', {
+      type: TYPE_AUTH_METHOD_LDAP,
+      name: 'LDAP Auth Method',
+      state: 'baz',
+      account_attribute_maps: [{ from: 'foo', to: 'bar' }],
+      urls: [{ value: 'ldap://ldap.example.com' }],
+      use_token_groups: false,
+      start_tls: true,
+      insecrure_tls: false,
+      discover_dn: true,
+      anon_group_search: true,
+      bind_dn: 'cn=users,dc=example,dc=com',
+      upn_domain: 'example.com',
+      user_dn: 'cn=users,dc=example,dc=com',
+      user_attr: 'uid',
+      user_filter: '({{.UserAttr}}={{.Username}})',
+      enable_groups: true,
+      group_dn: 'cn=groups,dc=example,dc=com',
+      group_attr: 'cn',
+      group_filter: '(member={{.UserDN}})',
+      certificates: [
+        { value: 'certificate-1234' },
+        { value: 'certificate-5678' },
+      ],
+      client_certificate: 'certificate-1234',
+      client_certificate_key: null,
+    });
+
+    let serializedRecord = record.serialize();
+
+    assert.deepEqual(serializedRecord, {
+      type: TYPE_AUTH_METHOD_LDAP,
+      name: 'LDAP Auth Method',
+      description: null,
+      attributes: {
+        account_attribute_maps: ['foo=bar'],
+        urls: ['ldap://ldap.example.com'],
+        use_token_groups: false,
+        start_tls: true,
+        insecure_tls: false,
+        discover_dn: true,
+        anon_group_search: true,
+        bind_dn: 'cn=users,dc=example,dc=com',
+        upn_domain: 'example.com',
+        user_dn: 'cn=users,dc=example,dc=com',
+        user_attr: 'uid',
+        user_filter: '({{.UserAttr}}={{.Username}})',
+        enable_groups: true,
+        group_dn: 'cn=groups,dc=example,dc=com',
+        group_attr: 'cn',
+        group_filter: '(member={{.UserDN}})',
+        certificates: ['certificate-1234', 'certificate-5678'],
+        client_certificate: 'certificate-1234',
+      },
+    });
+  });
+
   test('it serializes OIDC records with only state and version when `adapterOptions.state` is passed', function (assert) {
     assert.expect(1);
     const store = this.owner.lookup('service:store');
@@ -162,6 +284,30 @@ module('Unit | Serializer | auth method', function (hooks) {
     });
   });
 
+  test('it serializes LDAP records with only state and version when `adapterOptions.state` is passed', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('auth-method');
+    const record = store.createRecord('auth-method', {
+      type: TYPE_AUTH_METHOD_LDAP,
+      attributes: {
+        state: 'foo',
+      },
+      version: 1,
+    });
+    const snapshot = record._createSnapshot();
+    snapshot.adapterOptions = {
+      state: 'bar',
+    };
+    const serializedRecord = serializer.serialize(snapshot);
+    assert.deepEqual(serializedRecord, {
+      attributes: {
+        state: 'bar',
+      },
+      version: 1,
+    });
+  });
+
   test('it sorts primary first in normalizeArrayResponse', function (assert) {
     assert.expect(2);
     const store = this.owner.lookup('service:store');
@@ -184,19 +330,34 @@ module('Unit | Serializer | auth method', function (hooks) {
           {
             id: '2',
             type: 'auth-method',
-            attributes: { is_primary: true },
+            attributes: {
+              is_primary: true,
+              bind_password: '',
+              client_certificate_key: '',
+              client_secret: '',
+            },
             relationships: {},
           },
           {
             id: '1',
             type: 'auth-method',
-            attributes: { is_primary: false },
+            attributes: {
+              is_primary: false,
+              bind_password: '',
+              client_certificate_key: '',
+              client_secret: '',
+            },
             relationships: {},
           },
           {
             id: '3',
             type: 'auth-method',
-            attributes: { is_primary: false },
+            attributes: {
+              is_primary: false,
+              bind_password: '',
+              client_certificate_key: '',
+              client_secret: '',
+            },
             relationships: {},
           },
         ],
@@ -210,7 +371,6 @@ module('Unit | Serializer | auth method', function (hooks) {
 
     const apiUrlPrefix = 'protocol://host:port/foo';
     const clientId = 'id123';
-    const clientSecret = 'secret456';
     const disableDiscoveredConfigValidation = true;
     const dryRun = true;
     const issuer = 'http://www.example.net';
@@ -224,11 +384,11 @@ module('Unit | Serializer | auth method', function (hooks) {
         idp_ca_certs: ['certificate-1234', 'certificate-5678'],
         api_url_prefix: apiUrlPrefix,
         client_id: clientId,
-        client_secret: clientSecret,
         disable_discovered_config_validation: disableDiscoveredConfigValidation,
         dry_run: dryRun,
         issuer: issuer,
         max_age: maxAge,
+        client_secret: 'secret123',
       },
       version: 1,
       type: TYPE_AUTH_METHOD_OIDC,
@@ -262,7 +422,6 @@ module('Unit | Serializer | auth method', function (hooks) {
     ]);
     assert.strictEqual(record.api_url_prefix, apiUrlPrefix);
     assert.strictEqual(record.client_id, clientId);
-    assert.strictEqual(record.client_secret, clientSecret);
     assert.strictEqual(
       record.disable_discovered_config_validation,
       disableDiscoveredConfigValidation
@@ -270,5 +429,74 @@ module('Unit | Serializer | auth method', function (hooks) {
     assert.strictEqual(record.dry_run, dryRun);
     assert.strictEqual(record.issuer, issuer);
     assert.strictEqual(record.max_age, maxAge);
+    assert.strictEqual(record.client_secret, '');
+  });
+
+  test('it normalizes LDAP records', async function (assert) {
+    const store = this.owner.lookup('service:store');
+
+    const ldapUrl = 'ldap://ldap.example.com';
+    const certificates = ['certificate-1234', 'certificate-5678'];
+    const clientCerticate = 'certificate-1234';
+    const startTls = true;
+    const insecureTls = false;
+    this.server.get('/v1/auth-methods/ldap123', () => ({
+      attributes: {
+        account_attribute_maps: ['from=to', 'foo=bar'],
+        urls: [ldapUrl],
+        start_tls: startTls,
+        insecure_tls: insecureTls,
+        discover_dn: true,
+        anon_group_search: true,
+        bind_dn: 'cn=users,dc=example,dc=com',
+        bind_password: 'password123',
+        upn_domain: 'example.com',
+        user_dn: 'cn=users,dc=example,dc=com',
+        user_attr: 'uid',
+        user_filter: '({{.UserAttr}}={{.Username}})',
+        enable_groups: true,
+        group_dn: 'cn=groups,dc=example,dc=com',
+        group_attr: 'cn',
+        group_filter: '(member={{.UserDN}})',
+        certificates: certificates,
+        client_certificate: clientCerticate,
+        client_certificate_key: 'secret456',
+      },
+      version: 1,
+      type: TYPE_AUTH_METHOD_LDAP,
+      id: 'ldap123',
+    }));
+
+    const record = await store.findRecord('auth-method', 'ldap123');
+    const {
+      account_attribute_maps,
+      urls,
+      certificates: recordCertificates,
+    } = record;
+    assert.deepEqual(account_attribute_maps, [
+      { from: 'from', to: 'to' },
+      { from: 'foo', to: 'bar' },
+    ]);
+    assert.deepEqual(urls, [{ value: ldapUrl }]);
+    assert.deepEqual(recordCertificates, [
+      { value: 'certificate-1234' },
+      { value: 'certificate-5678' },
+    ]);
+    assert.strictEqual(record.start_tls, startTls);
+    assert.strictEqual(record.insecure_tls, insecureTls);
+    assert.true(record.discover_dn);
+    assert.true(record.anon_group_search);
+    assert.strictEqual(record.bind_dn, 'cn=users,dc=example,dc=com');
+    assert.strictEqual(record.upn_domain, 'example.com');
+    assert.strictEqual(record.user_dn, 'cn=users,dc=example,dc=com');
+    assert.strictEqual(record.user_attr, 'uid');
+    assert.strictEqual(record.user_filter, '({{.UserAttr}}={{.Username}})');
+    assert.true(record.enable_groups);
+    assert.strictEqual(record.group_dn, 'cn=groups,dc=example,dc=com');
+    assert.strictEqual(record.group_attr, 'cn');
+    assert.strictEqual(record.group_filter, '(member={{.UserDN}})');
+    assert.strictEqual(record.client_certificate, clientCerticate);
+    assert.strictEqual(record.client_certificate_key, '');
+    assert.strictEqual(record.bind_password, '');
   });
 });

ui/admin/tests/acceptance/auth-methods/create-test.js

@@ -96,7 +96,7 @@ module('Acceptance | auth-methods | create', function (hooks) {
   });
 
   test('Users can create new oidc auth method', async function (assert) {
-    assert.expect(13);
+    assert.expect(12);
     const count = getAuthMethodsCount();
 
     await visit(`${urls.authMethods}/new?type=oidc`);
@@ -127,7 +127,6 @@ module('Acceptance | auth-methods | create', function (hooks) {
     assert.strictEqual(authMethod.description, 'description');
     assert.strictEqual(authMethod.attributes.issuer, 'issuer');
     assert.strictEqual(authMethod.attributes.client_id, 'client_id');
-    assert.strictEqual(authMethod.attributes.client_secret, 'client_secret');
     assert.deepEqual(authMethod.attributes.signing_algorithms, ['RS384']);
     assert.deepEqual(authMethod.attributes.allowed_audiences, [
       'allowed_audiences',

ui/admin/tests/acceptance/auth-methods/update-test.js

@@ -71,7 +71,7 @@ module('Acceptance | auth methods | update', function (hooks) {
   });
 
   test('can update an oidc auth method and save changes', async function (assert) {
-    assert.expect(12);
+    assert.expect(11);
     instances.authMethod = this.server.create('auth-method', {
       scope: instances.scopes.org,
       type: TYPE_AUTH_METHOD_OIDC,
@@ -134,7 +134,6 @@ module('Acceptance | auth methods | update', function (hooks) {
     assert.strictEqual(authMethod.description, 'description');
     assert.strictEqual(authMethod.attributes.issuer, 'issuer');
     assert.strictEqual(authMethod.attributes.client_id, 'client_id');
-    assert.strictEqual(authMethod.attributes.client_secret, 'client_secret');
     assert.deepEqual(authMethod.attributes.signing_algorithms, ['RS384']);
     assert.deepEqual(authMethod.attributes.allowed_audiences, [
       'allowed_audiences',