feat(server): generic external auth callout - #4055
sansmoraxz wants to merge 4 commits into
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
|
hi, |
hubcio
added
S-waiting-on-author
hubcio
left a comment
hubcio
left a comment
There was a problem hiding this comment.
Needs another pass. The callout plumbing is mostly fine; nearly everything below is in the identity and ownership half.
Blockers are inline: the callout follows redirects and resends the credential body, the response cap runs after the body is buffered, HTTP recycles a synthetic id while its JWT is live, the counter is per process under a cluster-wide JWT key, a second Register splits identity from permissions, two paths leak ids until the 1M pool is gone, the binary transport skips the existence and Active checks HTTP enforces, and the metadata gate denies the consumer-group join/leave config.toml advertises.
Rest, on lines the diff does not cover or too small to inline:
permissions_global.rs:38- all ten global bools are mandatory and none of the three levels setsdeny_unknown_fields, so a typo silently weakens a grant and an 11th permission breaks every grant service. Parse into a#[serde(default)]DTO.dispatch/reads.rs:172-GET_MEungated: an all-false grant gets client id, transport, peer address, consumer-group memberships.dispatch/reads.rs:288-SYNC_CONSUMER_GROUPungated. Self-scoped, so nothing crosses callers, but still served to an all-false grant.http/handlers.rs:665- metrics route takes_identitywith no permission check. Predates this PR, but the PR creates a principal class with no permissions.http/jwt.rs:130- trusted issuers guarduser_id == 0but not the synthetic range, so an issuer mapped there inherits whatever grant holds that id.dispatch/authz.rs:578-gate_user_scopedreturnsOk(())on an unresolvable target while HTTP runs the predicate, soGET /users/me403s on HTTP and returns not-found on binary.external_auth.rs:160,171-urlis marked secret, then printed in full in the boot error and the plain-HTTP warning. Userinfo in the URL lands in the log.- Synthetic ids are persisted, against "never persisted" in the description:
Registerprepares carry them into the WAL, checkpoints and the recovered client table. The counter restarts atu32::MAX, so after a restart the same id means a different principal. common/types/user/mod.rs:25- the threshold and predicate are server-internal but sit in the publishediggy_common. The only outside consumer iscore/metadata, which already depends onserver_common.session_ops.rs:1317- external auth picks PAT vs password by first match, while the built-in path below deliberately falls back, with a comment saying the shapes collide. A colliding body fails here and succeeds there.config.toml:1108-forward_credentials = trueby default, so enabling the feature POSTs plaintext credentials unless the operator opts out, and the doc block does not say so. Worth flipping to opt-in.http/handlers.rs:198-client_addressis the raw TCP peer, so behind a proxy every login reports the proxy. Honour a trusted-proxy header or document which address it is.- No test covers a login on any transport, either authorization plane, the id lifecycle, or
on_error. The JSON tests assert on the DTO and never reach the decision mapping, so dropping the required-field checks leaves them green. Three blockers above only show up when you read two transports side by side.
Not blocking: single-use build closure at external_auth.rs:240; thread_local client duplicates http/jwks.rs:44; check_session_permission takes an Option all callers fill; mint and free-list logic written twice; get_connection, collect_expired_sessions, is_session_expired have no callers; session_permissions_for_user walks two maps when callers hold the connection id; stm/authz.rs:140 open-codes is_synthetic_user_id; handlers.rs:1910 re-imports MetadataHandle.
| /// request re-register cleanly through the barrier. | ||
| pub(in crate::http) fn forget_session(&self, session: &Rc<HttpSession>) { | ||
| let torn = forget_if_same(&mut self.sessions.borrow_mut(), session); | ||
| if torn.is_some() |
There was a problem hiding this comment.
critical: forget_if_same returns None when registry_token is unset even though it already removed the table entry, so this cleanup is skipped. any session that never did an acked produce leaks its permissions entry and burns an id.
There was a problem hiding this comment.
There should be proper cleanup now.
|
|
||
| /// Look up session-scoped permissions for a synthetic user ID. Returns | ||
| /// `None` for non-synthetic users or when no permissions are stored. | ||
| pub(in crate::http) fn get_synthetic_permissions(&self, user_id: u32) -> Option<Permissions> { |
There was a problem hiding this comment.
warning: this hands back the grant without checking expires_at, while the binary twin at session_manager.rs:449 does check it. on http an expired grant keeps authorizing until the token itself expires.
| #[serde_as(as = "DisplayFromStr")] | ||
| #[serde(default = "default_external_auth_timeout")] | ||
| #[config_env(leaf)] | ||
| pub timeout: IggyDuration, |
There was a problem hiding this comment.
warning: timeout = "unlimited" (also 0, none, disabled) parses to Duration::ZERO, and the compio timeout then fires on the first poll. with the default on_error = deny that is a total login outage, and nothing validates it.
There was a problem hiding this comment.
Zero timeout is now rejected at boot.
|
|
||
| /// Check if inline permissions allow sending messages to (stream, topic), | ||
| /// mirroring the `Permissioner::append_messages` inheritance chain. | ||
| pub fn can_send_messages(perms: &Permissions, stream_id: usize, topic_id: usize) -> bool { |
There was a problem hiding this comment.
simplification: these five predicates re-implement Permissioner rules by hand. init_permissions_for_user builds a one-user permissioner from the same Permissions, dropping ~100 lines plus the closure param threaded through the gates. watch the _ => false at line 211 - the permissioner arm allows there.
|
just to add: please wait with rebase until #4092 is merged. |
|
OK rethinking this, regarding the synthetic IDs probably better to rearchitect. I think we can have a synthetic ID provider (mapped to specific ID) and that basically enforces a custom ID that's provider generated. (Of course the provider here has to enforce the unique mapping). This can also remove all the extra plumbing necessary for rotating IDs and such. Especially in cluster mode. |
github-actions
Bot
removed
the
S-waiting-on-author
This would likely turn this PR into a breaking change. |
|
alright, you can ignore this comment for now. i'll check this from my side later this week. |
|
OK I rebased and went with a different approach now. There's one dedicated ID reserved for external users (which should not collide with local users) and is configurable (but not persisted). Default value if external users enables - Also the auth flow is now Both single node and cluster modes are validated to work without issues. Side note: |
2e44c1f to
12d4251
Compare
|
Also some pre-exsting lint issues in other languages (go etc.) Had to skip verify. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4055 +/- ##
============================================
- Coverage 86.01% 85.89% -0.13%
+ Complexity 1451 1448 -3
============================================
Files 1248 1248
Lines 194939 194866 -73
Branches 160243 160164 -79
============================================
- Hits 167669 167371 -298
- Misses 23163 23389 +226
+ Partials 4107 4106 -1
🚀 New features to boost your workflow:
|
2 participants
Which issue does this PR address?
Closes #3929
Rationale
Iggy supports only built-in credentials and HTTP-only JWT verification. There is no mechanism for an external service to authenticate a client and return its effective permissions, which blocks integration with centralized identity/policy systems (like Azure AD) or automated machine fleets.
What changed?
Login attempts on all transports (TCP, QUIC, WebSocket, HTTP) can forwarded to a configured HTTP endpoint. The service authenticates the client and returns one of three decisions: map to an existing Iggy user, grant a session-scoped identity with explicit permissions, or deny. Session-scoped identities use synthetic user IDs that are never persisted and are restricted to data-plane operations; (capacity defined as
1_000_000). The users ids within this pool are rotated (and next avaiable free user ids would be allocated). This can be adjusted if necessary.Request/response contract
Configuration
E2E validation
Built the server and validated the full external auth flow against a mock auth service.
Startup: Server loaded the config and logged the expected warning:
Test 1: Login
ext-alice(inline_grant): Mock received the callout withcredential_type,credential(forwarded),username,transport: "http",client_address. Responded withinline_grantgrantingread_streams,read_topics,poll_messages,send_messages. Server returned HTTP 200 with a JWT for syntheticuser_id: 4294967295(u32::MAX, first minted ID).Test 2: Login
ext-deny(deny): Mock responded{"decision":"deny","reason":"blocked by policy"}. Server returned HTTP 401.Test 3: GET /streams with alice's token (allowed): HTTP 200, body
[]. The grant'sread_streams: trueallowed the read.Test 4: POST /streams with alice's token (denied): HTTP 403. The grant's
manage_streams: falseblocked the mutation.Local Execution
AI Usage
If AI tools were used, please answer: