fix: updates for SignatureBouncer tests and voucher construction

contracts/access/SignatureBouncer.sol

@@ -1,6 +1,6 @@
 pragma solidity ^0.4.24;
 
-import "../ownership/Ownable.sol";
+import "../ownership/rbac/RBACOwnable.sol";
 import "../ownership/rbac/RBAC.sol";
 import "../ECRecovery.sol";
 
@@ -29,7 +29,7 @@ import "../ECRecovery.sol";
  * Also non fixed sized parameters make constructing the data in the signature
  * much more complex. See https://ethereum.stackexchange.com/a/50616 for more details.
  */
-contract SignatureBouncer is Ownable, RBAC {
+contract SignatureBouncer is RBACOwnable {
   using ECRecovery for bytes32;
 
   string public constant ROLE_BOUNCER = "bouncer";

contracts/ownership/rbac/RBACOwnable.sol

@@ -0,0 +1,41 @@
+pragma solidity ^0.4.24;
+
+import "./RBAC.sol";
+
+
+/**
+ * @title RBACOwnable
+ * @author Matt Condon (@shrugs)
+ * @dev Ownable logic using RBAC.
+ * @dev Use RBACOwnable if you could have many different owners and you're ok with
+ * @dev the security profile of any owner being able to add another owner.
+ * @dev Only difference from Ownable.sol is that the owners are not stored as public variables.
+ */
+contract RBACOwnable is RBAC {
+  string public constant ROLE_OWNER = "owner";
+
+  constructor()
+    public
+  {
+    addRole(msg.sender, ROLE_OWNER);
+  }
+
+  modifier onlyOwner() {
+    checkRole(msg.sender, ROLE_OWNER);
+    _;
+  }
+
+  function addOwner(address _owner)
+    onlyOwner
+    public
+  {
+    addRole(_owner, ROLE_OWNER);
+  }
+
+  function removeOwner(address _owner)
+    onlyOwner
+    public
+  {
+    removeRole(_owner, ROLE_OWNER);
+  }
+}

test/access/SignatureBouncer.test.js

@@ -1,7 +1,7 @@
 const { assertRevert } = require('../helpers/assertRevert');
 const { getBouncerSigner } = require('../helpers/sign');
 
-const Bouncer = artifacts.require('SignatureBouncerMock');
+const SignatureBouncer = artifacts.require('SignatureBouncerMock');
 
 require('chai')
   .use(require('chai-as-promised'))
@@ -13,15 +13,11 @@ const INVALID_SIGNATURE = '0xabcd';
 
 contract('Bouncer', ([_, owner, anyone, bouncerAddress, authorizedUser]) => {
   beforeEach(async function () {
-    this.bouncer = await Bouncer.new({ from: owner });
+    this.bouncer = await SignatureBouncer.new({ from: owner });
     this.roleBouncer = await this.bouncer.ROLE_BOUNCER();
   });
 
   context('management', () => {
-    it('has a default owner of self', async function () {
-      (await this.bouncer.owner()).should.eq(owner);
-    });
-
     it('allows the owner to add a bouncer', async function () {
       await this.bouncer.addBouncer(bouncerAddress, { from: owner });
       (await this.bouncer.hasRole(bouncerAddress, this.roleBouncer)).should.eq(true);

test/ownership/rbac/RBACOwnable.test.js

@@ -0,0 +1,34 @@
+import assertRevert from '../../helpers/assertRevert';
+
+const RBACOwnable = artifacts.require('RBACOwnable');
+
+require('chai')
+  .use(require('chai-as-promised'))
+  .should();
+
+contract('RBAC', function ([_, owner, newOwner, notOwner]) {
+  before(async function () {
+    this.ownable = await RBACOwnable.new({ from: owner });
+    this.roleOwner = await this.ownable.ROLE_OWNER();
+  });
+
+  it('should not allow notOwner to add or remove owners', async function () {
+    await assertRevert(
+      this.ownable.addOwner(newOwner, { from: notOwner })
+    );
+
+    await assertRevert(
+      this.ownable.removeOwner(owner, { from: notOwner })
+    );
+  });
+
+  it('should allow existing owner to add and remove a newOwner', async function () {
+    await this.ownable.addOwner(newOwner, { from: owner });
+    let hasRole = await this.ownable.hasRole(newOwner, this.roleOwner);
+    hasRole.should.eq(true);
+
+    await this.ownable.removeOwner(newOwner, { from: owner });
+    hasRole = await this.ownable.hasRole(newOwner, this.roleOwner);
+    hasRole.should.eq(false);
+  });
+});