docs: ADR-022 Deployment Mode Consolidation via Login Flow v2#523
docs: ADR-022 Deployment Mode Consolidation via Login Flow v2#523cbcoutinho wants to merge 2 commits intomasterfrom
Conversation
Proposes consolidating five deployment modes into two: - Single-User: App password in env vars (trusted environment) - Multi-User: Login Flow v2 for per-user app password acquisition Key changes: - Use Nextcloud Login Flow v2 (NC 16+) for delegated authentication - Application-level scope enforcement (app passwords have no native scopes) - MCP elicitation for seamless authorization prompting - Astrolabe front-end integration for scope management UI - Clear security posture documentation for administrators This removes the need for upstream Nextcloud OAuth patches and simplifies deployment while maintaining security through defense-in-depth. Related: #521 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PR Review: ADR-022 Deployment Mode Consolidation via Login Flow v2This is an excellent, comprehensive ADR that proposes a significant architectural simplification. The document is well-researched and thoughtfully addresses a real maintenance burden. Here's my detailed feedback: ✅ Strengths1. Clear Problem StatementThe ADR effectively articulates the pain points:
The motivation is compelling and well-documented. 2. Solid Technical Foundation
3. Security TransparencyThe ADR is admirably honest about limitations:
The security posture documentation (lines 822-866) is crucial and should be prominently displayed to administrators. This transparency is a strength, not a weakness. 4. Migration PathClear 3-phase deprecation plan with backward compatibility considerations during transition.
|
Changes based on review: 1. Add Nextcloud platform limitation section documenting OAuth/scope support by endpoint type (WebDAV supports OAuth, others don't) 2. Update MCP elicitation to show capability negotiation and graceful fallback - URL in error message when elicitation not supported 3. Simplify Smithery section - recommend self-hosted for privacy, don't detail platform changes 4. Expand re-auth section with scope merging behavior, scenarios table, and explicit design choice for tool-based re-auth over auto-elicitation 5. Make rate limiting configurable with environment variables and admin guidance by deployment size 6. Clarify OAuth alternative - keep simple now, revisit if Nextcloud adds scoped OAuth support 7. Expand verification steps with required tests, add recommended Nextcloud configuration, add required README security notice Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Comprehensive Review of ADR-022: Deployment Mode Consolidation via Login Flow v2Thank you for this comprehensive ADR! This represents a significant architectural shift with both benefits and important security considerations. Below is my detailed review. 🎯 Overall AssessmentVerdict: Strong proposal with excellent documentation, but requires critical updates before acceptance. Strengths:
Critical Issues:
🔴 Critical Issues1. OAuth Bearer Token Support Claims Need CorrectionLines 61-77: The "Nextcloud Platform Limitation" table contains incorrect information about OAuth bearer token support. Current claim:
Reality: The existing codebase successfully uses OAuth bearer tokens with CalDAV, CardDAV, Notes, and other APIs in the current OAuth modes (ADR-020 modes 3-4). See:
The real limitation is scope enforcement, not bearer token acceptance. Nextcloud's OAuth accepts bearer tokens but doesn't enforce scopes at the API level. Recommended correction: > **OAuth Bearer Token Support by Endpoint:**
> | Endpoint Type | OAuth Bearer Accepted | Scoped Access Enforced |
> |---------------|----------------------|------------------------|
> | OCS API | ✅ Yes | ❌ No |
> | WebDAV | ✅ Yes | ❌ No |
> | CalDAV/CardDAV | ✅ Yes | ❌ No |
> | Notes API | ✅ Yes | ❌ No |
> | Other App APIs | ✅ Yes | ❌ No |
>
> **Implications:**
> - OAuth bearer tokens ARE accepted by Nextcloud APIs
> - However, scopes in the token are NOT enforced by Nextcloud
> - App passwords have the SAME security posture as OAuth tokens (both grant full API access)
> - The choice between OAuth and app passwords is about deployment simplicity, not securityThis is critical because the current justification "OAuth modes don't work with most APIs" is factually incorrect and undermines the ADR's credibility. 2. MCP Elicitation URL Support AssumptionsLines 335-431: The ADR assumes MCP clients support URL elicitation, but this is unverified. Concerns:
Recommendation: Before acceptance, verify:
Alternative approach: If URL elicitation is unavailable, consider:
3. Scope Merging Logic May Confuse UsersLines 445-543: The re-authentication scope merging behavior is complex and potentially surprising. Issues:
Recommendations:
|
Addressed Reviewer FeedbackUpdated the ADR based on review comments: 1. Nextcloud Platform Limitation Documentation ✅Added a new section documenting OAuth/scope support by endpoint type:
2. MCP Elicitation with Graceful Fallback ✅
3. Simplified Smithery Section ✅
4. Expanded Re-auth Details ✅
5. Configurable Rate Limiting ✅
6. Keep Implementation Simple ✅
7. Testing Requirements ✅
|
Summary
This ADR proposes consolidating the five existing deployment modes into two simplified modes using Nextcloud's native Login Flow v2 for multi-user authentication:
Key Design Decisions
Login Flow v2 over OAuth Token Pass-through
Application-Level Scope Enforcement
MCP Elicitation for Authorization
Modes Being Deprecated
Security Considerations
The ADR explicitly documents that scope enforcement is defense-in-depth, not a security boundary. Administrators must understand:
Related
Review Questions
Test plan
This PR was generated with the help of AI, and reviewed by a Human