# ADR-014: Notification Service Uses External Service Authentication

> **⚠️ SUPERSEDED**: 2025-10-12
>
> This ADR applies to the original **Stateless HTTP API** design for the Notification Service.
>
> The service has been **redesigned as a CRD Controller** with NotificationRequest CRD for:
> - **Zero data loss** (etcd persistence)
> - **Complete audit trail** (all delivery attempts tracked)
> - **Automatic retry** (controller reconciliation)
> - **At-least-once delivery** (CRD-based state)
>
> **New Documentation**: [06-notification/](../../services/crd-controllers/06-notification/)
>
> **This ADR remains VALID** for the CRD-based design: External services still handle their own authentication.

---

**Date**: 2025-10-07
**Status**: ✅ Accepted (Superseded 2025-10-12, but principle remains valid)
**Related**: [Notification Controller](../../services/crd-controllers/06-notification/overview.md), BR-NOT-037

---

## Context

During initial design of the Notification Service, we considered implementing RBAC permission filtering to pre-filter notification action buttons based on each recipient's actual permissions in external services (GitHub, Kubernetes, Grafana, etc.).

The original approach required:
- Mapping email recipients to Kubernetes users/ServiceAccounts
- Querying Kubernetes SubjectAccessReview API for K8s permissions
- Querying Git provider APIs for repository permissions
- Permission caching with TTL-based invalidation
- Complex recipient mapping configuration

This added significant complexity (~500 lines of code) and tight coupling between Kubernaut and external services.

### Original Requirement (BR-NOT-037 - Incorrect)

> "Notification Service MUST filter action buttons based on recipient's actual permissions (RBAC) to prevent showing actions the recipient cannot perform."

### Problem Identified

**User Feedback**:
> "Why is there a UI/UX requirement when there is no UI provided? Notifications should provide links to external services (GitHub, GitLab, etc.) and those services should provide the authentication services, not Kubernaut."

**Assessment**: The feedback identified a major architectural misunderstanding. The original approach:
- ❌ **Unnecessary complexity** - External services already handle their own authentication
- ❌ **Wrong separation of concerns** - Authentication is the responsibility of the target service
- ❌ **Information hiding** - Pre-filtering prevents users from knowing actions exist
- ❌ **Tight coupling** - Kubernaut would need to understand multiple permission systems
- ❌ **Stale permissions** - Cached permissions could be outdated
- ❌ **External recipient failure** - No way to handle non-Kubernetes users (e.g., external vendors)

---

## Decision

**We will NOT implement RBAC permission filtering in the Notification Service.**

Instead, notifications will include **ALL recommended actions** as **links to external services**. When a user clicks a link:

1. User authenticates with the external service (GitHub, GitLab, Grafana, Kubernetes Dashboard, etc.)
2. External service enforces its own RBAC/permissions
3. User either:
   - Performs the action (if authorized)
   - Sees "Forbidden" or equivalent error from that service (if unauthorized)

### Revised Requirement (BR-NOT-037 - Correct)

> "Notification Service MUST include links to external services for all recommended actions. Authentication and authorization are enforced by the target service (GitHub, Grafana, Kubernetes API, etc.), not by Kubernaut."

---

## Example: Correct Notification Flow

### Email Notification (All Actions Visible)

```
Email to: developer@company.com

🚨 Alert: Pod OOMKilled in production namespace

Recommended Actions:

1. 📊 View Pod Logs
   → https://grafana.company.com/logs?pod=webapp-abc123
   (Grafana handles authentication when clicked)

2. 📈 View Prometheus Metrics
   → https://prometheus.company.com/graph?query=container_memory_usage{pod="webapp-abc123"}
   (Prometheus handles authentication when clicked)

3. 🔄 Restart Pod
   → https://k8s-dashboard.company.com/namespaces/production/pods/webapp-abc123/restart
   (Kubernetes Dashboard handles authentication when clicked)
   (If developer lacks permission, K8s Dashboard shows "Forbidden")

4. 📝 Approve GitOps PR
   → https://github.com/company/k8s-manifests/pull/123
   (GitHub handles authentication when clicked)
   (If developer lacks write permission, GitHub hides "Merge" button)
```

### Slack Notification (No RBAC Filtering)

```json
{
  "blocks": [
    {
      "type": "header",
      "text": {
        "type": "plain_text",
        "text": "🚨 Alert: Pod OOMKilled - webapp"
      }
    },
    {
      "type": "actions",
      "elements": [
        {
          "type": "button",
          "text": { "type": "plain_text", "text": "📊 View Logs" },
          "url": "https://grafana.company.com/logs?pod=webapp-abc123",
          "style": "primary"
        },
        {
          "type": "button",
          "text": { "type": "plain_text", "text": "🔄 Restart Pod" },
          "url": "https://k8s-dashboard.company.com/pods/webapp-abc123/restart"
        },
        {
          "type": "button",
          "text": { "type": "plain_text", "text": "📝 Approve PR" },
          "url": "https://github.com/company/k8s-manifests/pull/123"
        }
      ]
    }
  ]
}
```

**Note**: All buttons are shown. Authentication and authorization happen at the external service when clicked.

---

## Consequences

### Positive

1. **Reduced Complexity** (~500 lines of code eliminated)
   - No RecipientMapper interface
   - No Kubernetes SubjectAccessReview checks
   - No Git provider permission checks
   - No permission caching logic
   - No recipient-to-user mapping configuration

2. **Faster Notifications** (~50ms lower latency)
   - No external API calls to check permissions
   - No cache lookups or cache invalidation
   - Direct notification delivery

3. **Better Separation of Concerns**
   - Kubernaut focuses on notification delivery
   - External services own their authentication/authorization
   - Clear architectural boundaries

4. **Simpler Testing**
   - No mocking of permission systems
   - No testing of permission cache invalidation
   - No testing of recipient mapping logic

5. **Transparency**
   - Users see all possible actions
   - Users learn what capabilities exist
   - Users can request permissions if needed

6. **No Stale Permissions**
   - Permissions checked in real-time by external services
   - No cache synchronization issues

7. **External Recipient Support**
   - Works for users not in Kubernetes (vendors, contractors)
   - No special handling for external recipients

### Negative

1. **User May See Unavailable Actions**
   - Users may click actions they cannot perform
   - **Mitigation**: External service provides clear "Forbidden" or "Unauthorized" messages
   - **Assessment**: This is acceptable UX - users learn about capabilities and can request access

2. **No Permission-Based Action Prioritization**
   - Cannot sort actions by "actions you can perform" vs "actions you cannot"
   - **Mitigation**: Action prioritization based on AI confidence and risk level (not permissions)
   - **Assessment**: Permission-agnostic prioritization is more maintainable

---

## Alternatives Considered

### Alternative 1: RBAC Pre-Filtering (Rejected)

**Approach**: Kubernaut queries external services to check recipient permissions before including actions in notifications.

**Rejected Because**:
- Too complex (~500 lines of code)
- Tight coupling to external services
- Requires caching (stale permissions risk)
- Fails for external recipients
- Wrong separation of concerns

### Alternative 2: Hide Actions Without Permission Info (Rejected)

**Approach**: If Kubernaut cannot determine permissions, hide the action entirely.

**Rejected Because**:
- Information hiding (users don't know capabilities exist)
- Prevents users from requesting access
- Assumes Kubernaut knows all permission systems

### Alternative 3: Show All Actions with Warning (Considered, Not Needed)

**Approach**: Show all actions with a warning: "Some actions may require additional permissions."

**Not Needed Because**:
- External services already provide clear auth feedback
- Adding warnings adds unnecessary complexity
- Current approach is self-explanatory

---

## Implementation Changes

### Removed Components

1. ❌ `RecipientMapper` interface - No longer needed
2. ❌ `RBACChecker` interface - No longer needed
3. ❌ Kubernetes `SubjectAccessReview` checks - No longer needed
4. ❌ Git provider permission checks - No longer needed
5. ❌ Permission caching logic - No longer needed
6. ❌ Recipient-to-user mapping configuration - No longer needed

### Updated Data Structures

**Before (with RBAC filtering)**:
```go
type EscalationNotificationResponse struct {
    NotificationID       string
    RBACFiltering        RBACFilteringResult  // ❌ Remove
    VisibleActions       []Action             // ❌ Remove
    HiddenActions        []Action             // ❌ Remove
}
```

**After (no RBAC filtering)**:
```go
type EscalationNotificationResponse struct {
    NotificationID       string
    SanitizationApplied  []SanitizationResult
    DataFreshness        DataFreshnessResult
    DeliveryResults      []DeliveryResult
    ActionsIncluded      []Action  // All recommended actions (no filtering)
}
```

### Configuration Changes

**Before**: Complex recipient mapping
```yaml
recipients:
  "sre-oncall@company.com":
    k8s_user: "system:serviceaccount:kubernaut-system:sre-bot"
    git_user: "sre-oncall-bot"
    rbac_check: true
  "external-vendor@partner.com":
    rbac_check: false
```

**After**: No recipient mapping needed (removed entirely)

---

## Impact Analysis

| Metric | Before (RBAC Filtering) | After (External Auth) | Improvement |
|--------|------------------------|----------------------|-------------|
| **Code Complexity** | ~500 lines RBAC code | 0 lines | -100% |
| **Notification Latency** | ~150ms (with permission checks) | ~100ms | -50ms (-33%) |
| **External Dependencies** | K8s API + Git APIs | None | -100% |
| **Test Complexity** | High (mock permissions) | Low (no permission mocks) | -30% |
| **Configuration** | Complex recipient mapping | Simple (no mapping) | -100% |
| **External Recipient Support** | Limited | Full | +100% |

---

## Related Documents

- [Notification Service Overview](../../services/stateless/notification-service/overview.md)
- [Notification Service API Specification](../../services/stateless/notification-service/api-specification.md)
- [Business Requirement BR-NOT-037](../../requirements/06_INTEGRATION_LAYER.md#br-not-037)
- [Original Triage](../../services/stateless/notification-service/archive/triage/service-triage.md)

---

## Confidence Assessment

**Confidence in Decision**: **98%**

**Justification**:
- ✅ Aligns with industry best practices (external services own their auth)
- ✅ Follows separation of concerns principle
- ✅ Reduces complexity significantly (~500 lines removed)
- ✅ Improves performance (50ms faster notifications)
- ✅ Better UX (transparent, users can request permissions)
- ✅ Simpler testing and maintenance

**Remaining 2% Risk**:
- External service links might be incorrect (e.g., wrong Grafana URL)
- **Mitigation**: Configuration validation, E2E tests to verify link generation

---

## References

- Original Issue: CRITICAL-1 in [service-triage.md](../../services/stateless/notification-service/archive/triage/service-triage.md)
- Original Solution Document: [critical-issues.md](../../services/stateless/notification-service/archive/solutions/critical-issues.md)
- Original Revision Analysis: [critical-revisions.md](../../services/stateless/notification-service/archive/revisions/critical-revisions.md) (**deprecated** - content distributed to this ADR)

---

**Decision Made By**: Development Team
**Approved By**: Architecture Review
**Implementation Status**: ✅ Approved for implementation
