The vault from flaviovs

Remove the `secrets.pinged` column

App ping is mandatory for now.

The system does not work properly when installed in a subdirectory

(...)like installing it at https://example.com/vault instead of at https//example.com.

Pass the unlock key implicitly in the unlock URL

This will remove the need of the HE to copy and paste the unlock key. The secret will be unlocked by just clicking on the URL.

Remove unused VaultClient::INSTRUCTIONS_ALLOWED_TAGS constant

`htmlspecialchars()` is a bit fragile without `ENT_QUOTES`

By default, htmlspecialchars() only encodes double quotes. The views in vault only use double quotes in attributes, so it's not currently a problem, but the code is fragile: some future change might introduce a single quote.

Convert table "id" fields to the format "table_name_id"

Instructions/secret word wrap inconsistency

Instruction and secret textarea input does not specify any word wrapping. Long lines being input in these fields are being displayed using pre-formated output, which causes the text to overflow and break the layout.

Schema Questions

References

I don't think MySQL accepts REFERENCES outside of FOREIGN KEY statements. Can you look into this?

vault/lib/schema.php

Line 22 in 5455673

REFERENCES app,

LogLevel

Is this table necessary? Seems like it might be over-normalization.

Add a note about greylisting in the email feedback messages

Add an 'unlock' call to the API

Ping URL must be mandatory

There's no point in having an app without a ping URL, because the app is responsible to notify engineers upon secret input, and it can only get to know this by being pinged by the Vault.

Fix app (when available) not being recorded in the logs

Add a `secret add` command to the CLI

XSS when displaying instructions to user.

HEs can XSS users by including event attributes in the the instructions markup.

<b onmouseover=alert(1)>Hover over this</b>

This isn't problematic now, but it's unclear what tools in the future may accept user input to pre-generate instructions.

Refactor logging -- separate general request logging from audit records

Inconsistent Entity Properties?

Why are some App properties (key, secret, ...) assigned in the constructor and others (ping_url) not?

vault/lib/entity.php

Lines 13 to 18 in 5455673

    
           public function __construct( $key, $secret, $vault_secret, $name ) { 
        
           	$this->key = $key; 
        
           	$this->secret = $secret; 
        
           	$this->vault_secret = $vault_secret; 
        
           	$this->name = $name; 
        
           }

The same question can be asked of several of the other classes. Is there some guiding principal about when to put something in the constructor, when to set properties manually, and when to offer a setter method?

I can't create any secrets

vault/lib/repo.php

Lines 118 to 126 in 5455673

    
           $this->db->perform( 'INSERT into secrets ' 
        
                               . '(reqid, secret, mac, created) ' 
        
                               . 'VALUES (?, ?, ?, ?)', 
        
                               [ 
        
           	                    $secret->reqid, 
        
           	                    $secret->secret, 
        
           	                    $secret->mac, 
        
           	                    $secret->created->format( \DateTime::ISO8601 ), 
        
                               ] );

is failing because \DateTime::ISO8601 is not a valid datetime format according to MySQL.

[CRITICAL] www: exception 'PDOException' with message 'SQLSTATE[22007]: Invalid datetime format: 1292 Incorrect datetime value: '2016-03-03T07:34:08+00:00'

The +00:00 is what's breaking it for me. Changing the format to 'Y-m-d H:i:s' works for me.

What version of MySQL are you using?

Why rethrow `ConfigException` in `Mailer`?

vault/lib/util.php

Lines 47 to 57 in 5455673

    
           try { 
        
           	$from_address = $conf->get( 'mailer', 'from_address' ); 
        
           } catch ( ConfigException $ex ) { 
        
           	throw new VaultException( 'Missing from_address mailer configuration' ); 
        
           } 
        
           try { 
        
           	$from_name = $conf->get( 'mailer', 'from_name' ); 
        
           } catch ( ConfigException $ex ) { 
        
           	throw new VaultException( 'Missing from_name mailer configuration' ); 
        
           }

Could the message in ConfigException be expanded, or is it better to rethrow a new VaultException?

When should we use which?

Just 1 package, instead of 2
More easy to understand the whole system
Thus potentially easier to deploy
No loss of decoupling -- clients still communicate with the engine through API

Steps:

Move app to client/ folder
Move client-specific files in lib/
Check for common code
Update documentation

Cc @mdawaffe

	public function __construct( $key, $secret, $vault_secret, $name ) {
	$this->key = $key;
	$this->secret = $secret;
	$this->vault_secret = $vault_secret;
	$this->name = $name;
	}

	$this->db->perform( 'INSERT into secrets '
	. '(reqid, secret, mac, created) '
	. 'VALUES (?, ?, ?, ?)',
	[
	$secret->reqid,
	$secret->secret,
	$secret->mac,
	$secret->created->format( \DateTime::ISO8601 ),
	] );

	try {
	$from_address = $conf->get( 'mailer', 'from_address' );
	} catch ( ConfigException $ex ) {
	throw new VaultException( 'Missing from_address mailer configuration' );
	}

	try {
	$from_name = $conf->get( 'mailer', 'from_name' );
	} catch ( ConfigException $ex ) {
	throw new VaultException( 'Missing from_name mailer configuration' );
	}

flaviovs / vault Goto Github PK

vault's People

Contributors

Watchers

vault's Issues

References

LogLevel

Recommend Projects

Recommend Topics

Recommend Org