Fixed #11794 Admins Cannot View Encrypted Field

[inietov]

Description

Add permission to 'admin' roles to see encrypted fields values when looking for an asset (AssetsTransformer), and when viewing the details of a determined asset (resources/views/hardware/view.blade.php).

Fixes #11794

Type of change

• [x} Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Test Configuration:

• PHP version: 8.1
• MySQL version: 8.0.23
• Webserver version: PHP dev server
• OS version: Debian 11

[what-the-diff]

PR Summary

• Enhanced Security Checks in AssetsTransformer.php
  The transformAsset function now checks for both 'admin' and 'superadmin' user roles. This provides an additional layer of security, ensuring only privileged users have access to decrypted values. Should a field be formatted as 'DATE', the decrypted value will also be neatly formatted as a date if the user carries the 'admin' role.

• Role Permissions Update in view.blade.php
  Permissions directives have been updated, replacing @can with the @canany directive. Now the 'admin' role is included alongside the 'superuser' role. This will allow more types of users to engage with the app functionality, broadening the use-case scenarios without compromising security.

[snipe]

My expectation here would be that if you have the ability to edit an asset (and therefore edit the encrypted custom field) you should be able to see the value of that field.

[inietov]

@snipe So instead of looking the roles I should look for the correct permissions? or in addition to the roles I need to check permissions?

[snipe]

@snipe So instead of looking the roles I should look for the correct permissions? or in addition to the roles I need to check permissions?

That's an excellent question.

Right now it looks like only superusers can view encrypted custom fields. Right now it looks like only superusers can see encrypted custom fields in the list view and detail view, but it doesn't look like we check for a gate on the edit screen - which is exactly what the OP was reporting.

snipe-it/resources/views/hardware/edit.blade.php

Lines 76 to 93 in 74a5bcd

	<div id='custom_fields_content'>
	<!-- Custom Fields -->
	@if ($item->model && $item->model->fieldset)
	<?php $model = $item->model; ?>
	@endif
	@if (Request::old('model_id'))
	@php
	$model = \App\Models\AssetModel::find(old('model_id'));
	@endphp
	@elseif (isset($selected_model))
	@php
	$model = $selected_model;
	@endphp
	@endif
	@if (isset($model) && $model)
	@include("models/custom_fields_form",["model" => $model])
	@endif
	</div>

The problem here is that we're being inconsistent with the gating. If only superusers should be able to see encrypted custom fields in a list view or detail view, they should be the only ones that can edit them as well. However, this will likely break existing functionality.

We do check for that gate in the API responses:

snipe-it/app/Http/Transformers/AssetsTransformer.php

Line 104 in 74a5bcd

$value = (Gate::allows('superadmin')) ? $decrypted : strtoupper(trans('admin/custom_fields/general.encrypted'));

Stupidly, we check for admin permissions on the edit screen, so it seems we have 3 problems now.

snipe-it/resources/views/models/custom_fields_form.blade.php

Lines 56 to 60 in 74a5bcd

	@if (($field->field_encrypted=='0') || (Gate::allows('admin')))
	<input type="text" value="{{ Request::old($field->db_column_name(),(isset($item) ? Helper::gracefulDecrypt($field, $item->{$field->db_column_name()}) : $field->defaultValue($model->id))) }}" id="{{ $field->db_column_name() }}" class="form-control" name="{{ $field->db_column_name() }}" placeholder="Enter {{ strtolower($field->format) }} text">
	@else
	<input type="text" value="{{ strtoupper(trans('admin/custom_fields/general.encrypted')) }}" class="form-control disabled" disabled>
	@endif

Maybe we should add "view/edit encrypted fields" as an asset permission? It would mean a larger changeset but might be worth considering. My concern would potentially be that if ONLY superusers could see this before, it could potentially create a privilege escalation issue. Meaning, a "regular user" who is allowed to manage users but is not a superadmin could potentially allow asset managers who are not super admins to see data they previously could not. 🤔

snipe-it/resources/views/hardware/view.blade.php

Lines 474 to 485 in 74a5bcd

	@if ($field->isFieldDecryptable($asset->{$field->db_column_name()} ))
	@can('superuser')
	@if (($field->format=='URL') && ($asset->{$field->db_column_name()}!=''))
	<a href="{{ Helper::gracefulDecrypt($field, $asset->{$field->db_column_name()}) }}" target="_new">{{ Helper::gracefulDecrypt($field, $asset->{$field->db_column_name()}) }}</a>
	@elseif (($field->format=='DATE') && ($asset->{$field->db_column_name()}!=''))
	{{ \App\Helpers\Helper::gracefulDecrypt($field, \App\Helpers\Helper::getFormattedDateObject($asset->{$field->db_column_name()}, 'date', false)) }}
	@else
	{{ Helper::gracefulDecrypt($field, $asset->{$field->db_column_name()}) }}
	@endif
	@else
	{{ strtoupper(trans('admin/custom_fields/general.encrypted')) }}
	@endcan

@uberbrady do you have any additional thoughts here? (I know we've been workshopping this together offline just now.)

My thoughts here:

• Remove the superuser and admin checks
• Add a new permission for editing/viewing encrypted custom fields

I consider this a security bug actually, so I'd consider it higher priority than anything else on deck. If you think you can't tackle this today, let me know and I'll pick it up.

[inietov]

I think it now works as expected, let me know if something with the defined gate is not correct because I had a lot of fight and don't know if I created it correctly. Thanks for your patience!

[marcusmoore]

Assuming this is the direction we want to go in: this looks good and works 😄

[inietov]

Another kind ping @snipe