Merge branch 'feat/162829-162857-scope' into feat/162830-choose-scope-of-access-token

apps/app/src/server/middlewares/access-token-parser/access-token-parser.integ.ts → apps/app/src/server/middlewares/access-token-parser/access-token.integ.ts

@@ -8,15 +8,13 @@ import type Crowi from '~/server/crowi';
 import type UserEvent from '~/server/events/user';
 import { AccessToken } from '~/server/models/access-token';
 
+import { parserForAccessToken } from './access-token';
 import type { AccessTokenParserReq } from './interfaces';
 
-import { accessTokenParser } from '.';
-
-
 vi.mock('@growi/core/dist/models/serializers', { spy: true });
 
 
-describe('access-token-parser middleware', () => {
+describe('access-token-parser middleware for access token with scopes', () => {
 
   let User;
 
@@ -42,119 +40,13 @@ describe('access-token-parser middleware', () => {
     const resMock = mock<Response>();
     const nextMock = vi.fn();
 
-    expect(reqMock.user).toBeUndefined();
-
-    // act
-    await accessTokenParser()(reqMock, resMock, nextMock);
-
-    // assert
-    expect(reqMock.user).toBeUndefined();
-    expect(serializeUserSecurely).not.toHaveBeenCalled();
-    expect(nextMock).toHaveBeenCalled();
-  });
-
-  it('should call next if the given access token is invalid', async() => {
-    // arrange
-    const reqMock = mock<AccessTokenParserReq>({
-      user: undefined,
-    });
-    const resMock = mock<Response>();
-    const nextMock = vi.fn();
-
-    expect(reqMock.user).toBeUndefined();
-
-    // act
-    reqMock.query.access_token = 'invalidToken';
-    await accessTokenParser()(reqMock, resMock, nextMock);
-
-    // assert
-    expect(reqMock.user).toBeUndefined();
-    expect(serializeUserSecurely).not.toHaveBeenCalled();
-    expect(nextMock).toHaveBeenCalled();
-  });
-
-  it('should set req.user with a valid api token in query', async() => {
-    // arrange
-    const reqMock = mock<AccessTokenParserReq>({
-      user: undefined,
-    });
-    const resMock = mock<Response>();
-    const nextMock = vi.fn();
+    await parserForAccessToken([])(reqMock, resMock, nextMock);
 
     expect(reqMock.user).toBeUndefined();
-
-    // prepare a user with an access token
-    const targetUser = await User.create({
-      name: faker.person.fullName(),
-      username: faker.string.uuid(),
-      password: faker.internet.password(),
-      lang: 'en_US',
-      apiToken: faker.internet.password(),
-    });
-
-    // act
-    reqMock.query.access_token = targetUser.apiToken;
-    await accessTokenParser()(reqMock, resMock, nextMock);
-
-    // assert
-    expect(reqMock.user).toBeDefined();
-    expect(reqMock.user?._id).toStrictEqual(targetUser._id);
-    expect(serializeUserSecurely).toHaveBeenCalledOnce();
     expect(nextMock).toHaveBeenCalled();
   });
 
-  it('should set req.user with a valid api token in body', async() => {
-    // arrange
-    const reqMock = mock<AccessTokenParserReq>({
-      user: undefined,
-    });
-    const resMock = mock<Response>();
-    const nextMock = vi.fn();
-
-    expect(reqMock.user).toBeUndefined();
-
-    // prepare a user with an access token
-    const targetUser = await User.create({
-      name: faker.person.fullName(),
-      username: faker.string.uuid(),
-      password: faker.internet.password(),
-      lang: 'en_US',
-      apiToken: faker.internet.password(),
-    });
-
-    // act
-    reqMock.body.access_token = targetUser.apiToken;
-    await accessTokenParser()(reqMock, resMock, nextMock);
-
-    // assert
-    expect(reqMock.user).toBeDefined();
-    expect(reqMock.user?._id).toStrictEqual(targetUser._id);
-    expect(serializeUserSecurely).toHaveBeenCalledOnce();
-    expect(nextMock).toHaveBeenCalled();
-  });
-
-});
-
-
-describe('access-token-parser middleware for access token with scopes', () => {
-
-  let User;
-
-  beforeAll(async() => {
-    const crowiMock = mock<Crowi>({
-      event: vi.fn().mockImplementation((eventName) => {
-        if (eventName === 'user') {
-          return mock<UserEvent>({
-            on: vi.fn(),
-          });
-        }
-      }),
-    });
-    const userModelFactory = (await import('../../models/user')).default;
-    User = userModelFactory(crowiMock);
-  });
-
-  it('should authenticate with no scopes', async() => {
+  it('should not authenticate with no scopes', async() => {
     // arrange
     const reqMock = mock<AccessTokenParserReq>({
       user: undefined,
@@ -180,13 +72,12 @@ describe('access-token-parser middleware for access token with scopes', () => {
 
     // act
     reqMock.query.access_token = token;
-    await accessTokenParser()(reqMock, resMock, nextMock);
+    await parserForAccessToken([])(reqMock, resMock, nextMock);
 
     // assert
-    expect(reqMock.user).toBeDefined();
-    expect(reqMock.user?._id).toStrictEqual(targetUser._id);
-    expect(serializeUserSecurely).toHaveBeenCalledOnce();
-    expect(nextMock).toHaveBeenCalled();
+    expect(reqMock.user).toBeUndefined();
+    expect(serializeUserSecurely).not.toHaveBeenCalled();
+    expect(nextMock).not.toHaveBeenCalled();
   });
 
   it('should authenticate with specific scope', async() => {
@@ -216,7 +107,7 @@ describe('access-token-parser middleware for access token with scopes', () => {
 
     // act
     reqMock.query.access_token = token;
-    await accessTokenParser()(reqMock, resMock, nextMock);
+    await parserForAccessToken([SCOPE.READ.USER.INFO])(reqMock, resMock, nextMock);
 
     // assert
     expect(reqMock.user).toBeDefined();
@@ -253,12 +144,12 @@ describe('access-token-parser middleware for access token with scopes', () => {
 
     // act - try to access with write:user:info scope
     reqMock.query.access_token = token;
-    await accessTokenParser([SCOPE.WRITE.USER.INFO])(reqMock, resMock, nextMock);
+    await parserForAccessToken([SCOPE.WRITE.USER.INFO])(reqMock, resMock, nextMock);
 
     // // assert
     expect(reqMock.user).toBeUndefined();
     expect(serializeUserSecurely).not.toHaveBeenCalled();
-    expect(nextMock).toHaveBeenCalled();
+    expect(nextMock).not.toHaveBeenCalled();
   });
 
   it('should authenticate with write scope implying read scope', async() => {
@@ -288,7 +179,7 @@ describe('access-token-parser middleware for access token with scopes', () => {
 
     // act - try to access with read:user:info scope
     reqMock.query.access_token = token;
-    await accessTokenParser([SCOPE.READ.USER.INFO])(reqMock, resMock, nextMock);
+    await parserForAccessToken([SCOPE.READ.USER.INFO])(reqMock, resMock, nextMock);
 
     // assert
     expect(reqMock.user).toBeDefined();
@@ -322,7 +213,7 @@ describe('access-token-parser middleware for access token with scopes', () => {
 
     // act - try to access with read:user:info scope
     reqMock.query.access_token = token;
-    await accessTokenParser([SCOPE.READ.USER.INFO, SCOPE.READ.USER.API.ACCESS_TOKEN])(reqMock, resMock, nextMock);
+    await parserForAccessToken([SCOPE.READ.USER.INFO, SCOPE.READ.USER.API.ACCESS_TOKEN])(reqMock, resMock, nextMock);
 
     // assert
     expect(reqMock.user).toBeDefined();

apps/app/src/server/middlewares/access-token-parser/access-token-parser.ts → apps/app/src/server/middlewares/access-token-parser/access-token.ts

@@ -1,8 +1,6 @@
-import type { IUser, IUserHasId } from '@growi/core/dist/interfaces';
+import type { IUserHasId } from '@growi/core/dist/interfaces';
 import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
 import type { NextFunction, Response } from 'express';
-import type { HydratedDocument } from 'mongoose';
-import mongoose from 'mongoose';
 
 import type { Scope } from '~/interfaces/scope';
 import { AccessToken } from '~/server/models/access-token';
@@ -10,47 +8,42 @@ import loggerFactory from '~/utils/logger';
 
 import type { AccessTokenParserReq } from './interfaces';
 
-const logger = loggerFactory('growi:middleware:access-token-parser');
+const logger = loggerFactory('growi:middleware:access-token-parser:access-token');
 
-
-export const accessTokenParser = (scopes?: Scope[]) => {
+export const parserForAccessToken = (scopes: Scope[]) => {
   return async(req: AccessTokenParserReq, res: Response, next: NextFunction): Promise<void> => {
-  // TODO: comply HTTP header of RFC6750 / Authorization: Bearer
+
     const accessToken = req.query.access_token ?? req.body.access_token;
     if (accessToken == null || typeof accessToken !== 'string') {
       return next();
     }
-
-    logger.debug('accessToken is', accessToken);
-
-    // check the api token is valid
-    const User = mongoose.model<HydratedDocument<IUser>, { findUserByApiToken }>('User');
-    const userByApiToken: IUserHasId = await User.findUserByApiToken(accessToken);
-    if (userByApiToken != null) {
-      req.user = serializeUserSecurely(userByApiToken);
-      logger.debug('API token parsed.');
-      return next();
+    if (scopes == null || scopes.length === 0) {
+      logger.debug('scopes is empty');
+      return;
     }
 
     // check the access token is valid
     const userId = await AccessToken.findUserIdByToken(accessToken, scopes);
     if (userId == null) {
       logger.debug('The access token is invalid');
-      return next();
+      return;
     }
 
     // check the user is valid
-    const { user }: {user: IUserHasId} = await userId.populate('user');
-    if (user == null) {
+    const { user: userByAccessToken }: {user: IUserHasId} = await userId.populate('user');
+    if (userByAccessToken == null) {
       logger.debug('The access token\'s associated user is invalid');
-      return next();
+      return;
     }
 
     // transforming attributes
-    req.user = serializeUserSecurely(user);
+    req.user = serializeUserSecurely(userByAccessToken);
+    if (req.user == null) {
+      return;
+    }
 
     logger.debug('Access token parsed.');
-
     return next();
+
   };
 };

apps/app/src/server/middlewares/access-token-parser/api-token.integ.ts

@@ -0,0 +1,134 @@
+
+import { faker } from '@faker-js/faker';
+import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
+import type { Response } from 'express';
+import { mock } from 'vitest-mock-extended';
+
+import type Crowi from '~/server/crowi';
+import type UserEvent from '~/server/events/user';
+
+import { parserForApiToken } from './api-token';
+import type { AccessTokenParserReq } from './interfaces';
+
+
+vi.mock('@growi/core/dist/models/serializers', { spy: true });
+
+
+describe('access-token-parser middleware', () => {
+
+  let User;
+
+  beforeAll(async() => {
+    const crowiMock = mock<Crowi>({
+      event: vi.fn().mockImplementation((eventName) => {
+        if (eventName === 'user') {
+          return mock<UserEvent>({
+            on: vi.fn(),
+          });
+        }
+      }),
+    });
+    const userModelFactory = (await import('../../models/user')).default;
+    User = userModelFactory(crowiMock);
+  });
+
+  it('should call next if no access token is provided', async() => {
+    // arrange
+    const reqMock = mock<AccessTokenParserReq>({
+      user: undefined,
+    });
+    const resMock = mock<Response>();
+    const nextMock = vi.fn();
+
+    expect(reqMock.user).toBeUndefined();
+
+    // act
+    await parserForApiToken(reqMock, resMock, nextMock);
+
+    // assert
+    expect(reqMock.user).toBeUndefined();
+    expect(serializeUserSecurely).not.toHaveBeenCalled();
+    expect(nextMock).toHaveBeenCalled();
+  });
+
+  it('should call next if the given access token is invalid', async() => {
+    // arrange
+    const reqMock = mock<AccessTokenParserReq>({
+      user: undefined,
+    });
+    const resMock = mock<Response>();
+    const nextMock = vi.fn();
+
+    expect(reqMock.user).toBeUndefined();
+
+    // act
+    reqMock.query.access_token = 'invalidToken';
+    await parserForApiToken(reqMock, resMock, nextMock);
+
+    // assert
+    expect(reqMock.user).toBeUndefined();
+    expect(serializeUserSecurely).not.toHaveBeenCalled();
+    expect(nextMock).not.toHaveBeenCalled();
+  });
+
+  it('should set req.user with a valid api token in query', async() => {
+    // arrange
+    const reqMock = mock<AccessTokenParserReq>({
+      user: undefined,
+    });
+    const resMock = mock<Response>();
+    const nextMock = vi.fn();
+
+    expect(reqMock.user).toBeUndefined();
+
+    // prepare a user with an access token
+    const targetUser = await User.create({
+      name: faker.person.fullName(),
+      username: faker.string.uuid(),
+      password: faker.internet.password(),
+      lang: 'en_US',
+      apiToken: faker.internet.password(),
+    });
+
+    // act
+    reqMock.query.access_token = targetUser.apiToken;
+    await parserForApiToken(reqMock, resMock, nextMock);
+
+    // assert
+    expect(reqMock.user).toBeDefined();
+    expect(reqMock.user?._id).toStrictEqual(targetUser._id);
+    expect(serializeUserSecurely).toHaveBeenCalledOnce();
+    expect(nextMock).toHaveBeenCalled();
+  });
+
+  it('should set req.user with a valid api token in body', async() => {
+    // arrange
+    const reqMock = mock<AccessTokenParserReq>({
+      user: undefined,
+    });
+    const resMock = mock<Response>();
+    const nextMock = vi.fn();
+
+    expect(reqMock.user).toBeUndefined();
+
+    // prepare a user with an access token
+    const targetUser = await User.create({
+      name: faker.person.fullName(),
+      username: faker.string.uuid(),
+      password: faker.internet.password(),
+      lang: 'en_US',
+      apiToken: faker.internet.password(),
+    });
+
+    // act
+    reqMock.body.access_token = targetUser.apiToken;
+    await parserForApiToken(reqMock, resMock, nextMock);
+
+    // assert
+    expect(reqMock.user).toBeDefined();
+    expect(reqMock.user?._id).toStrictEqual(targetUser._id);
+    expect(serializeUserSecurely).toHaveBeenCalledOnce();
+    expect(nextMock).toHaveBeenCalled();
+  });
+
+});

apps/app/src/server/middlewares/access-token-parser/api-token.ts

@@ -0,0 +1,35 @@
+import type { IUser, IUserHasId } from '@growi/core/dist/interfaces';
+import { serializeUserSecurely } from '@growi/core/dist/models/serializers';
+import type { NextFunction, Response } from 'express';
+import type { HydratedDocument } from 'mongoose';
+import mongoose from 'mongoose';
+
+import loggerFactory from '~/utils/logger';
+
+import type { AccessTokenParserReq } from './interfaces';
+
+const logger = loggerFactory('growi:middleware:access-token-parser:api-token');
+
+export const parserForApiToken = async(req: AccessTokenParserReq, res: Response, next: NextFunction): Promise<void> => {
+  const accessToken = req.query.access_token ?? req.body.access_token;
+  if (accessToken == null || typeof accessToken !== 'string') {
+    return next();
+  }
+
+  logger.debug('accessToken is', accessToken);
+
+  const User = mongoose.model<HydratedDocument<IUser>, { findUserByApiToken }>('User');
+  const userByApiToken: IUserHasId = await User.findUserByApiToken(accessToken);
+
+  if (userByApiToken == null) {
+    return;
+  }
+
+  req.user = serializeUserSecurely(userByApiToken);
+  if (req.user == null) {
+    return;
+  }
+
+  logger.debug('Access token parsed.');
+  return next();
+};

apps/app/src/server/middlewares/access-token-parser/index.ts

@@ -1 +1,20 @@
-export * from './access-token-parser';
+import type { NextFunction, Response } from 'express';
+
+import type { Scope } from '~/interfaces/scope';
+
+import { parserForAccessToken } from './access-token';
+import { parserForApiToken } from './api-token';
+import type { AccessTokenParserReq } from './interfaces';
+
+export const accessTokenParser = (scopes?: Scope[]) => {
+  return async(req: AccessTokenParserReq, res: Response, next: NextFunction): Promise<void> => {
+    // TODO: comply HTTP header of RFC6750 / Authorization: Bearer
+
+    if (scopes != null) {
+      parserForAccessToken(scopes)(req, res, next);
+    }
+    parserForApiToken(req, res, next);
+
+    return next();
+  };
+};

apps/app/src/server/models/access-token.ts

@@ -12,7 +12,7 @@ import type { Scope } from '~/interfaces/scope';
 import loggerFactory from '~/utils/logger';
 
 import { getOrCreateModel } from '../util/mongoose-utils';
-import { extractAllScope, extractScopes } from '../util/scope-utils';
+import { extractScopes } from '../util/scope-utils';
 
 const logger = loggerFactory('growi:models:access-token');
 
@@ -44,7 +44,7 @@ export interface IAccessTokenModel extends Model<IAccessTokenDocument> {
   deleteTokenById: (tokenId: Types.ObjectId | string) => Promise<void>
   deleteAllTokensByUserId: (userId: Types.ObjectId | string) => Promise<void>
   deleteExpiredToken: () => Promise<void>
-  findUserIdByToken: (token: string, requiredScopes?: Scope[]) => Promise<HydratedDocument<IAccessTokenDocument> | null>
+  findUserIdByToken: (token: string, requiredScopes: Scope[]) => Promise<HydratedDocument<IAccessTokenDocument> | null>
   findTokenByUserId: (userId: Types.ObjectId | string) => Promise<HydratedDocument<IAccessTokenDocument>[] | null>
   validateTokenScopes: (token: string, requiredScopes: Scope[]) => Promise<boolean>
 }
@@ -102,14 +102,14 @@ accessTokenSchema.statics.deleteExpiredToken = async function() {
   await this.deleteMany({ expiredAt: { $lte: now } });
 };
 
-accessTokenSchema.statics.findUserIdByToken = async function(token: string, requiredScopes?: Scope[]) {
+accessTokenSchema.statics.findUserIdByToken = async function(token: string, requiredScopes: Scope[]) {
   const tokenHash = generateTokenHash(token);
   const now = new Date();
-  if (requiredScopes != null && requiredScopes.length > 0) {
-    const extractedScopes = requiredScopes.map(scope => extractAllScope(scope)).flat();
-    return this.findOne({ tokenHash, expiredAt: { $gt: now }, scopes: { $all: extractedScopes } }).select('user');
+  if (requiredScopes.length === 0) {
+    return;
   }
-  return this.findOne({ tokenHash, expiredAt: { $gt: now } }).select('user');
+  const extractedScopes = extractScopes(requiredScopes);
+  return this.findOne({ tokenHash, expiredAt: { $gt: now }, scopes: { $all: extractedScopes } }).select('user');
 };
 
 accessTokenSchema.statics.findTokenByUserId = async function(userId: Types.ObjectId | string) {
@@ -118,10 +118,7 @@ accessTokenSchema.statics.findTokenByUserId = async function(userId: Types.Objec
 };
 
 accessTokenSchema.statics.validateTokenScopes = async function(token: string, requiredScopes: Scope[]) {
-  const tokenHash = generateTokenHash(token);
-  const now = new Date();
-  const tokenData = await this.findOne({ tokenHash, expiredAt: { $gt: now }, scopes: { $all: requiredScopes } });
-  return tokenData != null;
+  return this.findUserIdByToken(token, requiredScopes) != null;
 };
 
 accessTokenSchema.methods.isExpired = function() {

apps/app/src/server/util/scope-util.spec.ts

@@ -0,0 +1,90 @@
+import { describe, it, expect } from 'vitest';
+
+import { SCOPE } from '../../interfaces/scope';
+
+import {
+  isValidScope, hasAllScope, extractAllScope, extractScopes,
+} from './scope-utils';
+
+describe('scope-utils', () => {
+  describe('isValidScope', () => {
+    it('should return true for valid scopes', () => {
+      expect(isValidScope(SCOPE.READ.USER.API.API_TOKEN)).toBe(true);
+      expect(isValidScope(SCOPE.WRITE.USER.API.ACCESS_TOKEN)).toBe(true);
+      expect(isValidScope(SCOPE.READ.ADMIN.APP)).toBe(true);
+    });
+
+    it('should return false for invalid scopes', () => {
+      expect(isValidScope('invalid:scope' as any)).toBe(false);
+      expect(isValidScope('read:invalid:path' as any)).toBe(false);
+      expect(isValidScope('write:user:invalid' as any)).toBe(false);
+    });
+  });
+
+  describe('hasAllScope', () => {
+    it('should return true for scopes ending with *', () => {
+      expect(hasAllScope(SCOPE.READ.USER.API.ALL)).toBe(true);
+      expect(hasAllScope(SCOPE.WRITE.ADMIN.ALL)).toBe(true);
+    });
+
+    it('should return false for specific scopes', () => {
+      expect(hasAllScope(SCOPE.READ.USER.API.API_TOKEN)).toBe(false);
+      expect(hasAllScope(SCOPE.WRITE.USER.API.ACCESS_TOKEN)).toBe(false);
+    });
+  });
+
+  describe('extractAllScope', () => {
+    it('should extract all specific scopes from ALL scope', () => {
+      const extracted = extractAllScope(SCOPE.READ.USER.API.ALL);
+      expect(extracted).toContain(SCOPE.READ.USER.API.API_TOKEN);
+      expect(extracted).toContain(SCOPE.READ.USER.API.ACCESS_TOKEN);
+      expect(extracted).not.toContain(SCOPE.READ.USER.API.ALL);
+    });
+
+    it('should return array with single scope for specific scope', () => {
+      const scope = SCOPE.READ.USER.API.API_TOKEN;
+      const extracted = extractAllScope(scope);
+      expect(extracted).toEqual([scope]);
+    });
+  });
+
+  describe('extractScopes', () => {
+    it('should return empty array for undefined input', () => {
+      expect(extractScopes()).toEqual([]);
+    });
+
+    it('should extract all implied scopes including READ permission for WRITE scopes', () => {
+      const scopes = [SCOPE.WRITE.USER.API.ACCESS_TOKEN];
+      const extracted = extractScopes(scopes);
+
+      expect(extracted).toContain(SCOPE.WRITE.USER.API.ACCESS_TOKEN);
+      expect(extracted).toContain(SCOPE.READ.USER.API.ACCESS_TOKEN);
+    });
+
+    it('should extract all specific scopes from ALL scope with implied permissions', () => {
+      const scopes = [SCOPE.WRITE.USER.API.ALL];
+      const extracted = extractScopes(scopes);
+
+      // Should include both WRITE and READ permissions for all specific scopes
+      expect(extracted).toContain(SCOPE.WRITE.USER.API.API_TOKEN);
+      expect(extracted).toContain(SCOPE.WRITE.USER.API.ACCESS_TOKEN);
+      expect(extracted).toContain(SCOPE.READ.USER.API.API_TOKEN);
+      expect(extracted).toContain(SCOPE.READ.USER.API.ACCESS_TOKEN);
+
+      // Should not include ALL scopes
+      expect(extracted).not.toContain(SCOPE.WRITE.USER.API.ALL);
+      expect(extracted).not.toContain(SCOPE.READ.USER.API.ALL);
+    });
+
+    it('should remove duplicate scopes', () => {
+      const scopes = [
+        SCOPE.WRITE.USER.API.ACCESS_TOKEN,
+        SCOPE.READ.USER.API.ACCESS_TOKEN, // This is implied by WRITE
+      ];
+      const extracted = extractScopes(scopes);
+
+      const accessTokenScopes = extracted.filter(s => s.endsWith('access_token'));
+      expect(accessTokenScopes).toHaveLength(2); // Only READ and WRITE, no duplicates
+    });
+  });
+});

apps/app/src/server/util/scope-utils.ts

@@ -71,7 +71,7 @@ export const extractAllScope = (scope: Scope): Scope[] => {
   getAllScopeValuesFromObj(obj).forEach((value) => {
     result.push(value);
   });
-  return result;
+  return result.filter(scope => !hasAllScope(scope));
 };
 
 
@@ -102,6 +102,5 @@ export const extractScopes = (scopes?: Scope[]): Scope[] => {
       result.add(extractedScope);
     });
   });
-  const resultArray = Array.from(result.values()).filter(scope => !hasAllScope(scope));
-  return resultArray;
+  return Array.from(result);
 };