Skip to content

Commit

Permalink
Encode TRNs in URLs
Browse files Browse the repository at this point in the history
Exposing a TRN in the URL opens us up to someone being able to enumerate
through the values and capture data on people.

We want to prevent this by encoding the values passed in the URL. This
ensures that the UI is the only way to access someone's record.

I opted to use a symmetric encoding process to allow us to encode/decode
using a secret that we control.

This is an easy to understand encryption that is as secure as our secret
key.
  • Loading branch information
felixclack committed Oct 13, 2023
1 parent 17d1e0f commit 0726d08
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 2 deletions.
3 changes: 2 additions & 1 deletion app/controllers/check_records/teachers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ module CheckRecords
class TeachersController < CheckRecordsController
def show
client = QualificationsApi::Client.new(token: ENV["QUALIFICATIONS_API_FIXED_TOKEN"])
@teacher = client.teacher(trn: params[:id])
trn = SecureIdentifier.decode(params[:id])
@teacher = client.teacher(trn:)
@npqs = @teacher.qualifications.filter(&:npq?)
@other_qualifications = @teacher.qualifications.filter { |qualification| !qualification.npq? }
rescue QualificationsApi::TeacherNotFoundError
Expand Down
32 changes: 32 additions & 0 deletions app/models/secure_identifier.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
class SecureIdentifier
SECRET_KEY = Rails.application.credentials.secret_key_base.byteslice(0..31)
CIPHER = 'aes-256-cbc'

def self.encode(value)
raise ArgumentError, "value cannot be nil" if value.nil?
return value if value.empty?

cipher = OpenSSL::Cipher.new(CIPHER)
cipher.encrypt
cipher.key = SECRET_KEY

encrypted = cipher.update(value) + cipher.final
Base64.urlsafe_encode64(encrypted).strip
end

def self.decode(value)
raise ArgumentError, "value cannot be nil" if value.nil?
return value if value.empty?

decipher = OpenSSL::Cipher.new(CIPHER)
decipher.decrypt
decipher.key = SECRET_KEY

begin
decoded = Base64.urlsafe_decode64(value)
decipher.update(decoded) + decipher.final
rescue ArgumentError => e
return value
end
end
end
2 changes: 1 addition & 1 deletion app/views/check_records/search/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<% @teachers.each do |teacher| %>
<div class="search-results__item">
<h2 class="govuk-heading-s">
<%= link_to teacher.name, check_records_teacher_path(teacher.trn), class: "govuk-link--no-visited-state" %>
<%= link_to teacher.name, check_records_teacher_path(SecureIdentifier.encode(teacher.trn)), class: "govuk-link--no-visited-state" %>
</h2>
<strong class="govuk-tag govuk-tag--<%= teacher.sanctions.any? ? "red" : "green" %>"%>
<%= teacher.sanctions.any? ? "Restrictions" : "No restrictions" %>
Expand Down
61 changes: 61 additions & 0 deletions spec/models/secure_identifier_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
require 'rails_helper'

RSpec.describe SecureIdentifier do
let(:plain_text) { 'Hello, World!' }

describe '.encode' do
subject { described_class.encode(plain_text) }

it { is_expected.not_to eq(plain_text) }

it 'returns a Base64 encoded string' do
is_expected.to eq(
Base64.strict_encode64(Base64.strict_decode64(SecureIdentifier.encode(plain_text)))
)
end

context 'when given an empty string' do
let(:plain_text) { '' }

it { is_expected.to eq(plain_text) }
end

context 'when given nil' do
let(:plain_text) { nil }

it 'raises an ArgumentError' do
expect { subject }.to raise_error(ArgumentError)
end
end
end

describe '.decode' do
subject { described_class.decode(encoded_text) }

let(:encoded_text) { described_class.encode(plain_text) }

it { is_expected.to eq(plain_text) }

context 'when given an invalid Base64 string' do
let(:encoded_text) { 'invalid' }

it 'returns the passed value' do
is_expected.to eq(encoded_text)
end
end

context 'when given an empty string' do
let(:encoded_text) { '' }

it { is_expected.to eq(encoded_text) }
end

context 'when given nil' do
let(:encoded_text) { nil }

it 'raises an ArgumentError' do
expect { subject }.to raise_error(ArgumentError)
end
end
end
end

0 comments on commit 0726d08

Please sign in to comment.