Device flow: single state resolver + shared terminal-state partial
The terminal states (expired / already-used / not-found) were duplicated as markup in show and result, and #verify re-implemented the nil→expired→!pending cascade that #show already had — a copy edit meant four touch points. - device_code_state(dc) resolves a code to :not_found / :expired / :already_handled / :ok. Both #show and #verify branch on it, so the cascade lives in one place. - _terminal_state partial renders the terminal heading/message/link once; result.html.erb now renders it instead of inlining the markup. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
e3f0bd4cab
commit
79a7524fda
@@ -20,15 +20,14 @@ class DeviceAuthorizationsController < ApplicationController
|
|||||||
# GET /device?user_code=WDJB-MJHT
|
# GET /device?user_code=WDJB-MJHT
|
||||||
def show
|
def show
|
||||||
@user_code = params[:user_code].to_s
|
@user_code = params[:user_code].to_s
|
||||||
@device_code = OidcDeviceCode.find_by_user_code(@user_code) if @user_code.present?
|
if @user_code.blank?
|
||||||
|
@state = :prompt
|
||||||
|
return render :show
|
||||||
|
end
|
||||||
|
|
||||||
if @device_code.nil?
|
@device_code = OidcDeviceCode.find_by_user_code(@user_code)
|
||||||
@state = @user_code.present? ? :not_found : :prompt
|
@state = device_code_state(@device_code)
|
||||||
elsif @device_code.expired?
|
if @state == :ok
|
||||||
@state = :expired
|
|
||||||
elsif !@device_code.pending?
|
|
||||||
@state = :already_handled
|
|
||||||
else
|
|
||||||
@state = :confirm
|
@state = :confirm
|
||||||
@application = @device_code.application
|
@application = @device_code.application
|
||||||
@scopes = granted_scopes(@device_code)
|
@scopes = granted_scopes(@device_code)
|
||||||
@@ -40,21 +39,8 @@ class DeviceAuthorizationsController < ApplicationController
|
|||||||
# POST /device
|
# POST /device
|
||||||
def verify
|
def verify
|
||||||
@device_code = OidcDeviceCode.find_by_user_code(params[:user_code].to_s)
|
@device_code = OidcDeviceCode.find_by_user_code(params[:user_code].to_s)
|
||||||
|
@state = device_code_state(@device_code)
|
||||||
if @device_code.nil?
|
return render :result unless @state == :ok
|
||||||
@state = :not_found
|
|
||||||
return render :result
|
|
||||||
end
|
|
||||||
|
|
||||||
if @device_code.expired?
|
|
||||||
@state = :expired
|
|
||||||
return render :result
|
|
||||||
end
|
|
||||||
|
|
||||||
unless @device_code.pending?
|
|
||||||
@state = :already_handled
|
|
||||||
return render :result
|
|
||||||
end
|
|
||||||
|
|
||||||
@application = @device_code.application
|
@application = @device_code.application
|
||||||
|
|
||||||
@@ -82,6 +68,17 @@ class DeviceAuthorizationsController < ApplicationController
|
|||||||
|
|
||||||
private
|
private
|
||||||
|
|
||||||
|
# Single resolver for the shared terminal-state cascade. Returns :not_found,
|
||||||
|
# :expired, :already_handled, or :ok (the code is live and actionable). Both
|
||||||
|
# show and verify branch on this so the cascade lives in one place, and the
|
||||||
|
# terminal states render through the shared _terminal_state partial.
|
||||||
|
def device_code_state(device_code)
|
||||||
|
return :not_found if device_code.nil?
|
||||||
|
return :expired if device_code.expired?
|
||||||
|
return :already_handled unless device_code.pending?
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
|
||||||
def granted_scopes(device_code)
|
def granted_scopes(device_code)
|
||||||
device_code.scope.to_s.split & OidcController::SUPPORTED_SCOPES
|
device_code.scope.to_s.split & OidcController::SUPPORTED_SCOPES
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -0,0 +1,19 @@
|
|||||||
|
<%#
|
||||||
|
Terminal device-authorization states, shared by the /device prompt (show) and
|
||||||
|
the result page. Pass `state:` as one of :expired, :already_handled, :not_found.
|
||||||
|
%>
|
||||||
|
<% headings = {
|
||||||
|
expired: "Code expired",
|
||||||
|
already_handled: "Code already used",
|
||||||
|
not_found: "Code not found"
|
||||||
|
} %>
|
||||||
|
<% messages = {
|
||||||
|
expired: "This device code has expired. Start again from your tool to get a fresh code.",
|
||||||
|
already_handled: "This device code has already been approved or denied. Start again from your tool if you need a new one.",
|
||||||
|
not_found: "We couldn't find that code. Check the code your tool is showing and try again."
|
||||||
|
} %>
|
||||||
|
<div class="text-center">
|
||||||
|
<h2 class="text-2xl font-bold text-gray-900 dark:text-gray-100"><%= headings[state] %></h2>
|
||||||
|
<p class="mt-3 text-sm text-gray-600 dark:text-gray-400"><%= messages[state] %></p>
|
||||||
|
<%= link_to "Enter a different code", device_verification_path, class: "mt-6 inline-block text-sm font-medium text-blue-600 hover:text-blue-500 dark:text-blue-400" %>
|
||||||
|
</div>
|
||||||
@@ -31,13 +31,7 @@
|
|||||||
</p>
|
</p>
|
||||||
|
|
||||||
<% else %>
|
<% else %>
|
||||||
<h2 class="text-2xl font-bold text-gray-900 dark:text-gray-100">
|
<%= render "terminal_state", state: @state %>
|
||||||
<%= @state == :expired ? "Code expired" : (@state == :already_handled ? "Code already used" : "Code not found") %>
|
|
||||||
</h2>
|
|
||||||
<p class="mt-3 text-sm text-gray-600 dark:text-gray-400">
|
|
||||||
Start again from your tool to get a fresh code.
|
|
||||||
</p>
|
|
||||||
<%= link_to "Enter a different code", device_verification_path, class: "mt-6 inline-block text-sm font-medium text-blue-600 hover:text-blue-500 dark:text-blue-400" %>
|
|
||||||
<% end %>
|
<% end %>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -280,6 +280,39 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest
|
|||||||
assert_redirected_to signin_path
|
assert_redirected_to signin_path
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# --- Terminal-state rendering (shared resolver + partial) ------------------
|
||||||
|
|
||||||
|
test "show prompts for a code when none is given" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
get "/device"
|
||||||
|
assert_response :success
|
||||||
|
assert_match(/Enter device code/i, @response.body)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "show renders the not-found terminal state for an unknown code" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
get "/device", params: {user_code: "ZZZZ9999"}
|
||||||
|
assert_response :success
|
||||||
|
assert_match(/Code not found/i, @response.body)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "show renders the expired terminal state" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
dc = OidcDeviceCode.create!(application: @cli, scope: "openid", expires_at: 1.minute.ago)
|
||||||
|
get "/device", params: {user_code: dc.user_code}
|
||||||
|
assert_response :success
|
||||||
|
assert_match(/Code expired/i, @response.body)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "verify renders the terminal state for an already-handled code" do
|
||||||
|
sign_in_as(@user)
|
||||||
|
dc = OidcDeviceCode.create!(application: @cli, scope: "openid")
|
||||||
|
dc.deny!
|
||||||
|
post "/device", params: {user_code: dc.user_code}
|
||||||
|
assert_response :success
|
||||||
|
assert_match(/Code already used/i, @response.body)
|
||||||
|
end
|
||||||
|
|
||||||
# Introspection is covered in depth in oidc_introspection_test.rb.
|
# Introspection is covered in depth in oidc_introspection_test.rb.
|
||||||
|
|
||||||
private
|
private
|
||||||
|
|||||||
Reference in New Issue
Block a user