Skip to content

Add LDAP login provider, provider-linked account lifecycle, and a shared provider-customizable login form - #1155

Open
devin-ai-integration[bot] wants to merge 123 commits into
developmentfrom
devin/1793737998-ldap-auth-provider
Open

devin-ai-integration[bot] wants to merge 123 commits into
developmentfrom
devin/1793737998-ldap-auth-provider

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

  • Let sites sign people in with their organization's LDAP directory (OpenLDAP, Active Directory, other LDAP directories) through a new ldap auth provider.
  • Keep each person's MMGIS account (ID, drawings, grants, API tokens, SuperAdmin) when they move to the directory, and take access away when they leave it.
  • Let username/password providers share one MMGIS login form with per-site wording, instead of each provider serving its own login page.

Proposed Changes

LDAP provider (plugins/core/auth/Ldap/, npm dependency ldapts declared in its plugin.json)

  • [ADD] Encryption: LDAPS or StartTLS, with LDAP_TLS_CA for internal CAs. Startup refuses plain ldap:// unless LDAP_ALLOW_INSECURE=true, which is for testing only.
  • [ADD] Two ways to find people:
    • search as a read-only service account (LDAP_BIND_DN, LDAP_USER_SEARCH_BASE/FILTER);
    • direct bind (LDAP_USER_DN_TEMPLATE).
  • [ADD] Input handling: usernames are escaped per RFC 4515/4514, and a search must match exactly one person.
  • [ADD] Accounts link by a permanent ID: entryUUID, or objectGUID on AD. Changeable attributes are allowed, but startup warns about them.
  • [ADD] Groups:
    • They come from memberOf or a group search, which covers AD nested groups.
    • They're reported by DN, or by name with LDAP_GROUP_NAME_ATTRIBUTE, and matched to MMGIS groups exactly.
    • SuperAdmin is never granted through a group.
  • [ADD] The service account is protected: sign-ins as it, by DN, UPN or DOMAIN\name, are refused before their password is tried.
  • [ADD] A startup self-check, and a log line naming the cause and fix for every refused sign-in.
  • [ADD] Form wording comes from LDAP_DISPLAY_NAME, LDAP_USERNAME_LABEL and LDAP_LOGIN_HINT.

Shared login forms

  • [ADD] Form-provider contract: the plugin.json fields login_form and login_icon, a loginForm(env) hook, and providers.loginForms() / providerLabel(). GET /api/auth also returns loginForms. local is now a form provider too.
  • [CHANGE] The login page, Configure sign-in and the in-app modal share one form:
    • One provider: a "Sign in with <name>" banner.
    • Several providers: a switch whose default is the first entry in AUTH_PROVIDERS. The last choice is remembered.
    • While waiting, the button reads "Signing in…".
    • After a wrong password, a hint names the other tab.
  • [CHANGE] With AUTH_LOCAL_LOGIN=admins, local accounts are offered only at Configure sign-in. A non-admin's correct local password now gets the same "Invalid username or password." as a wrong one, with the reason in the server log. Before, the message confirmed the password.

Provider-linked accounts (shared, in API/permissions/providers.js; any provider can use them)

  • [ADD] A provider sign-in links to an MMGIS account, which is created at first sign-in unless AUTH_PROVIDER_NEW_ACCOUNTS=false.
  • [ADD] Rechecks: provider-linked sessions and API tokens are rechecked every AUTH_RECHECK_MINUTES (lookup hook).
    • Removed or disabled people lose access, and their roles follow group changes.
    • During a directory outage, access continues, but accounts already found gone stay refused.
  • [ADD] One-account relinking (POST /api/accounts/allowRelink, /cancelRelink). An admin lets one provider sign-in, within 24 h, take over an existing account and keep everything it has.
    • The admin previews the directory entry first, and the relink is pinned to it (findSubject).
    • A provider offers relinking only if its plugin.json sets "relink": true. LDAP does; proxy doesn't.
  • [ADD] POST /api/accounts/recheckLinked checks every linked account now.
  • [ADD] Database: auth_identities.username, last_login_at and subject_setting, added at startup. Relink and not-found state is stored in the existing auth_meta table.

Sign-in hardening

  • [ADD] Failed-login limits per account (AUTH_MAX_FAILED_LOGINS), per address, and per directory entry.
  • [ADD] Some input is refused before the directory is contacted: empty passwords, and usernames with commas (Draw stores editors as a comma-separated list), control characters or more than 255 characters.
  • [ADD] Login, sign-up and logout refuse cross-site requests. CORS_ORIGINS sites are still allowed.
  • [FIX] Fixes:
    • Password-reset links now refuse provider-linked accounts. Before, a reset could bypass the provider.
    • First-admin setup stays open until a SuperAdmin exists.
    • Under SITE_ACCESS=optional, visitor sign-up closes once another provider is enabled, so nobody can claim a directory user's username first.
    • Reset tokens are redacted in logs.
    • Draw escapes owner and file names.

Configure > Users & Access

  • [ADD] People:
    • Each account shows how it signs in, its last provider login, and "not found since <date>".
    • A "Not found / needs relink" filter.
    • A Check linked accounts button.
  • [ADD] Person panel: Allow <provider name> relink for providers that opt in.
  • [ADD] Groups list their members from provider groups. Deleting an account can give its drawings to another account.
  • [CHANGE] Toolbars wrap on narrow screens. Provider display names replace technical IDs.
  • [ADD] An account button at the top right of every Configure page replaces the username and logout icon. It shows a role pill and an avatar with the provider's badge, and its popover lists:
    • who you are and how you signed in;
    • your permission level;
    • each role you hold and where it applies (site-wide, a mission, or N layers);
    • where each role comes from (your account, a group with its provider, or an audience).
      It also has Open the map and Sign out. Viewer grants are left out because they give no Configure access.
  • [ADD] GET /api/access/me?grants=true also returns grants and provider. Without the parameter, the response is unchanged.
  • [CHANGE] The SuperAdmin badge is plum (theme r[2], #7c1158) instead of pink. The account menu reuses the table badge styles, so role colors match everywhere.

Plugins, tests and CI

  • [ADD] A plugin can declare "ci": { "script": "tests/ci.sh", "variants": [...] } in plugin.json. A generic plugin-ci workflow job runs each declared script through scripts/plugin-ci.js and then that plugin's tests. LDAP (ldaps, starttls, against a real OpenLDAP) and proxy (network, secret) use it.
  • [CHANGE] Plugin unit tests are found by the @unit tag (npm run test:plugins:unit). Each plugin's tests live in that plugin's folder. Core tests use a stand-in provider, so removing a provider plugin breaks no core test.
  • [CHANGE] The CI jobs ldap and proxy are replaced by plugin-ci. Any required-check settings that name the old jobs need updating.

Settings and docs

  • [ADD] New settings:

    • AUTH_MAX_FAILED_LOGINS (5)
    • AUTH_RECHECK_MINUTES (15)
    • AUTH_PROVIDER_NEW_ACCOUNTS (true)
    • the LDAP_* settings

    All are in sample.env and docs/pages/Setup/ENVs/ENVs.md.

  • [CHANGE] Changed settings:

    • AUTH_PROVIDERS accepts "ldap".
    • AUTH_LOCAL_ALLOW_SIGNUP and SETUP_CODE behave as described under Fixes above.
  • [ADD] Docs:

    • New docs/pages/Setup/LDAP/LDAP.md, covering setup for OpenLDAP and AD, rollout, a one-account migration runbook, moving to a new directory, leavers, outages, rollback and troubleshooting.
    • The AuthProviders guide now documents the form contract, the lookup / locate / findSubject hooks and "relink": true.

Issues

Testing

  • Local runs on the latest commits:
    • PLAYWRIGHT_TEST_UNIT_ONLY=true playwright test tests/unit: 1770 passed.
    • npm run test:plugins:unit: 47 passed.
    • scripts/plugin-ci.js LDAP ldaps against OpenLDAP: 24 passed, 2 skipped.
  • scripts/plugin-ci.js on the earlier plugin-CI commit (5f91c16):
    • LDAP starttls: 24 passed, 2 skipped.
    • Proxy network: 8 passed.
    • Proxy secret: 8 passed.
  • Run before the move into plugin folders: LDAP e2e passed with direct bind (63) and with rechecks off (67).
  • AUTH_LOCAL_LOGIN=admins, checked against a running server: test_user with its correct password and with a wrong one both get "Invalid username or password.", and test_admin signs in.
  • Configure build succeeds, ESLint shows no errors, and all 63 plugin manifests pass validation.
  • UI: a recorded browser walkthrough on a production build with the OpenLDAP fixture. It covered the login forms, first LDAP sign-in, group roles, relinking a local account, a leaver caught by Check linked accounts, and the delete-account drawings offer.
  • Account menu: Playwright on a production Configure build with the OpenLDAP fixture. test_admin shows SuperAdmin. ldap_alice shows Data Admin site-wide and Layer Editor on a mission through an LDAP group; a whole-mission grant merged with a layer grant shows no layer count. Sign out returns to /. Unit tests: 1771 passed.
  • Not tested:
    • The latest Configure relink and display-name change hasn't been opened in a browser.
    • Expired-certificate behaviour.

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/408eb3c9ca61431cab68b9a361f23fdb
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/408eb3c9ca61431cab68b9a361f23fdb?variant=devin
Requested by: @tariqksoliman

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

✅ Version Already Updated

This PR includes a manual version update to 5.5.2-20261105

No automatic version bump needed.

devin-ai-integration Bot and others added 4 commits November 3, 2026 20:51
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Browser-tested the login UI with a production build and the LDAPS OpenLDAP fixture:

  • Login page: the outlined LDAP/MMGIS-account switch, the configured label, hint and password-help link, the choice remembered after reload, the wrong-password message, LDAP sign-in and local sign-in all work.
  • Configure: the provider switch, the "can't manage anything" notice for LDAP non-admins, local admin access, and the username-conflict message all work.
  • In-app modal: provider switching, LDAP sign-in and sign-out work. The LDAP-only banner works. With AUTH_LOCAL_LOGIN=admins, the public login page and modal show only LDAP, and Configure keeps both tabs.
  • Found and fixed (6b352e2): the account avatar is the React TopBar button, which showed only the username. It now reads "ldap_alice · signed in with JPL Directory"; local accounts still show the username only.
  • Not checked: the numeric 409 status for the conflict case. The message itself was confirmed on screen.

Separately, 61eb14a fixes "Check existing accounts survive the upgrade". The step seeded accounts by starting development with AUTH=local, which development now rejects since the auth refactor; it failed the same way on #1154. It now seeds from the commit before scripts/auth-migrate.js was added.

Written by Devin

devin-ai-integration Bot and others added 7 commits November 3, 2026 21:40
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…d provider, login accessibility

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…tch provider group names case-insensitively

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…apart; log the groups each login reports

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…artup, log why each login was refused, reject non-PEM CA files

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
github-advanced-security[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 7 commits November 4, 2026 01:20
…ogins per account (AUTH_MAX_FAILED_LOGINS); key the service-account backoff without hashing the password

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…her provider enabled, optional-mode sign-up follows AUTH_LOCAL_ALLOW_SIGNUP

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ep group names exact

- LDAP lookup(subject) and shared stillLinked(): every AUTH_RECHECK_MINUTES (default 15) group changes
  reach existing sessions and API tokens, and deleted/disabled/locked accounts lose access
- Provider group names match exactly, including case, for every provider (reverts the shared
  case-insensitive match); docs say to copy the DN from the login log
- syncGroups ignores duplicate memberships when two syncs race
- LDAP e2e runs in order and uses its own recheck-team group

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ovider renames without renaming the MMGIS account

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…refuse local passwords on provider-linked accounts

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
- Per-form failed-login limits keyed by NFKC-normalized username; only 401s count; reset on relink
- Relink names the provider username, pins the directory entry, runs in a transaction, ends sessions, replaces the local password, protects the last password SuperAdmin
- Case-insensitive username collisions, concurrent first-login retry
- LDAP group paging, duplicate-entry and size-limit diagnostics
- Docs: exact group matching, rename guidance, limiter behavior; restore CRLF

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
github-advanced-security[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 5 commits November 4, 2026 02:36
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ocal logins per address

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…CA/password rotation

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…links, recheck AD expiry, show the account-type hint only after wrong passwords

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…he reported form, default relink to the provider username

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
github-advanced-security[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits November 4, 2026 22:39
…the card instead of positioning it off the centered card

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…in the top bar and menu, 'Signed in with <provider>' in the menu, no hover title

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
@tariqksoliman tariqksoliman added the new feature New feature or request label Nov 5, 2026
devin-ai-integration Bot and others added 3 commits November 5, 2026 00:50
…e top-bar button

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…98-ldap-auth-provider

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>

# Conflicts:
#	configure/package.json
#	package.json
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

🤖 Version Auto-Bumped

The version has been automatically incremented to 5.5.2-20261105

This commit was added to your PR branch. When you merge this PR, the new version will be included.


If you want a different version, update package.json manually and push to this PR.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 3 commits November 5, 2026 00:52
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
… wording from plugin.json

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits November 5, 2026 01:34
…of reporting no groups

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ping warning and server notes shown in place instead of a toast

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 4 commits November 5, 2026 01:43
…does

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…embership-change warning, readable check errors with the provider's display name

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…ndler promises

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 2 commits November 5, 2026 17:52
…sernames after CORS

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…rAdmin badge in plum

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Devin Review found 3 new potential issues.

Devin Review

devin-ai-integration Bot and others added 4 commits November 5, 2026 19:49
… root without trailing-slash slip

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
…e column, marked with their source

Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
Co-Authored-By: tariq.k.soliman <tariqksoliman@gmail.com>
@sonarqubecloud

sonarqubecloud Bot commented Nov 5, 2026

Copy link
Copy Markdown

This branch has not been deployed

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

Labels

new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants