Split the tagging permission into two

- New permission flag: perm_tag_editor
  - taggers can tag events with existing tags
  - tag editors can create / edit / delete tags

- Fixed several misleading UI elements for tagging
  - tagging users that don't own an event and aren't creators thereof cannot tag them
  - this was enforced before but the UI elements were present and threw errors

- Migration is automatic
  - all existing tagger roles will automatically become tag editors
  - restricting current roles takes manual admin action, but the functionality should remain unchanged for those that just update

VERSION.json

@@ -1 +1 @@
-{"major":2, "minor":4, "hotfix":31}
+{"major":2, "minor":4, "hotfix":32}

app/Controller/AppController.php

@@ -273,6 +273,7 @@ class AppController extends Controller {
 			$this->set('isAclAuth', $role['perm_auth']);
 			$this->set('isAclRegexp', $role['perm_regexp_access']);
 			$this->set('isAclTagger', $role['perm_tagger']);
+			$this->set('isAclTagEditor', $role['perm_tag_editor']);
 			$this->set('isAclTemplate', $role['perm_template']);
 			$this->set('isAclSharingGroup', $role['perm_sharing_group']);
 			$this->userRole = $role;

app/Controller/TagsController.php

@@ -82,7 +82,7 @@ class TagsController extends AppController {
 	}
 	
 	public function add() {
-		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tagger']) throw new NotFoundException('You don\'t have permission to do that.');
+		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tag_editor']) throw new NotFoundException('You don\'t have permission to do that.');
 		if ($this->request->is('post')) {
 			if (isset($this->request->data['Tag']['request'])) $this->request->data['Tag'] = $this->request->data['Tag']['request']; 
 			if (!isset($this->request->data['Tag']['colour'])) $this->request->data['Tag']['colour'] = $this->Tag->random_color();
@@ -104,7 +104,7 @@ class TagsController extends AppController {
 	}
 	
 	public function quickAdd() {
-		if ((!$this->_isSiteAdmin() && !$this->userRole['perm_tagger']) || !$this->request->is('post')) throw new NotFoundException('You don\'t have permission to do that.');
+		if ((!$this->_isSiteAdmin() && !$this->userRole['perm_tag_editor']) || !$this->request->is('post')) throw new NotFoundException('You don\'t have permission to do that.');
 		if (isset($this->request->data['Tag']['request'])) $this->request->data['Tag'] = $this->request->data['Tag']['request'];
 		if ($this->Tag->quickAdd($this->request->data['Tag']['name'])) {
 			$this->Session->setFlash('The tag has been saved.');
@@ -115,7 +115,7 @@ class TagsController extends AppController {
 	}
 	
 	public function edit($id) {
-		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tagger']) {
+		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tag_editor']) {
 			throw new NotFoundException('You don\'t have permission to do that.');
 		}
 		$this->Tag->id = $id;
@@ -143,7 +143,7 @@ class TagsController extends AppController {
 	}
 	
 	public function delete($id) {
-		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tagger']) {
+		if (!$this->_isSiteAdmin() && !$this->userRole['perm_tag_editor']) {
 			throw new NotFoundException('You don\'t have permission to do that.');
 		}
 		if (!$this->request->is('post')) {
@@ -212,7 +212,7 @@ class TagsController extends AppController {
 				'conditions' => array(
 						'event_id' => $id
 				),
-				'contain' => 'Tag',
+				'contain' => array('Tag'),
 				'fields' => array('Tag.id', 'Tag.colour', 'Tag.name'),
 		));
 		$this->set('tags', $tags);
@@ -222,7 +222,11 @@ class TagsController extends AppController {
 			$tagNames[$v['Tag']['id']] = $v['Tag']['name'];
 		}
 		$this->set('allTags', $tagNames);
-		$event['Event']['id'] = $id;
+		$event = $this->Tag->EventTag->Event->find('first', array(
+				'recursive' => -1,
+				'fields' => array('Event.id', 'Event.orgc_id', 'Event.org_id', 'Event.user_id'),
+				'conditions' => array('Event.id' => $id)
+		));
 		$this->set('event', $event);
 		$this->layout = 'ajax';
 		$this->render('/Events/ajax/ajaxTags');

app/Model/AppModel.php

@@ -49,7 +49,7 @@ class AppModel extends Model {
 	// major -> minor -> hotfix -> requires_logout
 	public $db_changes = array(
 		2 => array(
-			4 => array(18 => false, 19 => false, 20 => false, 25 => false, 27 => false)
+			4 => array(18 => false, 19 => false, 20 => false, 25 => false, 27 => false, 32 => false)
 		)
 	);
 	
@@ -305,6 +305,10 @@ class AppModel extends Model {
 					PRIMARY KEY (`id`)
 					) ENGINE=InnoDB DEFAULT CHARSET=utf8;";
 				break;
+			case '2.4.32':
+				$sqlArray[] = "ALTER TABLE `roles` ADD `perm_tag_editor` tinyint(1) NOT NULL DEFAULT '0';";
+				$sqlArray[] = 'UPDATE `roles` SET `perm_tag_editor` = 1 WHERE `perm_tagger` = 1';
+				break;
 			case 'fixNonEmptySharingGroupID':
 				$sqlArray[] = 'UPDATE `events` SET `sharing_group_id` = 0 WHERE `distribution` != 4';
 				$sqlArray[] = 'UPDATE `attributes` SET `sharing_group_id` = 0 WHERE `distribution` != 4';

app/Model/Event.php

@@ -1781,7 +1781,10 @@ class Event extends AppModel {
 		if (!isset($data['Event']['EventTag'])) $data['Event']['EventTag'] = array();
 		if (isset($data['Event']['Tag'])) {
 			if (isset($data['Event']['Tag']['name'])) $data['Event']['Tag'] = array($data['Event']['Tag']);
-			foreach ($data['Event']['Tag'] as $tag) $data['Event']['EventTag'][] = array('tag_id' => $this->EventTag->Tag->captureTag($tag, $user));
+			foreach ($data['Event']['Tag'] as $tag) {
+				$tag_id = $this->EventTag->Tag->captureTag($tag, $user);
+				if ($tag_id) $data['Event']['EventTag'][] = array('tag_id' => $tag_id);
+			}
 			unset($data['Event']['Tag']);
 		}
 		return $data;
@@ -2036,17 +2039,22 @@ class Event extends AppModel {
 					if ($tag_id) {
 						$this->EventTag->attachTagToEvent($this->id, $tag_id);
 					} else {
-						$this->Log->create();
-						$this->Log->save(array(
-							'org' => $user['Organisation']['name'],
-							'model' => 'Event',
-							'model_id' => $this->id,
-							'email' => $user['email'],
-							'action' => 'edit',
-							'user_id' => $user['id'],
-							'title' => 'Failed create or attach Tag ' . $tag['name'] . ' to the event.',
-							'change' => ''
-						));
+						// If we couldn't attach the tag it is most likely because we couldn't create it - which could have many reasons
+						// However, if a tag couldn't be added, it could also be that the user is a tagger but not a tag editor
+						// In which case if no matching tag is found, no tag ID is returned. Logging these is pointless as it is the correct behaviour.
+						if ($user['Role']['perm_tag_editor']) {
+							$this->Log->create();
+							$this->Log->save(array(
+								'org' => $user['Organisation']['name'],
+								'model' => 'Event',
+								'model_id' => $this->id,
+								'email' => $user['email'],
+								'action' => 'edit',
+								'user_id' => $user['id'],
+								'title' => 'Failed create or attach Tag ' . $tag['name'] . ' to the event.',
+								'change' => ''
+							));
+						}
 					}
 				}
 			}

app/Model/Role.php

@@ -70,7 +70,8 @@ class Role extends AppModel {
 		'perm_audit' => array('id' => 'RolePermAudit', 'text' => 'Audit Actions'),
 		'perm_auth' => array('id' => 'RolePermAuth', 'text' => 'Auth key access'),
 		'perm_regexp_access' => array('id' => 'RolePermRegexpAccess', 'text' => 'Regex Actions'),
-		'perm_tagger' => array('id' => 'RolePermTagger', 'text' => 'Tag Editor'),
+		'perm_tagger' => array('id' => 'RolePermTagger', 'text' => 'Tagger'),
+		'perm_tag_editor' => array('id' => 'RolePermTagEditor', 'text' => 'Tag Editor'),
 		'perm_template' => array('id' => 'RolePermTemplate', 'text' => 'Template Editor'),
 		'perm_sharing_group' => array('id' => 'RolePermSharingGroup', 'text' => 'Sharing Group Editor'),
 	);

app/Model/Tag.php

@@ -109,14 +109,16 @@ class Tag extends AppModel {
 				'conditions' => array('LOWER(name)' => strtolower($tag['name']))
 		));
 		if (empty($existingTag)) {
-			$this->create();
-			$tag = array(
-					'name' => $tag['name'],
-					'colour' => $tag['colour'],
-					'exportable' => $tag['exportable'],
-			);
-			$this->save($tag);
-			return $this->id;
+			if ($user['Role']['perm_tag_editor']) {
+				$this->create();
+				$tag = array(
+						'name' => $tag['name'],
+						'colour' => $tag['colour'],
+						'exportable' => $tag['exportable'],
+				);
+				$this->save($tag);
+				return $this->id;
+			} else return false;
 		}
 		return $existingTag['Tag']['id'];
 	}

app/View/Elements/ajaxTags.ctp

@@ -3,13 +3,13 @@
 	<?php
 		foreach ($tags as $tag): ?>
 			<td style="padding-right:0px;">
-				<?php if ($isAclTagger): ?>
+				<?php if ($isAclTagger && $tagAccess): ?>
 					<a href="<?php echo $baseurl;?>/events/index/searchtag:<?php echo h($tag['Tag']['id']); ?>" class="tagFirstHalf" style="background-color:<?php echo h($tag['Tag']['colour']);?>;color:<?php echo $this->TextColour->getTextColour($tag['Tag']['colour']);?>"><?php echo h($tag['Tag']['name']); ?></a>
 				<?php else: ?>
 					<a href="<?php echo $baseurl;?>/events/index/searchtag:<?php echo h($tag['Tag']['id']); ?>" class=tag style="background-color:<?php echo h($tag['Tag']['colour']);?>;color:<?php echo $this->TextColour->getTextColour($tag['Tag']['colour']);?>"><?php echo h($tag['Tag']['name']); ?></a>
 				<?php endif; ?>
 			</td>
-			<?php if ($isAclTagger): ?>
+			<?php if ($isAclTagger && $tagAccess): ?>
 				<td style="padding-left:0px;padding-right:5px;">
 					<?php
 						echo $this->Form->create('Event', array('id' => 'removeTag_' . h($tag['Tag']['id']),  'url' => '/events/removeTag/' . h($event['Event']['id']) . '/' . h($tag['Tag']['id']), 'style' => 'margin:0px;'));
@@ -22,7 +22,7 @@
 			<?php endif; ?>
 			<?php 
 		endforeach;
-		if ($isAclTagger): ?>
+		if ($isAclTagger && $tagAccess): ?>
 			<td>
 				<button id="addTagButton" class="btn btn-inverse noPrint" style="line-height:10px; padding: 4px 4px;" onClick="getPopup('<?php echo h($event['Event']['id']); ?>', 'tags', 'selectTaxonomy');">+</button>
 			</td>

app/View/Elements/global_menu.ctp

@@ -33,7 +33,7 @@
 							<li><a href="<?php echo $baseurl;?>/events/proposalEventIndex">Events with proposals</a></li>
 							<li class="divider"></li>
 							<li><a href="<?php echo $baseurl;?>/tags/index">List Tags</a></li>
-							<?php if ($isAclTagger): ?>
+							<?php if ($isAclTagEditor): ?>
 							<li><a href="<?php echo $baseurl;?>/tags/add">Add Tag</a></li>
 							<?php endif; ?>
 							<li><a href="<?php echo $baseurl;?>/taxonomies/index">List Taxonomies</a></li>

app/View/Elements/side_menu.ctp

@@ -260,7 +260,7 @@
 				
 				case 'tags': ?>
 					<li id='liindex'><?php echo $this->Html->link('List Tags', array('action' => 'index'));?></li>
-					<?php if ($isAclTagger): ?>
+					<?php if ($isAclTagEditor): ?>
 					<li id='liadd'><?php echo $this->Html->link('Add Tag', array('action' => 'add'));?></li>
 					<?php 
 					endif;

app/View/Events/ajax/ajaxTags.ctp

@@ -1,3 +1,4 @@
 <?php 
-	echo $this->element('ajaxTags', array('event' => $event, 'tags' => $tags));
+	$mayModify = (($isAclModify && $event['Event']['user_id'] == $me['id'] && $event['Event']['orgc_id'] == $me['org_id']) || ($isAclModifyOrg && $event['Event']['orgc_id'] == $me['org_id']));
+	echo $this->element('ajaxTags', array('event' => $event, 'tags' => $tags, 'tagAccess' => ($isSiteAdmin || $mayModify || $me['org_id'] == $event['Event']['org_id'])));
 ?>

app/View/Events/view.ctp

@@ -74,7 +74,7 @@
 					?>
 					 
 				</dd>
-				<?php if (isset($event['User']['email']) && ($isSiteAdmin || ($isAdmin && $me['org'] == $event['Event']['org']))): ?>
+				<?php if (isset($event['User']['email']) && ($isSiteAdmin || ($isAdmin && $me['org_id'] == $event['Event']['org_id']))): ?>
 				<dt>Email</dt>
 				<dd>
 					<?php echo h($event['User']['email']); ?>
@@ -85,7 +85,7 @@
 					if (Configure::read('MISP.tagging')): ?>
 						<dt>Tags</dt>
 						<dd class="eventTagContainer">
-							<?php echo $this->element('ajaxTags', array('event' => $event, 'tags' => $event['EventTag'])); ?>
+							<?php echo $this->element('ajaxTags', array('event' => $event, 'tags' => $event['EventTag'], 'tagAccess' => ($isSiteAdmin || $mayModify || $me['org_id'] == $event['Event']['org_id']) )); ?>
 						</dd>
 				<?php endif; ?>
 				<dt>Date</dt>

app/View/Tags/index.ctp

@@ -23,7 +23,7 @@
 			<th><?php echo $this->Paginator->sort('name');?></th>
 			<th>Taxonomy</th>
 			<th>Tagged events</th>
-			<?php if ($isAclTagger): ?>
+			<?php if ($isAclTagEditor): ?>
 			<th class="actions"><?php echo __('Actions');?></th>
 			<?php endif; ?>
 	</tr><?php
@@ -41,7 +41,7 @@ foreach ($list as $item): ?>
 		&nbsp;
 		</td>
 		<td class="short"><?php echo h($item['Tag']['count']); ?>&nbsp;</td>
-		<?php if ($isAclTagger): ?>
+		<?php if ($isAclTagEditor): ?>
 		<td class="short action-links">
 			<?php echo $this->Html->link('', array('action' => 'edit', $item['Tag']['id']), array('class' => 'icon-edit', 'title' => 'Edit'));?>
 			<?php echo $this->Form->postLink('', array('action' => 'delete', $item['Tag']['id']), array('class' => 'icon-trash', 'title' => 'Delete'), __('Are you sure you want to delete "%s"?', $item['Tag']['name']));?>