You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by nir-sopher <gi...@git.apache.org> on 2017/03/15 20:59:54 UTC
[GitHub] incubator-trafficcontrol pull request #370: Adding a "create user" to the ap...
GitHub user nir-sopher opened a pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370
Adding a "create user" to the api
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nir-sopher/incubator-trafficcontrol create-user-rest-api
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafficcontrol/pull/370.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 #370
----
commit be444c5b593a21cf6585abe055345badfea6a2c7
Author: nir-sopher <ni...@gmail.com>
Date: 2017-03-15T14:48:24Z
User REST UPI - update, registration_sent cannot be 0/1
commit 53f7f47b1a94240e4b284adbb93bdb52c32da3e6
Author: nir-sopher <ni...@gmail.com>
Date: 2017-03-15T14:49:11Z
Adding user create API
----
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110310005
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
+ address_line2 => defined_or_default($params->{addressLine2}, ""),
+ city => defined_or_default($params->{city}, ""),
+ company => defined_or_default($params->{company}, ""),
+ country => defined_or_default($params->{country}, ""),
+ email => $params->{email},
+ full_name => $params->{fullName},
+ new_user => defined_or_default($params->{newUser}, 0),
+ phone_number => defined_or_default($params->{phoneNumber}, ""),
+ postal_code => defined_or_default($params->{postalCode}, ""),
+ public_ssh_key => defined_or_default($params->{publicSshKey}, ""),
+ registration_sent => undef,
+ role => $params->{role},
+ state_or_province => defined_or_default($params->{stateOrProvince}, ""),
+ username => $params->{username},
+ uid => 0,
+ gid => 0,
+ local_passwd => sha1_hex($params->{localPassword} ),
+ confirm_local_passwd => sha1_hex($params->{confirmLocalPassword} ),
+ tenant_id => $tenant_id,
+
+ };
+
+ my ( $is_valid, $result ) = $self->is_valid($values);
+
+ if ( !$is_valid ) {
+ return $self->alert($result);
+ }
+
+ my $insert = $self->db->resultset('TmUser')->create($values);
+ my $rs = $insert->insert();
+
+ if ($rs) {
+ my $response;
+ $response->{addressLine1} = $rs->address_line1;
+ $response->{addressLine2} = $rs->address_line2;
+ $response->{city} = $rs->city;
+ $response->{company} = $rs->company;
+ $response->{country} = $rs->country;
+ $response->{email} = $rs->email;
+ $response->{fullName} = $rs->full_name;
+ $response->{gid} = $rs->gid;
+ $response->{id} = $rs->id;
+ $response->{lastUpdated} = $rs->last_updated;
+ $response->{newUser} = \$rs->new_user;
+ $response->{phoneNumber} = $rs->phone_number;
+ $response->{postalCode} = $rs->postal_code;
+ $response->{publicSshKey} = $rs->public_ssh_key;
+ $response->{registrationSent} = \$rs->registration_sent;
+ $response->{role} = $rs->role->id;
+ $response->{roleName} = $rs->role->name;
+ $response->{stateOrProvince} = $rs->state_or_province;
+ $response->{uid} = $rs->uid;
+ $response->{username} = $rs->username;
+ $response->{tenantId} = $rs->tenant_id;
--- End diff --
can you return the tenant name as well?
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110309463
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
--- End diff --
I don't think you need to do all these defined_or_default()
it's fine if they leave out addressLine1 which will just set address_line1 to undef.
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110309859
--- Diff: docs/source/development/traffic_ops_api/v12/user.rst ---
@@ -214,6 +214,153 @@ Users
|
+
+**POST /api/1.2/users**
+
+ Create a user.
+
+ Authentication Required: Yes
+
+ Role(s) Required: admin or oper
+
+ **Request Properties**
+
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | Parameter | Type | Required | Description |
+ +=========================+========+==========+=================================================+
+ |``addressLine1`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``addressLine2`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``city`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``confirmLocalPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``company`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``country`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``email`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``fullName`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``localPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``newUser`` | bool | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``phoneNumber`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``postalCode`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``publicSshKey`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``role`` | int | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``stateOrProvince`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | ``tenantId`` | int | no | Owning tenant ID |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``username`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+
+
+ **Request Example** ::
+
+ {
+ "username": "tsimpson"
+ "tenantId": 1,
+ "fullName": "Tom Simpson"
+ "email": "email1@email.com"
+ "role": 6
+ "localPassword": "password"
+ "confirmLocalPassword": "password"
+ }
+
+|
+
+ **Response Properties**
+
+ +----------------------+--------+------------------------------------------------+
+ | Parameter | Type | Description |
+ +======================+========+================================================+
+ |``addressLine1`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``addressLine2`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``city`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``company`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``country`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``email`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``fullName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``gid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``id`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``lastUpdated`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``newUser`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``phoneNumber`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``postalCode`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``publicSshKey`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``registrationSent`` | bool | |
+ +----------------------+--------+------------------------------------------------+
+ |``role`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``roleName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``stateOrProvince`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``uid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ | ``tenantId`` | int | Owning tenant ID |
--- End diff --
nevermind, i guess this is checked in the is_valid() function.
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r113028771
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
--- End diff --
I'll remove the checks for full-name and email.
As "username" value is use for searching the DB later on, I prefer to validate it.
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r119400399
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
--- End diff --
I don't understand. fullname, email and username are validated here:
https://github.com/apache/incubator-trafficcontrol/blob/master/traffic_ops/app/lib/API/User.pm#L463
also, you should probably add role to line 463
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110366117
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -218,6 +218,115 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
--- End diff --
Indeed
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110309625
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
+ address_line2 => defined_or_default($params->{addressLine2}, ""),
+ city => defined_or_default($params->{city}, ""),
+ company => defined_or_default($params->{company}, ""),
+ country => defined_or_default($params->{country}, ""),
+ email => $params->{email},
+ full_name => $params->{fullName},
+ new_user => defined_or_default($params->{newUser}, 0),
--- End diff --
just set this to
new_user => \0, # false
it's confusing, i know but "new_user" is used for users that were created via a registrationSent.
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110366079
--- Diff: docs/source/development/traffic_ops_api/v12/user.rst ---
@@ -214,6 +214,153 @@ Users
|
+
+**POST /api/1.2/users**
+
+ Create a user.
+
+ Authentication Required: Yes
+
+ Role(s) Required: admin or oper
+
+ **Request Properties**
+
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | Parameter | Type | Required | Description |
+ +=========================+========+==========+=================================================+
+ |``addressLine1`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``addressLine2`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``city`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``confirmLocalPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``company`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``country`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``email`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``fullName`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``localPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``newUser`` | bool | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``phoneNumber`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``postalCode`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``publicSshKey`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``role`` | int | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``stateOrProvince`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | ``tenantId`` | int | no | Owning tenant ID |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``username`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+
+
+ **Request Example** ::
+
+ {
+ "username": "tsimpson"
+ "tenantId": 1,
+ "fullName": "Tom Simpson"
+ "email": "email1@email.com"
+ "role": 6
+ "localPassword": "password"
+ "confirmLocalPassword": "password"
+ }
+
+|
+
+ **Response Properties**
+
+ +----------------------+--------+------------------------------------------------+
+ | Parameter | Type | Description |
+ +======================+========+================================================+
+ |``addressLine1`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``addressLine2`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``city`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``company`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``country`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``email`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``fullName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``gid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``id`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``lastUpdated`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``newUser`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``phoneNumber`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``postalCode`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``publicSshKey`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``registrationSent`` | bool | |
+ +----------------------+--------+------------------------------------------------+
+ |``role`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``roleName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``stateOrProvince`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``uid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ | ``tenantId`` | int | Owning tenant ID |
--- End diff --
Ori plans to do so in a separate PR or on the merge with pr 434
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110659889
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
+ address_line2 => defined_or_default($params->{addressLine2}, ""),
+ city => defined_or_default($params->{city}, ""),
+ company => defined_or_default($params->{company}, ""),
+ country => defined_or_default($params->{country}, ""),
+ email => $params->{email},
+ full_name => $params->{fullName},
+ new_user => defined_or_default($params->{newUser}, 0),
--- End diff --
Done. "0" an not "\0" as setting it to "\0" failed.
Will need to look further into it to understand why
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r107505547
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -218,6 +218,115 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
--- End diff --
email is a unique column. do you want to check to make sure this email is not already in use? kind of like you did for username?
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r113005125
--- Diff: traffic_ops/app/t/api/1.2/user_admin.t ---
@@ -0,0 +1,126 @@
+package main;
+#
+#
+# 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 Mojo::Base -strict;
+use Test::More;
+use Test::Mojo;
+use DBI;
+use Data::Dumper;
+use strict;
+use warnings;
+use Schema;
+use Test::TestHelper;
+use Fixtures::TmUser;
+use Fixtures::Deliveryservice;
+use Digest::SHA1 qw(sha1_hex);
+use Data::Dumper;
+
+#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
+#no_transactions=>0 ==> delete fixtures after every execution
+
+BEGIN { $ENV{MOJO_MODE} = "test" }
+
+sub run_ut {
+ my $t = shift;
+ my $schema = shift;
+ my $login_user = shift;
+ my $login_password = shift;
+
+ Test::TestHelper->unload_core_data($schema);
+ Test::TestHelper->teardown( $schema, 'Log' );
+ Test::TestHelper->teardown( $schema, 'Role' );
+ Test::TestHelper->teardown( $schema, 'TmUser' );
+
+ Test::TestHelper->load_core_data($schema);
+
+ my $tenant_id = $schema->resultset('TmUser')->find( { username => $login_user } )->get_column('tenant_id');
+ my $tenant_name = defined ($tenant_id) ? $schema->resultset('Tenant')->find( { id => $tenant_id } )->get_column('name') : "null";
+
+ # Verify the user
+ ok my $user = $schema->resultset('TmUser')->find( { username => $login_user } ), 'Does the portal user exist?';
+
+ ok $t->post_ok( '/login', => form => { u => $login_user, p => $login_password} )->status_is(302);
+
+ #adding a user
+ my $addedUserName = "user1";
+ my $addedUserEmail = "abc\@z.com";
+
+ ok $t->post_ok('/api/1.2/users' => {Accept => 'application/json'} => json => {
+ "username" => $addedUserName, "fullName"=>"full name", "email" => $addedUserEmail, "localPassword" => "pass", "confirmLocalPassword"=> "pass", "role" => 4 })
+ ->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
+ ->json_is( "/response/username" => $addedUserName )
+ ->json_is( "/response/email" => $addedUserEmail)
+ ->json_is( "/response/tenantId" => $tenant_id) #tenant Id not set - getting the tenant id from the user
+ , 'Failed adding user?';
--- End diff --
this test message doesn't seem right either. Maybe "Successfully added user?" would be better.
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110650330
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
+ address_line2 => defined_or_default($params->{addressLine2}, ""),
+ city => defined_or_default($params->{city}, ""),
+ company => defined_or_default($params->{company}, ""),
+ country => defined_or_default($params->{country}, ""),
+ email => $params->{email},
+ full_name => $params->{fullName},
+ new_user => defined_or_default($params->{newUser}, 0),
+ phone_number => defined_or_default($params->{phoneNumber}, ""),
+ postal_code => defined_or_default($params->{postalCode}, ""),
+ public_ssh_key => defined_or_default($params->{publicSshKey}, ""),
+ registration_sent => undef,
+ role => $params->{role},
+ state_or_province => defined_or_default($params->{stateOrProvince}, ""),
+ username => $params->{username},
+ uid => 0,
+ gid => 0,
+ local_passwd => sha1_hex($params->{localPassword} ),
+ confirm_local_passwd => sha1_hex($params->{confirmLocalPassword} ),
+ tenant_id => $tenant_id,
+
+ };
+
+ my ( $is_valid, $result ) = $self->is_valid($values);
+
+ if ( !$is_valid ) {
+ return $self->alert($result);
+ }
+
+ my $insert = $self->db->resultset('TmUser')->create($values);
+ my $rs = $insert->insert();
+
+ if ($rs) {
+ my $response;
+ $response->{addressLine1} = $rs->address_line1;
+ $response->{addressLine2} = $rs->address_line2;
+ $response->{city} = $rs->city;
+ $response->{company} = $rs->company;
+ $response->{country} = $rs->country;
+ $response->{email} = $rs->email;
+ $response->{fullName} = $rs->full_name;
+ $response->{gid} = $rs->gid;
+ $response->{id} = $rs->id;
+ $response->{lastUpdated} = $rs->last_updated;
+ $response->{newUser} = \$rs->new_user;
+ $response->{phoneNumber} = $rs->phone_number;
+ $response->{postalCode} = $rs->postal_code;
+ $response->{publicSshKey} = $rs->public_ssh_key;
+ $response->{registrationSent} = \$rs->registration_sent;
+ $response->{role} = $rs->role->id;
+ $response->{roleName} = $rs->role->name;
+ $response->{stateOrProvince} = $rs->state_or_province;
+ $response->{uid} = $rs->uid;
+ $response->{username} = $rs->username;
+ $response->{tenantId} = $rs->tenant_id;
--- End diff --
Ori plans to add it in a separate CL
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r113034309
--- Diff: traffic_ops/app/t/api/1.2/user_admin.t ---
@@ -0,0 +1,126 @@
+package main;
+#
+#
+# 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 Mojo::Base -strict;
+use Test::More;
+use Test::Mojo;
+use DBI;
+use Data::Dumper;
+use strict;
+use warnings;
+use Schema;
+use Test::TestHelper;
+use Fixtures::TmUser;
+use Fixtures::Deliveryservice;
+use Digest::SHA1 qw(sha1_hex);
+use Data::Dumper;
+
+#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
+#no_transactions=>0 ==> delete fixtures after every execution
+
+BEGIN { $ENV{MOJO_MODE} = "test" }
+
+sub run_ut {
+ my $t = shift;
+ my $schema = shift;
+ my $login_user = shift;
+ my $login_password = shift;
+
+ Test::TestHelper->unload_core_data($schema);
+ Test::TestHelper->teardown( $schema, 'Log' );
+ Test::TestHelper->teardown( $schema, 'Role' );
+ Test::TestHelper->teardown( $schema, 'TmUser' );
+
+ Test::TestHelper->load_core_data($schema);
+
+ my $tenant_id = $schema->resultset('TmUser')->find( { username => $login_user } )->get_column('tenant_id');
+ my $tenant_name = defined ($tenant_id) ? $schema->resultset('Tenant')->find( { id => $tenant_id } )->get_column('name') : "null";
+
+ # Verify the user
+ ok my $user = $schema->resultset('TmUser')->find( { username => $login_user } ), 'Does the portal user exist?';
+
+ ok $t->post_ok( '/login', => form => { u => $login_user, p => $login_password} )->status_is(302);
+
+ #adding a user
+ my $addedUserName = "user1";
+ my $addedUserEmail = "abc\@z.com";
+
+ ok $t->post_ok('/api/1.2/users' => {Accept => 'application/json'} => json => {
+ "username" => $addedUserName, "fullName"=>"full name", "email" => $addedUserEmail, "localPassword" => "pass", "confirmLocalPassword"=> "pass", "role" => 4 })
+ ->status_is(200)->or( sub { diag $t->tx->res->content->asset->{content}; } )
+ ->json_is( "/response/username" => $addedUserName )
+ ->json_is( "/response/email" => $addedUserEmail)
+ ->json_is( "/response/tenantId" => $tenant_id) #tenant Id not set - getting the tenant id from the user
+ , 'Failed adding user?';
--- 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 pull request #370: Adding a "create user" to the ap...
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/370#discussion_r112562846
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
--- End diff --
you check for username, fullName and email in the is_valid method. so you don't have to do that here.
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r113004733
--- Diff: traffic_ops/app/t/api/1.2/user_admin.t ---
@@ -0,0 +1,126 @@
+package main;
+#
+#
+# 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 Mojo::Base -strict;
+use Test::More;
+use Test::Mojo;
+use DBI;
+use Data::Dumper;
+use strict;
+use warnings;
+use Schema;
+use Test::TestHelper;
+use Fixtures::TmUser;
+use Fixtures::Deliveryservice;
+use Digest::SHA1 qw(sha1_hex);
+use Data::Dumper;
+
+#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
+#no_transactions=>0 ==> delete fixtures after every execution
+
+BEGIN { $ENV{MOJO_MODE} = "test" }
+
+sub run_ut {
+ my $t = shift;
+ my $schema = shift;
+ my $login_user = shift;
+ my $login_password = shift;
+
+ Test::TestHelper->unload_core_data($schema);
+ Test::TestHelper->teardown( $schema, 'Log' );
+ Test::TestHelper->teardown( $schema, 'Role' );
+ Test::TestHelper->teardown( $schema, 'TmUser' );
+
+ Test::TestHelper->load_core_data($schema);
+
+ my $tenant_id = $schema->resultset('TmUser')->find( { username => $login_user } )->get_column('tenant_id');
+ my $tenant_name = defined ($tenant_id) ? $schema->resultset('Tenant')->find( { id => $tenant_id } )->get_column('name') : "null";
+
+ # Verify the user
+ ok my $user = $schema->resultset('TmUser')->find( { username => $login_user } ), 'Does the portal user exist?';
--- End diff --
Can you update this message? not sure what portal user has to do with this.
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r113034286
--- Diff: traffic_ops/app/t/api/1.2/user_admin.t ---
@@ -0,0 +1,126 @@
+package main;
+#
+#
+# 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 Mojo::Base -strict;
+use Test::More;
+use Test::Mojo;
+use DBI;
+use Data::Dumper;
+use strict;
+use warnings;
+use Schema;
+use Test::TestHelper;
+use Fixtures::TmUser;
+use Fixtures::Deliveryservice;
+use Digest::SHA1 qw(sha1_hex);
+use Data::Dumper;
+
+#no_transactions=>1 ==> keep fixtures after every execution, beware of duplicate data!
+#no_transactions=>0 ==> delete fixtures after every execution
+
+BEGIN { $ENV{MOJO_MODE} = "test" }
+
+sub run_ut {
+ my $t = shift;
+ my $schema = shift;
+ my $login_user = shift;
+ my $login_password = shift;
+
+ Test::TestHelper->unload_core_data($schema);
+ Test::TestHelper->teardown( $schema, 'Log' );
+ Test::TestHelper->teardown( $schema, 'Role' );
+ Test::TestHelper->teardown( $schema, 'TmUser' );
+
+ Test::TestHelper->load_core_data($schema);
+
+ my $tenant_id = $schema->resultset('TmUser')->find( { username => $login_user } )->get_column('tenant_id');
+ my $tenant_name = defined ($tenant_id) ? $schema->resultset('Tenant')->find( { id => $tenant_id } )->get_column('name') : "null";
+
+ # Verify the user
+ ok my $user = $schema->resultset('TmUser')->find( { username => $login_user } ), 'Does the portal user exist?';
--- 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 #370: Adding a "create user" to the api
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
Indeed
On Apr 7, 2017 6:16 AM, "Jeremy Mitchell" <no...@github.com> wrote:
> *@mitchell852* commented on this pull request.
> ------------------------------
>
> In traffic_ops/app/lib/API/User.pm
> <https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110310096>
> :
>
> > + my $name = $params->{username};
> + if ( !defined($name) ) {
> + return $self->alert("Username is required.");
> + }
> +
> + my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
> + if ($existing) {
> + return $self->alert("A user with username \"$name\" already exists.");
> + }
> +
> +
> + if ( !defined($params->{fullName}) ) {
> + return $self->alert("full-name is required.");
> + }
> +
> + if ( !defined($params->{email}) ) {
>
> nevermind, i guess email uniqueness is checked in the is_valid() function
>
> \u2014
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110310096>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AXGkYLXxSpb8U3fcfR6JyCEygD0RjcT5ks5rtaqDgaJpZM4MehR1>
> .
>
---
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 #370: Adding a "create user" to the api
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
@nir-sopher - can you fix this PR? "This branch cannot be rebased safely" - i'd like to get it merged if possible
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110310096
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -218,6 +218,115 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
--- End diff --
nevermind, i guess email uniqueness is checked in the is_valid() function
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r119400623
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -229,6 +229,112 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
--- End diff --
this is not necessary, it is validated in the is_valid method
---
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 #370: Adding a "create user" to the api
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
The change of the "role" broke the UT and was reverted.
It changes current behavior of user-update.
I prefer not to make such changes in this PR
---
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 #370: Adding a "create user" to the api
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
Rebase was done a PR review changes were made, except for a test for "username" in the beginning of the create function. The test is done as the value is in use by the function logic.
---
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 #370: Adding a "create user" to the ap...
Posted by nir-sopher <gi...@git.apache.org>.
Github user nir-sopher commented on a diff in the pull request:
https://github.com/apache/incubator-trafficcontrol/pull/370#discussion_r110659653
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -226,6 +226,120 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{fullName}) ) {
+ return $self->alert("full-name is required.");
+ }
+
+ if ( !defined($params->{email}) ) {
+ return $self->alert("email is required.");
+ }
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
+ return $self->alert("role is required.");
+ }
+
+ #setting tenant_id to the user's tenant if tenant is not set. TODO(nirs): remove when tenancy is no longer optional in the API
+ my $tenant_id = exists($params->{tenantId}) ? $params->{tenantId} : $self->current_user_tenant();
+
+ my $values = {
+ address_line1 => defined_or_default($params->{addressLine1}, ""),
--- 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 #370: Adding a "create user" to the api
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
you might want to run your code thru the perl formatter. I see some formatting errors. Here's more info about that:
https://github.com/apache/incubator-trafficcontrol/blob/master/CONTRIBUTING.md#code-formatting
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 issue #370: Adding a "create user" to the api
Posted by mitchell852 <gi...@git.apache.org>.
Github user mitchell852 commented on the issue:
https://github.com/apache/incubator-trafficcontrol/pull/370
yes, there is a UT failure:
t/user.t (Wstat: 256 Tests: 21 Failed: 1)
Failed test: 5
Non-zero exit status: 1
but i'm pretty sure this was fixed in master so i'll pull this in and if it's still broken, i can fix it.
it only changes the behavior of user-update in the sense that role is now checked which in my opinion is a very valid check. you should not be able to update a user and leave out a required field - role.
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r110309119
--- Diff: docs/source/development/traffic_ops_api/v12/user.rst ---
@@ -214,6 +214,153 @@ Users
|
+
+**POST /api/1.2/users**
+
+ Create a user.
+
+ Authentication Required: Yes
+
+ Role(s) Required: admin or oper
+
+ **Request Properties**
+
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | Parameter | Type | Required | Description |
+ +=========================+========+==========+=================================================+
+ |``addressLine1`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``addressLine2`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``city`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``confirmLocalPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``company`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``country`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``email`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``fullName`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``localPassword`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``newUser`` | bool | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``phoneNumber`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``postalCode`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``publicSshKey`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``role`` | int | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``stateOrProvince`` | string | no | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ | ``tenantId`` | int | no | Owning tenant ID |
+ +-------------------------+--------+----------+-------------------------------------------------+
+ |``username`` | string | yes | |
+ +-------------------------+--------+----------+-------------------------------------------------+
+
+
+ **Request Example** ::
+
+ {
+ "username": "tsimpson"
+ "tenantId": 1,
+ "fullName": "Tom Simpson"
+ "email": "email1@email.com"
+ "role": 6
+ "localPassword": "password"
+ "confirmLocalPassword": "password"
+ }
+
+|
+
+ **Response Properties**
+
+ +----------------------+--------+------------------------------------------------+
+ | Parameter | Type | Description |
+ +======================+========+================================================+
+ |``addressLine1`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``addressLine2`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``city`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``company`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``country`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``email`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``fullName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``gid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``id`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``lastUpdated`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``newUser`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``phoneNumber`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``postalCode`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``publicSshKey`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``registrationSent`` | bool | |
+ +----------------------+--------+------------------------------------------------+
+ |``role`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ |``roleName`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``stateOrProvince`` | string | |
+ +----------------------+--------+------------------------------------------------+
+ |``uid`` | int | |
+ +----------------------+--------+------------------------------------------------+
+ | ``tenantId`` | int | Owning tenant ID |
--- End diff --
Can you add tenant to this response?
---
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 #370: Adding a "create user" to the ap...
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/370#discussion_r119400773
--- Diff: traffic_ops/app/lib/API/User.pm ---
@@ -229,6 +229,112 @@ sub update {
}
+# Create
+sub create {
+ my $self = shift;
+ my $params = $self->req->json;
+
+ if ( !&is_oper($self) ) {
+ return $self->forbidden();
+ }
+
+ my $name = $params->{username};
+ if ( !defined($name) ) {
+ return $self->alert("Username is required.");
+ }
+
+ my $existing = $self->db->resultset('TmUser')->search( { username => $name } )->single();
+ if ($existing) {
+ return $self->alert("A user with username \"$name\" already exists.");
+ }
+
+
+ if ( !defined($params->{localPassword}) ) {
+ return $self->alert("local-password is required.");
+ }
+
+ if ( !defined($params->{confirmLocalPassword}) ) {
+ return $self->alert("confirm-local-password is required.");
+ }
+
+ if ($params->{localPassword} ne $params->{confirmLocalPassword}){
+ return $self->alert("local-password and confirmed-local-password mismatch.");
+ }
+
+ if ( !defined($params->{role}) ) {
--- End diff --
if you add 'role' to line 463, you can take this out as well....
---
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 #370: Adding a "create user" to the ap...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafficcontrol/pull/370
---
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.
---