From 20e77c7b54ce560b80b8a7ead0417bb2fb1eb616 Mon Sep 17 00:00:00 2001 From: Kirill Solomko Date: Thu, 8 Jul 2021 22:31:06 +0200 Subject: [PATCH] TT#129162 tighten user access checks * add additional centralised checks for inactive and read_only users. * use_userdata_from_session=0 now for all auth realms to cause the data re-fetched from the database, to avoid scenarios when a user is set as inactive or read_only and UI keeps using the cached data. the change only affects cookie and JWT subscriber based sessions as in all other cases, the auth data is fetched from the storage regardless. * add is_active=1 flag for the internal 'system' role, as otherwise access would be permanently denied for it. * default 403 error for denied api requests is changed to "Forbidden" instead of "Forbidden path". Change-Id: I1d6d3c765ca8e017e11845c1f5260243a3963c3b --- lib/NGCP/Panel.pm | 12 ++-- .../Panel/Authentication/Store/SystemRole.pm | 1 + lib/NGCP/Panel/Controller/Root.pm | 62 ++++++++++++------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/lib/NGCP/Panel.pm b/lib/NGCP/Panel.pm index c1b65ec7be..e4b9bbc238 100644 --- a/lib/NGCP/Panel.pm +++ b/lib/NGCP/Panel.pm @@ -142,7 +142,7 @@ __PACKAGE__->config( user_model => 'DB::admins', id_field => 'id', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - use_userdata_from_session => 1, + use_userdata_from_session => 0, } }, admin_bcrypt => { @@ -157,7 +157,7 @@ __PACKAGE__->config( user_model => 'DB::admins', id_field => 'id', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - use_userdata_from_session => 1, + use_userdata_from_session => 0, } }, admin_jwt => { @@ -189,7 +189,7 @@ __PACKAGE__->config( user_model => 'DB::admins', id_field => 'id', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - #use_userdata_from_session => 1, + use_userdata_from_session => 0, }, use_session => 0, }, @@ -205,7 +205,7 @@ __PACKAGE__->config( user_model => 'DB::admins', id_field => 'id', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - #use_userdata_from_session => 1, + use_userdata_from_session => 0, }, use_session => 0, }, @@ -255,7 +255,7 @@ __PACKAGE__->config( class => 'DBIx::Class', user_model => 'DB::provisioning_voip_subscribers', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - # use_userdata_from_session => 1, + use_userdata_from_session => 0, }, use_session => 0, }, @@ -319,7 +319,7 @@ __PACKAGE__->config( user_model => 'DB::provisioning_voip_subscribers', id_field => 'id', store_user_class => 'NGCP::Panel::Authentication::Store::RoleFromRealm', - use_userdata_from_session => 1, + use_userdata_from_session => 0, } } }, diff --git a/lib/NGCP/Panel/Authentication/Store/SystemRole.pm b/lib/NGCP/Panel/Authentication/Store/SystemRole.pm index 2495a09f58..471766ab5e 100644 --- a/lib/NGCP/Panel/Authentication/Store/SystemRole.pm +++ b/lib/NGCP/Panel/Authentication/Store/SystemRole.pm @@ -11,6 +11,7 @@ sub roles { } sub id { 0 }; +sub is_active { 1 }; sub is_system { 1 }; sub is_master { 1 }; sub is_superuser { 1 }; diff --git a/lib/NGCP/Panel/Controller/Root.pm b/lib/NGCP/Panel/Controller/Root.pm index ef022a00cb..9702f03c63 100644 --- a/lib/NGCP/Panel/Controller/Root.pm +++ b/lib/NGCP/Panel/Controller/Root.pm @@ -28,7 +28,7 @@ __PACKAGE__->config(namespace => ''); sub auto :Private { my($self, $c) = @_; - + $c->stash->{_request_start} = Time::HiRes::time; my $is_api_request = 0; @@ -136,8 +136,6 @@ sub auto :Private { return 1; } - #if($is_api_request or not $c->user_exists) { - if(index($c->controller->catalyst_component_name, 'NGCP::Panel::Controller::API') == 0) { $c->log->debug("++++++ Root::auto unauthenticated API request"); my $ssl_dn = $c->request->env->{SSL_CLIENT_M_DN} // ""; @@ -180,7 +178,7 @@ sub auto :Private { return; } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } elsif ($c->req->headers->header("NGCP-UserAgent") && $c->req->headers->header("NGCP-UserAgent") eq "NGCP::API::Client") { $c->log->debug("++++++ Root::auto API request with system auth"); @@ -193,7 +191,7 @@ sub auto :Private { } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } elsif ($c->req->headers->header("Authorization") && $c->req->headers->header("Authorization") =~ m/^Bearer(\s+)a=/) { $c->log->debug("++++++ Root::auto API request with admin JWT"); @@ -206,7 +204,7 @@ sub auto :Private { } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } elsif ($c->req->headers->header("Authorization") && $c->req->headers->header("Authorization") =~ m/^Bearer /) { $c->log->debug("++++++ Root::auto API request with JWT"); @@ -219,7 +217,7 @@ sub auto :Private { } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } elsif ($ngcp_api_realm eq "subscriber") { $c->log->debug("++++++ Root::auto API subscriber request with http auth"); my $realm = "api_subscriber_http"; @@ -257,7 +255,7 @@ sub auto :Private { return; } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } else { $c->log->debug("++++++ Root::auto API admin request with http auth"); my ($user, $pass) = $c->req->headers->authorization_basic; @@ -283,6 +281,7 @@ sub auto :Private { if($c->user->read_only && $c->req->method eq "POST" && $c->req->uri->path =~ m|^/api/admincerts/$|) { $c->log->info("let read-only user '".$c->user->login."' generate admin cert for itself"); + return 1; } elsif($c->user->read_only && !($c->req->method =~ /^(GET|HEAD|OPTIONS)$/)) { $c->log->error("invalid method '".$c->req->method."' for read-only user '".$c->user->login."', rejecting"); $c->user->logout; @@ -295,8 +294,7 @@ sub auto :Private { return; } $self->api_apply_fake_time($c); - #$c->log->debug("return 1"); - return 1; + return $self->check_user_access($c); } } elsif (!$c->user_exists && $c->req->headers->header("Authorization") && @@ -311,7 +309,7 @@ sub auto :Private { } $self->api_apply_fake_time($c); - return 1; + return $self->check_user_access($c); } elsif (!$c->user_exists) { # don't redirect to login page for ajax uris @@ -340,16 +338,6 @@ sub auto :Private { $c->log->debug("*** Root::auto grant access for authenticated user"); - # check for read_only on write operations - if($c->user->read_only && ( - $c->req->uri->path =~ /create/ - || $c->req->uri->path =~ /edit/ - || $c->req->uri->path =~ /delete/ - || !($c->req->method =~ /^(GET|HEAD|OPTIONS)$/) - )) { - $c->detach('/denied_page'); - } - if (exists $c->config->{external_documentation}{link} && 'ARRAY' ne ref $c->config->{external_documentation}{link}) { $c->config->{external_documentation}{link} = [$c->config->{external_documentation}{link}]; } @@ -372,7 +360,7 @@ sub auto :Private { $c->session->{created_objects} = {} unless(defined $c->session->{created_objects}); - return 1; + return $self->check_user_access($c); } sub root_index :Path :Args(0) { @@ -449,13 +437,41 @@ sub denied_page :Private { $c->log->error('Access denied to path ' . $c->request->path ); if($c->request->path =~ /^api\/.+/) { $c->response->content_type('application/json'); - $c->response->body(JSON::to_json({ code => 403, message => 'Path forbidden' })."\n"); + $c->response->body(JSON::to_json({ code => 403, message => 'Forbidden' })."\n"); } else { $c->stash(template => 'denied_page.tt'); } $c->response->status(403); } +sub check_user_access { + my ($self, $c) = @_; + + my $path = $c->req->uri->path; + + if ($path =~ /^\/(login|logout|login_jwt|admin_login_jwt)$/) { + return 1; + } + + # deny access to inactive users + if ($c->user_exists && !$c->user->uuid && !$c->user->is_active) { + $c->detach('/denied_page'); + return; + } + + # deny access to read-only users + if ($c->user_exists && $c->user->read_only && + ($path =~ /create/ || + $path =~ /edit/ || + $path =~ /delete/ || + $c->req->method =~ /^(POST|PUT|PATCH|DELETE)$/)) { + $c->detach('/denied_page'); + return; + } + + return 1; +} + sub emptyajax :Chained('/') :PathPart('emptyajax') :Args(0) { my ($self, $c) = @_;