Add initial implementation of OAuth2 server components#875
Add initial implementation of OAuth2 server components#875huangdijia merged 13 commits intofriendsofhyperf:mainfrom
Conversation
## Walkthrough
本次变更为项目引入了完整的 OAuth2 服务器组件,包括依赖配置、数据库迁移、核心实体、模型、命令行工具、控制器、事件、工厂、仓储、管理器、中间件、值对象、枚举、转换器、监听器及相关测试。通过 composer.json 配置,集成了 league/oauth2-server 及相关依赖,并实现了与 Hyperf 框架的深度集成。
## Changes
| 文件/分组 | 变更摘要 |
|-------------------------------------------------------------------|------------------------------------------------------------------|
| composer.json, src/oauth2-server/composer.json | 添加 OAuth2 相关依赖、autoload 配置、包元数据及 Hyperf 配置注册 |
| src/oauth2-server/.gitattributes, LICENSE, README.md | 新增归档忽略、MIT 协议、README |
| src/oauth2-server/databases/..._create_oauth_server.php | 新增 OAuth2 相关表的数据库迁移脚本 |
| src/oauth2-server/publish/oauth2-server.php | 新增 OAuth2 服务器配置文件,包含密钥、授权、作用域等配置 |
| src/oauth2-server/src/ConfigProvider.php | 新增配置提供者,注册命令、依赖、发布资源 |
| src/oauth2-server/src/Controller/*.php | 新增授权与令牌控制器 |
| src/oauth2-server/src/Command/*.php | 新增清理、创建、删除、列出、更新客户端及生成密钥对等命令 |
| src/oauth2-server/src/Entity/*.php | 新增 OAuth2 实体类(Token、Client、User、Scope、DeviceCode 等) |
| src/oauth2-server/src/Enums/*.php | 新增加密类型、授权类型、设备码状态枚举 |
| src/oauth2-server/src/Event/*.php | 新增事件类(授权请求解析、Token 请求解析、Scope 解析、用户解析等)|
| src/oauth2-server/src/Factory/*.php | 新增授权服务器、资源服务器、配置工厂 |
| src/oauth2-server/src/Interfaces/*.php | 新增配置接口、安全接口 |
| src/oauth2-server/src/Listener/*.php | 新增启动监听器,自动配置迁移路径和作用域 |
| src/oauth2-server/src/Manager/*.php | 新增访问令牌、授权码、客户端、刷新令牌、作用域、设备码管理器接口及实现 |
| src/oauth2-server/src/Manager/EloquentORM/*.php | 新增基于 Eloquent ORM 的实体管理器实现 |
| src/oauth2-server/src/Manager/InMemory/ScopeManager.php | 新增内存作用域管理器实现 |
| src/oauth2-server/src/Middleware/ResourceServerMiddleware.php | 新增资源服务器中间件 |
| src/oauth2-server/src/Model/*.php | 新增 OAuth2 数据模型及接口、属性转换器 |
| src/oauth2-server/src/Repository/*.php | 新增 OAuth2 相关仓储实现 |
| src/oauth2-server/src/ValueObject/*.php | 新增 Grant、RedirectUri、Scope 等值对象 |
| src/oauth2-server/src/Converter/*.php | 新增客户端转换器、作用域转换器及接口 |
| tests/Oauth2Server/Command/*.php | 新增命令行相关单元测试 |
| tests/Oauth2Server/InMemoryScopeManagerTest.php | 新增内存作用域管理器测试 |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant 用户
participant 控制器
participant 授权服务器
participant 事件分发器
participant 仓储/管理器
participant 数据库
用户->>控制器: 发起授权/令牌请求
控制器->>授权服务器: 转发请求
授权服务器->>仓储/管理器: 校验客户端/用户/作用域
仓储/管理器->>数据库: 查询/保存实体
授权服务器->>事件分发器: 触发事件(如授权解析、Token 解析)
事件分发器-->>授权服务器: 监听器处理,可能修改响应
授权服务器->>控制器: 返回响应
控制器->>用户: 返回授权/令牌结果Poem
|
There was a problem hiding this comment.
Actionable comments posted: 40
♻️ Duplicate comments (1)
src/oauth2-server/src/Model/AuthorizationCode.php (1)
91-95: getClient 方法存在相同问题与
AccessToken模型中的相同问题,firstOrFail()可能抛出异常。建议采用一致的异常处理方式。
🧹 Nitpick comments (21)
src/oauth2-server/README.md (1)
1-2: 补充 README 的使用与配置指引当前 README 仅有标题,无法帮助使用者快速了解组件的安装、配置步骤及常见授权流程示例。建议至少补充以下内容:
- 安装方式 (
composer require …)- 数据库迁移与配置发布命令
- 授权码 / 密码 / 客户端凭证等典型流程示例
- 生成密钥对、创建客户端等 CLI 指令说明
- 与 Hyperf 框架集成的配置片段
这将极大提升可用性与社区贡献度。
src/oauth2-server/src/Interfaces/SecurityInterface.php (1)
14-18: 避免命名冲突,建议调整别名或接口命名文件同时存在
FriendsOfHyperf\Oauth2\Server\Model\UserInterface与League\OAuth2\Server\Entities\UserEntityInterface,这里将后者直接as UserInterface容易造成阅读与类型提示混淆。-use League\OAuth2\Server\Entities\UserEntityInterface as UserInterface; +use League\OAuth2\Server\Entities\UserEntityInterface as OAuthUserInterface; ... -interface SecurityInterface -{ - public function getUser(): ?UserInterface; -} +interface SecurityInterface +{ + public function getUser(): ?OAuthUserInterface; +}或考虑把当前接口重命名为
SecurityUserResolverInterface等更具语义的名称,以进一步降低歧义。src/oauth2-server/src/Event/AuthorizationRequestResolveEventFactory.php (1)
22-30: 考虑改进trait使用模式的清晰度当前实现将依赖项分配给来自trait的属性,这降低了代码的可读性和可维护性。建议在构造函数参数中添加可见性修饰符,或者考虑更明确的实现方式。
建议改进实现:
public function __construct( - SecurityInterface $security, - ClientManagerInterface $clientManager, - ScopeConverterInterface $scopeConverter + private SecurityInterface $security, + private ClientManagerInterface $clientManager, + private ScopeConverterInterface $scopeConverter ) { - $this->clientManager = $clientManager; - $this->scopeConverter = $scopeConverter; - $this->security = $security; }或者,如果trait需要特定的属性名称,请考虑添加文档说明这些属性的来源。
src/oauth2-server/src/Manager/InMemory/ScopeManager.php (1)
24-26: 考虑移除空的构造函数空的构造函数没有提供任何功能,可以考虑移除以简化代码。
- public function __construct() - { - }src/oauth2-server/src/Repository/UserRepository.php (1)
54-58: 简化条件逻辑当前的 null 检查和返回逻辑可以简化。
- if ($user === null) { - return null; - } - - return $user; + return $user;src/oauth2-server/publish/oauth2-server.php (2)
32-33: 修复注释中的截断 URL第 32 行的注释包含截断的 URL,这可能会误导开发者。
- # The value should be a valid interval: http://ph + # The value should be a valid interval: http://php.net/manual/en/dateinterval.construct.php#refsect1-dateinterval.construct-parameters
65-68: 添加默认作用域配置指导空的作用域配置可能需要更明确的文档说明如何配置。
'scopes' => [ # Scopes that you wish to utilize in your application. # This should be a simple array of strings. + # Example: ['read', 'write', 'admin'] 'available' => [], # Scopes that will be assigned when no scope given. # This should be a simple array of strings. + # Example: ['read'] 'default' => [], ],src/oauth2-server/src/Listener/LeagueOAuth2ServerListener.php (1)
38-42: 移除未使用的参数或添加使用说明
process方法声明了$event参数但未使用。虽然这是接口要求的,但建议添加注释说明或使用@SuppressWarnings注解来抑制静态分析警告。-public function process(object $event): void +/** + * @SuppressWarnings(PHPMD.UnusedFormalParameter) + */ +public function process(object $event): voidsrc/oauth2-server/src/Event/AuthorizationRequestResolveEventFactoryTrait.php (1)
20-23:@internal注释与实际使用不一致此 trait 标记为
@internal,但被公共类AuthorizationRequestResolveEventFactory使用。这可能会造成混淆。建议移除
@internal注释,或将使用此 trait 的工厂类也标记为 internal。src/oauth2-server/src/Controller/TokenController.php (1)
29-29: 考虑使用更描述性的方法名方法名
index对于OAuth2令牌端点来说不够描述性。建议使用更明确的名称,如issueToken或handleTokenRequest。src/oauth2-server/src/Command/ListClientsCommand.php (1)
90-98: 可以优化数组映射逻辑当前的数组映射使用静态匿名函数,可以简化为更简洁的形式。
- ->addGrantCriteria(...array_map(static function (string $grant): Grant { - return new Grant($grant); - }, $grantStrings)) - ->addRedirectUriCriteria(...array_map(static function (string $redirectUri): RedirectUri { - return new RedirectUri($redirectUri); - }, $redirectUriStrings)) - ->addScopeCriteria(...array_map(static function (string $scope): Scope { - return new Scope($scope); - }, $scopeStrings)); + ->addGrantCriteria(...array_map(Grant::class, $grantStrings)) + ->addRedirectUriCriteria(...array_map(RedirectUri::class, $redirectUriStrings)) + ->addScopeCriteria(...array_map(Scope::class, $scopeStrings));src/oauth2-server/src/Entity/Client.php (1)
33-36: 数组参数类型注释不一致PHPDoc 注释声明参数为
string[],但方法参数类型为array。建议保持一致性。/** - * @param string[] $redirectUri + * @param array<string> $redirectUri */ public function setRedirectUri(array $redirectUri): voidsrc/oauth2-server/src/Factory/ResourceServerFactory.php (1)
38-38: 可能的容器污染将
BearerTokenValidator实例设置回容器中可能会导致意外的副作用,特别是在多次调用工厂方法时。- $this->container->set(BearerTokenValidator::class, $bearerTokenValidator); + // 移除容器设置,让容器管理实例生命周期src/oauth2-server/src/ConfigProvider.php (1)
88-88: 修正注释中的拼写错误将 "factroy" 改为 "factory"。
-// factroy +// factorysrc/oauth2-server/src/Command/CreateClientCommand.php (1)
119-119: 考虑使用更安全的标识符生成方法使用 MD5 哈希生成客户端标识符可能不是最佳选择。建议使用更现代的方法。
-$identifier = (string) $input->getArgument('identifier') ?: hash('md5', random_bytes(16)); +$identifier = (string) $input->getArgument('identifier') ?: bin2hex(random_bytes(16));src/oauth2-server/src/Command/UpdateClientCommand.php (1)
105-125: 集合更新逻辑实现巧妙!通过过滤和合并的方式处理添加/删除操作,验证了冲突的值。第 121 行的注释应该是
@var。- /* @var list<T> */ + /** @var list<T> */src/oauth2-server/src/Model/ClientInterface.php (1)
60-60: 考虑将工厂方法移至独立的工厂类在接口中定义实例工厂方法
newClientInstance不太常见。建议考虑将此方法移至专门的工厂类或使用静态工厂方法。src/oauth2-server/src/Repository/ScopeRepository.php (2)
55-77: finalizeScopes 方法需要处理未使用的参数方法实现整体正确,但是
$authCodeId参数未被使用。如果该参数是为了符合接口定义,建议添加注释说明;如果确实不需要,可以考虑移除。public function finalizeScopes( array $scopes, string $grantType, ClientEntityInterface $clientEntity, - string|int|null $userIdentifier = null, - ?string $authCodeId = null, + string|int|null $userIdentifier = null, + ?string $authCodeId = null, // 参数保留以符合接口定义 ): array {
85-110: setupScopes 方法逻辑正确但可以优化方法正确实现了 scope 验证逻辑,但可以考虑性能优化。当前实现在每次循环中都调用
array_map和in_array,可以预先计算。private function setupScopes(ClientInterface $client, array $requestedScopes): array { $clientScopes = $client->getScopes(); if (empty($clientScopes)) { return $requestedScopes; } if (empty($requestedScopes)) { return $clientScopes; } + $clientScopesAsStrings = array_map('strval', $clientScopes); $finalizedScopes = []; - $clientScopesAsStrings = array_map('strval', $clientScopes); foreach ($requestedScopes as $requestedScope) { $requestedScopeAsString = (string) $requestedScope; if (! \in_array($requestedScopeAsString, $clientScopesAsStrings, true)) { throw OAuthServerException::invalidScope($requestedScopeAsString); } $finalizedScopes[] = $requestedScope; } return $finalizedScopes; }src/oauth2-server/src/Event/AuthorizationRequestResolveEvent.php (1)
91-94: 方法名称可能引起混淆
isAuthorizationApproved()方法直接委托给$authorizationRequest,但这可能与当前事件的$authorizationResolution状态不一致。建议明确区分原始请求状态和事件解析状态。考虑重命名方法或添加文档说明:
+ /** + * Returns the original authorization approval status from the request. + * This may differ from the current event's authorization resolution. + */ public function isAuthorizationApproved(): bool { return $this->authorizationRequest->isAuthorizationApproved(); }src/oauth2-server/src/Factory/AuthorizationServerFactory.php (1)
86-97: 潜在的容器覆盖问题在启用授权码授权时,代码先使用
make()创建实例,然后使用set()将其存储回容器。这种模式在其他授权类型(如隐式授权)中也有使用,但对于密码授权和客户端凭据授权却没有。建议保持一致性。考虑统一处理所有授权类型的实例化和容器注册方式,或者在注释中解释为什么某些授权类型需要特殊处理。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (72)
composer.json(5 hunks)src/oauth2-server/.gitattributes(1 hunks)src/oauth2-server/LICENSE(1 hunks)src/oauth2-server/README.md(1 hunks)src/oauth2-server/composer.json(1 hunks)src/oauth2-server/databases/2025_07_15_044853_create_oauth_server.php(1 hunks)src/oauth2-server/publish/oauth2-server.php(1 hunks)src/oauth2-server/src/Command/ClearExpiredTokensCommand.php(1 hunks)src/oauth2-server/src/Command/CreateClientCommand.php(1 hunks)src/oauth2-server/src/Command/DeleteClientCommand.php(1 hunks)src/oauth2-server/src/Command/GenerateKeyPairCommand.php(1 hunks)src/oauth2-server/src/Command/ListClientsCommand.php(1 hunks)src/oauth2-server/src/Command/UpdateClientCommand.php(1 hunks)src/oauth2-server/src/ConfigProvider.php(1 hunks)src/oauth2-server/src/Controller/AuthorizationController.php(1 hunks)src/oauth2-server/src/Controller/TokenController.php(1 hunks)src/oauth2-server/src/Converter/ScopeConverter.php(1 hunks)src/oauth2-server/src/Converter/ScopeConverterInterface.php(1 hunks)src/oauth2-server/src/Entity/AccessToken.php(1 hunks)src/oauth2-server/src/Entity/AuthCode.php(1 hunks)src/oauth2-server/src/Entity/Client.php(1 hunks)src/oauth2-server/src/Entity/RefreshToken.php(1 hunks)src/oauth2-server/src/Entity/Scope.php(1 hunks)src/oauth2-server/src/Entity/User.php(1 hunks)src/oauth2-server/src/Enums/EncryptionKeyType.php(1 hunks)src/oauth2-server/src/Enums/OAuth2Grants.php(1 hunks)src/oauth2-server/src/Event/AuthorizationRequestResolveEvent.php(1 hunks)src/oauth2-server/src/Event/AuthorizationRequestResolveEventFactory.php(1 hunks)src/oauth2-server/src/Event/AuthorizationRequestResolveEventFactoryTrait.php(1 hunks)src/oauth2-server/src/Event/PreSaveClientEvent.php(1 hunks)src/oauth2-server/src/Event/ScopeResolveEvent.php(1 hunks)src/oauth2-server/src/Event/TokenRequestResolveEvent.php(1 hunks)src/oauth2-server/src/Event/UserResolveEvent.php(1 hunks)src/oauth2-server/src/Factory/AuthorizationServerFactory.php(1 hunks)src/oauth2-server/src/Factory/ConfigFactory.php(1 hunks)src/oauth2-server/src/Factory/ResourceServerFactory.php(1 hunks)src/oauth2-server/src/Interfaces/ConfigInterface.php(1 hunks)src/oauth2-server/src/Interfaces/SecurityInterface.php(1 hunks)src/oauth2-server/src/Listener/LeagueOAuth2ServerListener.php(1 hunks)src/oauth2-server/src/Manager/AccessTokenManagerInterface.php(1 hunks)src/oauth2-server/src/Manager/AuthorizationCodeManagerInterface.php(1 hunks)src/oauth2-server/src/Manager/ClientFilter.php(1 hunks)src/oauth2-server/src/Manager/ClientManagerInterface.php(1 hunks)src/oauth2-server/src/Manager/ElquentORM/AccessTokenManager.php(1 hunks)src/oauth2-server/src/Manager/ElquentORM/AuthorizationCodeManager.php(1 hunks)src/oauth2-server/src/Manager/ElquentORM/ClientManager.php(1 hunks)src/oauth2-server/src/Manager/ElquentORM/RefreshTokenManager.php(1 hunks)src/oauth2-server/src/Manager/InMemory/ScopeManager.php(1 hunks)src/oauth2-server/src/Manager/RefreshTokenManagerInterface.php(1 hunks)src/oauth2-server/src/Manager/ScopeManagerInterface.php(1 hunks)src/oauth2-server/src/Middleware/ResourceServerMiddleware.php(1 hunks)src/oauth2-server/src/Model/AccessToken.php(1 hunks)src/oauth2-server/src/Model/AccessTokenInterface.php(1 hunks)src/oauth2-server/src/Model/AuthorizationCode.php(1 hunks)src/oauth2-server/src/Model/AuthorizationCodeInterface.php(1 hunks)src/oauth2-server/src/Model/Casts/GrantsCast.php(1 hunks)src/oauth2-server/src/Model/Casts/RedirectsCast.php(1 hunks)src/oauth2-server/src/Model/Casts/ScopesCast.php(1 hunks)src/oauth2-server/src/Model/Client.php(1 hunks)src/oauth2-server/src/Model/ClientInterface.php(1 hunks)src/oauth2-server/src/Model/RefreshToken.php(1 hunks)src/oauth2-server/src/Model/RefreshTokenInterface.php(1 hunks)src/oauth2-server/src/Model/UserInterface.php(1 hunks)src/oauth2-server/src/Repository/AccessTokenRepository.php(1 hunks)src/oauth2-server/src/Repository/AuthCodeRepository.php(1 hunks)src/oauth2-server/src/Repository/ClientRepository.php(1 hunks)src/oauth2-server/src/Repository/RefreshTokenRepository.php(1 hunks)src/oauth2-server/src/Repository/ScopeRepository.php(1 hunks)src/oauth2-server/src/Repository/UserRepository.php(1 hunks)src/oauth2-server/src/ValueObject/Grant.php(1 hunks)src/oauth2-server/src/ValueObject/RedirectUri.php(1 hunks)src/oauth2-server/src/ValueObject/Scope.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (19)
src/oauth2-server/src/Entity/Scope.php (1)
src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)
src/oauth2-server/src/ValueObject/Scope.php (4)
src/oauth2-server/src/Entity/Scope.php (1)
Scope(18-22)src/oauth2-server/src/Model/AccessTokenInterface.php (1)
__toString(19-19)src/oauth2-server/src/ValueObject/Grant.php (1)
__toString(21-24)src/oauth2-server/src/ValueObject/RedirectUri.php (1)
__toString(26-29)
src/oauth2-server/src/Manager/ScopeManagerInterface.php (6)
src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Manager/AccessTokenManagerInterface.php (2)
find(18-18)save(20-20)src/oauth2-server/src/Manager/AuthorizationCodeManagerInterface.php (2)
find(18-18)save(20-20)src/oauth2-server/src/Manager/ClientManagerInterface.php (2)
find(22-22)save(18-18)src/oauth2-server/src/Manager/InMemory/ScopeManager.php (2)
find(28-31)save(33-36)src/oauth2-server/src/Manager/RefreshTokenManagerInterface.php (2)
find(18-18)save(20-20)
src/oauth2-server/src/Interfaces/ConfigInterface.php (1)
src/oauth2-server/src/Factory/ConfigFactory.php (1)
get(24-27)
src/oauth2-server/src/Listener/LeagueOAuth2ServerListener.php (6)
src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Factory/ConfigFactory.php (2)
__construct(19-22)get(24-27)src/oauth2-server/src/Interfaces/ConfigInterface.php (1)
get(16-16)src/oauth2-server/src/Manager/ElquentORM/AuthorizationCodeManager.php (1)
save(26-29)src/oauth2-server/src/Manager/InMemory/ScopeManager.php (1)
save(33-36)src/oauth2-server/src/Manager/ScopeManagerInterface.php (1)
save(20-20)
src/oauth2-server/src/Manager/ClientManagerInterface.php (2)
src/oauth2-server/src/Manager/ElquentORM/ClientManager.php (4)
save(31-37)remove(39-42)find(44-47)list(52-66)src/oauth2-server/src/Manager/ClientFilter.php (1)
ClientFilter(18-98)
src/oauth2-server/src/Manager/ElquentORM/RefreshTokenManager.php (5)
src/oauth2-server/src/Model/RefreshToken.php (1)
RefreshToken(29-83)src/oauth2-server/src/Manager/ElquentORM/AccessTokenManager.php (3)
find(27-33)save(35-41)clearExpired(43-51)src/oauth2-server/src/Manager/ElquentORM/AuthorizationCodeManager.php (3)
find(21-24)save(26-29)clearExpired(31-36)src/oauth2-server/src/Manager/RefreshTokenManagerInterface.php (3)
find(18-18)save(20-20)clearExpired(22-22)src/helpers/src/Functions.php (1)
now(371-374)
src/oauth2-server/src/Model/UserInterface.php (8)
src/oauth2-server/src/Model/AccessTokenInterface.php (1)
getIdentifier(21-21)src/oauth2-server/src/Model/AuthorizationCode.php (1)
getIdentifier(76-79)src/oauth2-server/src/Model/AuthorizationCodeInterface.php (1)
getIdentifier(21-21)src/oauth2-server/src/Model/AccessToken.php (1)
getIdentifier(71-74)src/oauth2-server/src/Model/RefreshToken.php (1)
getIdentifier(57-60)src/oauth2-server/src/Model/Client.php (1)
getIdentifier(59-62)src/oauth2-server/src/Model/RefreshTokenInterface.php (1)
getIdentifier(20-20)src/oauth2-server/src/Model/ClientInterface.php (1)
getIdentifier(23-23)
src/oauth2-server/databases/2025_07_15_044853_create_oauth_server.php (2)
src/oauth2-server/src/Controller/AuthorizationController.php (1)
index(32-56)src/oauth2-server/src/Controller/TokenController.php (1)
index(29-43)
src/oauth2-server/src/Converter/ScopeConverterInterface.php (3)
src/oauth2-server/src/Entity/Scope.php (1)
Scope(18-22)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Converter/ScopeConverter.php (4)
toDomain(20-23)toDomainArray(30-35)toLeague(37-43)toLeagueArray(50-55)
src/oauth2-server/src/Model/AccessTokenInterface.php (8)
src/oauth2-server/src/ValueObject/Scope.php (2)
Scope(14-25)__toString(21-24)src/oauth2-server/src/Model/AuthorizationCode.php (7)
__toString(66-69)getIdentifier(76-79)getUserIdentifier(86-89)getClient(91-95)getScopes(97-100)isRevoked(102-105)revoke(107-112)src/oauth2-server/src/Model/AuthorizationCodeInterface.php (7)
__toString(19-19)getIdentifier(21-21)getUserIdentifier(25-25)getClient(27-27)getScopes(32-32)isRevoked(34-34)revoke(36-36)src/oauth2-server/src/Model/AccessToken.php (8)
__toString(61-64)getIdentifier(71-74)getExpiry(76-79)getUserIdentifier(81-84)getClient(86-90)getScopes(92-95)isRevoked(97-100)revoke(102-107)src/oauth2-server/src/Model/RefreshToken.php (5)
__toString(47-50)getIdentifier(57-60)getExpiry(62-65)isRevoked(72-75)revoke(77-82)src/oauth2-server/src/Model/RefreshTokenInterface.php (5)
__toString(18-18)getIdentifier(20-20)getExpiry(22-22)isRevoked(26-26)revoke(28-28)src/oauth2-server/src/Model/ClientInterface.php (2)
getIdentifier(23-23)getScopes(46-46)src/oauth2-server/src/Model/UserInterface.php (1)
getIdentifier(16-16)
src/oauth2-server/src/Manager/ClientFilter.php (5)
src/oauth2-server/src/ValueObject/Grant.php (1)
Grant(14-25)src/oauth2-server/src/ValueObject/RedirectUri.php (1)
RedirectUri(16-30)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Model/Client.php (3)
getGrants(85-88)getRedirectUris(74-77)getScopes(96-99)src/oauth2-server/src/Model/ClientInterface.php (3)
getGrants(39-39)getRedirectUris(32-32)getScopes(46-46)
src/oauth2-server/src/Converter/ScopeConverter.php (3)
src/oauth2-server/src/Entity/Scope.php (1)
Scope(18-22)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Converter/ScopeConverterInterface.php (4)
toDomain(20-20)toDomainArray(27-27)toLeague(29-29)toLeagueArray(36-36)
src/oauth2-server/src/Manager/ElquentORM/AccessTokenManager.php (3)
src/oauth2-server/src/Model/AccessToken.php (1)
AccessToken(33-108)src/helpers/src/Functions.php (1)
now(371-374)src/oauth2-server/src/Interfaces/ConfigInterface.php (1)
get(16-16)
src/oauth2-server/src/Model/RefreshTokenInterface.php (3)
src/oauth2-server/src/Model/AccessTokenInterface.php (5)
__toString(19-19)getIdentifier(21-21)getExpiry(23-23)isRevoked(34-34)revoke(36-36)src/oauth2-server/src/Model/AuthorizationCodeInterface.php (4)
__toString(19-19)getIdentifier(21-21)isRevoked(34-34)revoke(36-36)src/oauth2-server/src/Model/RefreshToken.php (6)
__toString(47-50)getIdentifier(57-60)getExpiry(62-65)getAccessToken(67-70)isRevoked(72-75)revoke(77-82)
src/oauth2-server/src/Manager/ElquentORM/ClientManager.php (10)
src/oauth2-server/src/Event/PreSaveClientEvent.php (3)
PreSaveClientEvent(17-33)__construct(19-22)getClient(24-27)src/oauth2-server/src/Manager/ClientFilter.php (5)
ClientFilter(18-98)hasFilters(91-97)getGrants(70-73)getRedirectUris(78-81)getScopes(86-89)src/oauth2-server/src/Model/Client.php (4)
Client(36-142)getGrants(85-88)getRedirectUris(74-77)getScopes(96-99)src/oauth2-server/src/ValueObject/Grant.php (1)
Grant(14-25)src/oauth2-server/src/ValueObject/RedirectUri.php (1)
RedirectUri(16-30)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/compoships/src/Database/Query/Builder.php (1)
whereIn(18-38)src/oauth2-server/src/Model/Casts/GrantsCast.php (1)
get(20-31)src/oauth2-server/src/Model/Casts/RedirectsCast.php (1)
get(20-31)src/oauth2-server/src/Model/Casts/ScopesCast.php (1)
get(20-31)
src/oauth2-server/src/Repository/RefreshTokenRepository.php (6)
src/oauth2-server/src/Entity/RefreshToken.php (1)
RefreshToken(18-22)src/oauth2-server/src/Model/RefreshToken.php (6)
RefreshToken(29-83)getIdentifier(57-60)revoke(77-82)isRevoked(72-75)accessToken(52-55)getAccessToken(67-70)src/oauth2-server/src/Manager/AccessTokenManagerInterface.php (2)
find(18-18)save(20-20)src/oauth2-server/src/Manager/RefreshTokenManagerInterface.php (2)
find(18-18)save(20-20)src/oauth2-server/src/Model/AccessTokenInterface.php (3)
getIdentifier(21-21)revoke(36-36)isRevoked(34-34)src/oauth2-server/src/Model/RefreshTokenInterface.php (4)
getIdentifier(20-20)revoke(28-28)isRevoked(26-26)getAccessToken(24-24)
src/oauth2-server/src/Event/AuthorizationRequestResolveEvent.php (6)
src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Controller/AuthorizationController.php (1)
__construct(24-30)src/oauth2-server/src/Event/TokenRequestResolveEvent.php (3)
__construct(19-22)getResponse(24-27)setResponse(29-32)src/oauth2-server/src/Event/ScopeResolveEvent.php (3)
__construct(20-26)getClient(43-46)getScopes(28-31)src/oauth2-server/src/Interfaces/SecurityInterface.php (1)
getUser(18-18)src/oauth2-server/src/Model/ClientInterface.php (1)
getScopes(46-46)
src/oauth2-server/src/Factory/AuthorizationServerFactory.php (4)
src/oauth2-server/src/Factory/ConfigFactory.php (2)
__construct(19-22)get(24-27)src/oauth2-server/src/Factory/ResourceServerFactory.php (2)
__construct(22-27)__invoke(29-44)src/oauth2-server/src/ConfigProvider.php (1)
__invoke(58-102)src/oauth2-server/src/Interfaces/ConfigInterface.php (1)
get(16-16)
🪛 PHPMD (2.15.0)
src/oauth2-server/src/Listener/LeagueOAuth2ServerListener.php
38-38: Avoid unused parameters such as '$event'. (Unused Code Rules)
(UnusedFormalParameter)
src/oauth2-server/src/Model/Casts/ScopesCast.php
20-20: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
src/oauth2-server/src/Model/Casts/GrantsCast.php
20-20: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
src/oauth2-server/src/Model/Casts/RedirectsCast.php
20-20: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
src/oauth2-server/src/Repository/ScopeRepository.php
60-60: Avoid unused parameters such as '$authCodeId'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (61)
src/oauth2-server/.gitattributes (1)
1-4: 慎重忽略 tests 目录以避免发布包缺少示例将
tests目录标记为export-ignore会导致通过composer install --prefer-dist获得的发布包缺少测试用例与示例,降低二次开发者参考价值。若体积并不大,可考虑仅忽略大型 fixture,而保留核心测试文件。src/oauth2-server/LICENSE (1)
3-4: 确认版权年份与主体信息的一致性许可证中的年份为 2023,而本 PR 创建于 2025,且作者署名
D.J.Hwang与仓库所有者zds-s不一致。请确认:
- 年份是否需要更新为当前年份或范围(2023-2025)。
- 署名是否应为本项目实际版权主体,避免潜在法律歧义。
src/oauth2-server/src/Enums/EncryptionKeyType.php (1)
14-18: 补充注释说明各枚举含义并考虑命名风格统一
- 建议在每个 case 上方增加一行注释,说明
plain与defuse两种模式的适用场景,便于 IDE 悬浮提示。- 项目其它枚举(若有)若使用
UPPER_SNAKE命名风格,可保持一致性,如PLAIN、DEFUSE,以提升可读性。
[ suggest_nitpick ]src/oauth2-server/src/Model/UserInterface.php (1)
14-17: 接口设计一致性良好 👍方法签名与其它实体接口 (
AccessTokenInterface等) 保持一致,方便在仓库或服务中做统一处理。src/oauth2-server/src/Entity/User.php (1)
14-20: 实现简洁,符合 League 接口要求类只需要
EntityTrait即可满足UserEntityInterface的identifier要求,设计合理。src/oauth2-server/src/Entity/AuthCode.php (1)
14-24: 实现符合 OAuth2 规范组合三大 trait 已涵盖 AuthCode 所需字段与行为,代码整洁。
src/oauth2-server/src/Entity/Scope.php (1)
18-22: 实现正确,遵循了 OAuth2 服务器实体的最佳实践。该类正确实现了
ScopeEntityInterface接口并使用了合适的 traits,结构简洁明了。使用final关键字也是实体类的好习惯。src/oauth2-server/src/Entity/RefreshToken.php (1)
18-22: 实现正确,与其他实体类保持一致。该类遵循了与 Scope 实体相同的实现模式,正确使用了
RefreshTokenEntityInterface和相关的 traits。src/oauth2-server/src/Interfaces/ConfigInterface.php (1)
14-17: 接口设计简洁明了,符合单一职责原则。该接口为配置检索提供了清晰的抽象,方法签名合理,支持默认值回退机制。
src/oauth2-server/src/Manager/ScopeManagerInterface.php (1)
16-21: 接口设计合理,遵循了一致的管理器模式。该接口提供了标准的 CRUD 操作方法,使用 ValueObject\Scope 用于业务逻辑层是合适的选择,与其他管理器接口保持一致。
src/oauth2-server/src/Entity/AccessToken.php (1)
19-24: 实现正确,适当使用了访问令牌特有的 traits。该类正确实现了
AccessTokenEntityInterface接口,合理使用了三个 traits,其中TokenEntityTrait为访问令牌提供了特定的令牌功能。src/oauth2-server/src/Event/TokenRequestResolveEvent.php (1)
17-33: 实现看起来很好!这个事件类设计合理,遵循了标准的事件模式。使用
final关键字和严格的类型声明是好的实践。getter/setter模式允许监听器修改响应,这对于OAuth2令牌请求处理流程很有用。src/oauth2-server/src/Factory/ConfigFactory.php (1)
17-28: 配置工厂实现优秀!这个工厂类采用了清晰的适配器模式,通过自动添加
'oauth2-server.'前缀来简化OAuth2配置的访问。使用readonly属性和构造函数注入遵循了现代PHP最佳实践。src/oauth2-server/src/Event/PreSaveClientEvent.php (1)
17-33: 事件类实现规范!这个客户端预保存事件类的设计与其他事件类保持一致,使用了合适的封装和类型安全。允许在保存客户端之前进行修改的设计对于实现客户端管理生命周期的钩子很有用。
src/oauth2-server/src/Manager/RefreshTokenManagerInterface.php (1)
16-23: 接口设计良好,符合标准的管理器模式。该接口定义了刷新令牌管理的核心操作,方法签名清晰,返回类型恰当。
src/oauth2-server/src/Enums/OAuth2Grants.php (1)
14-40: 枚举实现规范,完全符合 OAuth2 标准。每个授权类型都有对应的 RFC 文档链接,便于理解和维护。字符串类型的枚举值与 OAuth2 规范完全一致。
src/oauth2-server/src/Manager/AccessTokenManagerInterface.php (1)
16-23: 接口设计与其他管理器接口保持一致。遵循了相同的方法模式,有利于代码的统一性和可维护性。
src/oauth2-server/src/Manager/AuthorizationCodeManagerInterface.php (1)
16-23: 接口设计延续了一致的管理器模式。与访问令牌和刷新令牌管理器接口保持相同的方法结构,确保了 API 的一致性。
src/oauth2-server/src/Model/Casts/GrantsCast.php (1)
20-20: 静态分析的未使用参数警告是误报。这些参数是由 CastsAttributes 接口要求的,即使在当前实现中未使用也不能移除。
Also applies to: 33-33
src/oauth2-server/src/Event/ScopeResolveEvent.php (1)
18-52: 实现正确,设计合理事件类的设计很好:
- 正确使用
readonly属性用于不可变的上下文信息- 允许修改
scopes数组以支持事件处理过程中的作用域解析- 类型声明完整且准确
src/oauth2-server/src/Model/Casts/ScopesCast.php (1)
18-42: 类型转换实现正确
ScopesCast类正确实现了作用域数据的双向转换:
get方法正确处理 JSON 字符串和数组输入set方法正确将 Scope 对象转换为字符串并编码为 JSON- 静态分析关于未使用参数的警告是误报,这些参数是
CastsAttributes接口的必需部分src/oauth2-server/src/Model/Casts/RedirectsCast.php (1)
18-42: 重定向 URI 类型转换实现正确
RedirectsCast类正确实现了重定向 URI 数据的双向转换:
get方法正确处理 JSON 字符串和数组输入set方法将单个RedirectUri包装在数组中,这确保了存储格式的一致性- 静态分析关于未使用参数的警告是误报,这些参数是接口契约的必需部分
src/oauth2-server/composer.json (2)
1-52: 整体包配置结构良好包配置结构完整,包含了必要的元数据、依赖项和自动加载配置。作者信息、支持链接和许可证信息都已正确配置。
29-29: 确认 league/oauth2-server 版本约束经核实,截至 2025 年 7 月中旬,league/oauth2-server 最新稳定版本为 9.2.0,composer.json 中使用的
"^9.2"约束已涵盖该版本(>=9.2.0 <10.0.0),无需做出调整。src/oauth2-server/src/Command/DeleteClientCommand.php (1)
19-27: 命令类实现良好命令类结构清晰,使用了适当的依赖注入和命名规范。构造函数正确调用了父类构造函数。
src/oauth2-server/src/Repository/UserRepository.php (1)
23-29: 依赖注入实现良好使用 readonly 属性和构造函数注入,符合现代 PHP 最佳实践。
src/oauth2-server/src/Event/UserResolveEvent.php (2)
23-29: 构造函数实现良好使用 readonly 属性进行依赖注入,确保了事件数据的不可变性,符合事件对象的最佳实践。
31-49: getter 方法实现良好所有 getter 方法都正确返回了相应的属性值,方法命名清晰明确。
src/oauth2-server/publish/oauth2-server.php (4)
48-48: 考虑隐式授权的安全性隐式授权 (implicit grant) 被禁用是一个好的安全实践,因为它存在已知的安全漏洞。
20-22: 敏感信息使用环境变量私钥和密码短语正确地从环境变量中获取,这是处理敏感信息的最佳实践。
33-39: 令牌生命周期设置合理访问令牌 1 小时,授权代码 10 分钟,刷新令牌 1 个月的生命周期设置是合理的默认值。
25-27: 验证 EncryptionKeyType 枚举支持的值已在 src/oauth2-server/src/Enums/EncryptionKeyType.php 中确认枚举定义如下:
- case Plain = 'plain'
- case Defuse = 'defuse'
默认值
'plain'和备用值'defuse'均已正确支持,无需额外修改。src/oauth2-server/src/Listener/LeagueOAuth2ServerListener.php (1)
44-54: 实现正确且符合最佳实践迁移路径配置和作用域初始化逻辑实现得当,正确使用了依赖注入和值对象模式。
src/oauth2-server/src/Model/AccessTokenInterface.php (1)
17-37: 接口设计良好
AccessTokenInterface接口设计清晰,包含了 OAuth2 访问令牌所需的所有必要方法。使用了适当的类型提示和 PHPDoc 注解。src/oauth2-server/src/Controller/AuthorizationController.php (1)
37-42: PKCE 安全检查实现得当正确实现了对 'plain' 代码挑战方法的安全检查,防止客户端使用不安全的 PKCE 方法,除非明确允许。
composer.json (1)
18-18: OAuth2 服务器集成配置正确所有变更都符合集成 OAuth2 服务器组件的要求:
- 正确添加了
ext-openssl扩展依赖(OAuth2 加密操作所需)- 适当地将
league/oauth2-server添加到开发依赖- PSR-4 自动加载映射和配置提供者注册都设置正确
Also applies to: 97-97, 152-152, 208-208, 293-293
src/oauth2-server/src/Manager/ElquentORM/AuthorizationCodeManager.php (1)
21-36: Eloquent 实现符合最佳实践除了命名空间拼写错误外,
AuthorizationCodeManager的实现正确:
find方法适当地使用 Eloquent 的查找功能save方法正确委托给模型的保存方法clearExpired方法正确使用 Carbon 进行时间比较并删除过期记录src/oauth2-server/src/Model/RefreshTokenInterface.php (1)
16-29: 接口设计良好!
RefreshTokenInterface的设计遵循了OAuth2规范,提供了完整的令牌管理方法。使用self返回类型支持流式接口调用,这是一个很好的设计选择。src/oauth2-server/src/Converter/ScopeConverterInterface.php (1)
18-37: 接口设计良好接口定义清晰,方法签名合理,PHPDoc 注释完整。使用
list<Type>注释明确表示数组类型,这是很好的实践。src/oauth2-server/src/Model/AuthorizationCodeInterface.php (1)
17-37: 接口设计符合OAuth2标准接口方法定义完整,涵盖了OAuth2授权码的核心功能。使用
DateTimeInterface而不是具体实现类是良好的实践。revoke()方法返回self支持链式调用。src/oauth2-server/src/Entity/Client.php (2)
25-28: ClientTrait 中的 name 属性可正常访问,无需修改
League\OAuth2\Server\Entities\Traits\ClientTrait在 trait 中将$name声明为protected,因此在Client类的setName()方法中访问$this->name是有效的。无需对属性可见性进行调整。Likely an incorrect or invalid review comment.
38-41: 错误的审核建议经检查仓库中并不存在名为
ClientTrait的 trait,且$isConfidential属性实际上是在src/oauth2-server/src/Entity/Client.php类内部定义并使用的,无需在不存在的 trait 中验证其可访问性。无需对现有
setConfidential()方法或属性访问做任何修改。Likely an incorrect or invalid review comment.
src/oauth2-server/src/Repository/AuthCodeRepository.php (1)
78-89: 代码实现良好!
buildAuthorizationCode方法正确地将 OAuth2 实体转换为领域模型,使用了tap函数使代码更加简洁。src/oauth2-server/src/Manager/ClientFilter.php (1)
18-98: 过滤器实现清晰且符合最佳实践
ClientFilter类设计良好:
- 使用了值对象模式
- 提供了流式接口(fluent interface)
- 类型声明准确
- 具有良好的封装性
src/oauth2-server/src/Command/CreateClientCommand.php (1)
91-114: 命令实现符合 Symfony Console 最佳实践代码结构清晰,错误处理得当,输出格式友好。使用
SymfonyStyle提供了良好的用户体验。src/oauth2-server/src/Converter/ScopeConverter.php (1)
18-56: 实现正确且简洁!转换器类正确实现了
ScopeConverterInterface接口,提供了 League OAuth2 Server 实体和领域模型之间的双向转换。使用了恰当的类型注解 (list<>) 有助于静态分析。src/oauth2-server/src/Repository/AccessTokenRepository.php (2)
37-50: Token 创建逻辑正确!
getNewToken方法正确处理了用户标识符的空值检查,并妥善地添加了所有作用域。
52-63: 唯一性约束检查实现良好!在持久化新 Token 前检查是否已存在相同标识符的 Token,避免了重复问题。
src/oauth2-server/src/Command/UpdateClientCommand.php (2)
36-56: 命令处理逻辑实现良好!错误处理恰当,使用了 Symfony Style 组件提供友好的用户界面反馈。
78-95: 激活状态验证逻辑完善!正确验证了冲突的选项(同时指定 activate 和 deactivate),避免了逻辑错误。
src/oauth2-server/src/Model/ClientInterface.php (1)
18-61: 接口设计清晰完整!接口定义了 OAuth2 客户端的完整契约,使用了值对象和恰当的类型注解。所有方法都遵循了良好的命名规范。
src/oauth2-server/src/Model/RefreshToken.php (1)
29-83: 模型实现规范且完整!RefreshToken 模型正确使用了 UUID 主键,定义了恰当的属性转换和关联关系。
revoke()方法遵循了流式接口模式。src/oauth2-server/src/Manager/ElquentORM/AccessTokenManager.php (1)
27-33: 修正反转的逻辑判断
find()方法的逻辑看起来是反的。当配置指示需要持久化访问令牌时(persistAccessToken()返回true),应该执行查找操作;当不需要持久化时,才应该返回null。建议修改为:
public function find(string $identifier): ?AccessTokenInterface { - if (! $this->persistAccessToken()) { + if ($this->persistAccessToken()) { - return null; + return AccessToken::find($identifier); } - return AccessToken::find($identifier); + return null; }Likely an incorrect or invalid review comment.
src/oauth2-server/src/Repository/ScopeRepository.php (2)
29-35: 构造函数实现正确依赖注入实现得当,所有依赖项都通过 readonly 属性正确注入。
37-46: getScopeEntityByIdentifier 方法实现合理方法逻辑简洁明了,正确处理了未找到 scope 的情况,并使用转换器将领域模型转换为 League OAuth2 实体。
src/oauth2-server/src/Manager/ElquentORM/ClientManager.php (1)
31-37: save 方法实现正确正确实现了事件驱动的保存逻辑,在保存前派发事件允许监听器修改客户端实体。
src/oauth2-server/src/Model/AccessToken.php (2)
33-59: 模型定义规范且合理模型正确实现了
AccessTokenInterface,使用了 UUID 主键,属性类型转换配置得当,特别是ScopesCast自定义转换器的使用。
76-79: getExpiry 方法实现正确正确将 Carbon 对象转换为不可变的 DateTimeInterface,这是良好的实践,避免了意外的时间修改。
src/oauth2-server/src/Model/Client.php (3)
36-57: 模型定义完善且规范模型正确实现了
ClientInterface,使用了 UUID 主键,自定义类型转换器配置得当,特别是对值对象的转换处理。
118-121: isConfidential 方法实现正确正确实现了客户端机密性判断逻辑,检查 secret 是否为 null 或空字符串。
134-141: newClientInstance 方法使用了正确的关键字使用
static关键字而不是self是正确的,这支持了继承场景下的后期静态绑定。
PR #875 代码审查:OAuth2 服务器组件实现概述此PR为FriendsOfHyperf组件仓库添加了一个全面的OAuth2服务器实现,包括:
代码质量分析架构
代码组织
实现质量
安全性考虑
|
…tokens, improve error handling in client retrieval, and enhance redirect URI validation
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (5)
src/oauth2-server/src/Model/Casts/GrantsCast.php (2)
20-31: 仍需增强错误处理和类型安全性。当前实现缺少对 JSON 解码失败和无效数据的处理,这些问题在之前的评审中已经指出但尚未解决。
public function get($model, string $key, $value, array $attributes) { + if ($value === null) { + return []; + } + if (is_string($value)) { - $value = Json::decode($value); + try { + $value = Json::decode($value); + } catch (\Throwable $e) { + return []; + } } if (is_array($value)) { - return array_map(fn (string $grant) => new Grant($grant), $value); + return array_map(function ($grant) { + if (!is_string($grant)) { + throw new \InvalidArgumentException('Grant must be a string'); + } + return new Grant($grant); + }, $value); } return []; }
33-42: set 方法仍缺乏输入验证。该方法需要验证输入数据的有效性,确保只接受预期的数据类型,这是之前评审中提到的问题。
public function set($model, string $key, $value, array $attributes) { + if ($value === null) { + return Json::encode([]); + } + if (is_array($value)) { - $value = array_map(fn (Grant $grant) => (string) $grant, $value); + $value = array_map(function ($grant) { + if (!$grant instanceof Grant) { + throw new \InvalidArgumentException('Array must contain only Grant objects'); + } + return (string) $grant; + }, $value); } elseif ($value instanceof Grant) { $value = [(string) $value]; + } else { + throw new \InvalidArgumentException('Value must be a Grant object or array of Grant objects'); } + return Json::encode($value); }src/oauth2-server/src/Command/DeleteClientCommand.php (1)
47-52: 确认提示实现良好删除确认提示已经正确实现,避免了意外删除客户端的风险。
--force选项的处理也很合理。src/oauth2-server/src/Command/GenerateKeyPairCommand.php (2)
54-56: 处理配置值可能为 null 的情况配置值可能返回 null,但属性被声明为 string 类型。这可能导致类型错误。
建议修改为:
-$this->secretKey = $config->get('authorization_server.private_key', ''); -$this->publicKey = $config->get('resource_server.public_key', ''); +$this->secretKey = (string) $config->get('authorization_server.private_key', ''); +$this->publicKey = (string) $config->get('resource_server.public_key', '');
159-165: 提供更有意义的错误信息与第 153 行类似,当
openssl_error_string()返回 false 时,应提供有意义的错误信息。建议修改为:
-throw new RuntimeException(openssl_error_string() ?: ''); +throw new RuntimeException(openssl_error_string() ?: 'Failed to export private key');-throw new RuntimeException(openssl_error_string() ?: ''); +throw new RuntimeException(openssl_error_string() ?: 'Failed to get public key details');
🧹 Nitpick comments (7)
src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php (1)
19-37: 考虑添加日志记录以改善可观测性作为 OAuth2 的核心组件,建议添加适当的日志记录来跟踪授权码的操作,这对于安全审计和故障排查很重要。
考虑在类中注入日志器并在关键操作点添加日志:
use Psr\Log\LoggerInterface; final class AuthorizationCodeManager implements AuthorizationCodeManagerInterface { public function __construct( private ?LoggerInterface $logger = null ) {} public function save(AuthorizationCodeInterface $authCode): void { try { $authCode->save(); $this->logger?->info('Authorization code saved', ['id' => $authCode->getId()]); } catch (\Exception $e) { $this->logger?->error('Failed to save authorization code', [ 'id' => $authCode->getId(), 'error' => $e->getMessage() ]); throw new \RuntimeException('Failed to save authorization code: ' . $e->getMessage(), 0, $e); } } }src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php (2)
21-24: find 方法实现正确,但建议增加参数验证find 方法的实现符合接口要求,使用 Eloquent 的 find 方法简洁明了。但建议考虑添加输入参数验证以提高健壮性。
public function find(string $identifier): ?RefreshTokenInterface { + if (empty($identifier)) { + return null; + } return RefreshToken::find($identifier); }
19-37: 整体架构良好,但需要与其他管理器保持一致性RefreshTokenManager 的实现整体上遵循了良好的设计原则,但与项目中其他管理器的实现存在不一致性。特别是:
- AccessTokenManager 有
persistAccessToken()条件检查- ClientManager 有事件分发机制
- 当前实现相对简单,可能需要根据业务需求补充相应功能
建议统一各个管理器的实现模式,确保代码库的一致性和可维护性。
src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php (1)
43-51: 建议使用辅助函数并添加错误处理可以使用项目中的
now()辅助函数来保持一致性,同时需要添加数据库操作的错误处理。参考
src/helpers/src/Functions.php中的now()函数,建议进行以下改进:public function clearExpired(): int { if (! $this->persistAccessToken()) { return 0; } - return AccessToken::query() - ->where('expires_at', '<', Carbon::now()) - ->delete(); + try { + return AccessToken::query() + ->where('expires_at', '<', now()) + ->delete(); + } catch (\Exception $e) { + throw new \RuntimeException('Failed to clear expired access tokens: ' . $e->getMessage(), 0, $e); + } }tests/Oauth2Server/Command/CreateClientCommandTest.php (1)
163-320: 建议重构重复的 mock 设置代码。虽然测试覆盖很全面,但存在大量重复的 mock 设置代码。建议提取共用的 mock 设置逻辑到辅助方法中,提高代码可维护性。
private function setupBasicClientMocks(): void { $this->clientManager->shouldReceive('save')->once(); $this->client->shouldReceive('newClientInstance')->once()->andReturnSelf(); $this->client->shouldReceive('setActive')->once()->andReturnSelf(); $this->client->shouldReceive('setAllowPlainTextPkce')->once()->andReturnSelf(); $this->client->shouldReceive('setRedirectUris')->once()->andReturnSelf(); $this->client->shouldReceive('setGrants')->once()->andReturnSelf(); $this->client->shouldReceive('setScopes')->once()->andReturnSelf(); $this->client->shouldReceive('getIdentifier')->once()->andReturn('custom-id'); $this->client->shouldReceive('getSecret')->once()->andReturn(null); }src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (1)
56-77: 考虑优化查询性能当前的过滤实现对每个条件使用
orWhereJsonContains,这在大数据集上可能会有性能问题。建议:
- 考虑为 JSON 字段添加索引
- 对于频繁查询的场景,可以考虑使用缓存
src/oauth2-server/src/Model/Client.php (1)
134-141: 工厂方法实现正确,考虑添加参数验证。使用
static关键字支持子类继承,方法签名符合接口要求。建议添加参数验证以确保数据完整性:public function newClientInstance(string $name, string $identifier, ?string $secret): ClientInterface { + if (empty($name) || empty($identifier)) { + throw new \InvalidArgumentException('Client name and identifier cannot be empty'); + } + return new static([ 'name' => $name, 'id' => $identifier, 'secret' => $secret, + 'active' => true, // 设置合理的默认值 ]); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (32)
src/oauth2-server/databases/2025_07_15_044853_create_oauth_server.php(1 hunks)src/oauth2-server/src/Command/ClearExpiredTokensCommand.php(1 hunks)src/oauth2-server/src/Command/CreateClientCommand.php(1 hunks)src/oauth2-server/src/Command/DeleteClientCommand.php(1 hunks)src/oauth2-server/src/Command/GenerateKeyPairCommand.php(1 hunks)src/oauth2-server/src/ConfigProvider.php(1 hunks)src/oauth2-server/src/Controller/AuthorizationController.php(1 hunks)src/oauth2-server/src/Controller/TokenController.php(1 hunks)src/oauth2-server/src/Event/AuthorizationRequestResolveEvent.php(1 hunks)src/oauth2-server/src/Event/UserResolveEvent.php(1 hunks)src/oauth2-server/src/Factory/AuthorizationServerFactory.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/ClientManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php(1 hunks)src/oauth2-server/src/Manager/InMemory/ScopeManager.php(1 hunks)src/oauth2-server/src/Middleware/ResourceServerMiddleware.php(1 hunks)src/oauth2-server/src/Model/AccessToken.php(1 hunks)src/oauth2-server/src/Model/AuthorizationCode.php(1 hunks)src/oauth2-server/src/Model/Casts/GrantsCast.php(1 hunks)src/oauth2-server/src/Model/Client.php(1 hunks)src/oauth2-server/src/Model/ClientInterface.php(1 hunks)src/oauth2-server/src/Repository/AccessTokenRepository.php(1 hunks)src/oauth2-server/src/Repository/ClientRepository.php(1 hunks)src/oauth2-server/src/Repository/UserRepository.php(1 hunks)src/oauth2-server/src/ValueObject/Grant.php(1 hunks)src/oauth2-server/src/ValueObject/RedirectUri.php(1 hunks)src/oauth2-server/src/ValueObject/Scope.php(1 hunks)tests/Oauth2Server/Command/ClearExpiredTokensCommandTest.php(1 hunks)tests/Oauth2Server/Command/CreateClientCommandTest.php(1 hunks)tests/Oauth2Server/Command/DeleteClientCommandTest.php(1 hunks)tests/Oauth2Server/Command/GenerateKeyPairCommand.php(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/oauth2-server/src/ValueObject/Scope.php
- src/oauth2-server/src/Command/ClearExpiredTokensCommand.php
🚧 Files skipped from review as they are similar to previous changes (16)
- src/oauth2-server/src/Event/UserResolveEvent.php
- src/oauth2-server/src/Repository/UserRepository.php
- src/oauth2-server/src/ConfigProvider.php
- src/oauth2-server/src/ValueObject/RedirectUri.php
- src/oauth2-server/src/Middleware/ResourceServerMiddleware.php
- src/oauth2-server/src/Manager/InMemory/ScopeManager.php
- src/oauth2-server/src/Repository/AccessTokenRepository.php
- src/oauth2-server/src/Controller/TokenController.php
- src/oauth2-server/src/Command/CreateClientCommand.php
- src/oauth2-server/src/Controller/AuthorizationController.php
- src/oauth2-server/src/Repository/ClientRepository.php
- src/oauth2-server/databases/2025_07_15_044853_create_oauth_server.php
- src/oauth2-server/src/Model/AccessToken.php
- src/oauth2-server/src/Event/AuthorizationRequestResolveEvent.php
- src/oauth2-server/src/Factory/AuthorizationServerFactory.php
- src/oauth2-server/src/Model/AuthorizationCode.php
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/oauth2-server/src/ValueObject/Grant.php (2)
src/oauth2-server/src/Event/UserResolveEvent.php (1)
__construct(23-29)src/oauth2-server/src/ValueObject/Scope.php (2)
__construct(16-19)__toString(21-24)
src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php (4)
src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php (3)
find(27-33)save(35-41)clearExpired(43-51)src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php (3)
find(21-24)save(26-29)clearExpired(31-36)src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (2)
find(44-47)save(31-37)src/helpers/src/Functions.php (1)
now(371-374)
src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php (4)
src/oauth2-server/src/Model/AccessToken.php (1)
AccessToken(34-115)src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php (3)
find(21-24)save(26-29)clearExpired(31-36)src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php (3)
find(21-24)save(26-29)clearExpired(31-36)src/helpers/src/Functions.php (1)
now(371-374)
src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (6)
src/oauth2-server/src/Event/PreSaveClientEvent.php (1)
PreSaveClientEvent(17-33)src/oauth2-server/src/Manager/ClientFilter.php (2)
ClientFilter(18-98)hasFilters(91-97)src/oauth2-server/src/Model/Client.php (4)
Client(36-142)getGrants(85-88)getRedirectUris(74-77)getScopes(96-99)src/oauth2-server/src/ValueObject/Grant.php (2)
Grant(17-31)__construct(19-25)src/oauth2-server/src/ValueObject/RedirectUri.php (2)
RedirectUri(16-54)__construct(18-24)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)
src/oauth2-server/src/Model/Client.php (7)
src/oauth2-server/src/Model/Casts/GrantsCast.php (1)
GrantsCast(18-42)src/oauth2-server/src/Model/Casts/RedirectsCast.php (1)
RedirectsCast(18-42)src/oauth2-server/src/Model/Casts/ScopesCast.php (1)
ScopesCast(18-42)src/oauth2-server/src/ValueObject/Grant.php (1)
Grant(17-31)src/oauth2-server/src/ValueObject/RedirectUri.php (1)
RedirectUri(16-54)src/oauth2-server/src/ValueObject/Scope.php (1)
Scope(14-25)src/oauth2-server/src/Model/ClientInterface.php (15)
getIdentifier(23-23)getName(25-25)getSecret(27-27)getRedirectUris(32-32)setRedirectUris(34-34)getGrants(39-39)setGrants(41-41)getScopes(46-46)setScopes(48-48)isActive(50-50)setActive(52-52)isConfidential(54-54)isPlainTextPkceAllowed(56-56)setAllowPlainTextPkce(58-58)newClientInstance(60-60)
🪛 PHPMD (2.15.0)
src/oauth2-server/src/Model/Casts/GrantsCast.php
20-20: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
20-20: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$model'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$key'. (Unused Code Rules)
(UnusedFormalParameter)
33-33: Avoid unused parameters such as '$attributes'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
🔇 Additional comments (17)
src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php (1)
21-24: 添加输入验证以提高安全性
find方法缺少对$identifier参数的验证,这可能导致安全风险。建议添加输入验证来确保标识符格式正确且安全。public function find(string $identifier): ?AuthorizationCodeInterface { + if (empty($identifier) || !is_string($identifier)) { + return null; + } + + // 验证标识符格式(例如:长度、字符集) + if (strlen($identifier) < 10 || strlen($identifier) > 255) { + return null; + } + return AuthorizationCode::find($identifier); }Likely an incorrect or invalid review comment.
src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php (2)
20-25: 构造函数实现良好使用了 readonly 关键字确保配置接口的不可变性,依赖注入模式正确。
53-56: 配置访问实现正确正确使用了默认值,确保配置缺失时的安全回退行为。
tests/Oauth2Server/Command/DeleteClientCommandTest.php (2)
25-37: 测试结构良好,但建议保持 mock 设置的一致性。测试用例覆盖了各种场景,mock 生命周期管理得当。不过注意到在不同测试中 mock 的设置方式略有不同,建议保持一致性。
81-101: 测试逻辑正确,验证了强制删除功能。该测试正确验证了
--force选项跳过确认的行为,mock 期望设置合理。tests/Oauth2Server/Command/ClearExpiredTokensCommandTest.php (2)
25-41: 测试设置完善,涵盖了多种场景。beforeEach 和 afterEach 的设置合理,正确管理了依赖注入和 mock 生命周期。
43-98: 测试用例覆盖全面,验证了各种清理场景。测试用例很好地覆盖了:
- 默认清理所有类型的令牌
- 选择性清理特定类型
- 多种令牌类型组合
- 无令牌清理的情况
每个测试都正确验证了相应管理器方法的调用次数和输出内容。
tests/Oauth2Server/Command/CreateClientCommandTest.php (3)
25-38: 测试设置良好,依赖管理得当。beforeEach 和 afterEach 正确设置了 mock 和容器清理,确保测试隔离。
48-78: 基础客户端创建测试覆盖完整。测试正确验证了最小参数下的客户端创建流程,mock 期望设置合理。
149-161: 公共客户端验证逻辑测试正确。正确测试了公共客户端不能设置密钥的业务规则,错误处理和退出码验证都很到位。
src/oauth2-server/src/Model/ClientInterface.php (1)
18-61: 接口设计良好接口定义清晰全面,使用值对象确保类型安全,setter 方法返回 self 支持链式调用,符合最佳实践。
tests/Oauth2Server/Command/GenerateKeyPairCommand.php (1)
24-144: 测试覆盖全面测试用例涵盖了各种场景,包括默认行为、错误处理、各种选项和密码短语功能。测试结构清晰,使用了适当的 setup 和 teardown。
src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (1)
31-37: 事件驱动设计良好在保存客户端之前分发
PreSaveClientEvent事件,允许监听器修改客户端数据,提供了良好的扩展性。src/oauth2-server/src/Model/Client.php (4)
36-57: 类结构设计合理,实现了完整的OAuth2客户端模型。模型正确继承了Hyperf的Model类并实现了ClientInterface接口,使用UUID作为主键是OAuth2客户端的最佳实践。自定义类型转换器的配置正确,能够在存储层的JSON数据和领域对象之间进行无缝转换。
59-77: Getter方法实现正确且简洁。所有的访问器方法都正确返回了相应的属性值,getIdentifier()使用getKey()符合Eloquent模型的最佳实践。
79-105: Setter方法设计优秀,支持流畅的API调用。使用变长参数语法允许灵活设置多个值对象,返回ClientInterface类型而非具体实现类符合依赖倒置原则,支持方法链式调用提升了API的易用性。
123-126: 注意plain text PKCE的安全风险。允许明文PKCE挑战可能带来安全风险。建议在生产环境中默认禁用此功能,并在文档中明确说明启用此选项的安全影响。
考虑添加验证逻辑来确保只有在安全的环境下才允许启用plain text PKCE:
public function isPlainTextPkceAllowed(): bool { + // 在生产环境中建议禁用plain text PKCE + if (config('app.env') === 'production' && $this->allow_plain_text_pkce) { + // 记录警告日志 + } return $this->allow_plain_text_pkce; }
…, authorization code, client, refresh token, and update client command managers
…nts in ListClientsCommand for grant, redirect URI, and scope options
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (1)
89-90: 修正返回类型注解返回类型注解中使用了
redirect_uris,但实际代码中使用的是redirects。建议修改为:
- * @return array{grants?: list<Grant>, redirect_uris?: list<RedirectUri>, scopes?: list<Scope>} + * @return array{grants?: list<Grant>, redirects?: list<RedirectUri>, scopes?: list<Scope>}Also applies to: 106-107
🧹 Nitpick comments (4)
tests/Oauth2Server/Command/ListClientsCommandTest.php (1)
143-143: 修正变量命名错误变量名
$g1看起来像是拼写错误,应该改为$scope以保持代码的可读性和一致性。- $scopes = array_map(fn (Scope $g1) => (string) $g1, $filter->getScopes()); + $scopes = array_map(fn (Scope $scope) => (string) $scope, $filter->getScopes());src/oauth2-server/src/Command/UpdateClientCommand.php (2)
42-42: 保持 sprintf 函数调用的一致性代码中对
sprintf函数的调用不一致,有些地方使用了\sprintf(第42行和第114行),而其他地方使用了sprintf。建议统一使用其中一种形式。建议统一使用不带反斜杠的形式:
- $io->error(\sprintf('OAuth2 client identified as "%s" does not exist.', $input->getArgument('identifier'))); + $io->error(sprintf('OAuth2 client identified as "%s" does not exist.', $input->getArgument('identifier')));- throw new RuntimeException(\sprintf('Cannot specify "%s" in either "--%s" and "--%s".', implode('", "', $colliding), $addArgument, $removeArgument)); + throw new RuntimeException(sprintf('Cannot specify "%s" in either "--%s" and "--%s".', implode('", "', $colliding), $addArgument, $removeArgument));Also applies to: 114-114
64-65: 修正选项描述中的语法错误在移除操作的描述中,应该使用 "from the client" 而不是 "to the client"。
- ->addOption('remove-redirect-uri', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed redirect uri to the client.', []) + ->addOption('remove-redirect-uri', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed redirect uri from the client.', []) - ->addOption('remove-grant-type', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed grant type to the client.', []) + ->addOption('remove-grant-type', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed grant type from the client.', []) - ->addOption('remove-scope', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed scope to the client.', []) + ->addOption('remove-scope', null, InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Remove allowed scope from the client.', [])Also applies to: 67-67, 70-70
src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (1)
42-43: 修正注释格式错误第42行有一个重复的注释标记
// //,应该修正为单个注释标记。- // // @phpstan-ignore-next-line + // @phpstan-ignore-next-line $client->delete();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/oauth2-server/src/Command/ListClientsCommand.php(1 hunks)src/oauth2-server/src/Command/UpdateClientCommand.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/ClientManager.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php(1 hunks)src/oauth2-server/src/Repository/ClientRepository.php(1 hunks)src/oauth2-server/src/ValueObject/Grant.php(1 hunks)src/oauth2-server/src/ValueObject/RedirectUri.php(1 hunks)tests/Oauth2Server/Command/ListClientsCommandTest.php(1 hunks)tests/Oauth2Server/Command/UpdateClientCommandTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/oauth2-server/src/ValueObject/Grant.php
- src/oauth2-server/src/Manager/EloquentORM/AuthorizationCodeManager.php
- src/oauth2-server/src/Manager/EloquentORM/AccessTokenManager.php
- src/oauth2-server/src/Repository/ClientRepository.php
- src/oauth2-server/src/ValueObject/RedirectUri.php
- src/oauth2-server/src/Command/ListClientsCommand.php
- src/oauth2-server/src/Manager/EloquentORM/RefreshTokenManager.php
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/Oauth2Server/Command/UpdateClientCommandTest.php (3)
src/oauth2-server/src/Command/UpdateClientCommand.php (1)
UpdateClientCommand(26-124)src/oauth2-server/src/ValueObject/Grant.php (2)
Grant(17-31)__toString(27-30)src/oauth2-server/src/ValueObject/RedirectUri.php (2)
RedirectUri(16-54)__toString(26-29)
src/oauth2-server/src/Manager/EloquentORM/ClientManager.php (6)
src/oauth2-server/src/Event/PreSaveClientEvent.php (1)
PreSaveClientEvent(17-33)src/oauth2-server/src/Manager/ClientFilter.php (2)
ClientFilter(18-98)hasFilters(91-97)src/oauth2-server/src/Model/Client.php (4)
Client(36-142)getGrants(85-88)getRedirectUris(74-77)getScopes(96-99)src/oauth2-server/src/ValueObject/Grant.php (2)
Grant(17-31)__construct(19-25)src/oauth2-server/src/ValueObject/RedirectUri.php (2)
RedirectUri(16-54)__construct(18-24)src/oauth2-server/src/ValueObject/Scope.php (2)
Scope(14-25)__construct(16-19)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/Oauth2Server/Command/UpdateClientCommandTest.php (1)
157-161: 修正测试逻辑错误当抛出异常时,后续的
expect语句不会被执行。第158-161行的断言是无法到达的代码。建议移除无法执行的断言:
$output = new BufferedOutput(); $this->expectExceptionMessage('Cannot specify "' . $uri . '" in either "--add-redirect-uri" and "--remove-redirect-uri"'); - $exitCode = $this->command->run($input, $output); - expect($exitCode)->toBe(1) - ->and($output->fetch())->toContain('Cannot specify "' . $uri . '" in either "--add-redirect-uri" and "--remove-redirect-uri"'); + $this->command->run($input, $output);
🧹 Nitpick comments (3)
tests/Oauth2Server/Command/UpdateClientCommandTest.php (3)
178-181: 简化 Mockery 匹配器的使用使用
withArgs方法可以更简洁地表达参数匹配逻辑,提高代码可读性。- $this->client->shouldReceive('setGrants')->once() - ->with(Mockery::on(function (...$arg) use ($newGrant) { - return count($arg) === 1 - && $arg[0]->__toString() === $newGrant; - })); + $this->client->shouldReceive('setGrants')->once() + ->withArgs(function (...$grants) use ($newGrant) { + expect($grants)->toHaveCount(1) + ->and((string) $grants[0])->toBe($newGrant); + return true; + });
238-241: 统一匹配器使用风格为了保持代码一致性,建议使用与其他测试相同的
withArgs方法风格。- $this->client->shouldReceive('setScopes')->once() - ->with(Mockery::on(function (...$arg) use ($newScope) { - return count($arg) === 1 - && $arg[0]->__toString() === $newScope; - }))->andReturnSelf(); + $this->client->shouldReceive('setScopes')->once() + ->withArgs(function (...$scopes) use ($newScope) { + expect($scopes)->toHaveCount(1) + ->and((string) $scopes[0])->toBe($newScope); + return true; + })->andReturnSelf();
325-325: 删除不必要的交互模式设置在单元测试中设置交互模式是不必要的,因为测试环境通常不需要用户交互。
- $input->setInteractive(false);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/Oauth2Server/Command/UpdateClientCommandTest.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Oauth2Server/Command/UpdateClientCommandTest.php (3)
src/oauth2-server/src/Command/UpdateClientCommand.php (1)
UpdateClientCommand(26-124)src/oauth2-server/src/ValueObject/Grant.php (2)
Grant(17-31)__toString(27-30)src/oauth2-server/src/ValueObject/RedirectUri.php (2)
RedirectUri(16-54)__toString(26-29)
🔇 Additional comments (1)
tests/Oauth2Server/Command/UpdateClientCommandTest.php (1)
43-346: 测试覆盖度良好测试套件全面覆盖了 UpdateClientCommand 的各种功能场景,包括:
- 参数验证
- 客户端更新成功流程
- 重定向 URI 的添加和删除
- 授权类型的添加和删除
- 作用域的添加和删除
- 客户端激活和停用
- 错误处理和冲突检测
测试逻辑清晰,模拟设置合理,能够有效验证命令的行为。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/oauth2-server/README.md (4)
52-67: 示例缺少 DateInterval 引用,复制即用会报错
代码片段中直接实例化new DateInterval(),但顶部未use DateInterval;,用户复制粘贴到实际项目时会导致 IDE 或 CS 报未导入类。
83-84: 注释被截断导致可读性差
# The value should be a valid interval: http://ph这一行被截断,链接失效且语义不完整,请补全至官方手册完整 URL。
34-40: 提醒配置私钥文件权限,减少泄露风险
建议在生成密钥示例后补一句:
“生成的oauth-private.key应设置chmod 600并确保不被提交到 VCS”。
有助于新手避免把私钥暴露到仓库或镜像中。
127-134: 命令列表缺少oauth2:update-client
前文功能介绍提到“client management (create/delete/list)”,代码中实则包含UpdateClientCommand,但表格未列出,容易让读者误以为缺少更新功能。
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/oauth2-server/README.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
…ies, converters, and repository
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (2)
src/oauth2-server/src/Repository/ClientRepository.php (1)
76-76: 之前的审查建议已被采纳很好!代码已经使用了严格比较模式
in_array($grant, $grants, true)。src/oauth2-server/src/Factory/AuthorizationServerFactory.php (1)
123-135: 未修复的逻辑错误:缺少启用检查且隐式授权设置错误此代码段存在之前评审中指出的问题仍未修复:
- 直接从容器获取授权类型实例时没有检查它们是否已启用,可能导致运行时错误
- 为 ImplicitGrant 设置刷新令牌 TTL 是错误的,因为隐式授权不支持刷新令牌
建议修复:
private function configureGrants(): void { $refreshTokenTTL = $this->config->get('authorization_server.refresh_token_ttl'); - $this->container->get(PasswordGrant::class) - ->setRefreshTokenTTL($refreshTokenTTL); - $this->container->get(RefreshTokenGrant::class) - ->setRefreshTokenTTL($refreshTokenTTL); - $this->container->get(AuthCodeGrant::class) - ->setRefreshTokenTTL($refreshTokenTTL); + + if ($this->config->get('authorization_server.enable_password_grant') && $this->container->has(PasswordGrant::class)) { + $this->container->get(PasswordGrant::class) + ->setRefreshTokenTTL($refreshTokenTTL); + } + + if ($this->config->get('authorization_server.enable_refresh_token_grant') && $this->container->has(RefreshTokenGrant::class)) { + $this->container->get(RefreshTokenGrant::class) + ->setRefreshTokenTTL($refreshTokenTTL); + } + + if ($this->config->get('authorization_server.enable_auth_code_grant') && $this->container->has(AuthCodeGrant::class)) { + $this->container->get(AuthCodeGrant::class) + ->setRefreshTokenTTL($refreshTokenTTL); + } + if (! $this->config->get('authorization_server.require_code_challenge_for_public_clients')) { - $this->container->get(AuthCodeGrant::class)->disableRequireCodeChallengeForPublicClients(); + if ($this->config->get('authorization_server.enable_auth_code_grant') && $this->container->has(AuthCodeGrant::class)) { + $this->container->get(AuthCodeGrant::class)->disableRequireCodeChallengeForPublicClients(); + } } - if ($this->config->get('authorization_server.enable_implicit_grant')) { - $this->container->get(ImplicitGrant::class) - ->setRefreshTokenTTL($refreshTokenTTL); - } + // 移除隐式授权的刷新令牌配置,因为它不支持刷新令牌 }
🧹 Nitpick comments (4)
src/oauth2-server/src/Repository/ClientRepository.php (1)
57-59: 考虑增强客户端密钥验证的安全性当前实现允许非机密客户端绕过密钥验证。建议添加更明确的逻辑分离。
-if (! $client->isConfidential() || hash_equals((string) $client->getSecret(), (string) $clientSecret)) { - return true; -} +// 非机密客户端不需要验证密钥 +if (! $client->isConfidential()) { + return true; +} + +// 机密客户端必须提供正确的密钥 +if ($clientSecret === null || ! hash_equals((string) $client->getSecret(), (string) $clientSecret)) { + return false; +} + +return true;src/oauth2-server/src/Model/DeviceCodeInterface.php (1)
26-26: 考虑返回 Scope 对象数组而非混合类型为了类型安全,建议明确指定返回类型为
Scope[]。-public function getScopes(): array; +/** @return Scope[] */ +public function getScopes(): array;src/oauth2-server/src/Repository/DeviceCodeRepository.php (1)
46-46: 参数名称可能引起混淆参数名
$deviceCodeEntity实际上是一个字符串,而不是实体对象。建议重命名以提高代码可读性。-public function getDeviceCodeEntityByDeviceCode(string $deviceCodeEntity): ?DeviceCodeEntityInterface +public function getDeviceCodeEntityByDeviceCode(string $deviceCode): ?DeviceCodeEntityInterface { - $deviceCodeModel = $this->manager->findByDeviceCode($deviceCodeEntity); + $deviceCodeModel = $this->manager->findByDeviceCode($deviceCode);src/oauth2-server/src/Model/Device.php (1)
166-170: 日期转换方法不一致
setLastPoll方法使用Carbon::instance()而setExpiry方法使用Carbon::createFromImmutable(),应保持一致性。统一日期转换方法:
public function setLastPoll(DateTimeImmutable $lastPoll): DeviceCodeInterface { - $this->last_poll_at = Carbon::instance($lastPoll); + $this->last_poll_at = Carbon::createFromImmutable($lastPoll); return $this; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/oauth2-server/src/ConfigProvider.php(1 hunks)src/oauth2-server/src/Controller/AuthorizationController.php(1 hunks)src/oauth2-server/src/Converter/ClientConverter.php(1 hunks)src/oauth2-server/src/Converter/ClientConverterInterface.php(1 hunks)src/oauth2-server/src/Entity/DeviceCode.php(1 hunks)src/oauth2-server/src/Enums/DeviceCodeStatus.php(1 hunks)src/oauth2-server/src/Factory/AuthorizationServerFactory.php(1 hunks)src/oauth2-server/src/Manager/DeviceCodeManagerInterface.php(1 hunks)src/oauth2-server/src/Manager/EloquentORM/DeviceCodeManager.php(1 hunks)src/oauth2-server/src/Model/Device.php(1 hunks)src/oauth2-server/src/Model/DeviceCodeInterface.php(1 hunks)src/oauth2-server/src/Repository/ClientRepository.php(1 hunks)src/oauth2-server/src/Repository/DeviceCodeRepository.php(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/oauth2-server/src/Entity/DeviceCode.php
- src/oauth2-server/src/Enums/DeviceCodeStatus.php
- src/oauth2-server/src/Converter/ClientConverterInterface.php
- src/oauth2-server/src/Manager/DeviceCodeManagerInterface.php
🚧 Files skipped from review as they are similar to previous changes (2)
- src/oauth2-server/src/Controller/AuthorizationController.php
- src/oauth2-server/src/ConfigProvider.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/oauth2-server/src/Manager/EloquentORM/DeviceCodeManager.php (2)
src/oauth2-server/src/Model/Device.php (1)
Device(37-194)src/oauth2-server/src/Manager/DeviceCodeManagerInterface.php (2)
save(18-18)findByDeviceCode(20-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
…ies, converters, and repository
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/oauth2-server/src/ConfigProvider.php (1)
32-36: 确认命名空间拼写的一致性注意到导入的命名空间使用了
EloquentORM,但之前的审查评论提到存在ElquentORM拼写错误。请确认目录结构和命名空间声明的一致性。如果目录名称仍为
ElquentORM,需要将其重命名为EloquentORM并更新所有相关的命名空间声明和自动加载配置。
🧹 Nitpick comments (1)
src/oauth2-server/src/ConfigProvider.php (1)
99-99: 修正注释中的拼写错误注释中的 "factroy" 应为 "factory"。
- // factroy + // factory
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/oauth2-server/src/ConfigProvider.php(1 hunks)src/oauth2-server/src/Controller/AuthorizationController.php(1 hunks)tests/Oauth2Server/InMemoryScopeManagerTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/oauth2-server/src/Controller/AuthorizationController.php
🧰 Additional context used
🪛 GitHub Actions: tests
tests/Oauth2Server/InMemoryScopeManagerTest.php
[error] 1-27: PHP CS Fixer: Multiple formatting issues including missing declare(strict_types=1), blank lines, no_unused_imports, cast_spaces, header_comment, blank_lines_before_namespace, single_blank_line_at_eof.
src/oauth2-server/src/ConfigProvider.php
[error] 71-71: PHP CS Fixer: Missing trailing comma in multiline array and other formatting issues (phpdoc_no_alias_tag, trailing_comma_in_multiline, header_comment, blank_lines_before_namespace).
🔇 Additional comments (1)
src/oauth2-server/src/ConfigProvider.php (1)
67-113: 配置结构设计良好整体配置提供器的结构清晰,正确注册了 OAuth2 服务器所需的所有组件:
- 命令行工具覆盖了客户端管理和令牌清理
- 依赖注入绑定完整,涵盖了模型、管理器、转换器、仓储和工厂
- 发布配置允许用户自定义配置文件
实现遵循了 Hyperf 框架的约定,使用了适当的接口抽象。
* Add initial implementation of OAuth2 server components * Refactor OAuth2 server components: update default values for revoked tokens, improve error handling in client retrieval, and enhance redirect URI validation * Add PHPStan ignore comments for potential type issues in access token, authorization code, client, refresh token, and update client command managers * Refactor: Mark Grant and RedirectUri classes as final, update type hints in ListClientsCommand for grant, redirect URI, and scope options * up * up * add README.md * feat: Implement Device Code flow with new Device and DeviceCode entities, converters, and repository * feat: Implement Device Code flow with new Device and DeviceCode entities, converters, and repository * up * feat: Add Device Code grant type to OAuth2Grants * refactor: Remove debug output and add UpdateClientCommand to ConfigProvider * Apply cs fix
Summary by CodeRabbit
新功能
文档
测试