More work on read-only views for admin_view permissions.

src/api-umbrella/admin-ui/app/components/admin-groups/record-form.js

@@ -1,6 +1,8 @@
 // eslint-disable-next-line ember/no-classic-components
 import Component from '@ember/component';
-import { action } from '@ember/object';
+import { action, computed } from '@ember/object';
+import { reads } from '@ember/object/computed';
+import { inject } from '@ember/service';
 import { tagName } from '@ember-decorators/component';
 // eslint-disable-next-line ember/no-mixins
 import Save from 'api-umbrella-admin-ui/mixins/save';
@@ -10,6 +12,17 @@ import escape from 'lodash-es/escape';
 @classic
 @tagName("")
 export default class RecordForm extends Component.extend(Save) {
+  @inject()
+  session;
+
+  @reads('session.data.authenticated.admin')
+  currentAdmin;
+
+  @computed('currentAdmin.permissions.admin_manage')
+  get isDisabled() {
+    return !this.currentAdmin.permissions.admin_manage;
+  }
+
   @action
   submitForm(event) {
     event.preventDefault();

src/api-umbrella/admin-ui/app/components/admins/record-form.js

@@ -1,6 +1,6 @@
 // eslint-disable-next-line ember/no-classic-components
 import Component from '@ember/component';
-import { action } from '@ember/object';
+import { action, computed } from '@ember/object';
 import { reads } from '@ember/object/computed';
 import { inject } from '@ember/service';
 import { tagName } from '@ember-decorators/component';
@@ -20,6 +20,11 @@ export default class RecordForm extends Component.extend(Save) {
   @reads('session.data.authenticated.admin')
   currentAdmin;
 
+  @computed('currentAdmin.permissions.admin_manage')
+  get isDisabled() {
+    return !this.currentAdmin.permissions.admin_manage;
+  }
+
   get usernameLabel() {
     return usernameLabel();
   }

src/api-umbrella/admin-ui/app/components/api-scopes/record-form.js

@@ -1,6 +1,6 @@
 // eslint-disable-next-line ember/no-classic-components
 import Component from '@ember/component';
-import { action } from '@ember/object';
+import { action, computed } from '@ember/object';
 import { reads } from '@ember/object/computed';
 import { inject } from '@ember/service';
 import { tagName } from '@ember-decorators/component';
@@ -18,6 +18,11 @@ export default class RecordForm extends Component.extend(Save) {
   @reads('session.data.authenticated.admin')
   currentAdmin;
 
+  @computed('currentAdmin.permissions.admin_manage')
+  get isDisabled() {
+    return !this.currentAdmin.permissions.admin_manage;
+  }
+
   @action
   submitForm(event) {
     event.preventDefault();

src/api-umbrella/admin-ui/app/components/api-users/record-form.js

@@ -1,6 +1,7 @@
 // eslint-disable-next-line ember/no-classic-components
 import Component from '@ember/component';
 import { action, computed } from '@ember/object';
+import { reads } from '@ember/object/computed';
 import { inject } from '@ember/service';
 // eslint-disable-next-line ember/no-mixins
 import Save from 'api-umbrella-admin-ui/mixins/save';
@@ -24,10 +25,12 @@ export default class RecordForm extends Component.extend(Save) {
     { id: false, name: 'Disabled' },
   ];
 
-  @computed('session.data.authenticated.admin')
+  @reads('session.data.authenticated.admin')
+  currentAdmin;
+
+  @computed('currentAdmin.permissions.user_manage')
   get isDisabled() {
-    const currentAdmin = this.session.data.authenticated.admin;
-    return !currentAdmin.permissions.user_manage;
+    return !this.currentAdmin.permissions.user_manage;
   }
 
   @action

src/api-umbrella/admin-ui/app/controllers/admin-groups/index.js

@@ -0,0 +1,10 @@
+import Controller from '@ember/controller';
+import { inject as service } from '@ember/service';
+
+export default class IndexController extends Controller {
+  @service session;
+
+  get currentAdmin() {
+    return this.session.data.authenticated.admin;
+  }
+}

src/api-umbrella/admin-ui/app/controllers/admins/index.js

@@ -0,0 +1,10 @@
+import Controller from '@ember/controller';
+import { inject as service } from '@ember/service';
+
+export default class IndexController extends Controller {
+  @service session;
+
+  get currentAdmin() {
+    return this.session.data.authenticated.admin;
+  }
+}

src/api-umbrella/admin-ui/app/controllers/api-scopes/index.js

@@ -0,0 +1,10 @@
+import Controller from '@ember/controller';
+import { inject as service } from '@ember/service';
+
+export default class IndexController extends Controller {
+  @service session;
+
+  get currentAdmin() {
+    return this.session.data.authenticated.admin;
+  }
+}

src/api-umbrella/admin-ui/app/templates/admin-groups/index.hbs

@@ -1,7 +1,9 @@
 <h1>Admin Groups</h1>
 
-<div class="button-actions button-actions-down">
-  <LinkTo @route="admin_groups.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New Admin Group</LinkTo>
-</div>
+{{#if this.currentAdmin.permissions.admin_manage}}
+  <div class="button-actions button-actions-down">
+    <LinkTo @route="admin_groups.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New Admin Group</LinkTo>
+  </div>
+{{/if}}
 
 <AdminGroups::IndexTable />

src/api-umbrella/admin-ui/app/templates/admins/index.hbs

@@ -1,7 +1,9 @@
 <h1>Admins</h1>
 
-<div class="button-actions button-actions-down">
-  <LinkTo @route="admins.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New Admin</LinkTo>
-</div>
+{{#if this.currentAdmin.permissions.admin_manage}}
+  <div class="button-actions button-actions-down">
+    <LinkTo @route="admins.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New Admin</LinkTo>
+  </div>
+{{/if}}
 
 <Admins::IndexTable />

src/api-umbrella/admin-ui/app/templates/api-scopes/index.hbs

@@ -1,7 +1,9 @@
 <h1>API Scopes</h1>
 
-<div class="button-actions button-actions-down">
-  <LinkTo @route="api_scopes.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New API Scope</LinkTo>
-</div>
+{{#if this.currentAdmin.permissions.admin_manage}}
+  <div class="button-actions button-actions-down">
+    <LinkTo @route="api_scopes.new" class="btn btn-primary"><FaIcon @icon="plus" /> Add New API Scope</LinkTo>
+  </div>
+{{/if}}
 
 <ApiScopes::IndexTable />

src/api-umbrella/admin-ui/app/templates/components/admin-groups/record-form.hbs

@@ -3,7 +3,7 @@
 
   <form {{on "submit" this.submitForm}}>
     <FieldsFor @model={{this.model}} @style="horizontal" as |f|>
-      <fieldset class="form-horizontal condensed">
+      <fieldset class="form-horizontal condensed" disabled={{this.isDisabled}}>
         {{f.text-field "name" label="Group Name"}}
         {{f.checkboxes-field "apiScopeIds" label="Scopes" options=this.apiScopeOptions}}
         {{f.checkboxes-field "permissionIds" label="Permissions" options=this.permissionOptions}}
@@ -25,7 +25,9 @@
 
       <div class="row">
         <div class="col-sm-6">
-          <button type="submit" class="btn btn-lg btn-primary save-button"><span class="btn-label">Save</span><span class="btn-loading-label"><FaIcon @icon="sync-alt" @spin={{true}} />Saving...</span></button>
+          {{#unless this.isDisabled}}
+            <button type="submit" class="btn btn-lg btn-primary save-button"><span class="btn-label">Save</span><span class="btn-loading-label"><FaIcon @icon="sync-alt" @spin={{true}} />Saving...</span></button>
+          {{/unless}}
         </div>
         <div class="col-sm-6 record-details">
           {{#if this.model.id}}
@@ -35,10 +37,12 @@
         </div>
       </div>
       {{#if this.model.id}}
-        <div class="form-extra-actions">
-          <a href="#" class="remove-action" {{action "delete"}}><FaIcon @icon="times" />Delete Admin Group</a>
-        </div>
+        {{#unless this.isDisabled}}
+          <div class="form-extra-actions">
+            <a href="#" class="remove-action" {{action "delete"}}><FaIcon @icon="times" />Delete Admin Group</a>
+          </div>
+        {{/unless}}
       {{/if}}
     </FieldsFor>
   </form>
-</div>
+</div>

src/api-umbrella/admin-ui/app/templates/components/admins/record-form.hbs

@@ -54,8 +54,8 @@
         </fieldset>
       {{/if}}
 
-      {{#if this.currentAdmin.permissions.admin_manage}}
-        <fieldset class="form-horizontal condensed">
+      {{#if (or this.currentAdmin.permissions.admin_manage this.currentAdmin.permissions.admin_view)}}
+        <fieldset class="form-horizontal condensed" disabled={{this.isDisabled}}>
           <legend>Permissions</legend>
 
           {{f.checkboxes-field "groupIds" label=(t "Groups") options=this.groupOptions}}

src/api-umbrella/admin-ui/app/templates/components/api-scopes/record-form.hbs

@@ -3,7 +3,7 @@
 
   <form {{on "submit" this.submitForm}}>
     <FieldsFor @model={{this.model}} @style="horizontal" as |f|>
-      <fieldset class="form-horizontal condensed">
+      <fieldset class="form-horizontal condensed" disabled={{this.isDisabled}}>
         {{f.text-field "name" label="Name"}}
         {{f.text-field "host" label="Host"}}
         {{f.text-field "pathPrefix" label="Path Prefix"}}
@@ -39,7 +39,9 @@
 
       <div class="row">
         <div class="col-sm-6">
-          <button type="submit" class="btn btn-lg btn-primary save-button"><span class="btn-label">Save</span><span class="btn-loading-label"><FaIcon @icon="sync-alt" @spin={{true}} />Saving...</span></button>
+          {{#unless this.isDisabled}}
+            <button type="submit" class="btn btn-lg btn-primary save-button"><span class="btn-label">Save</span><span class="btn-loading-label"><FaIcon @icon="sync-alt" @spin={{true}} />Saving...</span></button>
+          {{/unless}}
         </div>
         <div class="col-sm-6 record-details">
           {{#if this.model.id}}
@@ -49,10 +51,12 @@
         </div>
       </div>
       {{#if this.model.id}}
-        <div class="form-extra-actions">
-          <a href="#" class="remove-action" {{action "delete"}}><FaIcon @icon="times" />Delete API Scope</a>
-        </div>
+        {{#unless this.isDisabled}}
+          <div class="form-extra-actions">
+            <a href="#" class="remove-action" {{action "delete"}}><FaIcon @icon="times" />Delete API Scope</a>
+          </div>
+        {{/unless}}
       {{/if}}
     </FieldsFor>
   </form>
-</div>
+</div>

src/api-umbrella/web-app/models/admin.lua

@@ -246,7 +246,6 @@ Admin = model_ext.new_class("admins", {
       username = json_null_default(self.username),
       email = json_null_default(self.email),
       name = json_null_default(self.name),
-      notes = json_null_default(self.notes),
       superuser = json_null_default(self.superuser),
       current_sign_in_provider = json_null_default(self.current_sign_in_provider),
       last_sign_in_provider = json_null_default(self.last_sign_in_provider),
@@ -275,7 +274,12 @@ Admin = model_ext.new_class("admins", {
       version = 1,
     }
 
-    if ngx.ctx.current_admin and ngx.ctx.current_admin.id == self.id then
+    local current_admin = ngx.ctx.current_admin
+    if current_admin and current_admin:allows_permission("admin_manage") then
+      data["notes"] = json_null_default(self.notes)
+    end
+
+    if current_admin and current_admin.id == self.id then
       data["authentication_token"] = self:authentication_token_decrypted()
     end
 

test/apis/v1/admin_groups/test_admin_permissions.rb

@@ -112,14 +112,14 @@ def assert_admin_permitted(factory, admin)
     assert_admin_permitted_index(factory, admin)
     assert_admin_permitted_show(factory, admin)
     permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
-    if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
-      assert_admin_forbidden_create(factory, admin)
-      assert_admin_forbidden_update(factory, admin)
-      assert_admin_forbidden_destroy(factory, admin)
-    else
+    if admin.superuser? || permission_ids.include?("admin_manage")
       assert_admin_permitted_create(factory, admin)
       assert_admin_permitted_update(factory, admin)
       assert_admin_permitted_destroy(factory, admin)
+    else
+      assert_admin_forbidden_create(factory, admin)
+      assert_admin_forbidden_update(factory, admin)
+      assert_admin_forbidden_destroy(factory, admin)
     end
   end
 

test/apis/v1/admins/test_admin_permissions.rb

@@ -270,20 +270,47 @@ def test_permits_any_admin_to_view_but_not_edit_own_record
     assert_equal(0, active_count - initial_count)
   end
 
+  def test_notes_only_visible_to_admin_managers_and_superusers
+    record = FactoryBot.create(:google_admin, :notes => "Private notes")
+
+    superuser_admin = FactoryBot.create(:admin)
+    response = Typhoeus.get("https://127.0.0.1:9081/api-umbrella/v1/admins/#{record.id}.json", http_options.deep_merge(admin_token(superuser_admin)))
+    assert_response_code(200, response)
+    data = MultiJson.load(response.body)
+    assert_equal("Private notes", data.fetch("admin").fetch("notes"))
+
+    manager_admin = FactoryBot.create(:limited_admin, :groups => [
+      FactoryBot.create(:google_admin_group, :admin_view_and_manage_permission),
+    ])
+    response = Typhoeus.get("https://127.0.0.1:9081/api-umbrella/v1/admins/#{record.id}.json", http_options.deep_merge(admin_token(manager_admin)))
+    assert_response_code(200, response)
+    data = MultiJson.load(response.body)
+    assert_equal("Private notes", data.fetch("admin").fetch("notes"))
+
+    viewer_admin = FactoryBot.create(:limited_admin, :groups => [
+      FactoryBot.create(:google_admin_group, :admin_view_permission),
+    ])
+    response = Typhoeus.get("https://127.0.0.1:9081/api-umbrella/v1/admins/#{record.id}.json", http_options.deep_merge(admin_token(viewer_admin)))
+    assert_response_code(200, response)
+    data = MultiJson.load(response.body)
+    refute_includes(data.fetch("admin").keys, "notes")
+    refute_includes("Private notes", response.body)
+  end
+
   private
 
   def assert_admin_permitted(factory, admin)
     assert_admin_permitted_index(factory, admin)
     assert_admin_permitted_show(factory, admin)
     permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
-    if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
-      assert_admin_forbidden_create(factory, admin)
-      assert_admin_forbidden_update(factory, admin)
-      assert_admin_forbidden_destroy(factory, admin)
-    else
+    if admin.superuser? || permission_ids.include?("admin_manage")
       assert_admin_permitted_create(factory, admin)
       assert_admin_permitted_update(factory, admin)
       assert_admin_permitted_destroy(factory, admin)
+    else
+      assert_admin_forbidden_create(factory, admin)
+      assert_admin_forbidden_update(factory, admin)
+      assert_admin_forbidden_destroy(factory, admin)
     end
   end
 

test/apis/v1/api_scopes/test_admin_permissions.rb

@@ -74,14 +74,14 @@ def assert_admin_permitted(factory, admin)
     assert_admin_permitted_index(factory, admin)
     assert_admin_permitted_show(factory, admin)
     permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
-    if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
-      assert_admin_forbidden_create(factory, admin)
-      assert_admin_forbidden_update(factory, admin)
-      assert_admin_forbidden_destroy(factory, admin)
-    else
+    if admin.superuser? || permission_ids.include?("admin_manage")
       assert_admin_permitted_create(factory, admin)
       assert_admin_permitted_update(factory, admin)
       assert_admin_permitted_destroy(factory, admin)
+    else
+      assert_admin_forbidden_create(factory, admin)
+      assert_admin_forbidden_update(factory, admin)
+      assert_admin_forbidden_destroy(factory, admin)
     end
   end