my own indieAuth provider! indiko.dunkirk.sh/docs
indieauth oauth2-server

Add ldap support #1

closed
opened by avycado13.tngl.sh targeting main from [deleted fork]: main

LDAP support to create user if it does not exist in indiko but exists in linked ldap dir. Currently does not support deletion from ldap dir. if the user is deleted in ldap they will not be deleted in indiko

0
by avycado13.tngl.sh 2 comments
expand 1 commit
0a966e32
feat: add ldap account syncing (if you delete in ldap will not update in indiko)

Ran this through amp, and these are the main things it pointed out.

The rate limiting is fine, we should just document that in the readme that it should be rate limited. The other concerns seem reasonable, and then please also make sure you run this through biome as well :)

๐Ÿ”ด Security Issues (Must Fix)#

1. No rate limiting on ldapVerify#

This endpoint is effectively a password oracle against your LDAP directory. An attacker could brute-force LDAP credentials through this endpoint.

Recommendation: Add per-IP/username rate limiting, or document it as a deployment requirement at the reverse proxy layer.

2. Input validation missing#

No validation on username in ldapVerify. Should enforce:

  • Length limits (e.g., โ‰ค 128 chars)
  • Character restrictions (^[A-Za-z0-9._@-]+$) to prevent LDAP injection if the library interpolates values into DNs or filters
// Add validation at the start of ldapVerify
if (!username || username.length > 128 || !/^[A-Za-z0-9._@-]+$/.test(username)) {
    return Response.json(
        { error: "Invalid username format" },
        { status: 400 },
    );
}

3. Information leakage#

The error messages could reveal whether a username exists in LDAP vs incorrect password. Use generic "Invalid credentials" for all auth failures.

Current: Different error paths may leak info
Recommended: Single generic error for all LDAP auth failures

4. Race condition in invite usage#

current_uses < max_uses check followed by separate increment can race under concurrent requests.

Recommendation: Use atomic update:

UPDATE invites SET current_uses = current_uses + 1 
WHERE id = ? AND current_uses < max_uses

Then check affected rows โ€” if 0, the invite was already fully used.


๐ŸŸก Logic/Architecture Issues#

5. Orphaned accounts after LDAP deletion#

Users provisioned via LDAP will remain active even if deleted from LDAP. This is mentioned in the PR description but should be:

  • Prominently documented for admins
  • Consider adding a provisioned_via_ldap column on users for audit purposes
  • Recommend an admin review process or script for deprovisioning

6. No LDAP group/authorization check#

Any LDAP user matching the search base can self-provision. Consider adding optional group membership requirements via environment config (e.g., LDAP_REQUIRED_GROUP).


Required before merge:#

  • Add input validation on username in ldapVerify
  • Use generic error messages for LDAP auth failures
  • Fix race condition in invite usage with atomic UPDATE
  • Document rate limiting as a deployment requirement
  • Add provisioned_via_ldap flag on users table
  • Consider optional group membership requirement
sign up or login to add to the discussion
expand 2 commits
0a966e32
feat: add ldap account syncing (if you delete in ldap will not update in indiko)
c1e5cbdd
This update enhances LDAP integration by introducing group verification, automated account cleanup for orphaned LDAP users, and improved security practices.
sign up or login to add to the discussion
expand 3 commits
0a966e32
feat: add ldap account syncing (if you delete in ldap will not update in indiko)
c1e5cbdd
This update enhances LDAP integration by introducing group verification, automated account cleanup for orphaned LDAP users, and improved security practices.
fb66f8d9
my bad

@dunkirk.sh I made the fixes. let me know what you think

sign up or login to add to the discussion
expand 4 commits
0a966e32
feat: add ldap account syncing (if you delete in ldap will not update in indiko)
c1e5cbdd
This update enhances LDAP integration by introducing group verification, automated account cleanup for orphaned LDAP users, and improved security practices.
fb66f8d9
my bad
04eaa6b7
fixed this flaw

i'm adding this directly to the main branch to get around the merge conflicts; i'll sign the commit with your email so it credits you properly

closed without merging
sign up or login to add to the discussion
Labels

None yet.

assignee

None yet.

Participants 2
AT URI
at://did:plc:ew7nbkvshgu6lzv6uxl25lsh/sh.tangled.repo.pull/3mbm4nvk7h222