#1036 redux allow use of /login even when authN is delegated

lib/App/Netdisco/Web.pm

@@ -392,10 +392,6 @@ hook 'after' => sub {
     }
 };
 
-my $api_requires_key =
-  (setting('trust_remote_user') or setting('trust_x_remote_user') or setting('no_auth'))
-    eq '1' ? false : true;
-
 # setup for swagger API
 my $swagger = Dancer::Plugin::Swagger->instance;
 my $swagger_doc = $swagger->doc;
@@ -403,8 +399,8 @@ my $swagger_doc = $swagger->doc;
 $swagger_doc->{consumes} = 'application/json';
 $swagger_doc->{produces} = 'application/json';
 $swagger_doc->{tags} = [
-  ($api_requires_key ?
+  {name => 'General',
-    ({name => 'General', description => 'Log in and Log out'}) : ()),
+    description => 'Log in and Log out'},
   {name => 'Search',
     description => 'Search Operations'},
   {name => 'Objects',
@@ -415,15 +411,13 @@ $swagger_doc->{tags} = [
     description => 'Operations on the Job Queue'},
 ];
 
-if ($api_requires_key) {
+$swagger_doc->{securityDefinitions} = {
-    $swagger_doc->{securityDefinitions} = {
+  APIKeyHeader =>
-      APIKeyHeader =>
+    { type => 'apiKey', name => 'Authorization', in => 'header' },
-        { type => 'apiKey', name => 'Authorization', in => 'header' },
+  BasicAuth =>
-      BasicAuth =>
+    { type => 'basic'  },
-        { type => 'basic'  },
+};
-    };
+$swagger_doc->{security} = [ { APIKeyHeader => [] } ];
-    $swagger_doc->{security} = [ { APIKeyHeader => [] } ];
-}
 
 if (setting('trust_x_remote_user')) {
     foreach my $path (keys %{ $swagger_doc->{paths} }) {

lib/App/Netdisco/Web/Auth/Provider/DBIC.pm

@@ -46,10 +46,11 @@ sub get_user_details {
 
     # each of these settings permits no user in the database
     # so create a pseudo user entry instead
-    if (not $user and not setting('validate_remote_user')
+    if (not $user and
-                  and (setting('trust_remote_user')
+        (setting('no_auth') or
-                    or setting('trust_x_remote_user')
+          (not setting('validate_remote_user')
-                    or setting('no_auth'))) {
+           and (setting('trust_remote_user') or setting('trust_x_remote_user')) ))) {
+
         $user = $database->resultset($users_table)
           ->new_result({username => $username});
     }
@@ -73,7 +74,7 @@ sub validate_api_token {
       $database->resultset($users_table)->find({ $token_column => $token });
     };
 
-    return $user->username
+    return $user
       if $user and $user->in_storage and $user->token_from
         and $user->token_from > (time - setting('api_token_lifetime'));
     return undef;

lib/App/Netdisco/Web/AuthN.pm

@@ -16,6 +16,37 @@ hook 'before' => sub {
       ? request->uri : uri_for(setting('web_home'))->path);
 };
 
+# try to find a valid username according to headers
+# or configuration settings
+sub _get_delegated_authn_user {
+  my $username = undef;
+
+  if (setting('trust_x_remote_user')
+    and scalar request->header('X-REMOTE_USER')
+    and length scalar request->header('X-REMOTE_USER')) {
+
+      ($username = scalar request->header('X-REMOTE_USER')) =~ s/@[^@]*$//;
+  }
+  elsif (setting('trust_remote_user')
+    and defined $ENV{REMOTE_USER}
+    and length  $ENV{REMOTE_USER}) {
+
+      ($username = $ENV{REMOTE_USER}) =~ s/@[^@]*$//;
+  }
+  # this works for API calls, too
+  elsif (setting('no_auth')) {
+      $username = 'guest';
+  }
+
+  return unless $username;
+
+  # from the internals of Dancer::Plugin::Auth::Extensible
+  my $provider = Dancer::Plugin::Auth::Extensible::auth_provider('users');
+
+  # may synthesize a user if validate_remote_user=false
+  return $provider->get_user_details($username);
+}
+
 # Dancer will create a session if it sees its own cookie. For the API and also
 # various auto login options we need to bootstrap the session instead. If no
 # auth data passed, then the hook simply returns, no session is set, and the
@@ -29,9 +60,6 @@ hook 'before' => sub {
       or index(request->path, uri_for('/swagger-ui')->path) == 0
     );
 
-    # from the internals of Dancer::Plugin::Auth::Extensible
-    my $provider = Dancer::Plugin::Auth::Extensible::auth_provider('users');
-
     # Dancer will issue a cookie to the client which could be returned and
     # cause API calls to succeed without passing token. Kill the session.
     session->destroy if request_is_api;
@@ -39,40 +67,30 @@ hook 'before' => sub {
     # ...otherwise, we can short circuit if Dancer reads its cookie OK
     return if session('logged_in_user');
 
-    if (setting('trust_x_remote_user')
+    my $delegated = _get_delegated_authn_user();
-      and scalar request->header('X-REMOTE_USER')
-      and length scalar request->header('X-REMOTE_USER')) {
 
-        (my $user = scalar request->header('X-REMOTE_USER')) =~ s/@[^@]*$//;
+    # this ordering allows override of delegated authN if given creds
-        return if setting('validate_remote_user')
-          and not $provider->get_user_details($user);
 
-        session(logged_in_user => $user);
+    # protect against delegated authN config but no valid user
-        session(logged_in_user_realm => 'users');
+    if ((not $delegated) and
-    }
+      (setting('trust_x_remote_user') or setting('trust_remote_user'))) {
-    elsif (setting('trust_remote_user')
+        session->destroy;
-      and defined $ENV{REMOTE_USER}
+        request->path_info('/');
-      and length  $ENV{REMOTE_USER}) {
-
-        (my $user = $ENV{REMOTE_USER}) =~ s/@[^@]*$//;
-        return if setting('validate_remote_user')
-          and not $provider->get_user_details($user);
-
-        session(logged_in_user => $user);
-        session(logged_in_user_realm => 'users');
-    }
-    # this works for API calls, too
-    elsif (setting('no_auth')) {
-        session(logged_in_user => 'guest');
-        session(logged_in_user_realm => 'users');
     }
     # API calls must conform strictly to path and header requirements
     elsif (request_is_api) {
+        # from the internals of Dancer::Plugin::Auth::Extensible
+        my $provider = Dancer::Plugin::Auth::Extensible::auth_provider('users');
+
         my $token = request->header('Authorization');
         my $user = $provider->validate_api_token($token)
           or return;
 
-        session(logged_in_user => $user);
+        session(logged_in_user => $user->username);
+        session(logged_in_user_realm => 'users');
+    }
+    elsif ($delegated) {
+        session(logged_in_user => $delegated->username);
         session(logged_in_user_realm => 'users');
     }
     else {
@@ -81,9 +99,22 @@ hook 'before' => sub {
     }
 };
 
-my $login_sub = sub {
+# override default login_handler so we can log access in the database
+swagger_path {
+  description => 'Obtain an API Key',
+  tags => ['General'],
+  path => (setting('url_base') ? setting('url_base')->with('/login')->path : '/login'),
+  parameters => [],
+  responses => { default => { examples => {
+    'application/json' => { api_key => 'cc9d5c02d8898e5728b7d7a0339c0785' } } },
+  },
+},
+post '/login' => sub {
     my $api = ((request->accept and request->accept =~ m/(?:json|javascript)/) ? true : false);
 
+    # from the internals of Dancer::Plugin::Auth::Extensible
+    my $provider = Dancer::Plugin::Auth::Extensible::auth_provider('users');
+
     # get authN data from BasicAuth header used by API, put into params
     my $authheader = request->header('Authorization');
     if (defined $authheader and $authheader =~ /^Basic (.*)$/i) {
@@ -95,13 +126,21 @@ my $login_sub = sub {
     # validate authN
     my ($success, $realm) = authenticate_user(param('username'),param('password'));
 
-    if ($success) {
+    # or try to get user from somewhere else
-        my $user = schema('netdisco')->resultset('User')
+    my $delegated = _get_delegated_authn_user();
-          ->find({ username => { -ilike => quotemeta(param('username')) } });
+
+    if (($success and not
+          # protect against delegated authN config but no valid user (then must ignore params)
+          (not $delegated and (setting('trust_x_remote_user') or setting('trust_remote_user'))))
+        or $delegated) {
+
+        # this ordering allows override of delegated user if given creds
+        my $user = ($success ? $provider->get_user_details(param('username'))
+                             : $delegated);
 
         session logged_in_user => $user->username;
-        session logged_in_fullname => $user->fullname;
+        session logged_in_fullname => ($user->fullname || '');
-        session logged_in_user_realm => $realm;
+        session logged_in_user_realm => ($realm || 'users');
 
         schema('netdisco')->resultset('UserLog')->create({
           username => session('logged_in_user'),
@@ -112,9 +151,8 @@ my $login_sub = sub {
         $user->update({ last_on => \'LOCALTIMESTAMP' });
 
         if ($api) {
-            # from the internals of Dancer::Plugin::Auth::Extensible
-            my $provider = Dancer::Plugin::Auth::Extensible::auth_provider('users');
             header('Content-Type' => 'application/json');
+
             # if there's a current valid token then reissue it and reset timer
             $user->update({
               token_from => time,
@@ -150,7 +188,21 @@ my $login_sub = sub {
     }
 };
 
-my $logout_sub = sub {
+# ugh, *puke*, but D::P::Swagger has no way to set this with swagger_path
+# must be after the path is declared, above.
+Dancer::Plugin::Swagger->instance->doc
+  ->{paths}->{ (setting('url_base') ? setting('url_base')->with('/login')->path : '/login') }
+  ->{post}->{security}->[0]->{BasicAuth} = [];
+
+# we override the default login_handler, so logout has to be handled as well
+swagger_path {
+  description => 'Destroy user API Key and session cookie',
+  tags => ['General'],
+  path => (setting('url_base') ? setting('url_base')->with('/logout')->path : '/logout'),
+  parameters => [],
+  responses => { default => { examples => { 'application/json' => {} } } },
+},
+get '/logout' => sub {
     my $api = ((request->accept and request->accept =~ m/(?:json|javascript)/) ? true : false);
 
     # clear out API token
@@ -177,44 +229,6 @@ my $logout_sub = sub {
     redirect uri_for(setting('web_home'))->path;
 };
 
-my $api_requires_key =
-  (setting('trust_remote_user') or setting('trust_x_remote_user') or setting('no_auth'))
-    eq '1' ? false : true;
-
-if ($api_requires_key) {
-    # override default login_handler so we can log access in the database
-    swagger_path {
-      description => 'Obtain an API Key',
-      tags => ['General'],
-      path => (setting('url_base') ? setting('url_base')->with('/login')->path : '/login'),
-      parameters => [],
-      responses => { default => { examples => {
-        'application/json' => { api_key => 'cc9d5c02d8898e5728b7d7a0339c0785' } } },
-      },
-    },
-    post '/login' => $login_sub;
-
-    # ugh, *puke*, but D::P::Swagger has no way to set this with swagger_path
-    # must be after the path is declared, above.
-    Dancer::Plugin::Swagger->instance->doc
-      ->{paths}->{ (setting('url_base') ? setting('url_base')->with('/login')->path : '/login') }
-      ->{post}->{security}->[0]->{BasicAuth} = [];
-
-    # we override the default login_handler, so logout has to be handled as well
-    swagger_path {
-      description => 'Destroy user API Key and session cookie',
-      tags => ['General'],
-      path => (setting('url_base') ? setting('url_base')->with('/logout')->path : '/logout'),
-      parameters => [],
-      responses => { default => { examples => { 'application/json' => {} } } },
-    },
-    get '/logout' => $logout_sub;
-}
-else {
-    post '/login' => $login_sub;
-    get '/logout' => $logout_sub;
-}
-
 # user redirected here when require_role does not succeed
 any qr{^/(?:login(?:/denied)?)?} => sub {
     my $api = ((request->accept and request->accept =~ m/(?:json|javascript)/) ? true : false);