You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by naamashoresh <gi...@git.apache.org> on 2017/04/06 13:31:52 UTC

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

GitHub user naamashoresh opened a pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435

    Authorization model

    This PR adds basic functionality for implementing authorization in TO (or in the API GW).
    It includes:
    - DB tables (capability, apit_capability, role_capability & user_role)
    - Data seeding
    - CRUD APIs for capability & api_capability tables
    - Unit-tests for the new APIs
    - Documentation of the new APIs

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/naamashoresh/incubator-trafficcontrol api_capabilities

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafficcontrol/pull/435.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #435
    
----
commit 39f4ba74e28169363d734e9828322610b980bafe
Author: naamashoresh <na...@qwilt.com>
Date:   2017-04-06T11:49:50Z

    Authorization model -
    Adding tables: capability (list of available capabilities), api_capability mapping, role_capability mapping & user_role.
    Seeding capability & api_capability tables. Also seeding root role.

commit 9d059e9606d2752cd9e0e8b3508420759e5c363e
Author: naamashoresh <na...@qwilt.com>
Date:   2017-04-06T11:55:41Z

    Adding capabilities & api_capalities APIs

commit 9edc708d03d846a2a647afe17427040b95b4c423
Author: naamashoresh <na...@qwilt.com>
Date:   2017-04-06T11:57:36Z

    Unit tests for new capabilities & api_capabilities APIs

commit 909342f02d359cbf0233d8bae271c6717a6f4d6e
Author: naamashoresh <na...@qwilt.com>
Date:   2017-04-06T11:58:25Z

    Documentation for new capabilities & api_capabilities APIs.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474767
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    +
    +-- capabilities
    +insert into capability (name, description) values ('all-read', 'Full read access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('all-write', 'Full write access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('asn-read', 'View ASN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('asn-write', 'Create, edit or delete ASN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('basic-read', 'Basic read operations. Every user should have this capability') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('basic-write', 'Basic write operations. Every user should have this capability') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-config-files-read', 'View the generated cache configuration files') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-group-read', 'View cache-group configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-group-write', 'Create, edit or delete cache-group configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-stats-read', 'View Cache statistics read access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-config-snapshot-read', 'View config snapshot at CDN level') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-config-snapshot-write', 'Config snapshot write access at CDN level') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-health-read', 'View CDN health') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-read', 'View CDN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-write', 'Create, edit or delete CDN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-security-keys-read', 'View CDN DNSSEC keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-security-keys-write', 'Create, edit or delete CDN DNSSEC keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-stats-read', 'View CDN statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-stats-write', 'Create, edit or delete CDN statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('change-log-read', 'View change-log') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('change-log-write', 'Create change-log entries') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('division-read', 'View division configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('division-write', 'Create, edit or delete division configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-cache-read', 'View delivery-service cache assignment') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-cache-read', 'Create, edit or delete delivery-service cache assignment') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-health-read', 'View delivery-service health') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-read', 'View delivery-service configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-write', 'Create, edit or delete delivery-service configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-security-keys-read', 'View delivery-service security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-security-keys-write', 'Create, edit or delete delivery-service security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-stats-read', 'View delivery-service statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-steering-read', 'View delivery-service steering configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-steering-write', 'Create, edit or delete delivery-service steering configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('federation-routing-read', 'View federation routing') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('federation-routing-write', 'Create, edit or delete federation routing') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('job-read', 'View jobs') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('job-write', 'Create, edit or delete jobs') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('params-read', 'View parameters') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('params-write', 'Create, edit or delete parameters') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('phys-location-read', 'View physical location configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('phys-location-write', 'Create, edit or delete physical location configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('profile-read', 'View profiles') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('profile-write', 'Create, edit or delete profiles') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('queue-updates-write', 'Queue updates to caches') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('region-read', 'View region configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('region-write', 'Create, edit or delete region configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('role-read', 'View role configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('role-write', 'Create, edit or delete role configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('security-keys-read', 'View security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('security-keys-write', 'Create, edit or delete security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-pull-updates-read', 'Read server update indication') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-pull-updates-write', 'Write server update indication') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-read', 'View server configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-write', 'Create, edit or delete server configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('static-dns-read', 'View static DNS configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('static-dns-write', 'Create, edit or delete static DNS configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('status-read', 'View the list of defined statuses') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('to-extension-read', 'View Traffic Ops extensions') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('to-extension-write', 'Create, edit or delete Traffic Ops extensions') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('type-read', 'View types configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('type-write', 'Create, edit or delete type configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('user-read', 'View user configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('user-write', 'Create, edit or delete user configuration') ON CONFLICT DO NOTHING;
    +
    +-- roles_capabilities
    +insert into role_capability (role_id, cap_name) values (9, 'all-read') ON CONFLICT DO NOTHING;
    --- End diff --
    
    Done @56f8856
    (left it as root, for the moment, until we decide about the super-user role name).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113052674
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    +
    +-- capabilities
    +insert into capability (name, description) values ('all-read', 'Full read access') ON CONFLICT DO NOTHING;
    --- End diff --
    
    when you sync up with the new seeds.sql file, you'll see that the "ON CONFLICT DO NOTHING"s  have been changed to be more specific. For example, "ON CONFLICT (name) DO NOTHING;" . with both a proper unique constraint and a more specific "ON CONFLICT (whatever) DO NOTHING", we can avoid getting duplicates in the database if seeds.sql is run multiple times.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/435
  
    thanks @naamashoresh - i'll take a look at this asap.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474694
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    +
    +-- capabilities
    +insert into capability (name, description) values ('all-read', 'Full read access') ON CONFLICT DO NOTHING;
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113053641
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    --- End diff --
    
    can you rename this "index" to be consistent with other parts of the api? "index" is a list...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474961
  
    --- Diff: traffic_ops/app/lib/TrafficOpsRoutes.pm ---
    @@ -657,6 +657,23 @@ sub api_routes {
     	# Supports ?orderby=key
     	$r->get("/api/$version/roles")->over( authenticated => 1 )->to( 'Role#index', namespace => $namespace );
     
    +	# -- CAPABILITIES
    +	# Supports ?orderby=key
    +	$r->get("/api/$version/capabilities")->over( authenticated => 1 )->to( 'Capability#all', namespace => $namespace );
    +	$r->get("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#name', namespace => $namespace );
    +	$r->put("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#update', namespace => $namespace );
    +	$r->post("/api/$version/capabilities")->over( authenticated => 1 )->to( 'Capability#create', namespace => $namespace );
    +	$r->delete("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#delete', namespace => $namespace );
    +
    +	# -- API-CAPABILITIES
    +	# Supports ?orderby=key
    +	$r->get("/api/$version/api_capabilities")->over( authenticated => 1 )->to( 'ApiCapability#all', namespace => $namespace );
    +	$r->get("/api/$version/api_capabilities/:id")->over( authenticated => 1 )->to( 'ApiCapability#index', namespace => $namespace );
    +	$r->get("/api/$version/api_capabilities/capability/:name")->over( authenticated => 1 )->to( 'ApiCapability#capName', namespace => $namespace );
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474796
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113049164
  
    --- Diff: traffic_ops/app/db/migrations/20170406000001_create_capabilities_and_roles.sql ---
    @@ -0,0 +1,89 @@
    +/*
    +
    +    Licensed under the Apache License, Version 2.0 (the "License");
    +    you may not use this file except in compliance with the License.
    +    You may obtain a copy of the License at
    +
    +        http://www.apache.org/licenses/LICENSE-2.0
    +
    +    Unless required by applicable law or agreed to in writing, software
    +    distributed under the License is distributed on an "AS IS" BASIS,
    +    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +    See the License for the specific language governing permissions and
    +    limitations under the License.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- capability
    +CREATE TABLE capability (
    +    name text primary key UNIQUE NOT NULL,
    +    description text,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- http_method_t (enum)
    +CREATE TYPE http_method_t as ENUM ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +-- api_capability
    +
    +CREATE TABLE api_capability (
    +    id BIGSERIAL primary key NOT NULL,
    +    http_method http_method_t NOT NULL,
    +    route text NOT NULL,
    +    capability text NOT NULL,
    +    CONSTRAINT fk_capability FOREIGN KEY (capability) REFERENCES capability(name) ON DELETE RESTRICT,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON api_capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- role_capability
    +CREATE TABLE role_capability (
    +    role_id bigint NOT NULL,
    --- End diff --
    
    how about adding a unique constraint on role_id + cap_name so we don't end up with duplicate data in this table?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/435
  
    Hi Jeremy,
    
    Thanks a lot for your review.
    Will attend your comments and apply/respond.
    
    On Apr 24, 2017 9:57 PM, "Jeremy Mitchell" <no...@github.com> wrote:
    
    Hi @naamashoresh <https://github.com/naamashoresh> - I'm sorry to make you
    do this but can you reconcile your seeds.sql file? You might have to do a
    rebase as seeds.sql has changes significantly since you submitted this PR.
    Thanks!
    
    \u2014
    You are receiving this because you were mentioned.
    Reply to this email directly, view it on GitHub
    <https://github.com/apache/incubator-trafficcontrol/pull/435#issuecomment-296789761>,
    or mute the thread
    <https://github.com/notifications/unsubscribe-auth/AYQCvZcRmNs3qufrPxhKn3FDcbVPqwJWks5rzPCWgaJpZM4M1moi>
    .



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh closed the pull request at:

    https://github.com/apache/incubator-trafficcontrol/pull/435


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113701963
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    --- End diff --
    
    Well, I'm not sure.
    What we had in mind when we created the "root" role, was a super-user. A user that has unlimited capabilities in the system.
    If the admin role answers this concept, it may be enough.
    However, I'm not sure the name "admin" represents these unlimited capabilities well.
    What do you say?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474897
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    +	my $self = shift;
    +	my $capability = $self->param('name');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.capability' => $capability );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub index {
    +	my $self = shift;
    +	my $id = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.id' => $id );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub is_mapping_valid {
    +	my $self = shift;
    +	my $id = shift;
    +	my $http_method = shift;
    +	my $route = shift;
    +	my $capability = shift;
    +
    +	if ( !defined($http_method) ) {
    +		return ( undef, "HTTP method is required." );
    +	}
    +
    +	if ( !exists( $valid_http_methods{ $http_method } ) ) {
    +		return ( undef, "HTTP method \'$http_method\' is invalid. Valid values are: " . join(", ", sort keys %valid_http_methods ) );
    +	}
    +
    +	if ( !defined($route) or $route eq "" ) {
    +		return ( undef, "Route is required." );
    +	}
    +
    +	if ( !defined($capability) or $capability eq "" ) {
    +		return (undef, "Capability name is required." );
    +	}
    +	# check if capability exists
    +	my $rs_data = $self->db->resultset("Capability")->search( { 'name' => { 'like', $capability } } )->single();
    +	if (!defined($rs_data)) {
    +		return (undef, "Capability '$capability' does not exist." );
    +	}
    +
    +	# search a mapping for the same http_method & route
    +	$rs_data = $self->db->resultset("ApiCapability")->search( { 'route' => { 'like', $route } } )->search( {
    +		'http_method' => { '=', $http_method } } )->single();
    +	# if adding a new entry, make sure it is unique
    +	if ( !defined( $id ) ) {
    +		if (defined($rs_data)) {
    +			my $allocated_capability = $rs_data->capability->name;
    +			return (undef, "HTTP method '$http_method', route '$route' are already mapped to capability: $allocated_capability" );
    +		}
    +	}
    +	else {
    +		if (defined($rs_data)) {
    +			my $lid = $rs_data->id;
    +			if ($lid ne $id) {
    +				my $allocated_capability = $rs_data->capability->name;
    +				return (undef, "HTTP method '$http_method', route '$route' are already mapped to capability: $allocated_capability" );
    +			}
    +		}
    +	}
    +
    +	return ( 1, undef );
    +}
    +
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	my $http_method = $params->{httpMethod} if defined($params->{httpMethod});
    +	my $route = $params->{route} if defined($params->{route});
    +	my $capability = $params->{capName} if defined($params->{capName});
    +	my $id = undef;
    +
    +	my ( $is_valid, $errStr ) = $self->is_mapping_valid( $id, $http_method, $route, $capability );
    +	if ( !$is_valid ) {
    +		return $self->alert( $errStr );
    +	}
    +
    +	my $values = {
    +		id 			=> $self->db->resultset('ApiCapability')->get_column('id')->max() + 1,
    +		http_method	=> $http_method,
    +		route		=> $route,
    +		capability	=> $capability
    +	};
    +
    +	my $insert = $self->db->resultset('ApiCapability')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}				= $rs->id;
    --- End diff --
    
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/435
  
    PR got messed up with irrelevant commits.
    Will open a new one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474634
  
    --- Diff: traffic_ops/app/db/migrations/20170406000001_create_capabilities_and_roles.sql ---
    @@ -0,0 +1,89 @@
    +/*
    +
    +    Licensed under the Apache License, Version 2.0 (the "License");
    +    you may not use this file except in compliance with the License.
    +    You may obtain a copy of the License at
    +
    +        http://www.apache.org/licenses/LICENSE-2.0
    +
    +    Unless required by applicable law or agreed to in writing, software
    +    distributed under the License is distributed on an "AS IS" BASIS,
    +    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +    See the License for the specific language governing permissions and
    +    limitations under the License.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- capability
    +CREATE TABLE capability (
    +    name text primary key UNIQUE NOT NULL,
    +    description text,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- http_method_t (enum)
    +CREATE TYPE http_method_t as ENUM ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +-- api_capability
    +
    +CREATE TABLE api_capability (
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114385140
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    --- End diff --
    
    If possible, I would like to leverage our existing roles to ease the migration from role-based auth (which the current UI uses) to role/capability-based auth (which the API will use). But, I would like to make some small changes like so:
    
    role name, priv level, capabilities
    
    admin, 100, "all" capability <-- this should satisfy your "root" role
    migrations, 80, TBD
    steering, 80, TBD
    federation, 80, TBD
    deploy, 80, TBD
    operations, 70, TBD
    read-only, 60, TBD
    tenant-admin, 50, TBD <-- new role
    tenant-read-only, 40, TBD <-- renamed from "portal"
    disallowed, 0, none
    
    let me know what you think. i'm really trying to make sure the existing UI continues to function properly while we add roles/capabilities that the API can utilize.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474830
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    +	my $self = shift;
    +	my $capability = $self->param('name');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.capability' => $capability );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub index {
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113053856
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    --- End diff --
    
    calling a nested object will result in N+1 queries unless you do a "prefetch" or a join in your original query.
    
    https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/API/Region.pm#L39


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113058209
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    +	my $self = shift;
    +	my $capability = $self->param('name');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.capability' => $capability );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub index {
    +	my $self = shift;
    +	my $id = $self->param('id');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.id' => $id );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub is_mapping_valid {
    +	my $self = shift;
    +	my $id = shift;
    +	my $http_method = shift;
    +	my $route = shift;
    +	my $capability = shift;
    +
    +	if ( !defined($http_method) ) {
    +		return ( undef, "HTTP method is required." );
    +	}
    +
    +	if ( !exists( $valid_http_methods{ $http_method } ) ) {
    +		return ( undef, "HTTP method \'$http_method\' is invalid. Valid values are: " . join(", ", sort keys %valid_http_methods ) );
    +	}
    +
    +	if ( !defined($route) or $route eq "" ) {
    +		return ( undef, "Route is required." );
    +	}
    +
    +	if ( !defined($capability) or $capability eq "" ) {
    +		return (undef, "Capability name is required." );
    +	}
    +	# check if capability exists
    +	my $rs_data = $self->db->resultset("Capability")->search( { 'name' => { 'like', $capability } } )->single();
    +	if (!defined($rs_data)) {
    +		return (undef, "Capability '$capability' does not exist." );
    +	}
    +
    +	# search a mapping for the same http_method & route
    +	$rs_data = $self->db->resultset("ApiCapability")->search( { 'route' => { 'like', $route } } )->search( {
    +		'http_method' => { '=', $http_method } } )->single();
    +	# if adding a new entry, make sure it is unique
    +	if ( !defined( $id ) ) {
    +		if (defined($rs_data)) {
    +			my $allocated_capability = $rs_data->capability->name;
    +			return (undef, "HTTP method '$http_method', route '$route' are already mapped to capability: $allocated_capability" );
    +		}
    +	}
    +	else {
    +		if (defined($rs_data)) {
    +			my $lid = $rs_data->id;
    +			if ($lid ne $id) {
    +				my $allocated_capability = $rs_data->capability->name;
    +				return (undef, "HTTP method '$http_method', route '$route' are already mapped to capability: $allocated_capability" );
    +			}
    +		}
    +	}
    +
    +	return ( 1, undef );
    +}
    +
    +sub create {
    +	my $self = shift;
    +	my $params = $self->req->json;
    +
    +	if ( !&is_oper($self) ) {
    +		return $self->forbidden();
    +	}
    +
    +	if ( !defined($params) ) {
    +		return $self->alert("Parameters must be in JSON format.");
    +	}
    +
    +	my $http_method = $params->{httpMethod} if defined($params->{httpMethod});
    +	my $route = $params->{route} if defined($params->{route});
    +	my $capability = $params->{capName} if defined($params->{capName});
    +	my $id = undef;
    +
    +	my ( $is_valid, $errStr ) = $self->is_mapping_valid( $id, $http_method, $route, $capability );
    +	if ( !$is_valid ) {
    +		return $self->alert( $errStr );
    +	}
    +
    +	my $values = {
    +		id 			=> $self->db->resultset('ApiCapability')->get_column('id')->max() + 1,
    +		http_method	=> $http_method,
    +		route		=> $route,
    +		capability	=> $capability
    +	};
    +
    +	my $insert = $self->db->resultset('ApiCapability')->create($values);
    +	my $rs = $insert->insert();
    +	if ($rs) {
    +		my $response;
    +		$response->{id}				= $rs->id;
    --- End diff --
    
    it looks like you might have to run your code thru perltidy. http://trafficcontrol.apache.org/docs/latest/development/traffic_ops.html#perl-formatting-conventions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114584874
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    --- End diff --
    
    ok, you are probably right. let's not mess with the priv_level in this PR. i can do that in another PR once i verify that changing it won't impact anything negatively.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114476557
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    --- End diff --
    
    The variety and order looks good to me. However, I'm not sure what would be the implicaitons of changing the priv_level (currently the range is 0-30).



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113050424
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    +
    +-- capabilities
    +insert into capability (name, description) values ('all-read', 'Full read access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('all-write', 'Full write access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('asn-read', 'View ASN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('asn-write', 'Create, edit or delete ASN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('basic-read', 'Basic read operations. Every user should have this capability') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('basic-write', 'Basic write operations. Every user should have this capability') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-config-files-read', 'View the generated cache configuration files') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-group-read', 'View cache-group configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-group-write', 'Create, edit or delete cache-group configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cache-stats-read', 'View Cache statistics read access') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-config-snapshot-read', 'View config snapshot at CDN level') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-config-snapshot-write', 'Config snapshot write access at CDN level') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-health-read', 'View CDN health') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-read', 'View CDN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-write', 'Create, edit or delete CDN configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-security-keys-read', 'View CDN DNSSEC keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-security-keys-write', 'Create, edit or delete CDN DNSSEC keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-stats-read', 'View CDN statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('cdn-stats-write', 'Create, edit or delete CDN statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('change-log-read', 'View change-log') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('change-log-write', 'Create change-log entries') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('division-read', 'View division configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('division-write', 'Create, edit or delete division configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-cache-read', 'View delivery-service cache assignment') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-cache-read', 'Create, edit or delete delivery-service cache assignment') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-health-read', 'View delivery-service health') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-read', 'View delivery-service configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-write', 'Create, edit or delete delivery-service configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-security-keys-read', 'View delivery-service security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-security-keys-write', 'Create, edit or delete delivery-service security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-stats-read', 'View delivery-service statistics') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-steering-read', 'View delivery-service steering configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('ds-steering-write', 'Create, edit or delete delivery-service steering configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('federation-routing-read', 'View federation routing') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('federation-routing-write', 'Create, edit or delete federation routing') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('job-read', 'View jobs') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('job-write', 'Create, edit or delete jobs') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('params-read', 'View parameters') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('params-write', 'Create, edit or delete parameters') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('phys-location-read', 'View physical location configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('phys-location-write', 'Create, edit or delete physical location configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('profile-read', 'View profiles') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('profile-write', 'Create, edit or delete profiles') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('queue-updates-write', 'Queue updates to caches') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('region-read', 'View region configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('region-write', 'Create, edit or delete region configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('role-read', 'View role configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('role-write', 'Create, edit or delete role configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('security-keys-read', 'View security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('security-keys-write', 'Create, edit or delete security keys') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-pull-updates-read', 'Read server update indication') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-pull-updates-write', 'Write server update indication') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-read', 'View server configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('server-write', 'Create, edit or delete server configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('static-dns-read', 'View static DNS configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('static-dns-write', 'Create, edit or delete static DNS configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('status-read', 'View the list of defined statuses') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('to-extension-read', 'View Traffic Ops extensions') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('to-extension-write', 'Create, edit or delete Traffic Ops extensions') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('type-read', 'View types configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('type-write', 'Create, edit or delete type configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('user-read', 'View user configuration') ON CONFLICT DO NOTHING;
    +insert into capability (name, description) values ('user-write', 'Create, edit or delete user configuration') ON CONFLICT DO NOTHING;
    +
    +-- roles_capabilities
    +insert into role_capability (role_id, cap_name) values (9, 'all-read') ON CONFLICT DO NOTHING;
    --- End diff --
    
    if we use the existing "admin" role this could be changed to:
    
    insert into role_capability (role_id, cap_name) values ((select id from role where name = 'admin', 'all-read') ON CONFLICT DO NOTHING;
    insert into role_capability (role_id, cap_name) values ((select id from role where name = 'admin', 'all-write') ON CONFLICT DO NOTHING;
    
    once  you sync up with the new seeds.sql you'll also see that we are trying to get away from hardcoding id's into the insert statements.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113053307
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    +	my $self = shift;
    +	my $capability = $self->param('name');
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( 'me.capability' => $capability );
    +	$self->renderResults( $rs_data ) ;
    +}
    +
    +sub index {
    --- End diff --
    
    can you rename this "show" to be consistent with other parts of the api? "show" means show me one...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113048984
  
    --- Diff: traffic_ops/app/db/migrations/20170406000001_create_capabilities_and_roles.sql ---
    @@ -0,0 +1,89 @@
    +/*
    +
    +    Licensed under the Apache License, Version 2.0 (the "License");
    +    you may not use this file except in compliance with the License.
    +    You may obtain a copy of the License at
    +
    +        http://www.apache.org/licenses/LICENSE-2.0
    +
    +    Unless required by applicable law or agreed to in writing, software
    +    distributed under the License is distributed on an "AS IS" BASIS,
    +    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +    See the License for the specific language governing permissions and
    +    limitations under the License.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- capability
    +CREATE TABLE capability (
    +    name text primary key UNIQUE NOT NULL,
    +    description text,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- http_method_t (enum)
    +CREATE TYPE http_method_t as ENUM ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +-- api_capability
    +
    +CREATE TABLE api_capability (
    --- End diff --
    
    how about adding a unique constraint on capability + http_method + route so we don't end up with duplicate data in this table?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474817
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol issue #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:

    https://github.com/apache/incubator-trafficcontrol/pull/435
  
    Hi @naamashoresh - I'm sorry to make you do this but can you reconcile your seeds.sql file? You might have to do a rebase as seeds.sql has changes significantly since you submitted this PR. Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113056228
  
    --- Diff: traffic_ops/app/lib/TrafficOpsRoutes.pm ---
    @@ -657,6 +657,23 @@ sub api_routes {
     	# Supports ?orderby=key
     	$r->get("/api/$version/roles")->over( authenticated => 1 )->to( 'Role#index', namespace => $namespace );
     
    +	# -- CAPABILITIES
    +	# Supports ?orderby=key
    +	$r->get("/api/$version/capabilities")->over( authenticated => 1 )->to( 'Capability#all', namespace => $namespace );
    +	$r->get("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#name', namespace => $namespace );
    +	$r->put("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#update', namespace => $namespace );
    +	$r->post("/api/$version/capabilities")->over( authenticated => 1 )->to( 'Capability#create', namespace => $namespace );
    +	$r->delete("/api/$version/capabilities/:name")->over( authenticated => 1 )->to( 'Capability#delete', namespace => $namespace );
    +
    +	# -- API-CAPABILITIES
    +	# Supports ?orderby=key
    +	$r->get("/api/$version/api_capabilities")->over( authenticated => 1 )->to( 'ApiCapability#all', namespace => $namespace );
    +	$r->get("/api/$version/api_capabilities/:id")->over( authenticated => 1 )->to( 'ApiCapability#index', namespace => $namespace );
    +	$r->get("/api/$version/api_capabilities/capability/:name")->over( authenticated => 1 )->to( 'ApiCapability#capName', namespace => $namespace );
    --- End diff --
    
    so we typically use query params to filter data. so i don't think this is needed:
    
    $r->get("/api/$version/api_capabilities/capability/:name
    
    instead,  you can just do GET api/*/api_capabilities?capability=[whatever] but just make sure your "index" action can handle the query param to filter the result set.
    
    Here's an example where we use a query param to filter regions by division:
    
    https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/API/Region.pm#L28


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474648
  
    --- Diff: traffic_ops/app/db/migrations/20170406000001_create_capabilities_and_roles.sql ---
    @@ -0,0 +1,89 @@
    +/*
    +
    +    Licensed under the Apache License, Version 2.0 (the "License");
    +    you may not use this file except in compliance with the License.
    +    You may obtain a copy of the License at
    +
    +        http://www.apache.org/licenses/LICENSE-2.0
    +
    +    Unless required by applicable law or agreed to in writing, software
    +    distributed under the License is distributed on an "AS IS" BASIS,
    +    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +    See the License for the specific language governing permissions and
    +    limitations under the License.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- capability
    +CREATE TABLE capability (
    +    name text primary key UNIQUE NOT NULL,
    +    description text,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- http_method_t (enum)
    +CREATE TYPE http_method_t as ENUM ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +-- api_capability
    +
    +CREATE TABLE api_capability (
    +    id BIGSERIAL primary key NOT NULL,
    +    http_method http_method_t NOT NULL,
    +    route text NOT NULL,
    +    capability text NOT NULL,
    +    CONSTRAINT fk_capability FOREIGN KEY (capability) REFERENCES capability(name) ON DELETE RESTRICT,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON api_capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- role_capability
    +CREATE TABLE role_capability (
    +    role_id bigint NOT NULL,
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113049926
  
    --- Diff: traffic_ops/app/db/seeds.sql ---
    @@ -22,6 +22,257 @@ insert into role (id, name, description, priv_level) values (5, 'portal', 'Porta
     insert into role (id, name, description, priv_level) values (6, 'migrations', 'database migrations user - DO NOT REMOVE', 20) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (7, 'federation', 'Role for Secondary CZF', 15) ON CONFLICT DO NOTHING;
     insert into role (id, name, description, priv_level) values (8, 'steering', 'Role for Steering Delivery Services', 15) ON CONFLICT DO NOTHING;
    +insert into role (id, name, description, priv_level) values (9, 'root', 'Role for full capabilities - super-user ', 30) ON CONFLICT DO NOTHING;
    --- End diff --
    
    for phase 1, can we just use the "admin" role rather than creating a new role? I think this would simplify things.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113056848
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    +	my $self = shift;
    +	my @data;
    +	my $orderby = "capability";
    +	$orderby = $self->param('orderby') if ( defined $self->param('orderby') );
    +
    +	my $rs_data = $self->db->resultset("ApiCapability")->search( undef, { order_by => $orderby } );
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +
    +sub renderResults {
    +	my $self = shift;
    +	my $rs_data = shift;
    +
    +	my @data = ();
    +	while ( my $row = $rs_data->next ) {
    +		push(
    +			@data, {
    +				"id"          	=> $row->id,
    +				"httpMethod"	=> $row->http_method,
    +				"route" 		=> $row->route,
    +				"capName"   	=> $row->capability->name,
    +				"lastUpdated" 	=> $row->last_updated
    +			}
    +		);
    +	}
    +	$self->success( \@data );
    +}
    +
    +sub capName {
    --- End diff --
    
    see my comment in trafficopsroutes.pm. This action can go away as it can be rolled into the index action and coupled with a query param to get the same results.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r113049468
  
    --- Diff: traffic_ops/app/db/migrations/20170406000001_create_capabilities_and_roles.sql ---
    @@ -0,0 +1,89 @@
    +/*
    +
    +    Licensed under the Apache License, Version 2.0 (the "License");
    +    you may not use this file except in compliance with the License.
    +    You may obtain a copy of the License at
    +
    +        http://www.apache.org/licenses/LICENSE-2.0
    +
    +    Unless required by applicable law or agreed to in writing, software
    +    distributed under the License is distributed on an "AS IS" BASIS,
    +    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +    See the License for the specific language governing permissions and
    +    limitations under the License.
    +*/
    +
    +-- +goose Up
    +-- SQL in section 'Up' is executed when this migration is applied
    +
    +
    +-- capability
    +CREATE TABLE capability (
    +    name text primary key UNIQUE NOT NULL,
    +    description text,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- http_method_t (enum)
    +CREATE TYPE http_method_t as ENUM ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +-- api_capability
    +
    +CREATE TABLE api_capability (
    +    id BIGSERIAL primary key NOT NULL,
    +    http_method http_method_t NOT NULL,
    +    route text NOT NULL,
    +    capability text NOT NULL,
    +    CONSTRAINT fk_capability FOREIGN KEY (capability) REFERENCES capability(name) ON DELETE RESTRICT,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON api_capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- role_capability
    +CREATE TABLE role_capability (
    +    role_id bigint NOT NULL,
    +    CONSTRAINT fk_role_id FOREIGN KEY (role_id) REFERENCES role(id) ON DELETE CASCADE,  
    +    cap_name text NOT NULL,
    +    CONSTRAINT fk_cap_name FOREIGN KEY (cap_name) REFERENCES capability(name) ON DELETE RESTRICT,
    +    last_updated timestamp with time zone DEFAULT now()
    +);
    +
    +CREATE TRIGGER on_update_current_timestamp BEFORE UPDATE ON role_capability FOR EACH ROW EXECUTE PROCEDURE on_update_current_timestamp_last_updated();
    +
    +-- user_role
    +CREATE TABLE user_role (
    +    user_id bigint NOT NULL,
    --- End diff --
    
    what do you think if for phase 1, we only allow 1 role per user therefore this table would not be needed? Instead we could simply use the role found in the tm_user table.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafficcontrol pull request #435: Authorization model

Posted by naamashoresh <gi...@git.apache.org>.
Github user naamashoresh commented on a diff in the pull request:

    https://github.com/apache/incubator-trafficcontrol/pull/435#discussion_r114474777
  
    --- Diff: traffic_ops/app/lib/API/ApiCapability.pm ---
    @@ -0,0 +1,258 @@
    +package API::ApiCapability;
    +#
    +#
    +# Licensed under the Apache License, Version 2.0 (the "License");
    +# you may not use this file except in compliance with the License.
    +# You may obtain a copy of the License at
    +#
    +#     http://www.apache.org/licenses/LICENSE-2.0
    +#
    +# Unless required by applicable law or agreed to in writing, software
    +# distributed under the License is distributed on an "AS IS" BASIS,
    +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +# See the License for the specific language governing permissions and
    +# limitations under the License.
    +#
    +#
    +#
    +
    +use UI::Utils;
    +
    +use Mojo::Base 'Mojolicious::Controller';
    +use Data::Dumper;
    +
    +
    +
    +my $finfo = __FILE__ . ":";
    +
    +my %valid_http_methods = map { $_ => 1 } ('GET', 'POST', 'PUT', 'PATCH', 'DELETE');
    +
    +sub all {
    --- End diff --
    
    Done @56f8856


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---