Feat/auth provider interface

[Justy116]

Summary

Tenant safety

LogTide is multi-tenant. Confirm the following for any new/changed query, endpoint,
or background job (see docs/security/tenant-isolation-audit.md):

• Tenant tables are filtered by organization_id (and project_id where relevant).
• Joins enforce scoping at every level, not just the outer query.
• Updates/deletes verify scope before executing, not just trusting the filter to match.
• Cache keys include the organization id.
• Background jobs carry the org id and the consumer re-validates it.
• Ids from a URL parameter or request body are verified to belong to the requesting tenant before use.
• New data-access paths are added to the audit doc.
• npm run check:tenant-scoping passes (run from packages/backend).

Testing

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ Polliog
❌ Giustino Gragnaniello

---

Giustino Gragnaniello seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

❌ Patch coverage is 94.87179% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
packages/backend/src/modules/users/routes.ts	93.54%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Polliog]

Two findings from reviewing the auth-provider routing changes. The first is a security regression I'd like fixed before merge; the second is a minor robustness note.

1. User enumeration: disabled-account status is now disclosed without a valid password (blocking).
2. Non-atomic registration (minor, non-blocking).

Inline details below.

[Polliog]

Routing local login through LocalProvider.authenticate changes the order of the security checks and introduces a user-enumeration issue.

usersService.login verified the password (service.ts:173) before checking disabled (service.ts:179), so a disabled account with a wrong password returned Invalid email or password, indistinguishable from a non-existent user. LocalProvider.authenticate checks disabled (local-provider.ts:65) before bcrypt.compare (local-provider.ts:74), so a disabled account now returns This account has been disabled for any password. That message is sent to the client at routes.ts:162 and shown verbatim in the login UI, so anyone can probe which emails belong to disabled accounts without knowing the password. Skipping bcrypt for disabled accounts also adds a timing side-channel.

Two things that make this easy to miss: usersService.login no longer has any production callers after this change (the safe ordering now lives only in dead code), and the existing disabled-account tests (users-service.test.ts:258, local-provider.test.ts:114) both use the correct password, so they pass under either ordering.

Please restore the old ordering in LocalProvider.authenticate (run bcrypt.compare before the password_hash/disabled checks); the post-auth disabled re-check in findOrCreateUser (authentication-service.ts:260) still prevents a disabled user from getting a session. Please also add a test for 'disabled account + wrong password -> Invalid email or password' to lock the behavior in.

[Polliog]

Minor / non-blocking: createUser commits the user in its own transaction (service.ts:115) and does not create the user_identities row; the identity is created only here, during auto-login, which runs outside that transaction. If this call fails for a transient reason (DB or provider-cache error), the user row is already committed but /register returns a 500 (the catch only handles ZodError and already exists), with no session returned. The account is recoverable via a later /login (the identity is backfilled on the next successful auth), so severity is low, but consider catching an auto-login failure here and returning 201 with the created user and no session instead of propagating a 500.

For the record, disabling the local provider is not a viable trigger for this: it is explicitly blocked at provider-service.ts:250.

[Polliog]

Thanks for the update, the enumeration fix in LocalProvider is exactly what I asked for: password verified before the disabled check, generic error on wrong password, and a dedicated regression test. The register auto-login fallback is also a reasonable answer to the non-atomicity note.

A few remaining items before this can merge:

1. Frontend not updated for the optional session (blocking). packages/frontend/src/routes/register/+page.svelte (line 134) does authStore.setAuth(response.user, response.session.token) unguarded. With this PR the backend can now legitimately return 201 without a session, and the page would throw a TypeError instead of handling it. Please add a guard (e.g. redirect to /login with a message when session is absent).
2. CHANGELOG entry missing. Every merged branch gets an entry under [Unreleased] in CHANGELOG.md.
3. CLA check is failing: one of the commit author emails is not linked to a GitHub account, so the CLA bot cannot match it. Please link the email to your account (or rewrite the commits with the linked email) and re-run the check.

Non-blocking notes:

• usersService.login is now a parallel auth path used only by tests, and it does not carry the enumeration fix. Fine for this PR, but worth a follow-up to consolidate on the provider path so the two cannot drift.
• The 401 for a disabled account (correct password) and the "use your organization SSO" 401 are not recorded as auth.login_failed audit events. Pre-existing behavior, but since this PR now handles those cases explicitly it would be a good moment to audit them too.
• The "Please log in using your organization SSO" message is returned before password verification and reveals that an SSO account exists. Pre-existing, not a regression of this PR, just noting it.

Inline comments below for the code-level details.

[Polliog]

This catch swallows every failure silently, so a real problem on this path (DB error, provider cache failure) would be invisible in production. Please log it, e.g. request.log.warn({ err }, 'register auto-login failed'), so we can tell the difference between "never happens" and "happens and nobody notices".

Also a style nit: this comment uses an en dash; project convention is plain hyphens or rewording (no em/en dashes anywhere in the repo).

[Polliog]

Two issues with classifying errors by exact message string:

1. It is fragile: AuthenticationService.authenticateWithProvider throws a bare Error(result.error) and discards the provider's errorCode, so this route has to reconstruct semantics from strings. If any of these messages is ever reworded, the 401 silently becomes a 500. Suggestion: introduce an AuthError class (or rethrow with the errorCode attached) in AuthenticationService and switch on the code here.
2. There is an unhandled path that now produces a 500: if the local row in auth_providers is disabled (an SSO-only deployment), getProvider throws "Authentication provider 'local' not found or disabled", which is not in this whitelist, so every local login attempt returns a noisy 500 instead of a clean 401/403.

[Polliog]

AuthInput does not exist in the codebase; the interface takes credentials: unknown. Please reword this to match the actual contract (e.g. "a SAML provider can define its own credential shape for authenticate(credentials)").

Also, project convention is to avoid em dashes everywhere, including docs; this file uses them throughout. Please replace them with commas, colons, or parentheses.