docuseal/docs/qa/assessments/1.1.institution-admin-risk-20250103.md

# Risk Profile: Story 1.1 - Institution Admin Management

**Date:** 2025-01-03
**Reviewer:** Quinn (Test Architect)
**Story:** `docs/stories/1.1.institution-admin.md`

## Executive Summary

- **Total Risks Identified:** 8
- **Critical Risks:** 2 (SEC-001, SEC-002)
- **High Risks:** 3 (SEC-003, DATA-001, PERF-001, TECH-001)
- **Medium Risks:** 1 (OPS-001)
- **Low Risks:** 1 (BUS-001)
- **Overall Risk Score:** 23/100 (High Risk)
- **Quality Gate:** ⚠️ **CONCERNS** (requires mitigation before production)

## Critical Risks Requiring Immediate Attention

### 1. SEC-001: Cross-Institution Data Access Vulnerability

**Score: 9 (Critical)**
**Category:** Security
**Probability:** High (>70%) - Multiple access paths, complex enforcement
**Impact:** High - Complete multi-tenancy breach, legal liability, reputation damage

#### Description
The story requires "admins cannot access other institutions' data" but lacks specific enforcement mechanisms across API, UI, and database layers. Without comprehensive isolation, users could manipulate institution IDs to access unauthorized data.

#### Affected Components
- `app/controllers/api/v1/institutions_controller.rb`
- `app/controllers/cohorts/admin_controller.rb`
- `app/models/institution.rb` (scope methods)
- `app/javascript/cohorts/admin/` Vue components
- All API endpoints for cohort management

#### Mitigation Strategy (Preventive)

**Database-Level Controls:**
```ruby
# app/models/institution.rb
scope :for_user, ->(user) {
  where(id: user.institutions.select(:id))
}

# All queries MUST use this scope
def self.accessible_by(user)
  for_user(user).includes(:cohorts, :sponsors)
end
```

**API-Level Controls:**
```ruby
# app/controllers/api/api_base_controller.rb
before_action :verify_institution_access

def verify_institution_access
  return if current_user.institutions.exists?(id: params[:institution_id])
  render json: { error: 'Access denied' }, status: :forbidden
  SecurityEvent.log(:unauthorized_institution_access, current_user, params)
end
```

**UI-Level Controls:**
- Store institution context in Vuex with server-side validation
- Prevent manual URL manipulation with route guards
- Validate institution_id on every API call

#### Testing Requirements

**Security Testing:**
- [ ] Unit tests for `Institution.for_user(user)` scoping
- [ ] Request tests: Attempt cross-institution access via altered institution_id
- [ ] Integration tests: Concurrent users from different institutions
- [ ] Manual penetration testing: URL manipulation, JWT token tampering
- [ ] Load testing: 100+ concurrent users across institutions

**Test Scenarios:**
```ruby
# spec/requests/api/v1/institutions_spec.rb
describe 'Cross-institution access prevention' do
  it 'rejects access to institution not belonging to user' do
    other_institution = create(:institution)
    get api_v1_institution_path(other_institution), headers: auth_headers
    expect(response).to have_http_status(:forbidden)
  end

  it 'logs security event on unauthorized access' do
    expect {
      get api_v1_institution_path(other_institution), headers: auth_headers
    }.to change(SecurityEvent, :count).by(1)
  end
end
```

**Residual Risk:** Low - With comprehensive controls and testing

---

### 2. SEC-002: Invitation Token Security Flaws

**Score: 9 (Critical)**
**Category:** Security
**Probability:** High - Token generation, validation, expiration complexity
**Impact:** High - Unauthorized admin access, account takeover

#### Description
Token-based invitation system requires secure generation, single-use enforcement, expiration handling, and proper validation. Weaknesses could allow token reuse, brute-force attacks, or unauthorized access.

#### Affected Components
- `app/models/cohort_admin_invitation.rb`
- `app/jobs/cohort_admin_invitation_job.rb`
- `app/mailers/cohort_mailer.rb`
- Invitation acceptance flow (not yet specified in story)

#### Mitigation Strategy (Preventive)

**Token Generation & Storage:**
```ruby
# app/models/cohort_admin_invitation.rb
def generate_token
  raw_token = SecureRandom.urlsafe_base64(64)
  self.token = Digest::SHA256.hexdigest(raw_token)
  self.token_preview = raw_token[0..8] # For display/debug only
  raw_token # Return raw for email
end

# Redis for single-use enforcement
def validate_token(raw_token)
  return false if expired?
  return false if used?

  hashed = Digest::SHA256.hexdigest(raw_token)
  return false unless token == hashed

  # Mark as used atomically
  redis_key = "invitation:#{id}:used"
  used = Redis.current.set(redis_key, "1", nx: true, ex: 86400)
  return false unless used

  true
end
```

**Invitation Flow Security:**
- Rate limiting: 5 attempts per hour per email
- Email domain verification for invited admins
- Invalidate all tokens when super_admin role is revoked
- Audit trail for all invitation attempts

#### Testing Requirements

**Security Testing:**
- [ ] Token reuse attempts (should fail)
- [ ] Expired token usage (should fail)
- [ ] Token with wrong email (should fail)
- [ ] Race condition testing (concurrent token validation)
- [ ] Super_admin demotion cascades (invalidates pending tokens)
- [ ] Rate limiting verification

**Test Scenarios:**
```ruby
# spec/models/cohort_admin_invitation_spec.rb
describe 'Token security' do
  it 'rejects reused tokens' do
    invitation = create(:cohort_admin_invitation)
    raw_token = invitation.generate_token

    expect(invitation.validate_token(raw_token)).to be true
    expect(invitation.validate_token(raw_token)).to be false
  end

  it 'expires after 24 hours' do
    invitation = create(:cohort_admin_invitation, created_at: 25.hours.ago)
    raw_token = invitation.generate_token
    expect(invitation.validate_token(raw_token)).to be false
  end
end
```

**Residual Risk:** Low - With proper implementation and testing

---

## High Priority Risks

### 3. DATA-001: Migration Failure on Production Data

**Score: 6 (High)**
**Category:** Data
**Probability:** Medium - Schema changes on live data
**Impact:** High - Downtime, potential data loss

#### Mitigation
1. **Pre-migration backup** with point-in-time recovery
2. **Staging rehearsal** with production-like data volume
3. **Zero-downtime pattern**:
   ```ruby
   # Migration 1: Add nullable columns
   add_column :institutions, :super_admin_id, :bigint, null: true
   add_index :institutions, :super_admin_id

   # Code deployment
   # Backfill data in batches
   # Migration 2: Add constraints
   change_column_null :institutions, :super_admin_id, false
   ```
4. **Rollback plan tested** (as mentioned in story)

#### Testing
- [ ] Migration rollback on staging
- [ ] Data integrity verification post-migration
- [ ] Performance impact measurement

---

### 4. PERF-001: Query Performance Degradation

**Score: 6 (High)**
**Category:** Performance
**Probability:** High - Added joins for institution filtering
**Impact:** Medium - Must stay within 10% degradation threshold

#### Mitigation
1. **Index Strategy** (from Dev Notes):
   ```sql
   CREATE UNIQUE INDEX ON institutions (account_id);
   CREATE UNIQUE INDEX ON institutions (registration_number);
   CREATE INDEX ON institutions (super_admin_id);
   CREATE UNIQUE INDEX ON cohort_admin_invitations (token);
   CREATE INDEX ON account_access (role);
   ```

2. **Query Optimization**:
   - Use `EXPLAIN ANALYZE` on key queries
   - Eager loading: `includes(:institution, :account_access)`
   - Avoid N+1 queries in Vue components

3. **Benchmarking**:
   - Measure before/after for: login flow, dashboard, existing API endpoints
   - Target: <10% degradation on existing operations

#### Testing
- [ ] Benchmark existing user operations
- [ ] Load test with 1000+ users
- [ ] Monitor query execution times

---

### 5. TECH-001: DocuSeal Integration Conflicts

**Score: 6 (High)**
**Category:** Technical
**Probability:** Medium - Additive changes only
**Impact:** High - Breaking existing functionality

#### Mitigation
1. **Additive schema only** (no existing table modifications)
2. **Run existing test suite** before merge (IV1-IV3 tests)
3. **Feature flag** for phased rollout:
   ```ruby
   # config/initializers/features.rb
   Docuseal.enable_cohort_management = ENV['ENABLE_COHORT_MANAGEMENT'] == 'true'
   ```

#### Testing
- [ ] IV1: Existing authentication still works
- [ ] IV2: Role system compatibility
- [ ] IV3: Performance impact <10%

---

## Medium/Low Priority Risks

### 6. OPS-001: Missing Security Event Monitoring

**Score: 4 (Medium)**
**Category:** Operational
**Probability:** Medium - Not specified in story
**Impact:** Medium - Blind spots for security incidents

**Mitigation:**
- Implement `SecurityEvent` model for logging unauthorized access
- Add alerts for repeated failed access attempts
- Monitor token validation failures

---

### 7. BUS-001: Existing User Impact

**Score: 3 (Low)**
**Category:** Business
**Probability:** Low - Additive changes
**Impact:** High - Could affect existing DocuSeal customers

**Mitigation:**
- Extensive testing of existing flows (IV1-IV3)
- Feature flag for rollback capability
- Phased rollout with monitoring

---

## Risk-Based Testing Strategy

### Priority 1: Critical Risk Tests (P0 - Block Production)
- **Data Isolation**: All cross-institution access prevention scenarios
- **Token Security**: Reuse, expiration, validation, race conditions
- **Migration Rollback**: Tested and verified on production-like data

### Priority 2: High Risk Tests (P1 - Block Merge)
- **Performance**: Benchmark existing operations, load testing
- **Integration**: Existing DocuSeal test suite must pass
- **Authorization**: Role-based access at API and UI levels

### Priority 3: Medium Risk Tests (P2 - Pre-Deployment)
- **Security Monitoring**: Event logging and alerting
- **Error Handling**: Validation failures, edge cases

### Priority 4: Low Risk Tests (P3 - Post-Deployment)
- **UI/UX**: Role-based UI elements
- **Documentation**: Security controls documented

---

## Risk Acceptance Criteria

### ❌ **MUST FIX Before Production**
- All critical risks (SEC-001, SEC-002) fully mitigated
- Data isolation tested with 100% coverage
- Token security audit passed
- Migration rollback verified

### ⚠️ **CAN Deploy with Mitigation**
- High risks with compensating controls (performance monitoring, feature flags)
- Medium risks with monitoring in place

### ✅ **Accepted Risks**
- Low business risk (BUS-001) with extensive testing
- Documented and signed off by security team

---

## Monitoring Requirements

### Post-Deployment Metrics
1. **Security Events**: Unauthorized access attempts per hour
2. **Performance**: Query response times for institution-scoped queries
3. **Token Usage**: Validation success/failure rates
4. **Error Rates**: 403 Forbidden responses (should be 0 for legitimate users)

### Alert Thresholds
- **Critical**: >5 unauthorized access attempts/hour
- **High**: Query performance >10% degradation
- **Medium**: Token validation failure rate >20%

---

## Risk Review Triggers

Update this risk profile when:
- Architecture changes significantly
- Security vulnerabilities discovered in dependencies
- Performance issues reported in production
- Regulatory requirements change (GDPR, SOC2)
- New integration points added

---

## QA Gate Decision

**Gate Status:** ⚠️ **CONCERNS**

**Rationale:**
- 2 critical security risks require immediate attention
- Data isolation is foundational and must be perfect
- Token security requires security team review
- Overall risk score of 23/100 indicates high risk

**Required Actions:**
1. ✅ Security architecture review with Winston (per CLAUDE.md)
2. ✅ Implement database-level data isolation controls
3. ✅ Security audit of token system before merge
4. ✅ Performance benchmarking of existing operations
5. ✅ Migration rollback testing on production-like data

**Recommendation:** Proceed with implementation but require security review and comprehensive testing before production deployment. Consider feature flag for phased rollout.

---

## Next Steps for Development Team

1. **Before Coding**: Schedule security architecture review
2. **During Development**:
   - Code review on all authorization logic
   - Performance testing on each query addition
3. **Before Merge**:
   - Security audit of token system
   - Load testing of institution isolation
   - Run existing DocuSeal test suite
4. **Before Production**:
   - Feature flag ready for rollback
   - Monitoring dashboards deployed
   - Security team sign-off

---

**Document Location:** `docs/qa/assessments/1.1.institution-admin-risk-20250103.md`
**Related Files:**
- Story: `docs/stories/1.1.institution-admin.md`
- Gate: `docs/qa/gates/1.1.institution-admin.yml` (pending)
- Next Action: `*review @docs/stories/1.1.institution-admin.md`