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
31 changes: 30 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,38 @@
class ApplicationController < ActionController::Base
before_action :authenticate_user!
skip_forgery_protection if: -> { request.format.json? }
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Could we narrow this so CSRF stays enabled whenever the request is cookie-authenticated? Today it's safe because the JSON path requires a Bearer token, but if a future endpoint accepts JSON with cookie auth (or opts out of authenticate_request!), CSRF would silently disappear for it.

Suggested change
skip_forgery_protection if: -> { request.format.json? }
skip_forgery_protection if: -> { request.authorization.to_s.match?(/\ABearer\s/i) }


before_action :authenticate_request!

def execute_read_task
system('rake read')
flash[:notice] = "Read task executed!"
redirect_back_or_to root_path
end

private

def authenticate_request!
if request.format.json?
authenticate_with_personal_access_token!
else
authenticate_user!
end
Comment on lines +14 to +19
end

def authenticate_with_personal_access_token!
raw_token = bearer_token
pat = PersonalAccessToken.authenticate(raw_token)

if pat
pat.update_column(:last_used_at, Time.current)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This writes to the row on every authenticated API call, so a busy token would hot-spot. Could we debounce the same way Devise :trackable does?

Suggested change
pat.update_column(:last_used_at, Time.current)
if pat.last_used_at.nil? || pat.last_used_at < 1.minute.ago
pat.update_column(:last_used_at, Time.current)
end

sign_in pat.user, store: false
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Question: any reason to use Devise's sign_in here instead of just setting @current_user? Even with store: false, sign_in runs Warden's set_user callbacks. Harmless today, but it couples API auth to whichever Devise modules are enabled on User. If :trackable, :lockable, or :timeoutable get added later, every API request would mutate user state.

else
render json: { error: "Unauthorized" }, status: :unauthorized
end
end

Comment on lines +27 to +33
def bearer_token
header = request.headers["Authorization"].to_s
header.start_with?("Bearer ") ? header.split(" ", 2).last : nil
end
Comment on lines +34 to +37
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Two small parsing edges:

  • start_with?("Bearer ") is case-sensitive, but RFC 6750 says the scheme is case-insensitive.
  • "Bearer abc" (two spaces) returns " abc" with a leading space, so a near-valid header fails auth in a way that looks like a wrong token.
Suggested change
def bearer_token
header = request.headers["Authorization"].to_s
header.start_with?("Bearer ") ? header.split(" ", 2).last : nil
end
def bearer_token
request.authorization.to_s[/\ABearer\s+(.+)\z/i, 1]&.strip
end

end
36 changes: 36 additions & 0 deletions app/controllers/links_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,25 @@ def index
Link
end
@links = scope.order("published_at DESC NULLS LAST")

respond_to do |format|
format.html
format.json { render json: @links.map { |l| link_json(l) } }
end
end

# GET /links/1
def show
@grouped_social_media_snippets = @link.social_media_snippets.group_by(&:social_media_type)

respond_to do |format|
format.html
format.json do
render json: link_json(@link).merge(
social_media_snippets: @link.social_media_snippets.map { |s| snippet_json(s) }
)
end
end
end

private
Expand All @@ -26,4 +40,26 @@ def set_link
def link_params
params.require(:link).permit(:url, :title, :description)
end

def link_json(link)
{
id: link.id,
url: link.url,
title: link.title,
description: link.description,
open_graph_description: link.open_graph_description,
published_at: link.published_at,
created_at: link.created_at,
updated_at: link.updated_at
}
end

def snippet_json(snippet)
{
id: snippet.id,
social_media_type: snippet.social_media_type,
content: snippet.content,
created_at: snippet.created_at
}
end
end
32 changes: 32 additions & 0 deletions app/controllers/personal_access_tokens_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
class PersonalAccessTokensController < ApplicationController
def index
@personal_access_tokens = current_user.personal_access_tokens.order(created_at: :desc)
@new_token = PersonalAccessToken.new
end

def create
@new_token = current_user.personal_access_tokens.build(token_params)

if @new_token.save
@personal_access_tokens = current_user.personal_access_tokens.order(created_at: :desc)
@raw_token = @new_token.token
flash.now[:notice] = "Token created. Copy it now — it won't be shown again."
render :index, status: :created
else
@personal_access_tokens = current_user.personal_access_tokens.order(created_at: :desc)
render :index, status: :unprocessable_entity
end
end

def destroy
token = current_user.personal_access_tokens.find(params[:id])
token.destroy
redirect_to personal_access_tokens_path, notice: "Token revoked."
end

private

def token_params
params.require(:personal_access_token).permit(:name)
end
end
22 changes: 22 additions & 0 deletions app/controllers/shares_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ class SharesController < ApplicationController
# GET /shares
def index
@shares = @link.shares

respond_to do |format|
format.html
format.json { render json: @shares.map { |s| share_json(s) } }
end
end

# GET /shares/1
Expand Down Expand Up @@ -49,4 +54,21 @@ def set_share
def share_params
params.require(:share).permit(:link_id, :shortened_url, :utm_source, :utm_medium, :utm_campaign, :utm_term, :utm_content, :utm_id, :shared_link_name)
end

def share_json(share)
{
id: share.id,
link_id: share.link_id,
shortened_url: share.shortened_url,
utm_source: share.utm_source,
utm_medium: share.utm_medium,
utm_campaign: share.utm_campaign,
utm_term: share.utm_term,
utm_content: share.utm_content,
utm_id: share.utm_id,
shared_link_name: share.shared_link_name,
created_at: share.created_at,
updated_at: share.updated_at
}
end
end
30 changes: 30 additions & 0 deletions app/models/personal_access_token.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
require "digest"
require "securerandom"

class PersonalAccessToken < ApplicationRecord
belongs_to :user

validates :name, presence: true
validates :token_digest, presence: true, uniqueness: true

attr_reader :token

before_validation :assign_token, on: :create

def self.authenticate(raw_token)
return nil if raw_token.blank?
find_by(token_digest: digest_for(raw_token))
end
Comment on lines +14 to +17
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Could we enforce expires_at here so the column isn't a trap? Right now anyone setting expires_at (in the DB, or via a future UI) will think the token is disabled, but authenticate returns it anyway.

Suggested change
def self.authenticate(raw_token)
return nil if raw_token.blank?
find_by(token_digest: digest_for(raw_token))
end
def self.authenticate(raw_token)
return nil if raw_token.blank?
pat = find_by(token_digest: digest_for(raw_token))
return nil if pat && pat.expires_at && pat.expires_at <= Time.current
pat
end

Alternative: drop expires_at from the migration until enforcement lands, so the field doesn't exist in a half-working state.


def self.digest_for(raw_token)
Digest::SHA256.hexdigest(raw_token)
end

private

def assign_token
return if token_digest.present?
@token = SecureRandom.hex(32)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Suggestion: prefix the token with something recognizable so GitHub's secret scanning (and similar tools) can flag it if it leaks into a public repo. Costs nothing and pays off the first time someone pastes one into a gist.

Suggested change
@token = SecureRandom.hex(32)
@token = "lib_#{SecureRandom.hex(32)}"

self.token_digest = self.class.digest_for(@token)
end
end
2 changes: 2 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
class User < ApplicationRecord
include OmbuLabsAuthenticable

has_many :personal_access_tokens, dependent: :destroy
end
59 changes: 59 additions & 0 deletions app/views/personal_access_tokens/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<p style="color: green"><%= notice %></p>

<% if flash[:raw_token].present? %>
<div class="mb-6 p-4 border rounded bg-yellow-50">
<p class="font-semibold mb-2">Your new token (copy it now — it won't be shown again):</p>
<code class="block p-2 bg-white border rounded break-all"><%= flash[:raw_token] %></code>
</div>
<% end %>

<h1 class="text-2xl mb-4">Personal Access Tokens</h1>

<div class="mb-6 p-4 border rounded">
<h2 class="text-lg mb-2">Create a new token</h2>
<%= form_with model: @new_token, url: personal_access_tokens_path, local: true do |f| %>
<% if @new_token.errors.any? %>
<ul class="text-red-600 mb-2">
<% @new_token.errors.full_messages.each do |msg| %>
<li><%= msg %></li>
<% end %>
</ul>
<% end %>

<div class="mb-2">
<%= f.label :name, "Name" %>
<%= f.text_field :name, class: "border rounded px-2 py-1 ml-2", placeholder: "e.g. Local script" %>
</div>

<%= f.submit "Generate token", class: "inline-block border rounded py-1 px-3" %>
<% end %>
</div>

<h2 class="text-lg mb-2">Your tokens</h2>

<% if @personal_access_tokens.any? %>
<table class="w-full border-collapse">
<thead>
<tr class="border-b">
<th class="text-left py-2">Name</th>
<th class="text-left py-2">Created</th>
<th class="text-left py-2">Last used</th>
<th></th>
</tr>
</thead>
<tbody>
<% @personal_access_tokens.each do |token| %>
<tr class="border-b">
<td class="py-2"><%= token.name %></td>
<td class="py-2"><%= token.created_at.to_date %></td>
<td class="py-2"><%= token.last_used_at ? token.last_used_at.to_date : "Never" %></td>
<td class="py-2 text-right">
<%= button_to "Revoke", personal_access_token_path(token), method: :delete, data: { turbo_confirm: "Revoke this token?" }, class: "inline-block border rounded py-1 px-3" %>
</td>
</tr>
<% end %>
</tbody>
</table>
<% else %>
<p>No tokens yet.</p>
<% end %>
1 change: 1 addition & 0 deletions app/views/shared/_navigation_bar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

<li class="mr-3 ml-auto flex items-center">
<span><%= current_user.email %></span>
<%= link_to "API Tokens", personal_access_tokens_path, class: "inline-block border rounded py-1 px-3 ml-2" %>
<%= button_to "Sign out", ombu_labs_auth.destroy_user_session_path, method: :delete, class: "inline-block border rounded py-1 px-3" %>
</li>
</ul>
Expand Down
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,7 @@
end
post 'execute_read_task', to: 'application#execute_read_task'

resources :personal_access_tokens, only: [:index, :create, :destroy]

mount OmbuLabs::Auth::Engine, at: '/', as: 'ombu_labs_auth'
end
15 changes: 15 additions & 0 deletions db/migrate/20260429134259_create_personal_access_tokens.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class CreatePersonalAccessTokens < ActiveRecord::Migration[7.0]
def change
create_table :personal_access_tokens do |t|
t.references :user, null: false, foreign_key: true
t.string :name, null: false
t.string :token_digest, null: false
t.datetime :last_used_at
t.datetime :expires_at

t.timestamps
end

add_index :personal_access_tokens, :token_digest, unique: true
end
end
15 changes: 14 additions & 1 deletion db/schema.rb

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

6 changes: 6 additions & 0 deletions spec/factories/personal_access_tokens.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
FactoryBot.define do
factory :personal_access_token do
sequence(:name) { |n| "Token ##{n}" }
user
end
end
48 changes: 48 additions & 0 deletions spec/models/personal_access_token_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
require 'rails_helper'

RSpec.describe PersonalAccessToken, type: :model do
describe '#token' do
it 'is generated on create and exposed via attr_reader' do
pat = FactoryBot.create(:personal_access_token)
expect(pat.token).to be_present
expect(pat.token.length).to eq(64)
end

it 'is stored hashed, not plaintext' do
pat = FactoryBot.create(:personal_access_token)
expect(pat.token_digest).to eq(Digest::SHA256.hexdigest(pat.token))
expect(pat.token_digest).not_to eq(pat.token)
end

it 'is not retrievable after reload' do
pat = FactoryBot.create(:personal_access_token)
reloaded = PersonalAccessToken.find(pat.id)
expect(reloaded.token).to be_nil
end
end

describe '.authenticate' do
it 'returns the token record for a valid raw token' do
pat = FactoryBot.create(:personal_access_token)
expect(PersonalAccessToken.authenticate(pat.token)).to eq(pat)
end

it 'returns nil for an invalid raw token' do
FactoryBot.create(:personal_access_token)
expect(PersonalAccessToken.authenticate('not-a-real-token')).to be_nil
end

it 'returns nil for a blank token' do
expect(PersonalAccessToken.authenticate(nil)).to be_nil
expect(PersonalAccessToken.authenticate('')).to be_nil
end
end

describe 'validations' do
it 'requires a name' do
pat = PersonalAccessToken.new(user: FactoryBot.create(:user))
expect(pat).not_to be_valid
expect(pat.errors[:name]).to be_present
end
end
end
Loading