Modular: each streaming store decides what usernames are valid

lib/routes/S3Handler.js

@@ -11,6 +11,7 @@ const {
   ListObjectVersionsCommand,
   DeleteBucketCommand, GetBucketVersioningCommand, ListBucketsCommand
 } = require('@aws-sdk/client-s3');
+const { EMAIL_PATTERN, EMAIL_ERROR, PASSWORD_ERROR, PASSWORD_PATTERN } = require('../util/validations');
 const normalizeETag = require('../util/normalizeETag');
 const ParameterError = require('../util/ParameterError');
 const { dirname, basename } = require('path');
@@ -293,6 +294,9 @@ module.exports = function (endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SPQQ
     }
   );
 
+  const USER_NAME_PATTERN = new RegExp(`^[a-zA-Z0-9][a-zA-Z0-9-]{1,${61 - userNameSuffix.length}}[a-zA-Z0-9]$`);
+  const USER_NAME_ERROR = `User name must start and end with a letter or digit, contain only letters, digits & hyphens, and be 3–${63 - userNameSuffix.length} characters long.`;
+
   /**
    * Creates a versioned bucket with authentication & version data for the new user.
    * @param {Object} params
@@ -302,15 +306,21 @@ module.exports = function (endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SPQQ
    * @returns {Promise<string>} name of bucket
    */
   router.createUser = async function createUser (params) {
-    const { username, email, password } = params;
-    const bucketName = calcBucketName(username);
+    let { username, email, password } = params;
+    username = username.trim();
 
-    const errors = core.validateUser(params);
-    if (errors.length > 0) {
-      const msg = errors.map(err => err.message).join('|');
-      throw new Error(msg);
+    if (!USER_NAME_PATTERN.test(username)) {
+      throw new Error(USER_NAME_ERROR);
+    }
+    if (!EMAIL_PATTERN.test(email)) {
+      throw new Error(EMAIL_ERROR);
+    }
+    if (!PASSWORD_PATTERN.test(password)) {
+      throw new Error(PASSWORD_ERROR);
     }
 
+    const bucketName = calcBucketName(username);
+
     try {
       await s3client.send(new GetBucketVersioningCommand({ Bucket: bucketName }));
       throw new Error(`Username “${username}” is already taken`);
@@ -333,7 +343,7 @@ module.exports = function (endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SPQQ
       const hashedPasswordBlobPath = posix.join(AUTH_PREFIX, AUTHENTICATION_LOCAL_PASSWORD);
       await s3client.send(new PutObjectCommand({ Bucket: bucketName, Key: hashedPasswordBlobPath, Body: YAML.stringify(config), ContentType: 'application/yaml' }));
 
-      const metadata = { email };
+      const metadata = { username, email }; // username with original capitalization
       const metadataPath = posix.join(AUTH_PREFIX, USER_METADATA);
       await s3client.send(new PutObjectCommand({ Bucket: bucketName, Key: metadataPath, Body: YAML.stringify(metadata), ContentType: 'application/yaml' }));
 
@@ -393,7 +403,7 @@ module.exports = function (endPoint = 'play.min.io', accessKey = 'Q3AM3UQ867SPQQ
           }
         } catch (err) {
           if (err.name === 'NoSuchBucket') {
-            resolve();
+            resolve([numObjectsDeleted, numObjectsFailedToDelete]);
           } else {
             reject(err);
           }

lib/util/validations.js

@@ -0,0 +1,8 @@
+/** each streaming store decides what user names are valid, and may use its own password validation if desired. */
+
+module.exports = {
+  EMAIL_PATTERN: /[\p{L}\p{N}]@[a-zA-Z0-9][a-zA-Z0-9.]/u,
+  EMAIL_ERROR: 'Email must contain an alphanumeric, followed by an @-sign, followed by two ASCII alphanumerics',
+  PASSWORD_PATTERN: /\S{8,}/,
+  PASSWORD_ERROR: 'Password must contain at least 8 non-space characters'
+};

spec/account.spec.js

@@ -8,37 +8,37 @@ chai.use(require('chai-as-promised'));
 module.exports.shouldCreateDeleteAndReadAccounts = function () {
   describe('createUser', function () {
     before(function () {
-      this.username1 = 'automated-test-' + Math.round(Math.random() * Number.MAX_SAFE_INTEGER);
+      this.username = 'automated-test-' + Math.round(Math.random() * Number.MAX_SAFE_INTEGER);
     });
 
     after(async function () {
-      await this.store.deleteUser(this.username1);
+      await this.store.deleteUser(this.username);
     });
 
     it('rejects a user with too short a name', async function () {
-      const params = { username: 'a', email: '[email protected]', password: 'swordfish' };
-      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, 'Username must be at least 2 characters');
+      const params = { username: 'aa', email: '[email protected]', password: 'swordfish' };
+      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, /user\s?name/i);
     });
 
     it('rejects creating a user with illegal characters in username', async function () {
-      const params = { username: 'a+a', email: '[email protected]', password: 'swordfish' };
-      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, 'only contain lowercase letters, numbers, dots, dashes and underscores');
+      const params = { username: this.username + '\\q', email: '[email protected]', password: 'swordfish' };
+      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, /user\s?name/i);
     });
 
     it('rejects creating a user with bad email', async function () {
-      const params = { username: 'a1a', email: 'a@b', password: 'swordfish' };
-      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, 'Email is not valid');
+      const params = { username: this.username + 'j', email: 'a@b', password: 'swordfish' };
+      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, /email/i);
     });
 
     it('rejects creating a user without password', async function () {
-      const params = { username: 'a2a', email: '[email protected]', password: '' };
-      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, 'Password must not be blank');
+      const params = { username: this.username + 'h', email: '[email protected]', password: '' };
+      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, /password/i);
     });
 
     it('creates a user & rejects creating a new user with an existing name', async function () {
-      const params1 = { username: this.username1, email: '[email protected]', password: 'swordfish' };
-      await expect(this.store.createUser(params1)).to.eventually.eql(this.username1);
-      const params2 = { username: this.username1, email: '[email protected]', password: 'iloveyou' };
+      const params1 = { username: this.username, email: '[email protected]', password: 'swordfish' };
+      await expect(this.store.createUser(params1)).to.eventually.eql(this.username + this.USER_NAME_SUFFIX);
+      const params2 = { username: this.username, email: '[email protected]', password: 'iloveyou' };
       await expect(this.store.createUser(params2)).to.be.rejectedWith(Error, 'is already taken');
     });
   });
@@ -54,7 +54,7 @@ module.exports.shouldCreateDeleteAndReadAccounts = function () {
 
     it('deletes a user', async function () {
       const params = { username: this.username2, email: '[email protected]', password: 'swordfish' };
-      await expect(this.store.createUser(params)).to.eventually.eql(this.username2);
+      await expect(this.store.createUser(params)).to.eventually.eql(this.username2 + this.USER_NAME_SUFFIX);
       const result = await this.store.deleteUser(this.username2);
       await expect(result?.[0]).to.be.greaterThanOrEqual(2);
       await expect(result?.[1]).to.equal(0);

spec/streaming_handler/S3Handler.spec.js

@@ -2,6 +2,8 @@
 /* eslint-env mocha, chai, node */
 /* eslint-disable no-unused-expressions */
 
+const chai = require('chai');
+const expect = chai.expect;
 const s3handler = require('../../lib/routes/S3Handler');
 const { shouldStoreStream } = require('../streaming_handler.spec');
 const { configureLogger } = require('../../lib/logger');
@@ -10,12 +12,20 @@ const { shouldCreateDeleteAndReadAccounts } = require('../account.spec');
 describe('S3 streaming handler', function () {
   before(function () {
     configureLogger({ stdout: [], log_dir: './test-log', log_files: ['debug'] });
+    this.USER_NAME_SUFFIX = '-java.extraordinary.org';
     // If the environment variables aren't set, tests are run using a shared public account on play.min.io
-    this.handler = s3handler(process.env.S3_ENDPOINT, process.env.S3_ACCESS_KEY, process.env.S3_SECRET_KEY);
+    this.handler = s3handler(process.env.S3_ENDPOINT, process.env.S3_ACCESS_KEY, process.env.S3_SECRET_KEY, undefined, this.USER_NAME_SUFFIX);
     this.store = this.handler;
   });
 
   shouldCreateDeleteAndReadAccounts();
 
+  describe('createUser (S3-specific)', function () {
+    it('rejects a user with too long a name for the user name suffix', async function () {
+      const params = { username: 'a-------10--------20--------30--------40-', email: '[email protected]', password: 'swordfish' };
+      await expect(this.store.createUser(params)).to.be.rejectedWith(Error, '3–40 characters long');
+    });
+  });
+
   shouldStoreStream();
 });