Add CSV export functionality to organization members page

[maxkpower]

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-25534

📔 Objective

Adds CSV export button to organization members page with email, name, status, role, two-step login, account recovery, Secrets Manager access, and groups.

📸 Screenshots

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude[bot]]

Claude finished @maxkpower's task —— View job

---

Code Review Complete ✅

I've completed a comprehensive review of PR #17342 that adds CSV export functionality to the organization members page. Overall, this is a well-implemented feature with good test coverage, but there are some security and permission concerns that should be addressed.

---

Summary

Quality Rating: Good with Security Concerns

What's Working Well:

• ✅ Clean, well-structured code with proper separation of concerns
• ✅ Comprehensive unit tests (88% coverage on core service)
• ✅ Proper i18n usage throughout
• ✅ Good reuse of existing patterns (ExportHelper, papaparse)
• ✅ Previous review feedback addressed (default case in switch statement)
• ✅ Follows Angular best practices and ADRs

Key Issues Found: 4 inline comments posted

---

Critical Issues (Must Address)

1. Missing Permission Check ⚠️

Location: members.component.html:114

The export button is visible to all users without checking permissions. Since the export includes sensitive data (emails, names, roles, groups, 2FA status), this could be a privacy concern.

Recommendation: Add permission check like *ngIf="organization().canViewMembers || organization().canManageUsers"

Also consider:

• Should exports be logged in the audit trail?
• Should there be an org-level setting to enable/disable exports?

2. CSV Injection Vulnerability ⚠️

Location: member.export.ts:38

User-controlled data (names, emails, group names) is exported without sanitization. Values starting with =, +, @, -, or | can be interpreted as formulas in Excel/Calc.

Attack Example: User sets name to =cmd|'/c calc'!A1

Recommendation: Implement CSV injection prevention by prefixing dangerous characters with a single quote. See inline comment for implementation details.

---

Minor Issues

3. UI Alignment with Magic Number 📝

Location: members.component.html:366

The tw-w-[32px] spacer is brittle. Consider adding a comment or using a more maintainable approach.

4. Missing Test Coverage 🎨

Location: member-export.service.spec.ts:151

Add tests for:

• getFileName() method
• Special CSV characters handling (quotes, commas, newlines)

---

Security Checklist

• ✅ No unencrypted vault data sent to APIs
• ✅ No secrets or credentials in code
• ✅ No logging of PII
• ⚠️ CSV injection prevention needed
• ⚠️ Permission check needed

---

Recommendations Priority

1. HIGH: Implement CSV injection prevention (security issue)
2. HIGH: Evaluate and add permission checks (privacy concern)
3. MEDIUM: Add missing test coverage
4. LOW: Improve UI alignment documentation

---

Additional Considerations

Questions to discuss with the team:

• Should member exports require a specific permission?
• Should exports be logged/audited in the event log?
• Should there be rate limiting on exports?
• Is there a feature flag strategy for this feature?

The code quality is solid and follows established patterns well. The main blockers are the security concerns around CSV injection and permissions, which should be addressed before merging.

---

Review Status: Changes requested for security issues

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – 246b508e-4302-4de9-9d75-d3dc5487d16d

Great job! No new security vulnerabilities introduced in this pull request

[codecov[bot]]

Codecov Report

:x: Patch coverage is 56.45161% with 27 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 41.85%. Comparing base (f89c9b0) to head (70c9bc3). :warning: Report is 63 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...console/organizations/members/members.component.ts	0.00%	16 Missing :warning:
...le/organizations/members/pipes/user-status.pipe.ts	84.61%	1 Missing and 1 partial :warning:
...anizations/members/services/member-export/index.ts	0.00%	2 Missing :warning:
...rs/services/member-export/member-export.service.ts	88.23%	2 Missing :warning:
...src/app/tools/event-export/event-export.service.ts	0.00%	2 Missing :warning:
...c/app/admin-console/organizations/members/index.ts	0.00%	1 Missing :warning:
...in-console/organizations/members/members.module.ts	0.00%	1 Missing :warning:
...in-console/organizations/members/services/index.ts	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #17342      +/-   ##
==========================================
+ Coverage   41.70%   41.85%   +0.14%     
==========================================
  Files        3571     3594      +23     
  Lines      103817   104217     +400     
  Branches    15605    15721     +116     
==========================================
+ Hits        43301    43624     +323     
- Misses      58681    58741      +60     
- Partials     1835     1852      +17     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.