feat: exp scheme #9650
feat: exp scheme #9650
Conversation
📝 WalkthroughWalkthroughThis set of changes introduces a new Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
commit: |
| // Add `exp` and `appwrite-callback-{projectId}` schemes | ||
| if (!$project->isEmpty() && $project->getId() !== 'console') { | ||
| $project->setAttribute('platforms', [ | ||
| '$collection' => ID::custom('platforms'), | ||
| 'type' => Platform::TYPE_SCHEME, | ||
| 'name' => 'Expo', | ||
| 'key' => 'exp', | ||
| ], Document::SET_TYPE_APPEND); | ||
| $project->setAttribute('platforms', [ | ||
| '$collection' => ID::custom('platforms'), | ||
| 'type' => Platform::TYPE_SCHEME, | ||
| 'name' => 'Appwrite Callback', | ||
| 'key' => 'appwrite-callback-' . $project->getId(), | ||
| ], Document::SET_TYPE_APPEND); |
There was a problem hiding this comment.
Core feature: here is where we add exp and appwrite-callback-projectId schemes to the platform config.
| public const CLIENT_TYPE_UNKNOWN = 'unknown'; | ||
| public const CLIENT_TYPE_WEB = 'web'; | ||
| public const CLIENT_TYPE_FLUTTER_IOS = 'flutter-ios'; | ||
| public const CLIENT_TYPE_FLUTTER_ANDROID = 'flutter-android'; | ||
| public const CLIENT_TYPE_FLUTTER_MACOS = 'flutter-macos'; | ||
| public const CLIENT_TYPE_FLUTTER_WINDOWS = 'flutter-windows'; | ||
| public const CLIENT_TYPE_FLUTTER_LINUX = 'flutter-linux'; | ||
| public const CLIENT_TYPE_FLUTTER_WEB = 'flutter-web'; | ||
| public const CLIENT_TYPE_APPLE_IOS = 'apple-ios'; | ||
| public const CLIENT_TYPE_APPLE_MACOS = 'apple-macos'; | ||
| public const CLIENT_TYPE_APPLE_WATCHOS = 'apple-watchos'; | ||
| public const CLIENT_TYPE_APPLE_TVOS = 'apple-tvos'; | ||
| public const CLIENT_TYPE_ANDROID = 'android'; | ||
| public const CLIENT_TYPE_UNITY = 'unity'; | ||
| public const CLIENT_TYPE_REACT_NATIVE_IOS = 'react-native-ios'; | ||
| public const CLIENT_TYPE_REACT_NATIVE_ANDROID = 'react-native-android'; |
There was a problem hiding this comment.
These constants now live in Appwrite/Network/Platform.php
✨ Benchmark results
⚡ Benchmark Comparison
|
|
Hi Folks, This is causing iOS appwrite auth to fail. please merge in asap! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/init/resources.php (1)
124-167: LGTM: Core feature implementation for React Native OAuth support.The resource refactor correctly:
- Renames from 'clients' to 'platforms' for consistency
- Uses
Platform::TYPE_WEBconstant instead of the old Origin constant- Adds
expandappwrite-callback-{projectId}schemes for React Native OAuth- Returns merged platform arrays from console and project
This aligns with the PR objectives to support React Native OAuth development.
src/Appwrite/Network/Validator/Origin.php (1)
5-5: LGTM: Platform class import added.Correct import for the new Platform utility class that centralizes platform handling.
🧹 Nitpick comments (2)
app/controllers/api/account.php (1)
1185-1186: Good security improvement with conditional validationThe conditional validator logic properly implements different validation rules for development vs production environments. Using
Redirectvalidator in production helps prevent open redirect attacks by validating against the platform list.Consider extracting the repeated conditional validator pattern into a helper function to improve maintainability, since this pattern is used across multiple endpoints:
private function createRedirectValidator(): callable { return fn ($platforms, $devKey) => $devKey->isEmpty() ? new Redirect($platforms) : new URL(); }src/Appwrite/Network/Validator/Origin.php (1)
61-72: Consider improving the error message for better user experience.The error message could be more informative when no platform is found for the scheme:
public function getDescription(): string { - $platform = $this->scheme ? Platform::getNameByScheme($this->scheme) : null; + $platform = $this->scheme ? Platform::getNameByScheme($this->scheme) : 'Web'; $host = $this->host ? '(' . $this->host . ')' : ''; if (empty($this->host) && empty($this->scheme)) { return 'Invalid Origin.'; } return 'Invalid Origin. Register your new client ' . $host . ' as a new ' - . $platform . ' platform on your project console dashboard'; + . ($platform ?? 'platform') . ' platform on your project console dashboard'; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/config/console.php(2 hunks)app/controllers/api/account.php(6 hunks)app/controllers/api/projects.php(2 hunks)app/controllers/api/teams.php(2 hunks)app/controllers/api/vcs.php(2 hunks)app/controllers/general.php(5 hunks)app/init/resources.php(3 hunks)app/realtime.php(2 hunks)src/Appwrite/Network/Platform.php(1 hunks)src/Appwrite/Network/Validator/Origin.php(2 hunks)src/Appwrite/Network/Validator/Redirect.php(1 hunks)tests/e2e/Services/Realtime/RealtimeBase.php(2 hunks)tests/unit/Network/Validators/OriginTest.php(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Benchmark
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build SDK
- GitHub Check: scan
🔇 Additional comments (35)
src/Appwrite/Network/Platform.php (2)
56-59: LGTM! Well-designed utility method.The
getNameBySchememethod provides a clean way to get user-friendly platform names with proper fallback handling.
97-113: LGTM! Clean implementation for scheme extraction.The
getSchemesmethod correctly filters platforms by TYPE_SCHEME and returns unique schemes.app/config/console.php (2)
8-8: LGTM! Correct import update.The import change from
OrigintoPlatformaligns with the new centralized platform utility class.
26-26: LGTM! Consistent platform type constant usage.The update from
Origin::CLIENT_TYPE_WEBtoPlatform::TYPE_WEBis consistent with the broader refactoring to centralize platform cons 8000 tants.app/realtime.php (2)
535-535: LGTM! Correct resource reference update.The change from 'console' to 'platforms' resource aligns with the broader refactoring to use platform-based validation.
560-564: LGTM! Improved origin validation logic.The changes provide two improvements:
- Uses the new platform-based Origin validator
- Adds a guard to skip validation when origin is empty, which is appropriate for non-web platforms
src/Appwrite/Network/Validator/Redirect.php (1)
7-25: LGTM! Well-designed validator with user-friendly error messages.The
Redirectvalidator effectively extendsOriginwhile providing more informative error messages. The implementation correctly:
- Uses
Platform::getNameByScheme()to get user-friendly platform names- Handles edge cases with appropriate guards for empty host/scheme
- Provides actionable guidance for users to register platforms
app/controllers/api/teams.php (2)
14-14: LGTM! Correct import for improved redirect validation.The change from
HosttoRedirectvalidator import supports better error messaging for redirect URL validation failures.
469-469: LGTM! Enhanced redirect validation with better UX.The parameter validation changes provide two improvements:
- Uses
Redirectvalidator instead ofHostfor more informative error messages- Updates dependency from 'clients' to 'platforms' aligning with the refactoring
This maintains security while providing better user guidance when redirect URLs are invalid.
tests/unit/Network/Validators/OriginTest.php (6)
5-5: LGTM: Import addition aligns with platform refactor.The import of the
Platformclass is consistent with the broader refactor to centralize platform type definitions.
18-36: LGTM: Platform type constant migration is correct.The migration from
Origin::CLIENT_TYPE_*toPlatform::TYPE_*constants is consistent with the centralization of platform types in the newPlatformclass.
39-50: LGTM: Scheme-based platform entries support React Native OAuth.The addition of scheme-based platform types (
Platform::TYPE_SCHEME) with keys 'exp' and 'appwrite-callback-123' aligns with the PR objective to support React Native OAuth development using theexp://scheme.
53-55: LGTM: Empty and root path validation tests.Adding tests for empty string and root path ('/') ensures proper validation of invalid origins.
76-81: LGTM: Comprehensive scheme validation tests.The tests cover various scheme formats (
exp://,exp:///,exp://index,appwrite-callback-123://) and properly validate both registered and unregistered schemes. The test forappwrite-callback-456://correctly expects failure for unregistered schemes.
83-96: LGTM: Platform-specific error message validation.The tests verify that appropriate error messages are returned for unregistered platform schemes (iOS, Android, macOS, Linux, Windows), ensuring good user experience with clear guidance.
app/controllers/api/vcs.php (2)
7-7: LGTM: Improved security with Redirect validator.The change from
HosttoRedirectvalidator provides better security for URL validation in OAuth flows, ensuring proper redirect URL validation rather than just hostname validation.
427-428: LGTM: Consistent parameter naming and validation.The update to use the
Redirectvalidator with 'platforms' parameter aligns with the broader refactor from client-based to platform-based validation. This ensures consistent validation across the codebase.app/controllers/api/projects.php (2)
11-11: LGTM: Import addition aligns with platform constants refactoring.The addition of the Platform class import is necessary for using the new
Platform::TYPE_*constants in the platform creation endpoint.
1793-1793: LGTM: Platform type constants successfully migrated.The change from
Origin::CLIENT_TYPE_*toPlatform::TYPE_*constants aligns with the refactoring to centralize platform-related logic in the Platform class. The validation logic remains intact while using the new constant source.tests/e2e/Services/Realtime/RealtimeBase.php (3)
24-30: LGTM: Improved WebSocket client header handling.Removing the automatic Origin header injection makes the method more flexible and aligns with the refactored origin validation approach using the Platform class. Tests now have better control over headers without hardcoded assumptions.
45-45: LGTM: Explicit empty channels parameter improves clarity.Explicitly passing an empty array for channels makes the test intent clearer and ensures the parameter is not omitted.
60-60: LGTM: Centralized WebSocket client creation improves consistency.Using the
getWebsocketmethod with theprojectIdparameter instead of directWebSocketClientinstantiation centralizes client creation and removes code duplication while maintaining the same test behavior.app/controllers/api/account.php (5)
24-24: LGTM: New Redirect validator importThe import of the new
Redirectvalidator aligns with the security improvements to prevent open redirect attacks.
1782-1783: Consistent implementation across OAuth2 endpointsThe OAuth2 token endpoints correctly implement the same conditional validation pattern as the session endpoints, maintaining security consistency across related functionality.
1863-1863: Proper security validation for magic URL redirectsThe magic URL endpoint correctly implements the conditional redirect validation, ensuring URLs are properly validated against the platform list in production while maintaining development flexibility.
3149-3149: Critical security improvement for password recoveryPassword recovery flows are particularly vulnerable to redirect attacks. The conditional validation ensures production deployments properly validate redirect URLs against the platform list, preventing potential account takeover attempts through malicious redirects.
3416-3416: Complete security coverage for authentication redirectsThe email verification endpoint properly implements the conditional redirect validation, completing the security improvements across all authentication-related redirect parameters. This ensures consistent protection against open redirect attacks throughout the authentication flow.
app/init/resources.php (1)
22-22: LGTM: Import updated to use new Platform class.The import statement correctly reflects the refactor from Origin to Platform class.
app/controllers/general.php (4)
14-14: LGTM: Platform class import added.Correct import statement for the new Platform utility class.
797-808: LGTM: Resource injection updated to use platforms.The function signature correctly updates from 'clients' to 'platforms' injection, maintaining consistency with the broader refactor.
947-948: LGTM: Hostname extraction uses Platform utility.The code correctly uses
Platform::getHostnames($platforms)instead of direct array access, centralizing platform handling logic.
1062-1066: LGTM: Improved origin validation with defensive programming.The validation logic correctly:
- Uses the new platforms array with the Origin validator
- Adds a defensive check for empty origins before validation
- Maintains the existing security validation flow
This is a good improvement that prevents potential edge cases.
src/Appwrite/Network/Validator/Origin.php (3)
11-25: LGTM: Constructor refactored to use Platform class.The refactor correctly:
- Updates properties to use typed arrays and nullable strings
- Accepts an array of platforms instead of hardcoded values
- Uses
Platform::getHostnames()andPlatform::getSchemes()for dynamic extractionThis centralizes platform logic and improves maintainability.
33-55: LGTM: Validation logic simplified and improved.The
isValidmethod is well-refactored:
- Properly initializes state variables
- Uses the new
parseSchememethod for robust scheme extraction- Validates schemes against the dynamic schemes array
- Falls back to hostname validation for HTTP/HTTPS schemes
The logic maintains security while adding flexibility for new platform types.
92-115: LGTM: Robust scheme parsing implementation.The
parseSchememethod is well-implemented:
- Handles edge cases like empty strings
- Uses
parse_urlas the primary method- Falls back to regex parsing for malformed URIs
- Follows RFC 3986 scheme naming conventions in the regex pattern
This provides robust scheme extraction that handles various URI formats.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Appwrite/Network/Platform.php (1)
100-116: Apply defensive programming practices for scheme handling.Based on the past review comments, consider adding
strtolower()and scheme validation to make the method more robust:public static function getSchemes(array $platforms): array { $schemes = []; foreach ($platforms as $platform) { $type = $platform['type'] ?? self::TYPE_UNKNOWN; - $scheme = $platform['key'] ?? ''; + $scheme = strtolower($platform['key'] ?? ''); switch ($type) { case self::TYPE_SCHEME: - $schemes[] = $scheme; + if (!empty($scheme) && preg_match('/^[a-z][a-z0-9+.-]*$/', $scheme)) { + $schemes[] = $scheme; + } break; default: break; } } return array_unique($schemes); }This addresses the concerns about case sensitivity and validates schemes according to RFC 3986.
🧹 Nitpick comments (1)
src/Appwrite/Network/Platform.php (1)
38-48: Consider adding missing platform schemes to the names mapping.The
$namesarray only includes scheme constants (HTTP, HTTPS, and platform-specific schemes) but doesn't cover some platform types that might need user-friendly names. Consider if schemes likeexpor custom callback schemes mentioned in the PR objectives should be included.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Appwrite/Network/Platform.php(1 hunks)src/Appwrite/Network/Validator/Redirect.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Appwrite/Network/Validator/Redirect.php
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (2)
src/Appwrite/Network/Platform.php (2)
7-34: Well-structured platform and scheme constants.The platform type and scheme constants are comprehensive and follow a consistent naming convention. The separation between platform types and schemes provides good clarity.
61-98: Good handling of TYPE_UNITY and empty value filtering.The
getHostnamesmethod correctly addresses the previous review concerns:
TYPE_UNITYis now explicitly handled in the switch statement (line 88)- Empty hostnames and keys are filtered out using
!empty()checksThe implementation is solid and handles all platform types appropriately.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/Appwrite/Network/Platform.php (1)
38-48: Consider extending the names array for custom schemes.The
$namesarray only includes predefined schemes but doesn't account for custom schemes that might be used withTYPE_SCHEME. This could limit the utility ofgetNameByScheme()for dynamically registered schemes.Consider adding a fallback mechanism or extending the array to handle custom schemes:
public static function getNameByScheme(?string $scheme): string { - return self::$names[$scheme] ?? ''; + $name = self::$names[$scheme] ?? ''; + if (empty($name) && !empty($scheme)) { + // For custom schemes, return a formatted version + return ucfirst(str_replace(['-', '_'], ' ', $scheme)); + } + return $name; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Network/Platform.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Linter
- GitHub Check: scan
🔇 Additional comments (4)
src/Appwrite/Network/Platform.php (4)
7-34: Well-structured platform and scheme constants.The platform type and scheme constants are comprehensive and follow a consistent naming convention. The separation between platform types and schemes provides good clarity.
61-98: Excellent improvement addressing previous review feedback.The
getHostnamesmethod now properly handlesTYPE_UNITY(line 88) and includes empty value validation for both hostnames and keys. The defensive use ofstrtolower()and comprehensive switch statement coverage are well implemented.
100-118: Robust scheme validation with proper regex.The
getSchemesmethod implements proper URI scheme validation using the regex/^[a-z][a-z0-9+.-]*$/which correctly enforces the RFC 3986 standard for scheme syntax. The defensive programming approach withstrtolower()and empty checks is consistent with the other methods.
1-119: Strong architectural improvement centralizing platform logic.This class successfully centralizes platform-related functionality that was previously scattered across the codebase. The design follows good PHP practices with appropriate constant visibility, static utility methods, and consistent error handling. The implementation addresses the stated PR objectives of supporting React Native OAuth development with
exp://scheme handling.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/Appwrite/Network/Platform.php (1)
38-48: Consider adding missing scheme mappings.The
$namesarray only includes predefined scheme constants but doesn't cover custom schemes that might be used withTYPE_SCHEME. Consider how custom schemes should be handled for user-friendly display.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Appwrite/Network/Platform.php(1 hunks)src/Appwrite/Network/Validator/Origin.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Appwrite/Network/Validator/Origin.php
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: scan-pr / osv-scan
- GitHub Check: scan
🔇 Additional comments (4)
src/Appwrite/Network/Platform.php (4)
7-23: Well-structured platform type constants.The platform type constants provide a clean, centralized definition of supported platforms. The naming convention is consistent and covers all major platforms comprehensively.
25-33: Comprehensive scheme constants definition.The scheme constants provide a good foundation for platform-specific URL scheme handling. The naming convention follows the platform hierarchy clearly.
61-98: Excellent defensive programming with proper validation.The method correctly handles the previous issues mentioned in past reviews - TYPE_UNITY is now explicitly included (line 88) and empty values are properly filtered out (lines 72, 89). The use of
strtolower()provides good defensive programming.
108-112: Robust scheme validation with proper regex pattern.The regex pattern
/^[a-z][a-z0-9+.-]*$/correctly validates URI scheme syntax according to RFC 3986. The combination with the empty check and lowercase conversion provides comprehensive validation.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Appwrite/Network/Platform.php (2)
116-119: Add HTTPS support for web platforms.As mentioned in a previous review comment, web platforms should support both HTTP and HTTPS schemes for complete coverage. Currently only HTTP is supported.
Apply this diff to include HTTPS support:
case self::TYPE_WEB: case self::TYPE_FLUTTER_WEB: $schemes[] = self::SCHEME_HTTP; + $schemes[] = self::SCHEME_HTTPS; break;
135-137: Document Unity platform scheme mapping rationale.Unity is mapped to
SCHEME_WINDOWSwhich may be confusing since Unity is a cross-platform game engine. This mapping decision should be documented to explain why Unity specifically uses the Windows scheme rather than having its own scheme or supporting multiple schemes.Consider adding a comment explaining this mapping decision or revisiting whether Unity should have its own scheme constant.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Network/Platform.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Unit Test
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (2)
src/Appwrite/Network/Platform.php (2)
112-114: Scheme validation regex looks correct.The regex pattern
/^[a-z][a-z0-9+.-]*$/correctly validates URI schemes according to RFC 3986, ensuring they start with a letter and contain only allowed characters.
90-94: Good fix for TYPE_UNITY handling.The explicit handling of
TYPE_UNITYin the switch statement addresses the previous review comment about the missing case. The empty key validation also prevents adding empty values to the hostnames array.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Appwrite/Network/Platform.php (1)
132-135: Unity platform scheme mapping may need reconsideration.Unity is mapped to
SCHEME_WINDOWS, but Unity is a cross-platform engine that can target multiple platforms. This mapping might be too restrictive for Unity applications targeting non-Windows platforms.Consider one of these approaches:
- Create a dedicated
SCHEME_UNITYconstant- Map Unity to multiple schemes based on target platform
- Document why Unity specifically maps to Windows scheme if intentional
🧹 Nitpick comments (1)
src/Appwrite/Network/Platform.php (1)
38-48: Consider adding scheme-to-name mappings for custom schemes.The current mapping only covers built-in schemes. For platforms using custom schemes (like React Native with
exp://or custom callback schemes), there's no user-friendly name mapping available.Consider extending the mapping to handle dynamic scheme names or provide a fallback mechanism:
private static array $names = [ self::SCHEME_HTTP => 'Web', self::SCHEME_HTTPS => 'Web', self::SCHEME_IOS => 'iOS', self::SCHEME_MACOS => 'macOS', self::SCHEME_WATCHOS => 'watchOS', self::SCHEME_TVOS => 'tvOS', self::SCHEME_ANDROID => 'Android', self::SCHEME_WINDOWS => 'Windows', self::SCHEME_LINUX => 'Linux', + 'exp' => 'Expo', ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/controllers/api/vcs.php(2 hunks)app/controllers/general.php(3 hunks)app/init/resources.php(6 hunks)src/Appwrite/Network/Platform.php(1 hunks)src/Appwrite/Network/Validator/Origin.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/controllers/api/vcs.php
- app/init/resources.php
- src/Appwrite/Network/Validator/Origin.php
- app/controllers/general.php
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (Users)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Unit Test
- GitHub Check: Benchmark
- GitHub Check: scan
🔇 Additional comments (5)
src/Appwrite/Network/Platform.php (5)
7-33: Platform constants are well-structured and comprehensive.The platform type and scheme constants provide good coverage of supported platforms. The naming convention is consistent and follows a logical pattern.
61-98: Hostname extraction logic correctly handles all platform types.The method properly handles different platform types, filters empty values, and includes the previously missing
TYPE_UNITYcase. The implementation addresses the concerns from past reviews.
109-109: Scheme validation regex is RFC 3986 compliant.The regex pattern
^[a-z][a-z0-9+.-]*$correctly validates URI schemes according to RFC 3986 standards, ensuring schemes start with a letter and contain only valid characters.
113-117: Web platforms correctly support both HTTP and HTTPS schemes.Good improvement from the previous version - both HTTP and HTTPS schemes are now included for web platforms, ensuring complete protocol coverage.
100-150: Scheme extraction method is well-implemented with proper validation.The method correctly handles different platform types, validates custom schemes with regex, and ensures uniqueness in the returned array. The logic is comprehensive and handles edge cases appropriately.
|
To facilitate React Native OAuth development, where App callback links are
exp://, I've addedexpandappwrite-callback-${projectId}origins by default.Platform::TYPE_SCHEMEhas been added.Originvalidator now permits any hostname if there is aPlatform::TYPE_SCHEMEmatch.Redirectvalidator extendsOriginvalidator, simply overriding the error mesage.