enh: better use of account creation metadata

Store account creation information in a JSON.
Display this information in `accountInfo` for auditors.

bin/admin/install

@@ -1127,6 +1127,49 @@ if [ "$nothing" = 0 ]; then
         action_done
     fi
 
+    # The Bastion > v3.04.00
+    action_doing "Migrating account creation metadata to new format where applicable"
+    at_least_one_changed=0
+    at_least_one_error=0
+    for accounthome in $(getent passwd | grep ":$basedir/bin/shell/osh.pl$" | cut -d: -f6); do
+        # if the old AND the new files exists, don't do anything: we won't delete the old file
+        # because we want our users to be able to roll back to an old version, and as this file doesn't
+        # hurt anything (it'll just get ignored by new versions), no point in inserting an incompatible change here.
+        if [ -f "$accounthome/accountCreate.comment" ] && ! [ -f "$accounthome/config.creation_info" ]; then
+            at_least_one_changed=$((at_least_one_changed + 1))
+            # build a config.creation_info file from the data stored in the legacy accountCreate.comment
+            perl -Mstrict -MJSON -mPOSIX -e '
+                my %h;
+                open(my $oldfh, "<", $ARGV[0]) or die $!;
+                while (<$oldfh>) {
+                    chomp;
+                    my @f = split("=", $_, 2);
+                    $h{$f[0]}=$f[1] if @f == 2;
+                }
+                close($oldfh);
+                open(my $newfh, ">", $ARGV[1]) or die $!;
+                print $newfh encode_json({
+                    by => $h{CREATED_BY},
+                    bastion_version => $h{BASTION_VERSION},
+                    datetime_utc => POSIX::strftime("%a %Y-%m-%d %H:%M:%S UTC", gmtime($h{CREATION_TIMESTAMP})),
+                    datetime_local => POSIX::strftime("%a %Y-%m-%d %H:%M:%S %Z", localtime($h{CREATION_TIMESTAMP})),
+                    timestamp => $h{CREATION_TIMESTAMP}+0,
+                    comment => (($h{COMMENT} && $h{COMMENT} ne "(no_comment_provided)") ? $h{COMMENT} : undef),
+                });
+                close($newfh);
+            ' "$accounthome/accountCreate.comment" "$accounthome/config.creation_info"; ret=$?
+            [ -f "$accounthome/config.creation_info" ] && chmod 644 "$accounthome/config.creation_info"
+            [ "$ret" != 0 ] && at_least_one_error=1
+        fi
+    done
+    if [ "$at_least_one_error" != 0 ]; then
+        action_error
+    elif [ "$at_least_one_changed" = 0 ]; then
+        action_na
+    else
+        action_done "Migrated $at_least_one_changed files"
+    fi
+
     # checking whether we have things to install from the install.d directory
     # shellcheck disable=SC2034
     STARTED_BY_MAIN_INSTALL=1

bin/helper/osh-accountCreate

@@ -9,6 +9,8 @@
 use common::sense;
 use Getopt::Long;
 use Sys::Hostname ();
+use JSON;
+use POSIX ();
 
 use File::Basename;
 use lib dirname(__FILE__) . '/../../lib/perl';
@@ -310,21 +312,27 @@ if (ref $config->{'accountCreateDefaultPersonalAccesses'} eq 'ARRAY' && $type eq
     }
 }
 
-if (not defined $comment) {
+# store some metadata
-    $comment = '(no_comment_provided)';
+my $creation_time = time();
-}
+my %metadata      = (
-
+    by              => $self,
-$comment = "CREATED_BY=$self\nBASTION_VERSION=" . $OVH::Bastion::VERSION . "\nCREATION_TIME=" . localtime() . "\nCREATION_TIMESTAMP=" . time() . "\nCOMMENT=" . $comment . "\n";
+    bastion_version => $OVH::Bastion::VERSION,
-
+    datetime_utc    => POSIX::strftime("%a %Y-%m-%d %H:%M:%S UTC", gmtime($creation_time)),
-if (open(my $fh_comment, '>>', $homedir . '/accountCreate.comment')) {
+    datetime_local  => POSIX::strftime("%a %Y-%m-%d %H:%M:%S %Z", localtime($creation_time)),
-    print $fh_comment $comment;
+    timestamp       => $creation_time,
-    close $fh_comment;
+    comment         => $comment,
-    chmod 0644, $homedir . '/accountCreate.comment';
+);
+$fnret = OVH::Bastion::account_config(account => $account, key => "creation_info", value => encode_json(\%metadata));
+if (!$fnret) {
+    osh_warn("Couldn't set creation_info account metadata, continuing anyway");
+    warn_syslog("While creating account '$account', couldn't set creation_info metadata: " . $fnret->msg);
 }
 
+# also store the bare creation timestamp by itself, it's used by osh.pl for TTL accounts
 $fnret = OVH::Bastion::account_config(account => $account, key => "creation_timestamp", value => time());
 if (!$fnret) {
     osh_warn("Couldn't store creation timestamp (" . $fnret->msg . "), continuing anyway");
+    warn_syslog("While creating account '$account', couldn't store creaation timestamp: " . $fnret->msg);
 }
 
 if ($ttl) {
@@ -332,7 +340,8 @@ if ($ttl) {
     osh_info sprintf("Setting this account TTL (will expire in %s)", $fnret->value->{'human'});
     $fnret = OVH::Bastion::account_config(account => $account, key => "account_ttl", value => $ttl);
     if (!$fnret) {
-        osh_warn("Couldn't store account TTL (" . $fnret->msg . "), this account will NOT expire!! Continuing anyway");
+        osh_warn("Couldn't store account TTL (" . $fnret->msg . "), this account will NOT expire, continuing anyway");
+        warn_syslog("Couldn't store account TTL (" . $fnret->msg . ") while creating account '$account', this account will NOT expire, continuing anyway");
     }
 }
 
@@ -340,6 +349,7 @@ if ($alwaysActive || $type eq 'realm') {
     $fnret = OVH::Bastion::account_config(account => $account, key => OVH::Bastion::OPT_ACCOUNT_ALWAYS_ACTIVE, value => "yes", public => 1);
     if (!$fnret) {
         osh_warn("Couldn't store always_active flag (" . $fnret->msg . "), continuing anyway");
+        warn_syslog("Couldn't store always_active flag (" . $fnret->msg . ") while creating account '$account', continuing anyway");
     }
 }
 
@@ -409,7 +419,7 @@ if ($type eq 'realm') {
 }
 else {
     osh_info "==> alias $bastionName='$bastionCommand'";
-    osh_info "To test his access, ask this user to set the above alias in his .bash_aliases, then run `$bastionName --osh info'";
+    osh_info "To test his access, ask this user to set the above alias in their .bash_aliases, then run `$bastionName --osh info'";
 }
 
 OVH::Bastion::syslogFormatted(

bin/plugin/restricted/accountInfo

@@ -3,6 +3,8 @@
 use common::sense;
 use Sys::Hostname ();
 use Term::ANSIColor;
+use JSON;
+use POSIX ();
 
 use File::Basename;
 use lib dirname(__FILE__) . '/../../../lib/perl';
@@ -27,6 +29,9 @@ my $fnret;
 
 $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account);
 $fnret or osh_exit $fnret;
+$account = $fnret->value->{'account'};
+my $sysaccount    = $fnret->value->{'sysaccount'};
+my $remoteaccount = $fnret->value->{'remoteaccount'};
 
 $fnret = OVH::Bastion::get_plugin_list(restrictedOnly => 1);
 $fnret or osh_exit $fnret;
@@ -110,7 +115,8 @@ else {
 }
 
 if (OVH::Bastion::is_auditor(account => $self)) {
-    $fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $account);
+
+    $fnret = OVH::Bastion::is_account_nonexpired(sysaccount => $sysaccount, remoteaccount => $remoteaccount);
     if ($fnret->is_ok) {
         osh_info "This account is " . colored('not expired', 'green');
         $ret{'is_expired'} = 0;
@@ -128,23 +134,58 @@ if (OVH::Bastion::is_auditor(account => $self)) {
         osh_info "As a consequence, this account " . ($canConnect ? colored("can", 'green') : colored("CANNOT", 'red')) . " connect to this bastion\n\n";
         $ret{'can_connect'} = $canConnect;
 
-        my $seenBefore = 1;
         if ($fnret->value->{'already_seen_before'}) {
-            osh_info "This account has already been used " . colored('at least once', 'green');
             $ret{'already_seen_before'} = 1;
+            if (defined $fnret->value->{'seconds'}) {
+                $fnret = OVH::Bastion::duration2human(seconds => $fnret->value->{'seconds'}, tense => "past");
+                if ($fnret) {
+                    my $seenBeforeStr = $fnret->value->{'datetime_utc'};
+                    if ($fnret->value->{'datetime_local'} && $fnret->value->{'datetime_utc'} ne $fnret->value->{'datetime_local'}) {
+                        $seenBeforeStr .= " / " . $fnret->value->{'datetime_local'};
+                    }
+                    $seenBeforeStr = sprintf("Last seen on %s (%s ago)", colored($seenBeforeStr, 'magenta'), $fnret->value->{'duration'},);
+                    osh_info $seenBeforeStr;
+                    $ret{'last_activity'}{$_}          = $fnret->value->{$_} for qw{ datetime_local datetime_utc };
+                    $ret{'last_activity'}{'ago'}       = $fnret->value->{'duration'};
+                    $ret{'last_activity'}{'timestamp'} = time() - $fnret->value->{'seconds'};
+                }
+            }
         }
         else {
             osh_info "This account has " . colored('NEVER', 'red') . " been used (yet)";
-            $seenBefore = 0;
             $ret{'already_seen_before'} = 0;
         }
+    }
 
-        if (defined $fnret->value->{'seconds'}) {
+    $fnret = OVH::Bastion::account_config(account => $account, key => "creation_info");
-            $fnret = OVH::Bastion::duration2human(seconds => $fnret->value->{'seconds'}, tense => "past");
+    if ($fnret) {
-            if ($fnret) {
+        my $creation_info;
-                my $seenBeforeStr = sprintf("%s on %s (%s ago)", $seenBefore ? "Last seen" : "Created", colored($fnret->value->{'date'}, 'magenta'), $fnret->value->{'duration'});
+        eval { $creation_info = decode_json($fnret->value); };
-                osh_info $seenBeforeStr;
+        if ($@) {
-                $ret{'last_activity_date'} = $seenBeforeStr;
+            osh_warn("While reading creation metadata information for account '$account', couldn't decode JSON: $@");
+        }
+        else {
+            $ret{'creation_information'} = $creation_info;
+            if ($creation_info->{'time_utc'}) {
+                my $createdOnStr = $creation_info->{'time_utc'};
+                if ($creation_info->{'time_local'} && $creation_info->{'time_utc'} ne $creation_info->{'time_local'}) {
+                    $createdOnStr .= " / " . $creation_info->{'time_local'};
+                }
+                $createdOnStr = sprintf(
+                    "Created on %s (%s ago)",
+                    colored($createdOnStr, 'magenta'),
+                    OVH::Bastion::duration2human(seconds => time() - $creation_info->{'timestamp'})->value->{'duration'}
+                );
+                osh_info $createdOnStr;
+            }
+            if ($creation_info->{'by'}) {
+                osh_info "Created by " . colored($creation_info->{'by'}, 'magenta');
+            }
+            if ($creation_info->{'bastion_version'}) {
+                osh_info "Created using The Bastion " . colored('v' . $creation_info->{'bastion_version'}, 'magenta');
+            }
+            if ($creation_info->{'comment'}) {
+                osh_info "Creation with the following comment: " . colored($creation_info->{'comment'}, 'magenta');
             }
         }
     }

lib/perl/OVH/Bastion/allowdeny.inc

@@ -472,7 +472,8 @@ sub duration2human {
     my $tense  = $params{'tense'};
 
     require POSIX;
-    my $date = POSIX::strftime("%a %Y-%m-%d %H:%M:%S %Z", localtime(time() + ($tense eq 'past' ? -$s : $s)));
+    my $date_local = POSIX::strftime("%a %Y-%m-%d %H:%M:%S %Z", localtime(time() + ($tense eq 'past' ? -$s : $s)));
+    my $date_utc = POSIX::strftime("%a %Y-%m-%d %H:%M:%S UTC", gmtime(time() + ($tense eq 'past' ? -$s : $s)));
 
     my $d = int($s / 86400);
     $s -= $d * 86400;
@@ -482,7 +483,9 @@ sub duration2human {
     $s -= $m * 60;
 
     my $duration = $d ? sprintf('%dd+%02d:%02d:%02d', $d, $h, $m, $s) : sprintf('%02d:%02d:%02d', $h, $m, $s);
-    return R('OK', value => {duration => $duration, date => $date, human => "$duration ($date)"});
+
+    # we keep the 'date' key for backwards compat, it's the same as 'datetime_local'
+    return R('OK', value => {duration => $duration, date => $date_local, datetime_local => $date_local, datetime_utc => $date_utc, human => "$duration ($date_local)"});
 }
 
 sub print_acls {

tests/functional/tests.d/325-accountinfo.sh

@@ -0,0 +1,62 @@
+# vim: set filetype=sh ts=4 sw=4 sts=4 et:
+# shellcheck shell=bash
+# shellcheck disable=SC2086,SC2016,SC2046
+# below: convoluted way that forces shellcheck to source our caller
+# shellcheck source=tests/functional/launch_tests_on_instance.sh
+. "$(dirname "${BASH_SOURCE[0]}")"/dummy
+
+testsuite_accountinfo()
+{
+    grant accountCreate
+    # create regular account to compare info access between auditor and non auditor
+    success 325-accountinfo a0_create_a1 $a0 --osh accountCreate --always-active --account $account1 --uid $uid1 --public-key "\"$(cat $account1key1file.pub)\""
+    json .error_code OK .command accountCreate .value null
+
+    # create another target account we'll use for accountInfo
+    success 325-accountinfo a0_create_a2 $a0 --osh accountCreate --always-active --account $account2 --uid $uid2 --public-key "\"$(cat $account2key1file.pub)\"" --comment "\"'this is a comment'\""
+    json .error_code OK .command accountCreate .value null
+    revoke accountCreate
+
+    # grant account0 as admin
+    success 325-accountinfo set_a0_as_admin $r0 "\". $opt_remote_basedir/lib/shell/functions.inc; add_user_to_group_compat $account0 osh-admin\""
+    configchg 's=^\\\\x22adminAccounts\\\\x22.+=\\\\x22adminAccounts\\\\x22:[\\\\x22'"$account0"'\\\\x22],='
+
+    # grant account1 as auditor
+    success 325-accountinfo a0_grant_a1_as_auditor $a0 --osh accountGrantCommand --command auditor --account $account1
+
+    # grant accountInfo to a0 and a1
+    success 325-accountinfo a0_grant_a0_accountinfo $a0 --osh accountGrantCommand --command accountInfo --account $account0
+    success 325-accountinfo a0_grant_a1_accountinfo $a0 --osh accountGrantCommand --command accountInfo --account $account1
+
+    # a0 should see basic info about a2
+    success 325-accountinfo a0_accountinfo_a2_basic $a0 --osh accountInfo --account $account2
+    json_document '{"error_message":"OK","command":"accountInfo","error_code":"OK","value":{"always_active":1,"is_active":1,"allowed_commands":[],"groups":{}}}'
+
+    # a1 should see detailed info about a2
+    success 325-accountinfo a1_accountinfo_a2_detailed $a1 --osh accountInfo --account $account2
+    json .error_code OK .command accountInfo .value.always_active 1 .value.is_active 1 .value.allowed_commands "[]" .value.groups "{}"
+    json .value.ingress_piv_policy null .value.personal_egress_mfa_required none .value.pam_auth_bypass 0
+    json .value.password.min_days 0 .value.password.warn_days 7 .value.password.user "$account2" .value.password.password locked
+    json .value.password.inactive_days -1 .value.password.date_disabled null .value.password.date_disabled_timestamp 0 .value.password.date_changed $(date +%Y-%m-%d)
+    json .value.ingress_piv_enforced 0 .value.always_active 1 .value.creation_information.by "$account0"
+    json .value.creation_information.comment "this is a comment"
+    json .value.already_seen_before 0 .value.last_activity null
+
+    # a2 connects, which will update already_seen_before
+    success 325-accountinfo a2_connects $a2 --osh info
+    json .command info .error_code OK
+
+    # a1 should see the updated fields
+    success 325-accountinfo a1_accountinfo_a2_detailed2 $a1 --osh accountInfo --account $account2
+    json .value.already_seen_before 1
+    contain "Last seen on"
+
+    # delete account1 & account2
+    grant accountDelete
+    success 325-accountinfo a0_delete_a1 $a0 --osh accountDelete --account $account1 --no-confirm
+    success 325-accountinfo a0_delete_a2 $a0 --osh accountDelete --account $account2 --no-confirm
+    revoke accountDelete
+}
+
+testsuite_accountinfo
+unset -f testsuite_accountinfo