Skip to content

feat: implement Dex user provisioning and password hashing functionality - #1077

Open
CasLubbers wants to merge 19 commits into
mainfrom
APL-2079
Open

CasLubbers wants to merge 19 commits into
mainfrom
APL-2079

Conversation

@CasLubbers

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI lite review requested due to automatic review settings September 3, 2026 11:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Dex UpdatePassword request construction risks unintentionally clearing credentials/fields by always sending empty defaults, and should be corrected before merge.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds support for provisioning and managing users in Dex (as an alternative to Keycloak), including hashing plaintext passwords for Dex’s password store and mapping Otomi user roles/teams to Dex group strings.

Changes:

  • Introduces AUTH_PROVIDER and DEX_GRPC_ADDRESS env configuration to switch between Keycloak and Dex provisioning paths.
  • Adds a Dex gRPC client, group mapping helpers, and bcrypt-based password hashing utilities.
  • Updates user CRUD flows, JWT group parsing, and OpenAPI requirements to support Dex-backed users and optional first/last names.
File summaries
File Description
src/validators.ts Adds AUTH_PROVIDER and DEX_GRPC_ADDRESS validators.
src/utils/userUtils.ts Adds Dex group derivation + Dex Password→User mapping.
src/utils/userUtils.test.ts Tests for Dex group derivation and mapping helpers.
src/utils/passwordUtils.ts Adds bcrypt hashing helper for provisioning Dex password records.
src/utils/passwordUtils.test.ts Tests for bcrypt hashing behavior (verify + salted).
src/proto/dex/api.proto Adds Dex admin API proto for TS client generation.
src/clients/dexClient.ts Implements Dex gRPC client wrapper (create/update/list/delete password records).
src/clients/dexClient.test.ts Unit tests for the Dex client wrapper behavior.
src/clients/dexClient.integration.test.ts Optional integration test for real Dex gRPC endpoint.
src/otomi-stack.ts Adds Dex-backed implementations for user CRUD + team membership edits.
src/otomi-stack.test.ts Adds tests covering Dex provisioning and Dex-mode user operations.
src/openapi/user.yaml Makes firstName/lastName optional in the User schema.
src/middleware/jwt.ts Ignores Dex no-groups sentinel when mapping JWT groups→roles/teams.
src/middleware/jwt.test.ts Adds coverage for sentinel behavior in JWT claim mapping.
package.json Adds dependencies and build/postinstall steps to generate Dex client code.
package-lock.json Locks new deps for Dex gRPC client generation and bcrypt hashing.
eslint.config.mjs Excludes src/generated/* from linting.
.gitignore Ignores generated Dex client output under src/generated/.
Review details

Suppressed comments (1)

src/otomi-stack.ts:1547

  • When userData.id is missing, the error message currently becomes "User undefined not found", which is misleading and makes debugging harder. Return an explicit “id is required” message for this validation failure.
      if (!userData.id) {
        throw new NotExistError(`User ${userData.id} not found`)
      }
  • Files reviewed: 16/18 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/clients/dexClient.ts Outdated
Comment thread package.json Outdated
Comment thread package.json
Comment thread src/clients/dexClient.ts Outdated
Comment thread src/middleware/jwt.ts Outdated
Comment thread src/otomi-stack.ts
Copilot AI review requested due to automatic review settings September 3, 2026 11:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

package.json currently includes a duplicate JSON key (breaking/ambiguous metadata) and there are remaining production-readiness concerns around pulling gRPC client deps into middleware plus plaintext gRPC credentials.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

src/utils/userUtils.test.ts:2

  • This test hard-codes the Dex "no groups" sentinel string. Importing and using DEX_NO_GROUPS_SENTINEL avoids drift if the sentinel value ever changes.

This issue also appears on line 70 of the same file.

package.json:128

  • package.json contains a duplicate "name" property, which makes the JSON ambiguous and can confuse tooling (only the last key wins). Remove the duplicate entry.
  "name": "@redkubes/otomi-api",
  "name": "@redkubes/otomi-api",

package.json:152

  • The test scripts rely on src/generated/dex being present (generated by postinstall), but running git clean -fdx or deleting ignored files can remove it and npm test won't regenerate it. Consider generating the Dex client as part of the test scripts too, similar to the build script.
    "postinstall": "npm run build:models && npm run gen:dex-client",

src/utils/userUtils.test.ts:72

  • Use the shared DEX_NO_GROUPS_SENTINEL constant instead of the literal 'no_groups' so the test stays consistent with production behavior.
  it('strips the no-groups sentinel and treats it as no groups at all', () => {
    expect(dexPasswordToUser(password({ groups: ['__no_groups__'] }))).toMatchObject({
      isPlatformAdmin: false,

src/middleware/jwt.ts:4

  • jwt middleware imports DEX_NO_GROUPS_SENTINEL from src/clients/dexClient, which also pulls in @grpc/grpc-js and env parsing on every server startup even when AUTH_PROVIDER=keycloak. Consider moving the sentinel constant into a lightweight constants module (imported by both dexClient and jwt) to avoid unnecessary dependencies in the hot-path middleware.
import { DEX_NO_GROUPS_SENTINEL } from 'src/clients/dexClient'

src/clients/dexClient.ts:42

  • DexClient is created with ChannelCredentials.createInsecure(), which sends credentials and password hashes over plaintext. If this will be used outside strictly local/dev networks, please wire TLS (e.g., createSsl() with configurable CA/hostname) or gate insecure mode behind an explicit environment flag.
    // TODO(#3536): createInsecure() is a known temporary gap pending TLS wiring in apl-core.
    client = new DexClient(env.DEX_GRPC_ADDRESS, ChannelCredentials.createInsecure())
  }
  • Files reviewed: 17/19 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/otomi-stack.ts Outdated
Copilot AI review requested due to automatic review settings September 8, 2026 14:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are correctness/operational blockers in the current diff (e.g., Dex-mode editUser can return an email change that cannot actually be persisted, and package.json has a duplicate key) that should be resolved before approval.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

src/otomi-stack.ts:1395

  • In Dex mode, editUser can accept an updated email but the Dex UpdatePassword API uses the existing email as an immutable lookup key, so the change will not persist. The method currently returns a User object with the new email anyway, which makes the API response inconsistent with subsequent reads. Also, password updates in Dex mode should enforce the same minimum length as createUser to avoid accepting weak passwords.

package.json:129

  • package.json contains the "name" property twice; duplicate JSON keys can lead to unpredictable behavior across tooling (some parsers reject duplicates). Remove the duplicate entry so the package metadata is unambiguous.
  "main": "dist/src/app.js",
  "name": "@redkubes/otomi-api",
  "name": "@redkubes/otomi-api",
  "publishConfig": {

src/clients/dexClient.ts:41

  • Dex gRPC is currently instantiated with ChannelCredentials.createInsecure(), which means traffic (including password hashes and group membership updates) is sent without transport security. If Dex is reachable beyond a strictly trusted network boundary, this can enable interception or tampering. Consider wiring TLS/mTLS (or explicitly limiting the address to a local/cluster-internal endpoint) before enabling AUTH_PROVIDER=dex in production.
    throw new DexProvisionError('DEX_GRPC_ADDRESS must be set when AUTH_PROVIDER=dex')
  }
  if (!client) {
    // TODO(#3536): createInsecure() is a known temporary gap pending TLS wiring in apl-core.
    client = new DexClient(env.DEX_GRPC_ADDRESS, ChannelCredentials.createInsecure())

package.json:153

  • The Dex client types are generated into src/generated/dex, and postinstall/build now run gen:dex-client, but the test scripts still only run build:models. If installs are performed with --ignore-scripts (skipping postinstall) or src/generated is cleaned, npm test / npm run test:pattern will fail to compile. Consider running gen:dex-client as part of the test scripts too.
    "lint-staged": "lint-staged",
    "postinstall": "npm run build:models && npm run gen:dex-client",
    "pre-release:client": "npm version prerelease --preid rc --no-commit-hooks --no-git-tag-version && bin/release-client.sh",
  • Files reviewed: 17/19 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 9, 2026 08:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It introduces a new authentication/user-provisioning backend (Dex) plus password-handling and gRPC integration that should receive final human verification.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/proto/dex/api.proto:243

  • Typo in comment: "disovery" should be "discovery".

src/otomi-stack.ts:1524

  • This comment suggests the Dex team update batch is all-or-nothing ("each update either lands in Dex or the whole request rejects"), but the implementation applies updates sequentially and can leave partial updates persisted if a later call fails (see the for-loop below and the corresponding test case that expects two calls then a rejection). Please update the comment (or implement rollback/transaction semantics) to reflect the actual behavior.
  // Dex has no Git counterpart to keep in sync, so there's no two-pass ordering to worry about
  // here — each update either lands in Dex or the whole request rejects.
  private async editDexTeamUsers(

src/otomi-stack.ts:1544

  • When userData.id is missing, this throws NotExistError with message User undefined not found, which is misleading (the input is invalid, not a missing user record). Consider returning a 400 with a clear message that the id is required.
    if (!userData.id) {
      throw new NotExistError(`User ${userData.id} not found`)
    }
  • Files reviewed: 17/19 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 9, 2026 12:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are a few misleading comments/docs and an unclear error message in newly added Dex-mode code paths that should be corrected to match actual behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

src/otomi-stack.ts:1524

  • The comment implies the batch is all-or-nothing, but updates are applied sequentially and a failure partway through can leave earlier users updated in Dex. This is misleading for future maintainers and incident triage.
  // Dex has no Git counterpart to keep in sync, so there's no two-pass ordering to worry about
  // here — each update either lands in Dex or the whole request rejects.
  private async editDexTeamUsers(

src/otomi-stack.ts:1544

  • When userData.id is missing, the error message interpolates to "User undefined not found", which is confusing and makes debugging bad requests harder.
    if (!userData.id) {
      throw new NotExistError(`User ${userData.id} not found`)
    }
  • Files reviewed: 17/19 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/clients/dexClient.test.ts
Comment thread src/openapi/user.yaml Outdated
Copilot AI review requested due to automatic review settings September 10, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There is a confirmed JWT group-to-team mapping bug (team-admin can be misinterpreted as team admin) plus a couple of test-side issues that should be addressed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/otomi-stack.ts:1544

  • When userData.id is missing, the error message becomes User undefined not found, which is misleading (this is an invalid request rather than a missing user). Use a clearer message for the missing-id case.
    if (!userData.id) {
      throw new NotExistError(`User ${userData.id} not found`)
    }
  • Files reviewed: 17/19 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/clients/dexClient.test.ts
Comment thread src/middleware/jwt.ts
Comment thread src/utils/userUtils.test.ts
Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI review requested due to automatic review settings September 11, 2026 12:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical correctness and security issues remain in Dex error handling, metadata persistence, team updates, and bcrypt input handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (7)

src/clients/dexClient.integration.test.ts:12

  • This fixture is not a complete bcrypt hash: it is 56 characters, whereas the bcrypt encoding stored by Dex is 60 characters. A real Dex instance may reject it or persist a password that can never authenticate, so the integration test should use a known valid bcrypt hash.
      passwordHash: '$2a$10$abcdefghijklmnopqrstuuVGm5ZQeXk6b2ZQeXk6b2ZQeXk6b',

src/clients/dexClient.ts:39

  • callWithRetry retries every thrown error, but the callbacks below intentionally throw DexProvisionError for non-transient alreadyExists and notFound responses. That produces four identical RPCs for deterministic failures and can make a successful create whose response is lost look like a failed duplicate after retry; restrict retries to transient gRPC status codes so application-level responses are not retried.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })

src/clients/dexClient.ts:120

  • listDexPasswords is the read path for deduplication, getUser, getAllUsers, and edits, but dexClient.test.ts never mocks or calls listPasswords; the current mock client does not define it. Add success and RPC-error tests so regressions in the request/response mapping and retry behavior cannot pass unnoticed.
export async function listDexPasswords(): Promise<Password[]> {
  const dex = getDexClient()
  return callWithRetry(
    () =>
      new Promise<Password[]>((resolve, reject) => {

src/openapi/user.yaml:118

  • editDexUser also accepts data.initialPassword and hashes it at lines 1406-1410 to reset a Dex password, so saying this may be set only "on create" is inconsistent with the edit behavior introduced here. Update the description to document password resets on edit and that the field is ignored for other providers.
      description: The initial password of the user. With Dex as issuer, an admin may set this on create; otherwise one is generated.

src/otomi-stack.test.ts:833

  • When originalAuthProvider is undefined, assigning it back with process.env.AUTH_PROVIDER = originalAuthProvider stores the string "undefined" rather than removing the variable in Node. That can leak an invalid provider value into later tests and make subsequent cleanEnv calls fail; delete the property when the original value was absent.
    afterEach(() => {
      process.env.AUTH_PROVIDER = originalAuthProvider
      jest.clearAllMocks()

src/otomi-stack.ts:1551

  • editDexTeamUsers does not validate the requested team IDs before writing groups, unlike createUser and editDexUser. A platform admin can therefore store team-<nonexistent> in Dex; dexPasswordToUser reports that team while JWT processing ignores it as unknown, leaving the API and authorization views inconsistent. Validate updatedUser before the RPC.
    const updatedUser: User = { ...existingUser, teams: userData.teams }
    await updateDexPassword({ email: match.email, newGroups: deriveDexGroups(updatedUser) })

src/validators.ts:81

  • Because DEX_GRPC_ADDRESS is optional in cleanEnv and this check runs only inside getDexClient, a deployment with AUTH_PROVIDER=dex but no address starts successfully and only fails later when a user operation is attempted. Since the validator documents this value as required for Dex, validate the combination during application/stack initialization instead of deferring the configuration error to requests.
export const DEX_GRPC_ADDRESS = str({
  desc: 'host:port of the Dex gRPC API. Required when AUTH_PROVIDER=dex.',
  example: 'dex-grpc.dex.svc:5557',
  devDefault: 'localhost:5557',
  default: undefined,
  • Files reviewed: 18/20 changed files
  • Comments generated: 6
  • Review effort level: Lite

Comment thread src/clients/dexClient.ts
Comment thread src/otomi-stack.ts
Comment thread src/otomi-stack.ts
Comment thread src/otomi-stack.ts
Comment thread src/utils/passwordUtils.ts
Comment thread src/utils/userUtils.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Dex team-update path can persist nonexistent teams, and the newly exposed issuer setting is not wired to runtime provider or JWT configuration.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/otomi-stack.ts:1571

  • This new Dex branch never validates the requested team IDs before persisting them. Unlike create/edit user, applyDexTeamUpdate can therefore write groups such as team-does-not-exist; JWT mapping later ignores unknown teams, leaving the Dex record and API membership inconsistent. Validate updatedUser with validateUserTeamsExist before calling updateDexPassword.
    src/clients/dexClient.ts:164
  • The new identity-list/delete RPC path has no client-level tests: otomi-stack.test.ts mocks these exported functions, while the integration test covers only password create/update/delete. A wrong generated-method request or error mapping here would still let the stack tests pass while leaving sessions or identities undeleted. Add unit coverage for the list/filter and DeleteUserIdentity request/error behavior.

src/clients/dexClient.ts:49

  • async-retry retries every rejected promise, including the AlreadyExists and NotExistError values intentionally produced by the RPC wrappers below. A duplicate create or stale update will therefore make four non-transient RPCs before returning 409/404, adding avoidable latency and load; restrict retries to transient transport errors or bail on domain errors.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })
}
  • Files reviewed: 21/23 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread src/openapi/settings.yaml
Copilot AI review requested due to automatic review settings September 18, 2026 05:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Dex operations retry non-transient outcomes and perform full password-hash scans, creating correctness and scalability risks.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/clients/dexClient.ts:48

  • callWithRetry retries every rejected promise, including the deliberate AlreadyExists and NotExistError results returned by createDexPassword and updateDexPassword. That turns normal duplicate/404 responses into four RPCs, and if a create commits but its response is lost, the retries can end by reporting a 409 even though the user was created. Restrict retries to transient gRPC failures or mark domain errors as non-retryable.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })
  • Files reviewed: 21/23 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/clients/dexClient.ts Outdated
Comment thread src/utils/passwordUtils.ts
Copilot AI review requested due to automatic review settings September 18, 2026 05:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Password validation and Dex retry/configuration handling have unresolved correctness and reliability issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/clients/dexClient.ts:49

  • async-retry retries every rejected promise, including the deliberate AlreadyExists and NotExistError rejections produced by the RPC wrappers. A duplicate create or missing update therefore makes four RPC calls and adds retry delay; worse, a successful create whose response is lost can be reported as a final conflict after retries. Restrict retries to transient gRPC failures and bail immediately for domain errors.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })
}
  • Files reviewed: 21/23 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/clients/dexClient.ts
Comment thread src/otomi-stack.ts
Comment thread src/clients/dexClient.ts Outdated
Copilot AI review requested due to automatic review settings September 18, 2026 06:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It changes security-sensitive authentication, password-reset, and account-deletion paths while introducing a generated gRPC contract; human validation is required.

Review details

Suppressed comments (1)

src/clients/dexClient.ts:49

  • callWithRetry retries every rejection, including the deliberate AlreadyExists/NotExistError responses and the non-idempotent CreatePassword call. A duplicate create therefore makes four unnecessary RPCs, and a lost response after Dex committed a create can be retried until the client receives a conflict even though the user now exists. Restrict retries to transient transport failures and make create reconciliation/idempotency explicit.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })
}
  • Files reviewed: 21/23 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/otomi-stack.ts
Comment thread src/proto/dex/api.proto
Copilot AI review requested due to automatic review settings September 18, 2026 06:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Dex team updates bypass team validation, while retry and identity enumeration behavior introduce correctness and scalability risks.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/clients/dexClient.ts:48

  • This retry helper is used for CreatePassword as well as reads and updates, so every thrown error—including AlreadyExists—is retried. A create can be committed by Dex while its response is lost; replaying it then yields alreadyExists and the API reports a conflict even though the user was provisioned. Restrict retries to transient, safe-to-replay failures or reconcile the create operation before retrying.
function callWithRetry<T>(fn: () => Promise<T>): Promise<T> {
  return retry(fn, { retries: 3, minTimeout: 200 })

src/clients/dexClient.ts:179

  • The new identity-list and identity-delete wrappers are not covered by dexClient.test.ts; the stack tests mock these helpers, so they do not verify request fields, userId filtering, or RPC error handling for the full account purge. Add unit tests for both wrappers before relying on this deletion path.
export async function deleteDexUserIdentity(userId: string, connectorId: string): Promise<void> {
  • Files reviewed: 21/23 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment thread src/clients/dexClient.ts Outdated
Comment thread src/otomi-stack.ts
Comment on lines +1571 to +1572
const updatedUser: User = { ...existingUser, teams: userData.teams }
await updateDexPassword({ email: match.email, newGroups: deriveDexGroups(updatedUser) })
Comment thread src/otomi-stack.ts
Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI review requested due to automatic review settings September 23, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Three moderate test and integration issues remain unresolved.

Review effort: Lite
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (4)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Fixture uses an invalid bcrypt hash

src/​clients/​dexClient.integration.test.ts:12

This fixture is not a valid bcrypt hash: the value is only 56 characters, whereas a bcrypt hash must be 60 characters. Dex may accept the record write, but any subsequent password verification will fail, so the integration test does not exercise a usable credential. Use a known valid 60-character bcrypt hash (or generate one in the test).

Copilot AI review requested due to automatic review settings September 23, 2026 14:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical deletion-flow behavior and three moderate Dex client/test issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 High severity · 1 Medium severity

Open (3)

Comment thread src/otomi-stack.ts
Comment on lines +1468 to +1472
const identities = await listUserIdentitiesByUserId(match.userId)
for (const identity of identities) {
await deleteDexUserIdentity(identity.userId, identity.connectorId)
}
await deleteDexPassword(match.email)
Signed-off-by: Cas Lubbers <clubbers@akamai.com>
Copilot AI review requested due to automatic review settings September 24, 2026 11:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved moderate findings affect request timeouts, test validity, schema accuracy, and team membership consistency.

Review effort: Lite
Findings: 2 High severity · 1 Medium severity

Open (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity OpenAPI maxLength mismatches UTF-8 byte limit

src/​openapi/​user.yaml:120

maxLength in OpenAPI is a character-count constraint, while the service enforces Buffer.byteLength(password, 'utf-8') <= 72. A 72-character multibyte password therefore passes schema validation but is rejected by the API, and the schema cannot actually express the documented byte limit. Remove this misleading maxLength or add a byte-length-specific validation extension so client-side validation matches the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants