fix(vue3): 添加配置authService的功能以支持严格权限判断#353
Merged
Merged
Conversation
Contributor
概览该变更扩展了认证服务的能力，通过暴露内部资源映射和引入服务配置回调，使得外部实现能够自定义权限验证逻辑。变更涉及三个文件，核心是添加可配置的权限检查机制。 变更清单
预估代码审查工作量🎯 3 (中等难度) | ⏱️ ~25 分钟 诗文
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/vue3/source/src/common/plugins/auth/index.js (2)
31-38: 使用for...of替代while+shift()提升可读性当前实现使用
shift()会修改原数组且时间复杂度较高（虽然对短路径影响不大）。使用for...of循环更加简洁。♻️ 建议的重构
- while (parentAuthPaths.length > 0) { - const path = parentAuthPaths.shift(); - if (!_map.has(path)) { - hasPermission = false; - console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); - break; - } - } + for (const path of parentAuthPaths) { + if (!_map.has(path)) { + hasPermission = false; + console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); + break; + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue3/source/src/common/plugins/auth/index.js` around lines 31 - 38, Replace the while (parentAuthPaths.length > 0) { const path = parentAuthPaths.shift(); ... } pattern with a for...of loop over parentAuthPaths to avoid mutating the array and improve readability: iterate using for (const path of parentAuthPaths) { if (!_map.has(path)) { hasPermission = false; console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); break; } } while keeping the same variables (_map, parentAuthPaths, hasPermission) and preserving the break behavior and warning.
17-18: 生产环境中console.warn可能造成日志污染当前实现在权限检查失败时会输出
console.warn，在生产环境中如果有大量权限检查失败，会造成控制台日志污染。建议考虑：
- 添加环境判断，仅在开发模式下输出警告
- 或使用节流（throttle）减少重复警告
♻️ 开发环境判断示例
+ const isDev = process.env.NODE_ENV === 'development'; + service.has = function(authPath) { const _map = service._getResourceMap(); if (!_map) { - console.warn('权限资源未获取到，请检查权限资源接口'); + if (isDev) console.warn('权限资源未获取到，请检查权限资源接口'); return false; } // ... if (!_map.has(path)) { hasPermission = false; - console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); + if (isDev) console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); break; }Also applies to: 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vue3/source/src/common/plugins/auth/index.js` around lines 17 - 18, Replace the bare console.warn in the auth plugin with an environment-guarded or throttled logger: wrap the warning in a dev-only check (e.g. if (process.env.NODE_ENV !== 'production') console.warn(...)) or route through a throttled helper so repeated permission failures don't spam logs; update the specific console.warn occurrences found in this module (the permission-check warning and the other occurrence referenced) to use the chosen approach and ensure the message and context remain the same.packages/basic/src/init/auth/authService.ts (1)
51-51: 接口返回类型与实际实现不完全匹配
_getResourceMap的返回类型声明为Map<string, any>，但实际上_map可能是null（在start()中设置）或undefined（初始状态）。建议将返回类型改为可选类型以更准确地反映实际情况。♻️ 建议的修改
- _getResourceMap?: () => Map<string, any>; + _getResourceMap?: () => Map<string, any> | null | undefined;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/basic/src/init/auth/authService.ts` at line 51, _getResourceMap 的声明目前是 Map<string, any> 但实现中 _map 在 start() 里可能被设为 null，初始也可能为 undefined，导致类型不匹配；将 _getResourceMap 的返回类型改为可选/联合类型（例如 Map<string, any> | null | undefined 或 Map<string, any> | undefined）以反映实际可能值，或者在实现中保证始终返回 Map 并移除 null/undefined 的赋值；定位符：_getResourceMap、_map、start() 并同步更新相关调用处的类型检查或空值处理。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/vue3/source/src/common/plugins/auth/index.js`:
- Around line 23-29: The code builds parentAuthPaths from authPathSegments but
doesn't handle the empty-path edge case: when authPath is '' or '/',
authPathSegments becomes [] and parentAuthPaths is empty which makes the
permission check incorrectly return true; update the auth check to explicitly
handle empty or root paths before computing authPathSegments (e.g., in the
function where authPath is used, add a guard like if (!authPath ||
authPath.trim() === '' || authPath === '/') return false or, if root should be
allowed, add a clear comment documenting that behavior), and keep references to
authPath, authPathSegments and parentAuthPaths when making the change so the
intended behavior is explicit.
- Around line 7-41: The current configureAuthService override replaces
service.has with a strict parent-path check that differs from Vue2 and can break
migrations; change configureAuthService to accept a configurable mode (e.g.,
authStrictMode or strategy) and make service.has branch on that flag: when in
permissive mode keep the original behavior (only check
service._getResourceMap().has(authPath)), and when in strict mode run the
existing parent-path iteration; ensure the new flag defaults to the
Vue2-compatible permissive behavior, reference configureAuthService, service.has
and service._getResourceMap to locate code, and update the docs/upgrade guide to
mention the new option and default.
---
Nitpick comments:
In `@packages/basic/src/init/auth/authService.ts`:
- Line 51: _getResourceMap 的声明目前是 Map<string, any> 但实现中 _map 在 start() 里可能被设为
null，初始也可能为 undefined，导致类型不匹配；将 _getResourceMap 的返回类型改为可选/联合类型（例如 Map<string,
any> | null | undefined 或 Map<string, any> | undefined）以反映实际可能值，或者在实现中保证始终返回 Map
并移除 null/undefined 的赋值；定位符：_getResourceMap、_map、start() 并同步更新相关调用处的类型检查或空值处理。
In `@packages/vue3/source/src/common/plugins/auth/index.js`:
- Around line 31-38: Replace the while (parentAuthPaths.length > 0) { const path
= parentAuthPaths.shift(); ... } pattern with a for...of loop over
parentAuthPaths to avoid mutating the array and improve readability: iterate
using for (const path of parentAuthPaths) { if (!_map.has(path)) { hasPermission
= false; console.warn(`权限资源：缺少权限 ${path}，请确认是否已配置该权限项`); break; } } while
keeping the same variables (_map, parentAuthPaths, hasPermission) and preserving
the break behavior and warning.
- Around line 17-18: Replace the bare console.warn in the auth plugin with an
environment-guarded or throttled logger: wrap the warning in a dev-only check
(e.g. if (process.env.NODE_ENV !== 'production') console.warn(...)) or route
through a throttled helper so repeated permission failures don't spam logs;
update the specific console.warn occurrences found in this module (the
permission-check warning and the other occurrence referenced) to use the chosen
approach and ensure the message and context remain the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3f7fc4f4-2db3-480b-a6d5-cf2f02865f0d
📒 Files selected for processing (3)
packages/basic/src/init/auth/authService.tspackages/basic/src/init/auth/index.tspackages/vue3/source/src/common/plugins/auth/index.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit