reviewing-security-architecture

name	reviewing-security-architecture
description	This skill should be used when the user asks to "review the security architecture", "check authentication patterns", "evaluate trust boundaries", "review encryption implementation", "assess authorization design", or needs to evaluate system designs for authentication, authorization, data protection, or cryptographic correctness.

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.