reviewing-security-architecture

Authentication Architecture

Token Handling

Review these aspects of token-based authentication:

| Aspect | Secure Pattern | Anti-Pattern | | -------------------- | ----------------------------------------------------------------- | ---------------------------------------------------------------------- | | Issuance | Short-lived tokens with refresh mechanism | Long-lived tokens that never expire | | Validation | Validate signature, issuer, audience, and expiry on every request | Validate only the signature, or skip validation for "internal" calls | | Storage (server) | Stateless JWT or server-side session store | Token stored in querystring or URL | | Storage (client) | HttpOnly Secure cookies or secure platform storage | localStorage, sessionStorage, or cookies without HttpOnly/Secure flags | | Refresh | Refresh token rotation (old refresh token invalidated on use) | Reusable refresh tokens with no rotation | | Revocation | Token blocklist or short expiry + refresh rotation | No revocation mechanism for compromised tokens |

Session Management

• Server-side sessions should have absolute timeouts (maximum session duration) and idle timeouts
• Session identifiers must be cryptographically random and sufficiently long (128+ bits of entropy)
• Regenerate session ID after authentication state changes (login, privilege escalation)
• Bind sessions to client properties where possible (IP range, user agent) for anomaly detection

Credential Storage

• Passwords must be hashed with a modern KDF: Argon2id (preferred), bcrypt, or PBKDF2 with high work factor and a unique salt
• Never use raw cryptographic hash functions alone for password hashing (too fast, no salt by default)
• Salts should be unique per credential to prevent rainbow-tables from accelerating brute-force attacks

Authorization Patterns

Role-Based Access Control (RBAC)

// CORRECT — explicit role check at the API layer
[Authorize(Roles = "Admin")]
public async Task<IActionResult> DeleteUser(Guid userId)

// WRONG — checking role in business logic with string comparison
if (currentUser.Role == "admin") // Fragile, case-sensitive, easy to bypass

Object-Level Authorization

// WRONG — trusts the userId from the route, no ownership check
public async Task<Cipher> GetCipher(Guid cipherId) {
    return await _cipherRepository.GetByIdAsync(cipherId);
}

// CORRECT — verify the requesting user owns the resource
public async Task<Cipher> GetCipher(Guid cipherId) {
    var cipher = await _cipherRepository.GetByIdAsync(cipherId);
    if (cipher.UserId != _currentContext.UserId)
        throw new NotFoundException();
    return cipher;
}

Authorization Principles

• Check at every layer. API controller, service layer, and data access should all enforce authorization. Don't rely on a single checkpoint.
• Least privilege. Grant the minimum permissions needed. Default to deny.
• Fail closed. If an authorization check fails or throws an exception, deny access. Never fail open.
• Don't trust client-side authorization. UI visibility controls are UX, not security. Always enforce server-side.

Data Protection

Encryption at Rest

• All sensitive data must be encrypted at rest using AES-256 or equivalent
• Cryptographic keys MUST NEVER be stored directly accessible in a database, without being wrapped by another key
• Use envelope encryption: data encrypted with a data encryption key (DEK), DEK encrypted with a key encryption key (KEK) in a key management system
• Bitwarden's end-to-end encryption ensures vault data is encrypted before leaving the client

Encryption in Transit

• TLS 1.2 minimum, TLS 1.3 preferred
• Disable older protocols (SSL 3.0, TLS 1.0, TLS 1.1)
• Use strong cipher suites (ECDHE for key exchange, AES-GCM for encryption)
• Certificate pinning for mobile apps where appropriate
• Internal service-to-service communication should also use TLS

Data Classification

When reviewing architecture, identify data by classification:

| Classification | Examples | Required Protection | | ---------------- | --------------------------------------------- | ----------------------------------------------- | | Critical | Encryption keys, master passwords, vault data | End-to-end encryption, HSM key storage | | Confidential | PII, email addresses, billing info | Encryption at rest + in transit, access logging | | Internal | Organizational settings, feature flags | Encryption in transit, role-based access | | Public | Marketing content, public API docs | Integrity protection |

Trust Boundaries

A trust boundary exists wherever data crosses between components with different levels of trust. Every crossing must be validated.

Common Trust Boundaries

Client ←→ API Gateway         (user-controlled → server-controlled)
API Gateway ←→ Backend Service (internet-facing → internal)
Backend Service ←→ Database    (application → data store)
Service ←→ External API        (internal → third-party)
Browser ←→ Browser Extension   (page context → extension context)
Main Thread ←→ Web Worker      (different execution contexts)

Validation at Trust Boundaries

At each boundary crossing:

1. Validate all input — type, format, range, length. Don't trust upstream validation.
2. Authenticate the caller — verify identity before processing requests.
3. Authorize the action — verify the caller has permission for this specific operation.
4. Sanitize output — encode/escape data appropriate to the destination context.
5. Log the crossing — security-relevant boundary crossings should be auditable.

Zero-Trust Principles

• Don't trust internal network location as a proxy for authentication
• Every service-to-service call should be authenticated and authorized
• Assume the network is compromised — encrypt all internal communication
• Validate data from internal services just as rigorously as external input

Architecture Decision Alignment

Before evaluating a design, check Bitwarden's Architecture Decision Records for existing decisions relevant to the components under review — see ${CLAUDE_PLUGIN_ROOT}/references/adr-alignment.md for the ground rules (conflict = finding, undocumented significant decision = gap, verify status before citing). Applied to an architecture review specifically:

• Cite it, don't just flag it. When a design conflicts with an accepted ADR, name the ADR and state whether the implementation should change or the deviation needs its own ADR justifying the exception.
• Watch for these gap triggers. New trust boundaries, new auth patterns, new data stores, or other consequential choices with no corresponding ADR are exactly the kind of significant decision that should be flagged so it gets recorded, not just implemented.

Reference Material

For detailed lookup tables and code examples, consult:

• references/crypto-algorithms.md — Algorithm selection table (recommended vs. deprecated) and common crypto anti-pattern code examples
• references/architectural-anti-patterns.md — Common security architecture anti-patterns (implicit trust, single points of failure, insecure defaults, monolithic auth) with fixes

Connection to Threat Modeling

Architecture security review directly feeds into the threat modeling process:

• Trust boundary identification informs where to draw boundaries in data flow diagrams
• Architectural weaknesses become threats in the threat catalog
• Security properties (auth, encryption, access control) map to security goals in security definitions
• Anti-patterns found become candidates for Bitwarden's engagement model Phase 1 initial security assessment

When conducting architecture review, consider whether the findings warrant engaging the AppSec team (#team-eng-appsec) for a full threat modeling session.