Skip to content

Conversation

@s-aga-r
Copy link
Collaborator

@s-aga-r s-aga-r commented Jan 6, 2026

No description provided.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables authentication and authorization controls for the mail application by activating the auth_hooks configuration and implementing path-based access validation to restrict non-System Users from accessing certain framework endpoints.

Key Changes:

  • Enabled the auth_hooks in the mail app configuration to activate authentication validation
  • Implemented path-based authorization logic that blocks framework API endpoints for non-System Users while allowing specific whitelisted paths

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
mail/hooks.py Uncommented the auth_hooks configuration to enable the mail.auth.validate authentication hook
mail/auth.py Added new authentication validation module with path-based access control logic for restricting non-System Users from accessing framework endpoints

Critical Issues Identified:

The implementation contains several critical security vulnerabilities that should be addressed:

  1. Logic flaw allowing unrestricted access: Paths that don't start with "/api/" bypass validation entirely (except for explicit DENIED_PATHS), potentially allowing unauthorized access to non-API routes.

  2. Path normalization bypass vulnerability: The code doesn't normalize paths before validation, allowing potential bypasses through double slashes, path traversal, or URL encoding.

  3. Missing guest user handling: The code doesn't explicitly check for Guest users before database queries, which could lead to unexpected behavior.

  4. Insufficient documentation: The security-critical function and constants lack documentation explaining the security model and usage guidelines.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@s-aga-r s-aga-r force-pushed the block-framework-endpoints branch from a34e7d9 to e60038b Compare January 7, 2026 05:17
@s-aga-r s-aga-r marked this pull request as ready for review January 7, 2026 05:42
@s-aga-r s-aga-r requested a review from krantheman as a code owner January 7, 2026 05:42
@s-aga-r s-aga-r merged commit ffb51d0 into frappe:develop Jan 7, 2026
3 checks passed
@s-aga-r s-aga-r deleted the block-framework-endpoints branch January 7, 2026 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant