-
-
Notifications
You must be signed in to change notification settings - Fork 0
Update to support Pro level #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the verification system by replacing a single hash validator with a two-tiered validator approach (Supporter and Pro), updates the status enum from PLUS_EDITION to PRO_EDITION with a new SIGNATURE_EDITION option, introduces a new VerifyProStatus middleware, adds a console command for checking license keys, and includes configuration and test updates to reflect the new system. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks❌ Failed checks (1 warning)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
docs/README.md (1)
93-95: Update outdated method reference.The documentation references
is_plus(), but according to the AI summary, this method was removed in favor ofis_pro()andis_signature(). Update the example to reflect the current API.Apply this diff to update the documentation:
-// Check if the user has premium status -if ($verify->is_plus()) { - // Provide premium features +// Check if the user has pro status +if ($verify->is_pro()) { + // Provide pro features }src/Contract/Status.php (1)
6-6: Update comment to reflect four levels.The comment states "3 levels" but the enum now defines four cases: FREE_EDITION, SUPPORTER_EDITION, PRO_EDITION, and SIGNATURE_EDITION.
Apply this diff:
-/** - * Defines the 3 levels of installation of Lychee. - */ +/** + * Defines the 4 levels of installation of Lychee. + */src/Verify.php (2)
84-92: Update outdated docblock terminology.The docblock references "plus registered user" which is outdated terminology from the previous system.
/** - * Returns true if the user is a supporter (or plus registered user). + * Returns true if the user is a supporter (or pro/signature user). * * @return bool */ public function is_supporter(): bool
94-102: Update outdated docblock terminology.The docblock references "plus registered user" which should be "pro user".
/** - * Return true of the user is a plus registered user. + * Return true if the user is a pro user (or signature user). * * @return bool */ public function is_pro(): bool
🧹 Nitpick comments (5)
src/Console/Commands/CheckKeyCommand.php (1)
32-36: Unnecessary check for required argument.
hasArgument('key')will always returntruefor a required argument defined in the signature. Laravel ensures required arguments are present beforehandle()is called.public function handle(): int { - if (!$this->hasArgument('key')) { - $this->error('No key provided. Please provide a license key to check.'); - - return Command::FAILURE; - } - if (!is_string($this->argument('key'))) {tests/Verify/VerifyTest.php (4)
14-29: Good coverage of new PRO/SIGNATURE tiers in default flow; consider also assertingauthorize(Status::SIGNATURE_EDITION)throws.
This would fully exercise the new tieredauthorize()behavior alongside the new status checks.
31-50: Supporter-path assertions look consistent withVerify::check(); consider adding anauthorize(Status::SIGNATURE_EDITION)negative assertion too.
Right now this test only coversauthorize(Status::PRO_EDITION)failing.
52-69:testVerifyPro()is solid; consider addingauthorize(Status::PRO_EDITION)success +authorize(Status::SIGNATURE_EDITION)failure to lock policy in.
This would catch accidental privilege changes for Pro vs Signature.
89-111: Make the validation test fail clearer by asserting the config shape beforeforeach.
This avoids a noisy “invalid argument” warning if config is missing/mis-typed and improves diagnostics./** @var array<class-string,string> $checks */ $checks = config('verify.validation'); + self::assertIsArray($checks); + self::assertNotEmpty($checks); foreach ($checks as $class => $value) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
config/verify.php(1 hunks)docs/README.md(3 hunks)generate-key.php(1 hunks)phpstan.neon(1 hunks)src/Console/Commands/CheckKeyCommand.php(1 hunks)src/Contract/Status.php(1 hunks)src/Contract/VerifyInterface.php(1 hunks)src/Exceptions/BaseVerifyException.php(1 hunks)src/Exceptions/SupporterOnlyOperationException.php(1 hunks)src/Http/Middleware/VerifyProStatus.php(1 hunks)src/Validators/ValidatePro.php(1 hunks)src/Validators/ValidateSignature.php(1 hunks)src/Validators/ValidateSupporter.php(1 hunks)src/Verify.php(6 hunks)src/VerifyServiceProvider.php(3 hunks)tests/Constants.php(1 hunks)tests/Verify/Validators/ValidateProTest.php(1 hunks)tests/Verify/Validators/ValidateSupporterTest.php(1 hunks)tests/Verify/VerifyTest.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/Contract/VerifyInterface.php (1)
src/Verify.php (3)
is_supporter(89-92)is_pro(99-102)is_signature(109-112)
src/Validators/ValidateSignature.php (3)
src/Validators/ValidatePro.php (1)
grant(38-41)src/Validators/ValidateSupporter.php (1)
grant(38-41)src/Contract/ValidatorInterface.php (1)
grant(25-25)
tests/Verify/Validators/ValidateSupporterTest.php (1)
src/Validators/ValidateSupporter.php (2)
ValidateSupporter(12-42)validate(24-31)
src/Validators/ValidateSupporter.php (2)
src/Verify.php (1)
__construct(23-35)src/Validators/ValidatePro.php (1)
__construct(16-19)
src/Validators/ValidatePro.php (6)
src/Exceptions/SupporterOnlyOperationException.php (1)
__construct(12-20)src/Verify.php (3)
__construct(23-35)validate(154-177)check(68-82)src/Validators/ValidateSupporter.php (3)
__construct(16-19)validate(24-31)grant(38-41)src/Exceptions/BaseVerifyException.php (1)
__construct(54-62)src/Http/Middleware/VerifyProStatus.php (1)
__construct(19-22)src/Contract/VerifyInterface.php (2)
validate(76-76)check(23-23)
src/Console/Commands/CheckKeyCommand.php (4)
src/Validators/ValidatePro.php (2)
ValidatePro(12-42)validate(24-31)src/Validators/ValidateSupporter.php (2)
ValidateSupporter(12-42)validate(24-31)src/Contract/VerifyInterface.php (1)
validate(76-76)src/Verify.php (1)
validate(154-177)
src/VerifyServiceProvider.php (1)
src/Console/Commands/CheckKeyCommand.php (1)
CheckKeyCommand(11-74)
src/Verify.php (5)
src/Validators/ValidatePro.php (4)
ValidatePro(12-42)__construct(16-19)validate(24-31)grant(38-41)src/Validators/ValidateSignature.php (4)
ValidateSignature(16-60)__construct(20-23)validate(28-49)grant(56-59)src/Validators/ValidateSupporter.php (4)
ValidateSupporter(12-42)__construct(16-19)validate(24-31)grant(38-41)src/Contract/VerifyInterface.php (4)
validate(76-76)is_pro(37-37)check(23-23)is_signature(44-44)src/Contract/ValidatorInterface.php (2)
validate(18-18)grant(25-25)
tests/Verify/VerifyTest.php (3)
src/Contract/VerifyInterface.php (6)
check(23-23)is_supporter(30-30)is_pro(37-37)authorize(56-56)get_status(14-14)is_signature(44-44)src/Verify.php (7)
check(68-82)is_supporter(89-92)is_pro(99-102)authorize(124-129)Verify(15-178)get_status(42-59)is_signature(109-112)src/Exceptions/SupporterOnlyOperationException.php (1)
SupporterOnlyOperationException(7-21)
config/verify.php (7)
src/Http/Middleware/VerifyProStatus.php (1)
VerifyProStatus(15-45)src/Http/Middleware/VerifySupporterStatus.php (1)
VerifySupporterStatus(15-45)src/Validators/ValidatePro.php (1)
ValidatePro(12-42)src/Validators/ValidateSignature.php (1)
ValidateSignature(16-60)src/Validators/ValidateSupporter.php (1)
ValidateSupporter(12-42)src/Verify.php (1)
Verify(15-178)src/VerifyServiceProvider.php (1)
VerifyServiceProvider(8-36)
🪛 Gitleaks (8.30.0)
tests/Constants.php
[high] 14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (20)
src/Exceptions/BaseVerifyException.php (1)
63-63: EOF newline change is safe and improves file hygiene.
No functional impact; constructor/exception behavior remains unchanged.phpstan.neon (1)
5-13: Raising PHPStan strictness is good, but confirm it won’t break CI unexpectedly.
level: 9is a solid move; just ensure the repo’s pinned PHPStan version supports it and that the CI phpstan step runs this config and is green (or that new findings are intentionally handled via updates elsewhere /ignoreErrors).generate-key.php (2)
9-28: LGTM! Secure key generation implementation.The implementation correctly uses
random_int()for cryptographic security, providing approximately 129 bits of entropy (25 characters from a 36-character alphabet). The key format and character set are well-chosen for manual entry and visual distinction.
30-35: LGTM! Proper key hashing.The use of
PASSWORD_BCRYPTis appropriate for hashing license keys, and displaying both the plaintext key and hash is necessary for distribution and storage.tests/Constants.php (1)
13-14: LGTM! Test constants for PRO validation.The new constants mirror the existing HASH/HASH_KEY pattern and provide necessary test fixtures for PRO edition validation coverage.
src/Contract/Status.php (1)
10-13: LGTM! Status enum updated correctly.The new PRO_EDITION and SIGNATURE_EDITION cases appropriately replace PLUS_EDITION, establishing a clear hierarchy for the two-tiered premium validation system.
src/Exceptions/SupporterOnlyOperationException.php (1)
14-18: LGTM! Exception messages updated for new status levels.The match expression correctly maps SIGNATURE_EDITION and PRO_EDITION to appropriate user-facing strings, maintaining backward compatibility with the default case.
src/VerifyServiceProvider.php (2)
21-21: LGTM! Minor optimization using self:: for constant reference.The change from
static::CONFIGtoself::CONFIGuses early binding, which is appropriate for a path constant that doesn't require runtime resolution.
30-34: LGTM! Proper console command registration.The command registration follows Laravel conventions, correctly gating the registration behind the
runningInConsole()check.src/Validators/ValidateSupporter.php (3)
12-12: LGTM! Class renamed to reflect supporter tier.The rename from
ValidateHashtoValidateSupporterclarifies the validator's purpose in the two-tiered system.
38-40: LGTM! Returns correct status for supporter tier.The grant method appropriately returns
SUPPORTER_EDITIONfor hash-validated supporters.
16-19: The default hash value is intentional and follows proper design patterns.The hardcoded default hash is used when
ValidateSupporter()is instantiated without parameters (e.g., inCheckKeyCommand). The class properly supports dependency injection via the constructor parameter, allowing callers to override it when needed (as done inVerify.php). Theconfig/verify.phpfile contains validation checksums, not hash overrides. This design is appropriate: the embedded default serves as a fallback for command-line usage while allowing injection for other contexts.src/Validators/ValidateSignature.php (1)
56-58: LGTM! Returns correct status for signature validation.The grant method now appropriately returns
SIGNATURE_EDITIONfor cryptographically validated premium users.tests/Verify/Validators/ValidateSupporterTest.php (1)
7-24: LGTM!The test class is correctly renamed to match the new
ValidateSupportervalidator. Both test methods properly exercise the valid and invalid hash scenarios using the appropriate constants.src/Contract/VerifyInterface.php (1)
39-44: LGTM!The new
is_signature()method is properly defined with clear documentation, completing the three-tier status check API.tests/Verify/Validators/ValidateProTest.php (1)
1-26: LGTM!The test class properly mirrors the structure of
ValidateSupporterTestwhile using distinct constants (HASH2,HASH2_KEY) for pro-level validation. Both valid and invalid scenarios are covered.src/Console/Commands/CheckKeyCommand.php (1)
52-66: Validation order differs fromVerify::get_status().This command checks Pro first, then Supporter. However,
Verify::get_status()insrc/Verify.php(lines 46-52) checks Supporter first, then Pro. While this doesn't affect correctness (they use different hashes), the inconsistency could cause confusion when debugging.Please confirm this ordering difference is intentional, or align the order with
Verify::get_status()for consistency:- // Check Pro edition first (highest tier) - if ($validatePro->validate($verifiable, $key)) { + // Check Supporter edition first + if ($validateSupporter->validate($verifiable, $key)) { $this->info('Key: ' . $key); - $this->info('Sponsorship Level: ' . Status::PRO_EDITION->value . ' (Pro Edition)'); + $this->info('Sponsorship Level: ' . Status::SUPPORTER_EDITION->value . ' (Supporter Edition)'); return Command::SUCCESS; } - // Check Supporter edition - if ($validateSupporter->validate($verifiable, $key)) { + // Check Pro edition + if ($validatePro->validate($verifiable, $key)) { $this->info('Key: ' . $key); - $this->info('Sponsorship Level: ' . Status::SUPPORTER_EDITION->value . ' (Supporter Edition)'); + $this->info('Sponsorship Level: ' . Status::PRO_EDITION->value . ' (Pro Edition)'); return Command::SUCCESS; }src/Verify.php (2)
46-52: Verify validation order reflects intended tier hierarchy.The current order checks Supporter first, then Pro, then Signature. Since the method returns on first match, a license that matches the Supporter hash will never be checked against Pro or Signature validators.
Given the
check()method's hierarchy (Signature > Pro > Supporter), consider whether higher tiers should be checked first to avoid edge cases where a license could match multiple tiers.
76-81: LGTM!The
check()method correctly implements the tier hierarchy using a clean match expression. SIGNATURE_EDITION grants access to all tiers, PRO_EDITION grants access to Pro and Supporter features, and SUPPORTER_EDITION grants access only to Supporter features.config/verify.php (1)
12-20: Verify SHA1 hashes match file contents.The validation map has been updated with new classes and their corresponding hashes. Please ensure these SHA1 hashes were generated from the normalized file contents (with Unix line endings) as expected by
Verify::validate().#!/bin/bash # Verify SHA1 hashes match actual file contents echo "Checking ValidateSupporter..." cat src/Validators/ValidateSupporter.php | sed 's/\r$//' | sha1sum echo "Checking ValidatePro..." cat src/Validators/ValidatePro.php | sed 's/\r$//' | sha1sum echo "Checking ValidateSignature..." cat src/Validators/ValidateSignature.php | sed 's/\r$//' | sha1sum echo "Checking Verify..." cat src/Verify.php | sed 's/\r$//' | sha1sum echo "Checking VerifySupporterStatus..." cat src/Http/Middleware/VerifySupporterStatus.php | sed 's/\r$//' | sha1sum echo "Checking VerifyProStatus..." cat src/Http/Middleware/VerifyProStatus.php | sed 's/\r$//' | sha1sum echo "Checking VerifyServiceProvider..." cat src/VerifyServiceProvider.php | sed 's/\r$//' | sha1sum
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.