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.

Sign up for GitHub

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

Jump to bottom

feat: add lastUsedAt for classic token #547

Merged
merged 1 commit into from
Jul 12, 2023
Merged

feat: add lastUsedAt for classic token #547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 2 additions & 7 deletions app/core/service/TokenService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,9 @@ export class TokenService extends AbstractService {
return null;
}

public async checkTokenExpired(token: Token) {
// skip classic token
if (!isGranularToken(token)) {
return true;
}

public async checkTokenStatus(token: Token) {
// check for expires
if (dayjs(token.expiredAt).isBefore(new Date())) {
if (isGranularToken(token) && dayjs(token.expiredAt).isBefore(new Date())) {
throw new UnauthorizedError('Token expired');
}

fengmk2 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion app/port/UserRoleManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export class UserRoleManager {
}

// check token expired & set lastUsedAt
await this.tokenService.checkTokenExpired(authorizedUserAndToken.token);
await this.tokenService.checkTokenStatus(authorizedUserAndToken.token);
this.currentAuthorizedToken = authorizedUserAndToken.token;
this.currentAuthorizedUser = authorizedUserAndToken.user;
ctx.userId = authorizedUserAndToken.user.userId;
Copy link

Choose a reason for hiding this comment

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

这段代码是一个UserRoleManager类中的一部分。代码检查了令牌是否过期，并设置了lastUsedAt的值。

在修改的代码行中，将调用checkTokenExpired方法改为了checkTokenStatus方法。

对于代码的风险和改进建议，我目前无法提供详细信息，因为我无法看到完整的代码上下文和相关代码。但是，根据给出的部分代码，没有明显的错误或问题，而且修改似乎只是用不同的方法名替换了原有的方法名。

如果您提供更多的上下文或代码细节，我可能能够给出更具体的反馈。

Expand Down
19 changes: 19 additions & 0 deletions test/port/controller/TokenController/listTokens.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { AuthAdapter } from '../../../../app/infra/AuthAdapter';
import assert from 'assert';
import { app, mock } from 'egg-mock/bootstrap';
import { TestUtil } from '../../../../test/TestUtil';
import dayjs from 'dayjs';

describe('test/port/controller/TokenController/listTokens.test.ts', () => {
describe('[GET /-/npm/v1/tokens] listTokens()', () => {
Expand Down Expand Up @@ -35,6 +36,24 @@ describe('test/port/controller/TokenController/listTokens.test.ts', () => {
assert(tokens[0].updated);
});

it('should update lastUsedAt', async () => {
const { authorization } = await TestUtil.createUser();
const now = Date.now();

let res = await app.httpRequest()
.get('/-/whoami')
.set('authorization', authorization)
.expect(200);

res = await app.httpRequest()
.get('/-/npm/v1/tokens')
.set('authorization', authorization)
.expect(200);

const lastUsedAt = res.body.objects[0].lastUsedAt;
assert(dayjs(lastUsedAt).isAfter(now));
});

it('should 401 when readonly token access', async () => {
const { authorization } = await TestUtil.createUser({ tokenOptions: { readonly: true } });
const res = await app.httpRequest()
Copy link

Choose a reason for hiding this comment

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

这段代码看起来是一个关于TokenController的测试。

潜在的问题和改进建议如下：

  1. 引入dayjs库之后没有执行npm install命令，可能会导致缺少依赖。需要确保安装了dayjs库。
  2. 在第二个测试用例中，通过调用TestUtil.createUser()创建用户并获取authorization。然而，在之后没有清理创建的用户或回滚对数据库的更改。建议在测试运行结束后进行清理操作，以确保数据库的一致性。
  3. 第二个测试用例验证了lastUsedAt属性是否在当前时间之后，但是使用了Date.now()来获取当前时间。为了准确比较时间，建议使用dayjs库提供的方法来获取当前时间，例如dayjs().valueOf()。

总体上看，代码比较简洁，没有明显的错误。以上是一些建议和潜在问题，供参考。

Expand Down