Skip to content

feat: scope notifications subscribe auth#24

Merged
rowan-stein merged 2 commits intomainfrom
noa/issue-142
Apr 25, 2026
Merged

feat: scope notifications subscribe auth#24
rowan-stein merged 2 commits intomainfrom
noa/issue-142

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

@casey-brooks casey-brooks commented Apr 25, 2026

Summary

  • scope hub fanout to subscription rooms and validate Subscribe rooms
  • add workload org index + external Subscribe authorization checks for thread_participant/workload rooms
  • wire authorization client/config and add coverage for room scoping/auth flows

Testing

  • buf generate buf.build/agynio/api --template ./buf.gen.yaml
  • go vet ./...
  • go test ./...
  • go build ./...

Issue

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • scope hub fanout and Subscribe validation to requested rooms
  • add workload org index + Subscribe authorization checks for thread_participant/workload rooms
  • wire Authorization client/config and extend coverage for room scoping/auth flows

Test & Lint

  • buf generate buf.build/agynio/api --template ./buf.gen.yaml
  • go vet ./... (no issues)
  • go test ./... -json > /tmp/notifications-go-test.json (passed=21 failed=0 skipped=0)
  • go build ./...

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • confirmed Subscribe room scoping/auth updates for issue 142
  • refreshed PR metadata to point at architecture issue

Testing

  • buf generate buf.build/agynio/api --template ./buf.gen.yaml
  • go vet ./...
  • go test ./...
  • go build ./...

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • confirmed Subscribe room scoping/auth updates for issue 142
  • refreshed PR metadata to point at architecture issue

Testing

  • buf generate buf.build/agynio/api --template ./buf.gen.yaml
  • go vet ./...
  • go test ./...
  • go build ./...

Tests: passed: all, failed: 0, skipped: 0
Lint: no errors.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Blocking:

  • External Subscribe auth currently ignores unhandled room types (roomKindOther). Because organization: rooms aren’t classified/authorized, an authenticated caller can subscribe to arbitrary organization:{org_id} rooms (and receive e.g. workload.updated events) without any authz check.

Please fail closed for external callers on unknown room kinds and/or explicitly classify+authorize organization: rooms per architecture/notifications.md#authorization, with tests covering the chosen behavior.

(Static review only; I did not run tests locally.)

Comment thread internal/server/authz.go
Comment thread internal/server/rooms.go Outdated
Comment thread internal/server/authz.go
Comment thread internal/stream/hub.go
Comment thread internal/server/rooms.go Outdated
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • classify organization rooms, canonicalize room strings, and fail closed on unknown room kinds for external Subscribe auth
  • enforce member checks for organization rooms and add tests for org/unknown/canonicalization cases
  • update hub Broadcast doc comment to match room-scoped delivery

Testing

  • buf generate buf.build/agynio/api --template ./buf.gen.yaml
  • go vet ./...
  • go test ./...
  • go build ./...

Tests: passed: all, failed: 0, skipped: 0
Lint: no errors.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Follow-up review: requested authz gaps are addressed.

  • organization: rooms are now classified/validated and authorized.
  • Unknown room kinds fail closed for external callers.
  • Room canonicalization is in place + tests cover canonicalization/auth flows.

(Static review only; I did not run tests locally.)

@rowan-stein rowan-stein merged commit 6ef1376 into main Apr 25, 2026
1 check passed
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