Procházet zdrojové kódy

Two factors or nothing

TOTP was optional, which meant it was a thing most people did not have.  Now
everybody has it: a login with no enrolment still gets a session -- it has to,
because the QR is the shared secret and a login is what you present to be shown
it -- but it lands on /totp, and the dispatcher refuses every other route behind
a login until there is a secret to check against.  The exemption for the
enrolment page and its QR is written on those routes rather than in a list in
TCMS.pm which the routing table knows nothing about.

The page to turn it off is gone with it, along with the change request type, the
mail templates and Trog::Auth::clear_totp.  Losing an authenticator is not a
reason to stop having a second factor, and it was never the recovery path
anyway: bin/totp reads a user their enrolment back out of band, and since the
secret does not change that is the same enrolment rather than a new one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADwKSaBSvUTdA3RtGQeNau
George Baugh před 1 měsícem
rodič
revize
9ad58bf016

+ 8 - 3
design.md

@@ -401,9 +401,14 @@ against it -- which is how private uploads under `www/assets/private` are gated
 without tCMS being in the path of the file itself. That protection is real and it
 is invisible from inside the application, so code that reasons about what is
 reachable should not assume it and should not try to replace it.
-TOTP two-factor is supported. The first user to register becomes the
-administrator and registration then closes; further users are made by an admin
-through the `about` post type -- because a user *is* their profile page.
+TOTP two-factor is mandatory. A login without an enrolment gets a session and is
+sent to `/totp`, and the dispatcher refuses every other route behind a login
+until they have one -- that window exists at all because the QR *is* the shared
+secret, so a login is what you have to present to be shown it. There is no way to
+turn it back off; `bin/totp` reads a user their existing enrolment back out of
+band, which is what covers a lost authenticator. The first user to register
+becomes the administrator and registration then closes; further users are made by
+an admin through the `about` post type -- because a user *is* their profile page.
 
 **Config** (`Trog::Config`) is a thin wrapper over `Config::Simple` reading
 `config/main.cfg` and falling back to the shipped `config/default.cfg`. It is

+ 27 - 0
lib/TCMS.pm

@@ -126,6 +126,11 @@ sub build_routes {
             # Now that we have firmed up the actual routing, let's validate.
             return $tpsgi->forbidden($query) if exists $query->{dispatcher}{auth} && !$active_user;
 
+            if ( needs_enrolment( $active_user, $query->{dispatcher} ) ) {
+                INFO("$active_user has no TOTP enrolment; sending them to /totp");
+                return $tpsgi->see_also('/totp');
+            }
+
             no strict 'refs';
             $callback->($query);
             use strict;
@@ -141,6 +146,28 @@ sub build_routes {
     return [%routes_adj];
 }
 
+=head2 needs_enrolment($user, $dispatcher) = BOOL
+
+Whether this request should be turned around and sent to /totp.
+
+Two factors or nothing.  A login is what gets you to the enrolment page -- the QR
+is the shared secret, so it cannot be handed to whoever asks for it -- which
+leaves a window where somebody is logged in and has no second factor yet.  This
+is what makes that window one page wide: every route a login can reach, and no
+route a logged out visitor can, is refused until they have enrolled.
+
+The enrolment page and the QR on it say so themselves, with totp_exempt, because
+the exemption belongs next to the route rather than in a list over here that the
+routing table knows nothing about.
+
+=cut
+
+sub needs_enrolment ( $user, $dispatcher ) {
+    return 0 unless $user;
+    return 0 unless $dispatcher && $dispatcher->{auth} && !$dispatcher->{totp_exempt};
+    return Trog::Auth::has_totp($user) ? 0 : 1;
+}
+
 # Override the generic error handler to look spiffy
 sub generic_route ( $rname, $code, $title, $query ) {
     $query->{code} = $code;

+ 16 - 14
lib/Trog/Auth.pm

@@ -249,16 +249,26 @@ sub _totp {
     return $totp;
 }
 
-=head2 clear_totp
+=head2 has_totp(user) = BOOL
 
-Clear the totp codes for provided user
+Whether this user has enrolled in TOTP yet.
+
+Everybody has to, so this is the question the dispatcher asks on the way to
+every page a login can reach: a user who has not is sent to /totp and can go
+nowhere else until they have.
+
+There is no way to turn it back off.  Losing an authenticator is not a reason to
+stop having a second factor, and the answer to it is bin/totp, which re-reads a
+user their enrolment out of band -- the secret is still in the database, so it
+is the same enrolment rather than a new one.
 
 =cut
 
-sub clear_totp ($user) {
-    my $dbh = _dbh();
-    my $res = $dbh->do( "UPDATE user SET totp_secret=null WHERE name=?", undef, $user ) or die "Could not clear user TOTP secrets";
-    return !!$res;
+sub has_totp ($user) {
+    my $dbh  = _dbh();
+    my $rows = $dbh->selectall_arrayref( "SELECT totp_secret FROM user WHERE name=?", { Slice => {} }, $user );
+    return 0 unless ref $rows eq 'ARRAY' && @$rows;
+    return $rows->[0]{totp_secret} ? 1 : 0;
 }
 
 =head2 mksession(user, pass, token) = STRING
@@ -393,14 +403,6 @@ sub process_change_request ($token) {
             killsession($user);
             return "Password set to $pass for $user";
         },
-        clear_totp => sub {
-            my ($user) = @_;
-            clear_totp($user) or do {
-                return '';
-            };
-            killsession($user);
-            return "TOTP auth turned off for $user";
-        },
     );
     my $res = $dispatch{$type}->( $user, $secret );
     $dbh->do( "UPDATE change_request SET processed=1 WHERE token=?", undef, $token ) or do {

+ 28 - 40
lib/Trog/Routes/HTML.pm

@@ -78,9 +78,12 @@ our %routes = (
         noindex  => 1,
     },
     '/totp' => {
-        method   => 'GET',
-        auth     => 1,
-        callback => \&Trog::Routes::HTML::totp,
+        method => 'GET',
+        auth   => 1,
+
+        # The one page a user who has not enrolled yet is allowed to reach.
+        totp_exempt => 1,
+        callback    => \&Trog::Routes::HTML::totp,
     },
     '/post/save' => {
         method   => 'POST',
@@ -136,12 +139,6 @@ our %routes = (
         noindex  => 1,
         nocache  => 1,
     },
-    '/request_totp_clear' => {
-        method   => 'POST',
-        callback => \&Trog::Routes::HTML::do_totp_clear,
-        noindex  => 1,
-        nocache  => 1,
-    },
     '/processed' => {
         method   => 'GET',
         callback => \&Trog::Routes::HTML::processed,
@@ -275,9 +272,12 @@ our %routes = (
         callback => \&Trog::Routes::HTML::icon,
     },
     '/totp_qr/(.*)' => {
-        auth     => 1,
-        method   => 'GET',
-        callback => \&Trog::Routes::HTML::totp_qr,
+        auth   => 1,
+        method => 'GET',
+
+        # The QR on that page, so it has to be reachable from it.
+        totp_exempt => 1,
+        callback    => \&Trog::Routes::HTML::totp_qr,
     },
     '/styles/rss-style.xsl' => {
         method   => 'GET',
@@ -586,6 +586,11 @@ sub setup ($query) {
 Enable 2 factor auth via TOTP for the currently authenticated user.
 Returns a page with a QR code & TOTP uri for pasting into your authenticator app of choice.
 
+Every user has to enrol, so this is where a login without one lands and the only
+page it can reach until it has one.  Showing the same enrolment again is the
+point rather than a fallback: a user who lost their authenticator scans it again
+and is back where they were.
+
 =cut
 
 sub totp ($query) {
@@ -657,6 +662,12 @@ sub login ($query) {
         my $cookie = Trog::Auth::mksession( $query->{username}, $query->{password}, $query->{token}, $query->{ip} // '' );
         if ($cookie) {
 
+            # A login is how you get to enrol, since the QR is not something to
+            # hand out to whoever asks for it -- so a user with no enrolment yet
+            # gets a session and goes straight there rather than where they were
+            # headed.  The dispatcher keeps them there until they have one.
+            $query->{to} = '/totp' unless Trog::Auth::has_totp( $query->{username} );
+
             # TODO secure / sameSite cookie to kill csrf, maybe do rememberme with Expires=~0
             my $secure = '';
             $secure  = '; Secure' if $query->{scheme} eq 'https';
@@ -901,10 +912,13 @@ sub _config_title ($name) {
 
 =head2 do_resetpass
 
-=head2 do_totp_clear
-
 Routes for user service of their authentication details.
 
+There is no route here for turning TOTP off.  Everybody has it, and losing an
+authenticator is not a reason to stop having a second factor -- bin/totp reads a
+user their existing enrolment back out of band, which is the same enrolment
+rather than a new one.
+
 =cut
 
 sub resetpass ($query) {
@@ -957,32 +971,6 @@ sub do_resetpass ($query) {
     return $query->{tpsgi}->see_also("/processed");
 }
 
-sub do_totp_clear ($query) {
-    my $user = $query->{username};
-
-    # User Does not exist
-    return $query->{tpsgi}->forbidden($query) if !Trog::Auth::user_exists($user);
-
-    # User exists, but is not logged in this session
-    return $query->{tpsgi}->forbidden($query) if !$query->{user} && Trog::Auth::user_has_session($user);
-
-    my $token = Trog::Utils::uuid();
-    my $res   = Trog::Auth::add_change_request( type => 'clear_totp', user => $user, token => $token );
-    die "Could not add auth change request!" unless $res;
-
-    # If the user is logged in, just do the deed, otherwise send them the token in an email
-    if ( $query->{user} ) {
-        return $query->{tpsgi}->see_also("/api/auth_change_request/$token");
-    }
-    Trog::Email::contact(
-        $user,
-        "root\@$query->{domain}",
-        "$query->{domain}: Password reset URL for $user",
-        { uri => "$query->{scheme}://$query->{domain}/api/auth_change_request/$token", template => 'totp_reset.tx' }
-    );
-    return $query->{tpsgi}->see_also("/processed");
-}
-
 sub _get_series ( $edit = 0 ) {
     state $data;
     $data //= Trog::Data->new( Trog::Config::get() );

+ 139 - 0
t/Trog-Auth-totp-mandatory.t

@@ -0,0 +1,139 @@
+#!/usr/bin/env perl
+
+# TOTP is not optional here.  These tests are about the window between "logged
+# in" and "enrolled", which exists because a login is what gets you the QR --
+# and about the fact that there is no longer any way to turn 2fa back off.
+
+use strict;
+use warnings;
+
+use FindBin;
+use lib "$FindBin::Bin/../lib";
+
+use Test::More;
+use Test::MockModule qw{strict};
+
+our ( $REPO, $ROOT, $OLDCWD );
+
+BEGIN {
+    require Cwd;
+    require File::Copy;
+    require File::Path;
+    require File::Temp;
+
+    $REPO   = Cwd::abs_path("$FindBin::Bin/..");
+    $OLDCWD = Cwd::getcwd();
+    $ROOT   = File::Temp::tempdir( 'tcms-totp-XXXXXX', TMPDIR => 1, CLEANUP => 1 );
+
+    # Every path in this app is cwd relative, the auth db included, so the tests
+    # stand in an instance root of their own rather than in the checkout.  This
+    # is a BEGIN because TCMS.pm builds its routing table at file scope, against
+    # whatever cwd is current when it loads.
+    #
+    # Text::Xslate works out its cache directory from HOME once, at load time.
+    $ENV{HOME} = $ROOT;
+    File::Path::make_path( map { "$ROOT/$_" } qw{config schema data/files logs totp www/statics www/themes} );
+
+    # Hardcoded paths in Auth.pm, TagIndex.pm, the data models and Log.pm.
+    File::Copy::copy( "$REPO/schema/$_", "$ROOT/schema/$_" ) or die $! for qw{auth.schema flatfile.schema log.schema};
+
+    open( my $fh, '>', "$ROOT/config/default.cfg" ) or die $!;
+    print {$fh} "[general]\n    data_model=FlatFile\n    hostname=totp.example.com\n";
+    close $fh;
+
+    # Otherwise login() reads the instance as a fresh install and registers
+    # whoever knocks first as the admin, which is a different test.
+    open( $fh, '>', "$ROOT/config/has_users" ) or die $!;
+    close $fh;
+
+    chdir($ROOT) or die "could not chdir to the sandbox: $!";
+}
+
+END {
+    chdir($OLDCWD) if $OLDCWD;
+}
+
+require_ok('Trog::Auth') or BAIL_OUT("Can't find SUT");
+require_ok('TCMS')       or BAIL_OUT("Can't load the dispatcher");
+require_ok('Trog::Routes::HTML');
+
+Trog::Auth::useradd( 'bob', 'Bob Bobson', 'hunter2', ['admin'], 'bob@example.com' )
+  or BAIL_OUT('could not make a user to test with');
+
+subtest 'a user starts out unenrolled' => sub {
+    is( Trog::Auth::has_totp('bob'),           0, 'a fresh user has no second factor' );
+    is( Trog::Auth::has_totp('nobody-at-all'), 0, 'nor does somebody who does not exist' );
+
+    # Which is the only reason logging in without a token works at all: it is
+    # how you get to the page that gives you one.
+    ok( Trog::Auth::mksession( 'bob', 'hunter2', '', '127.0.0.1' ), 'and can still log in, with no token' );
+};
+
+subtest 'and cannot go anywhere but /totp until they enrol' => sub {
+    my %page = ( auth => 1 );
+    my %public;
+    my %enrol = ( auth => 1, totp_exempt => 1 );
+
+    is( TCMS::needs_enrolment( 'bob', \%page ),   1, 'a page behind a login is refused' );
+    is( TCMS::needs_enrolment( 'bob', \%enrol ),  0, 'the enrolment page is not, or there would be nowhere to go' );
+    is( TCMS::needs_enrolment( 'bob', \%public ), 0, 'and a public page is nobody\'s business' );
+
+    # Nothing to enrol, and nothing to protect.
+    is( TCMS::needs_enrolment( '', \%page ), 0, 'a logged out visitor is left alone' );
+
+    my ( $uri, $qr, $failure, $message ) = Trog::Auth::totp( 'bob', 'totp.example.com' );
+    ok( $uri, 'enrolling gives them a URI' ) or diag($message);
+    is( Trog::Auth::has_totp('bob'), 1, 'and they are enrolled' );
+
+    is( TCMS::needs_enrolment( 'bob', \%page ), 0, 'after which every page opens up' );
+};
+
+subtest 'the enrolment does not change' => sub {
+
+    # A user who lost their authenticator scans the same code into the next one,
+    # which is what makes having no way to turn 2fa off a supportable position.
+    my ( $uri, $qr, $failure ) = Trog::Auth::totp( 'bob', 'totp.example.com' );
+    my ( $again, $qr2 ) = Trog::Auth::totp( 'bob', 'totp.example.com' );
+    is( $again,   $uri, 'asking again gives back the enrolment they already have' );
+    is( $qr2,     $qr,  'and the same QR' );
+    is( $failure, -1,   'reported as nothing new having been generated' );
+};
+
+subtest 'and there is no way to turn it off' => sub {
+    ok( !Trog::Auth->can('clear_totp'),                             'Trog::Auth cannot clear a secret' );
+    ok( !Trog::Routes::HTML->can('do_totp_clear'),                  'nor is there a route handler to ask it to' );
+    ok( !exists $Trog::Routes::HTML::routes{'/request_totp_clear'}, 'nor a route' );
+
+    # The two the gate lets through, which is the whole of the exemption.
+    ok( $Trog::Routes::HTML::routes{'/totp'}{totp_exempt},         'the enrolment page says it is exempt' );
+    ok( $Trog::Routes::HTML::routes{'/totp_qr/(.*)'}{totp_exempt}, 'and so does the QR on it' );
+    my @exempt = grep { $Trog::Routes::HTML::routes{$_}{totp_exempt} } keys(%Trog::Routes::HTML::routes);
+    is( scalar(@exempt), 2, 'and nothing else is' );
+};
+
+subtest 'a login without an enrolment is sent to enrol' => sub {
+    my $tpsgi = bless {}, 'FakeTPSGI';
+    {
+        no warnings qw{once};
+        *FakeTPSGI::see_also = sub { return [ 303, [ Location => $_[1] ], [''] ] };
+    }
+
+    # login() renders rather than redirects on success -- the redirect is the
+    # 'to' the page then sends the browser to -- so that is what to look at.
+    my $rendered;
+    my $renderer = Test::MockModule->new('Trog::Renderer');
+    $renderer->redefine( render => sub { my ( $class, %args ) = @_; $rendered = $args{data}; return [ 200, [], [''] ] } );
+
+    my $auth = Test::MockModule->new('Trog::Auth');
+    $auth->redefine( mksession => sub { return 'a-session' } );
+    $auth->redefine( has_totp  => sub { return 0 } );
+
+    Trog::Routes::HTML::login( { username => 'bob', password => 'hunter2', to => '/config', tpsgi => $tpsgi, scheme => 'http', route => '/auth' } );
+    is( $rendered->{to}, '/totp', 'a login with nothing enrolled goes to enrol, not where it was headed' );
+
+    $auth->redefine( has_totp => sub { return 1 } );
+    Trog::Routes::HTML::login( { username => 'bob', password => 'hunter2', to => '/config', tpsgi => $tpsgi, scheme => 'http', route => '/auth' } );
+    is( $rendered->{to}, '/config', 'and one with a second factor goes where it was headed' );
+};
+
+done_testing();

+ 7 - 10
www/templates/html/components/resetpass.tx

@@ -13,13 +13,10 @@
 </form>
 
 <br /><br />
-<h3>Turn off TOTP 2fa</h3>
-<form id="resettotp" action="/request_totp_clear" method="POST">
-: if ($user) {
-    <input type="hidden" name="username" value="<: $user :>" />
-: } else {
-    <label for="username">Username:</label>
-    <input type="text" class="cooltext" name="username" placeholder="WhoWasPhone" />
-: }
-    <input type="submit" value="Reset TOTP">
-</form>
+<h3>Lost your authenticator?</h3>
+<p>
+    TOTP cannot be turned off &mdash; losing a phone is not a reason to stop
+    having a second factor.  Your enrolment is still on the server, so ask
+    whoever runs this site to read it back to you with <code>bin/totp</code>,
+    and scan it into a new authenticator.
+</p>

+ 5 - 0
www/templates/html/components/totp.tx

@@ -1,4 +1,9 @@
 <div id="backoffice">
+    <p>
+        Two factor auth is required here, and this is your enrolment.  It does
+        not change, so if you lose your authenticator you scan this same code
+        into the next one.
+    </p>
     Use the following URI, or scan the below QR code into your authenticator Application of choice:<br />
     <: $uri :>
     <br />

+ 1 - 1
www/templates/html/login.tx

@@ -26,7 +26,7 @@
       </div>
       <br />
 : if ($has_users) {
-      TOTP 2FA code <a style="cursor:help" title="only needed if 2FA enabled">ℹ️ </a><br />
+      TOTP 2FA code <a style="cursor:help" title="from your authenticator app; leave this blank only on the login where you enrol">ℹ️ </a><br />
       <div class="input-group">
         <label for="token">🔒</label>
         <input name="token" id="token" placeholder="7779311" value="" type="password"></input>

+ 2 - 2
www/templates/html/sysbar.tx

@@ -7,8 +7,8 @@
         <a href="/metrics?period=hour&num_periods=24"     title="Metrics"       class="topbar">Metrics</a>
         <a href="/admin/sessions"  title="Session Audit Log" class="topbar">Sessions</a>
         <a href="/admin/wyzzerdd"  title="Post Type wizard" class="topbar">Post Types</a>
-        <a href="/totp"        title="TOTP"          class="topbar">Enable/View TOTP 2fa</a>
-        <a href="/password_reset" title="Reset Auth" class="topbar">Reset Password/TOTP</a>
+        <a href="/totp"        title="TOTP"          class="topbar">View TOTP 2fa</a>
+        <a href="/password_reset" title="Reset Password" class="topbar">Reset Password</a>
         <a href="/logout"      title="Logout"        class="topbar">🚪</a>
     </span>
 </div>

+ 0 - 1
www/templates/html/totp_reset.tx

@@ -1 +0,0 @@
-Your TOTP 2FA can be disabled by following <a href="<: $uri :>">this</a> link.

+ 0 - 3
www/templates/text/totp_reset.tx

@@ -1,3 +0,0 @@
-Your TOTP 2FA can be disabled via following this link:
-
-<: $uri :>