Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
INTERNAL_API_TOKEN=supersecrettoken
4 changes: 4 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ group :development, :staging do
gem 'slim_lint', require: false
end

group :development, :test do
gem 'dotenv-rails'
end

group :test do
gem 'capybara'
gem 'rails-controller-testing'
Expand Down
7 changes: 7 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ GEM
docker-api (2.4.0)
excon (>= 0.64.0)
multi_json
dotenv (3.2.0)
dotenv-rails (3.2.0)
dotenv (= 3.2.0)
railties (>= 6.1)
drb (2.2.3)
erb (5.1.1)
erubi (1.13.1)
Expand Down Expand Up @@ -690,6 +694,7 @@ DEPENDENCIES
charlock_holmes
csv
docker-api
dotenv-rails
eventmachine
factory_bot_rails
faraday
Expand Down Expand Up @@ -810,6 +815,8 @@ CHECKSUMS
diff-lcs (1.6.2) sha256=9ae0d2cba7d4df3075fe8cd8602a8604993efc0dfa934cff568969efb1909962
docile (1.4.1) sha256=96159be799bfa73cdb721b840e9802126e4e03dfc26863db73647204c727f21e
docker-api (2.4.0) sha256=824be734f4cc8718189be9c8e795b6414acbbf7e8b082a06f959a27dd8dd63e6
dotenv (3.2.0) sha256=e375b83121ea7ca4ce20f214740076129ab8514cd81378161f11c03853fe619d
dotenv-rails (3.2.0) sha256=657e25554ba622ffc95d8c4f1670286510f47f2edda9f68293c3f661b303beab
drb (2.2.3) sha256=0b00d6fdb50995fe4a45dea13663493c841112e4068656854646f418fda13373
erb (5.1.1) sha256=b2c26e7924551d9efbae998e17ddbef220937b6422b1d2ec7ae71417b5a1f4ec
erubi (1.13.1) sha256=a082103b0885dbc5ecf1172fede897f9ebdb745a4b97a5e8dc63953db1ee4ad9
Expand Down
18 changes: 18 additions & 0 deletions app/controllers/api/api_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

module Api
class ApiController < ActionController::API
include ActionController::HttpAuthentication::Token::ControllerMethods

before_action :authenticate!

def authenticate!
authenticate_or_request_with_http_token do |token, _options|
ActiveSupport::SecurityUtils.secure_compare(
token,
ENV.fetch('INTERNAL_API_TOKEN', nil)
)
end
end
end
end
16 changes: 16 additions & 0 deletions app/controllers/api/internal/users_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module Api
module Internal
class UsersController < Api::ApiController
def destroy
user = ExternalUser.where(external_id: params[:id]).first
Comment thread
Janis4411 marked this conversation as resolved.

return head :not_found unless user

user.soft_delete
head :ok
end
end
end
end
4 changes: 4 additions & 0 deletions app/models/external_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ def displayname
name.presence || "#{model_name.human} #{id}"
end

def soft_delete
update!(name: 'Deleted User', email: nil, deleted_at: Time.zone.now)
end

def webauthn_name
"#{consumer.name}: #{displayname}"
end
Expand Down
6 changes: 6 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@
mount ActionCable.server => '/cable'
mount RailsAdmin::Engine => '/rails_admin', as: 'rails_admin'

namespace :api do
namespace :internal do
resources :users, only: :destroy
end
end

# Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500.
# Can be used by load balancers and uptime monitors to verify that the app is live.
get 'up', to: 'rails/health#show', as: :rails_health_check
Expand Down
2 changes: 1 addition & 1 deletion db/cable_schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions db/migrate/20260421114010_add_deleted_at_to_external_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class AddDeletedAtToExternalUsers < ActiveRecord::Migration[8.0]
def change
add_column :external_users, :deleted_at, :datetime, null: true, default: nil
end
end
2 changes: 1 addition & 1 deletion db/queue_schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions docs/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ The following environment variables are specifically support in CodeOcean and ar
| `LISTEN_ADDRESS` | `127.0.0.1` in `development` | Specifies the IP address the Vagrant VM server should attach to during development |
| `HEADLESS` | `false` | Enables the test environment to work without a window manager for feature tests (e.g., using Vagrant) |
| `BROWSER` | `chrome` | Specifies the browser to be used for system tests. Supported are `chrome` or `firefox` |
| `INTERNAL_API_TOKEN` | ` ` | Bearer token for authenticating requests from openHPI for user anonymization |
6 changes: 4 additions & 2 deletions lib/tasks/gdpr_delete.rake
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,11 @@ namespace :gdpr do
next
end

if user.update(name: 'Deleted User', email: nil)
begin
user.soft_delete
users_deleted += 1
else
rescue StandardError => e
warn "An error occurred while anonymizing user #{user_id}: #{e.message}"
errored_user_ids << user_id
end
end
Expand Down
61 changes: 61 additions & 0 deletions spec/controllers/api/api_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# frozen_string_literal: true

require 'rails_helper'

RSpec.describe Api::ApiController do
controller(described_class) do
def index
head :ok
end
end

before do
routes.draw do
get 'index' => 'api/api#index'
end
end

describe 'authentication' do
context 'with valid token' do
it 'allows the request' do
Comment on lines +19 to +20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nits] A context with only one example is not really worth it. I think a single it assertion here is clearer and more readable.

Suggested change
context 'with valid token' do
it 'allows the request' do
it 'allows the request with valid token' do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this context follows the same pattern as the other contexts.

E.g.

context 'with valid token' do, context 'with invalid token' do, context 'without token' do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of these contexts. I think a context should not only include one test. A context with one test is just one individual test. This is not a must for me. I approved.

request.headers['Authorization'] = 'Bearer supersecrettoken'

get :index

expect(response).to have_http_status(:ok)
end
end

context 'with invalid token' do
it 'returns 401 Unauthorized' do
request.headers['Authorization'] = 'Bearer invalid_token'

get :index

expect(response).to have_http_status(:unauthorized)
end
end

context 'without token' do
Comment thread
arkirchner marked this conversation as resolved.
it 'returns 401 Unauthorized' do
get :index

expect(response).to have_http_status(:unauthorized)
end
end

context 'without the INTERNAL_API_TOKEN being set' do
it 'returns 401 Unauthorized' do
allow(ENV)
.to receive(:fetch)
.with('INTERNAL_API_TOKEN', nil)
.and_return(nil)

request.headers['Authorization'] = "Bearer #{ENV.fetch('INTERNAL_API_TOKEN', nil)}"
get :index

expect(response).to have_http_status(:unauthorized)
end
end
end
end
15 changes: 15 additions & 0 deletions spec/models/external_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,19 @@
expect(build(:external_user).learner?).to be true
end
end

describe '#soft_delete' do
let(:user) { create(:external_user, name: 'Test User', email: 'testmail@gmail.com') }

it 'sets the name to "Deleted User" and email to nil' do
user.soft_delete
expect(user.name).to eq('Deleted User')
expect(user.email).to be_nil
end

it 'raises an error if the update fails' do
allow(user).to receive(:update!).and_raise(ActiveRecord::RecordInvalid.new(user))
expect { user.soft_delete }.to raise_error(ActiveRecord::RecordInvalid)
end
end
end
26 changes: 26 additions & 0 deletions spec/request/api/internal/users/destroy_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true
Comment thread
arkirchner marked this conversation as resolved.

include ActiveSupport::Testing::TimeHelpers

Check warning on line 3 in spec/request/api/internal/users/destroy_spec.rb

View workflow job for this annotation

GitHub Actions / lint

[rubocop] reported by reviewdog 🐶 `include` is used at the top level. Use inside `class` or `module`. Raw Output: spec/request/api/internal/users/destroy_spec.rb:3:1: C: Style/MixinUsage: `include` is used at the top level. Use inside `class` or `module`.

Check warning on line 3 in spec/request/api/internal/users/destroy_spec.rb

View workflow job for this annotation

GitHub Actions / lint

[rubocop] reported by reviewdog 🐶 `include` is used at the top level. Use inside `class` or `module`. Raw Output: spec/request/api/internal/users/destroy_spec.rb:3:1: C: Style/MixinUsage: `include` is used at the top level. Use inside `class` or `module`.

require 'rails_helper'

RSpec.describe 'DELETE Users API', type: :request do
let(:headers) { {'Authorization' => 'Bearer supersecrettoken'} }

describe 'DELETE /api/internal/users' do
it 'soft deletes a user' do
user = create(:external_user, external_id: '123456')

freeze_time

expect { delete '/api/internal/users/123456', headers: headers }
.to change { user.reload.deleted_at }.to(Time.zone.now)
end

it 'returns an error if the user is not found' do
delete '/api/internal/users/123456',
headers: headers
expect(response).to have_http_status(:not_found)
end
end
end
Loading