Skip to content
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

feat: adaptive username #536

Merged
merged 2 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/common/PackageUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ export function cleanUserPrefix(username: string): string {
return username.replace(/^.*:/, '');
}

export function getPrefixedName(prefix: string, username: string): string {
return prefix ? `${prefix}${username}` : username;
}

export async function calculateIntegrity(contentOrFile: Uint8Array | string) {
let integrityObj;
if (typeof contentOrFile === 'string') {

Choose a reason for hiding this comment

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

在给定的代码补丁中,有一个新的函数getPrefixedName被添加到代码中。这个函数将一个前缀字符串和一个用户名字符串作为参数,并返回一个带有前缀的用户名字符串。

代码的修改看起来是正确的,没有明显的错误或缺陷风险。然而,以下是一些建议改进:

  1. getPrefixedName函数中,可以添加输入参数的类型检查器,确保传递正确的参数类型。例如,为prefixusername添加类型注解string
  2. calculateIntegrity函数中,你可以添加返回类型注解(可能是Promise<void>)来指示异步函数的返回值类型。
  3. 如有可能,对函数和变量使用更具描述性的命名,以提高代码的可读性。

如果代码的上下文中还有其他内容,或者有其他方面需要注意的限制,请提供更多信息,以便我能够作出更准确的建议。

Choose a reason for hiding this comment

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

代码看起来没有明显的错误风险,但以下是一些建议:

  1. 在代码中添加适当的注释,以解释每个函数和条件的用途和意图。这将提高可读性并帮助其他开发人员理解代码。

  2. 在导出的函数上添加适当的类型注解。这将使代码更具可读性,并允许编辑器捕获潜在的类型错误。

  3. 对于 calculateIntegrity 函数,请确保在可能的情况下处理错误和异常情况。例如,如果传入的参数既不是 Uint8Array 类型也不是字符串类型,可能需要抛出一个错误或返回一个默认值。

  4. 针对每个函数编写一些单元测试,以确保其正确性和预期行为。这有助于捕获潜在的错误和边界情况。

总体而言,代码看起来相当简单且功能齐全。建议继续进行更准确的测试以确保正确的行为。

Expand Down
27 changes: 27 additions & 0 deletions app/core/service/UserService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import { WebauthnCredential as WebauthnCredentialEntity } from '../entity/Webaut
import { LoginResultCode } from '../../common/enum/User';
import { integrity, checkIntegrity, randomToken, sha512 } from '../../common/UserUtil';
import { AbstractService } from '../../common/AbstractService';
import { RegistryManagerService } from './RegistryManagerService';
import { getPrefixedName } from '../../common/PackageUtil';
import { Registry } from '../entity/Registry';

type Optional<T, K extends keyof T> = Omit < T, K > & Partial<T> ;

Expand Down Expand Up @@ -59,12 +62,36 @@ type CreateWebauthnCredentialOptions = {
export class UserService extends AbstractService {
@Inject()
private readonly userRepository: UserRepository;
@Inject()
private readonly registryManagerService: RegistryManagerService;

checkPassword(user: UserEntity, password: string): boolean {
const plain = `${user.passwordSalt}${password}`;
return checkIntegrity(plain, user.passwordIntegrity);
}

async findUserByNameOrDisplayName(name: string) {
const hasPrefix = name.includes(':');
if (hasPrefix) {
return await this.findUserByName(name);
}

const selfRegistry = await this.registryManagerService.ensureSelfRegistry();
const selfUser = await this.findUserByName(getPrefixedName(selfRegistry.userPrefix, name));
if (selfUser) {
return selfUser;
}

const defaultRegistry = await this.registryManagerService.ensureDefaultRegistry();
const defaultUser = await this.findUserByName(getPrefixedName(defaultRegistry.userPrefix, name));

return defaultUser;
}

async findInRegistry(registry:Registry, name: string): Promise<UserEntity | null> {
return await this.findUserByName(getPrefixedName(registry.userPrefix, name));
}

async findUserByName(name: string): Promise<UserEntity | null> {
return await this.userRepository.findUserByName(name);
}

Choose a reason for hiding this comment

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

这段代码对UserService进行了补丁。下面是一些代码审查的建议和改进建议:

  1. 在导入语句处,可以把所有的import语句按照一定规则进行分组,以提高可读性。

  2. 对于findUserByNameOrDisplayName方法:

    • 增加了在包含前缀的情况下直接调用findUserByName的逻辑。
    • 通过registryManagerService来获取默认的注册表(defaultRegistry)和当前注册表(selfRegistry)。
    • 使用getPrefixedName函数生成带前缀的用户名,并在默认注册表和当前注册表中进行用户查找。
  3. 添加了findInRegistry方法,该方法在给定的注册表中查找用户。

  4. 建议在代码中添加注释,解释每个方法的功能和参数。

  5. 确保在使用async/await时处理异常情况,可以使用try-catch块来捕获和处理可能发生的错误。

  6. 检查是否有必要对参数进行验证和边界条件检查。

这些建议都是基于代码片段本身,更全面的代码审查还需要结合整个项目上下文来进行。如果有其他具体问题或需求,请提供更多细节。

Expand Down
12 changes: 6 additions & 6 deletions app/port/controller/UserController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class UserController extends AbstractController {
ctx.status = 201;
return {
ok: true,
id: `org.couchdb.user:${result.user?.name}`,
id: `org.couchdb.user:${result.user?.displayName}`,
rev: result.user?.userId,
token: result.token?.token,
};
Expand All @@ -113,7 +113,7 @@ export class UserController extends AbstractController {
ctx.status = 201;
return {
ok: true,
id: `org.couchdb.user:${userEntity.name}`,
id: `org.couchdb.user:${userEntity.displayName}`,
rev: userEntity.userId,
token: token.token,
};
Expand All @@ -140,14 +140,14 @@ export class UserController extends AbstractController {
method: HTTPMethodEnum.GET,
})
async showUser(@Context() ctx: EggContext, @HTTPParam() username: string) {
const user = await this.userRepository.findUserByName(username);
const user = await this.userService.findUserByNameOrDisplayName(username);
if (!user) {
throw new NotFoundError(`User "${username}" not found`);
}
const authorized = await this.userRoleManager.getAuthorizedUserAndToken(ctx);
return {
_id: `org.couchdb.user:${user.name}`,
name: user.name,
_id: `org.couchdb.user:${user.displayName}`,
name: user.displayName,
email: authorized ? user.email : undefined,
};
}
Expand Down Expand Up @@ -209,7 +209,7 @@ export class UserController extends AbstractController {
// "pending": false,
// "mode": "auth-only"
// },
name: authorizedUser.name,
name: authorizedUser.displayName,
email: authorizedUser.email,
email_verified: false,
created: authorizedUser.createdAt,
Expand Down
25 changes: 25 additions & 0 deletions test/port/controller/UserController/showUser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,31 @@ describe('test/port/controller/UserController/showUser.test.ts', () => {
});
});

it('should clean prefix info', async () => {
const { authorization, email } = await TestUtil.createUser({ name: 'npm:banana' });
const res = await app.httpRequest()
.get(`/-/user/org.couchdb.user:${encodeURIComponent('npm:banana')}`)
.set('authorization', authorization)
.expect(200);
assert.deepEqual(res.body, {
_id: 'org.couchdb.user:banana',
name: 'banana',
email,
});
});

it('should return self user first', async () => {
const { authorization, email } = await TestUtil.createUser({ name: 'npm:apple' });
await TestUtil.createUser({ name: 'apple' });
const res = await app.httpRequest()
.get('/-/user/org.couchdb.user:apple')
.set('authorization', authorization)
.expect(200);

assert.equal(res.body.email, email);
});


it('should 404 when user not exists', async () => {
const { authorization, name } = await TestUtil.createUser();
let res = await app.httpRequest()

Choose a reason for hiding this comment

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

这段代码的主要目标是测试用户控制器(UserController)中的一个方法。以下是对代码进行的简要代码审查:

  1. 在第23行和第34行之间添加了两个新的测试用例。

    • 第一个测试用例验证了当请求带有特定前缀信息时是否正确清除该前缀,并返回相应的用户信息。
    • 第二个测试用例验证了当请求自身用户时是否返回正确的用户信息。
  2. 需要确保在每个测试用例中都提供有效的授权头部(authorization)。

  3. 没有立即发现明显的错误或潜在的风险。

针对改进的建议:

  • 在测试用例中提供更详细的注释,以解释每个测试的目的和预期结果。
  • 添加边界测试用例,例如尝试使用无效的授权头部或无效的请求路径来测试系统的鲁棒性。
  • 确保在删除前缀信息的操作上进行适当的错误处理,例如在前缀信息为空或不存在时返回合适的错误响应。

请注意,这只是对给定代码片段的初步审查,可能还有其他方面需要检查。最佳实践是进行全面而彻底的代码审查,包括阅读和理解相关的功能实现和错误处理逻辑。

Expand Down