-
Notifications
You must be signed in to change notification settings - Fork 0
Add JSON API endpoints and personal access tokens #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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? } | ||||||||||||||||
|
|
||||||||||||||||
| 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) | ||||||||||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
|
||||||||||||||||
| sign_in pat.user, store: false | ||||||||||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: any reason to use Devise's |
||||||||||||||||
| 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two small parsing edges:
Suggested change
|
||||||||||||||||
| end | ||||||||||||||||
| 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 |
| 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we enforce
Suggested change
Alternative: drop |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 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) | ||||||||||||||||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||||||||||||||||||||||
| self.token_digest = self.class.digest_for(@token) | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
| end | ||||||||||||||||||||||
| 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 |
| 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 %> |
| 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 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| 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 |
| 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 |
There was a problem hiding this comment.
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.