Improve typing post review

Issue: ARSN-462

lib/auth/auth.ts

@@ -17,8 +17,11 @@ import validateAuthConfig from './backends/in_memory/validateAuthConfig';
 import AuthLoader from './backends/in_memory/AuthLoader';
 import Vault, { AuthV2RequestParams, AuthV4RequestParams } from './Vault';
 
+export type AuthResult<T> = { err: ArsenalError } | { err: null, params: T };
+
 let vault: Vault | null = null;
 const auth = {};
+
 const checkFunctions = {
     v2: {
         headers: v2.header.check,
@@ -61,7 +64,7 @@ function extractParams(
     log: Logger,
     awsService: string,
     data: { [key: string]: string }
-): { err: null | Error | ArsenalError, params?: AuthV2RequestParams | AuthV4RequestParams | AuthInfo } {
+): AuthResult<AuthV2RequestParams | AuthV4RequestParams | AuthInfo> {
     log.trace('entered', { method: 'Arsenal.auth.server.extractParams' });
     const authHeader = request.headers.authorization;
     let version: 'v2' |'v4' | null = null;
@@ -126,7 +129,8 @@ function doAuth(
     const res = extractParams(request, log, awsService, request.query);
     if (res.err) {
         return cb(res.err);
-    } else if (res.params instanceof AuthInfo) {
+    }
+    if (res.params instanceof AuthInfo) {
         return cb(null, res.params);
     }
     if (requestContexts) {
@@ -145,12 +149,12 @@ function doAuth(
     }
 
     // Corner cases managed, we're left with normal auth
-    res.params!.log = log;
-    if (res.params!.version === 2) {
-        return vault!.authenticateV2Request(res.params!, requestContexts, cb);
+    res.params.log = log;
+    if (res.params.version === 2) {
+        return vault!.authenticateV2Request(res.params, requestContexts, cb);
     }
-    if (res.params!.version === 4) {
-        return vault!.authenticateV4Request(res.params!, requestContexts, cb);
+    if (res.params.version === 4) {
+        return vault!.authenticateV4Request(res.params, requestContexts, cb);
     }
 
     log.error('authentication method not found', {

lib/auth/v2/headerAuthCheck.ts

@@ -1,16 +1,17 @@
 import { Logger } from 'werelogs';
-import errors, { ArsenalError } from '../../errors';
+import errors from '../../errors';
 import * as constants from '../../constants';
 import constructStringToSign from './constructStringToSign';
 import checkRequestExpiry from './checkRequestExpiry';
 import algoCheck from './algoCheck';
 import { AuthV2RequestParams } from '../Vault';
+import { AuthResult } from '../auth';
 
 export function check(
     request: any,
     log: Logger,
     data: { [key: string]: string },
-): { err: null | ArsenalError | Error, params?: AuthV2RequestParams } {
+): AuthResult<AuthV2RequestParams> {
     log.trace('running header auth check');
     const headers = request.headers;
 

lib/auth/v2/queryAuthCheck.ts

@@ -1,9 +1,10 @@
 import { Logger } from 'werelogs';
-import errors, { ArsenalError } from '../../errors';
+import errors from '../../errors';
 import * as constants from '../../constants';
 import algoCheck from './algoCheck';
 import constructStringToSign from './constructStringToSign';
 import { AuthV2RequestParams } from '../Vault';
+import { AuthResult } from '../auth';
 
 export const PRE_SIGN_URL_EXPIRY = process.env.PRE_SIGN_URL_EXPIRY ?
     Number.parseInt(process.env.PRE_SIGN_URL_EXPIRY, 10) :
@@ -13,7 +14,7 @@ export function check(
     request: any,
     log: Logger,
     data: { [key: string]: string },
-): { err: null | ArsenalError | Error, params?: AuthV2RequestParams } {
+): AuthResult<AuthV2RequestParams> {
     log.trace('running query auth check');
     if (request.method === 'POST') {
         log.debug('query string auth not supported for post requests');

lib/auth/v4/constructStringToSign.ts

@@ -17,7 +17,7 @@ export default function constructStringToSign(params: {
     log?: Logger;
     proxyPath?: string;
     awsService: string;
-}): string | Error {
+}): string {
     const {
         request,
         signedHeaders,

lib/auth/v4/headerAuthCheck.ts

@@ -13,6 +13,7 @@ import {
     areSignedHeadersComplete,
 } from './validateInputs';
 import { AuthV4RequestParams } from '../Vault';
+import { AuthResult } from '../auth';
 
 /**
  * V4 header auth check
@@ -27,7 +28,7 @@ export function check(
     log: Logger,
     data: { [key: string]: string },
     awsService: string
-): { err: null | ArsenalError | Error, params?: AuthV4RequestParams } {
+): AuthResult<AuthV4RequestParams> {
     log.trace('running header auth check');
 
     const token = request.headers['x-amz-security-token'];
@@ -102,7 +103,7 @@ export function check(
 
     const validationResult = validateCredentials(credentialsArr, timestamp,
         log);
-    if (validationResult instanceof Error) {
+    if (validationResult instanceof ArsenalError) {
         log.debug('credentials in improper format', { credentialsArr,
             timestamp, validationResult });
         return { err: validationResult };
@@ -156,10 +157,6 @@ export function check(
         proxyPath,
     });
     log.trace('constructed stringToSign', { stringToSign });
-    if (stringToSign instanceof Error) {
-        return { err: stringToSign };
-    }
-
 
     return {
         err: null,

lib/auth/v4/queryAuthCheck.ts

@@ -6,6 +6,7 @@ import { checkTimeSkew, convertAmzTimeToMs } from './timeUtils';
 import { validateCredentials, extractQueryParams } from './validateInputs';
 import { areSignedHeadersComplete } from './validateInputs';
 import { AuthV4RequestParams } from '../Vault';
+import { AuthResult } from '../auth';
 
 /**
  * V4 query auth check
@@ -17,7 +18,7 @@ export function check(
     request: any,
     log: Logger,
     data: { [key: string]: string },
-): { err: null | ArsenalError | Error, params?: AuthV4RequestParams } {
+): AuthResult<AuthV4RequestParams> {
     const authParams = extractQueryParams(data, log);
 
     if (Object.keys(authParams).length !== 5) {
@@ -45,7 +46,7 @@ export function check(
 
     const validationResult = validateCredentials(credential, timestamp,
         log);
-    if (validationResult instanceof Error) {
+    if (validationResult instanceof ArsenalError) {
         log.debug('credentials in improper format', { credential,
             timestamp, validationResult });
         return { err: validationResult };
@@ -99,9 +100,6 @@ export function check(
         awsService: service,
         proxyPath,
     });
-    if (stringToSign instanceof Error) {
-        return { err: stringToSign };
-    }
     log.trace('constructed stringToSign', { stringToSign });
     return {
         err: null,

lib/auth/v4/validateInputs.ts

@@ -1,5 +1,5 @@
 import { Logger } from 'werelogs';
-import errors from '../../../lib/errors';
+import errors, { ArsenalError } from '../../../lib/errors';
 
 /**
  * Validate Credentials
@@ -13,7 +13,7 @@ export function validateCredentials(
     credentials: [string, string, string, string, string],
     timestamp: string,
     log: Logger
-): Error | {} {
+): ArsenalError | {} {
     if (!Array.isArray(credentials) || credentials.length !== 5) {
         log.warn('credentials in improper format', { credentials });
         return errors.InvalidArgument;

lib/models/BucketPolicy.ts

@@ -63,7 +63,7 @@ export type BucketPolicyMetadata = {
 
 export default class BucketPolicy {
     private json: string;
-    private policy: BucketPolicyMetadata | null = null;
+    private policy?: BucketPolicyMetadata;
 
     /**
      * Create a Bucket Policy instance

lib/models/WebsiteConfiguration.ts

@@ -22,7 +22,7 @@ export type Condition = {
     httpErrorCodeReturnedEquals?: string;
 };
 
-export type RoutingRuleParams = { redirect: Redirect; condition?: Condition };
+export type RoutingRuleParams = { redirect?: Redirect; condition?: Condition };
 
 export class RoutingRule {
     _redirect?: Redirect;
@@ -35,7 +35,7 @@ export class RoutingRule {
     * @param params.redirect - specifies how to redirect requests
     * @param [params.condition] - specifies conditions for a redirect
     */
-    constructor(params?: RoutingRuleParams) {
+    constructor(params?: { redirect?: Redirect; condition?: Condition }) {
         if (params) {
             this._redirect = params.redirect;
             this._condition = params.condition;
@@ -80,7 +80,7 @@ export type WebsiteConfigurationParams = {
     indexDocument?: string;
     errorDocument?: string;
     redirectAllRequestsTo?: RedirectAllRequestsTo;
-    routingRules?: RoutingRule[] | any[],
+    routingRules?: RoutingRuleParams[],
 };
 
 export class WebsiteConfiguration {
@@ -185,7 +185,7 @@ export class WebsiteConfiguration {
     * Set the whole RoutingRules array
     * @param array - array to set as instance's RoutingRules
     */
-    setRoutingRules(array?: (RoutingRule | RoutingRuleParams)[]) {
+    setRoutingRules(array?: RoutingRuleParams[]) {
         if (array) {
             this._routingRules = array.map(rule => {
                 if (rule instanceof RoutingRule) {
@@ -200,7 +200,7 @@ export class WebsiteConfiguration {
      * Add a RoutingRule instance to routingRules array
      * @param obj - rule to add to array
      */
-    addRoutingRule(obj?: RoutingRule | RoutingRuleParams) {
+    addRoutingRule(obj?: RoutingRule) {
         if (!this._routingRules) {
             this._routingRules = [];
         }
@@ -215,7 +215,7 @@ export class WebsiteConfiguration {
      * Get routing rules
      * @return - array of RoutingRule instances
      */
-    getRoutingRules() {
-        return this._routingRules;
+    getRoutingRules(): RoutingRuleParams[] {
+        return this._routingRules ? this._routingRules.map(r => r.getRuleObject()) : [];
     }
 }

tests/unit/models/WebsiteConfiguration.spec.js

@@ -160,9 +160,9 @@ describe('WebsiteConfiguration class', () => {
             const websiteConfig = new WebsiteConfiguration();
             const routingRules = [testRoutingRuleParams];
             websiteConfig.setRoutingRules(routingRules);
-            assert.strictEqual(websiteConfig.getRoutingRules()[0]._condition,
+            assert.strictEqual(websiteConfig.getRoutingRules()[0].condition,
                 routingRules[0].condition);
-            assert.strictEqual(websiteConfig.getRoutingRules()[0]._redirect,
+            assert.strictEqual(websiteConfig.getRoutingRules()[0].redirect,
                 routingRules[0].redirect);
             assert(websiteConfig._routingRules[0] instanceof RoutingRule);
             done();