From 79a7524fda8c319d0f0be53a3d186c63b4e15d8f Mon Sep 17 00:00:00 2001 From: Dan Milne Date: Sun, 19 Jul 2026 14:10:04 +1000 Subject: [PATCH] Device flow: single state resolver + shared terminal-state partial MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01F7cwhwDJp3MJJDoNPVE6zq --- .../device_authorizations_controller.rb | 43 +++++++++---------- .../_terminal_state.html.erb | 19 ++++++++ .../device_authorizations/result.html.erb | 8 +--- .../oidc_device_flow_controller_test.rb | 33 ++++++++++++++ 4 files changed, 73 insertions(+), 30 deletions(-) create mode 100644 app/views/device_authorizations/_terminal_state.html.erb diff --git a/app/controllers/device_authorizations_controller.rb b/app/controllers/device_authorizations_controller.rb index 92b6fde..f4bfbf4 100644 --- a/app/controllers/device_authorizations_controller.rb +++ b/app/controllers/device_authorizations_controller.rb @@ -20,15 +20,14 @@ class DeviceAuthorizationsController < ApplicationController # GET /device?user_code=WDJB-MJHT def show @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? - @state = @user_code.present? ? :not_found : :prompt - elsif @device_code.expired? - @state = :expired - elsif !@device_code.pending? - @state = :already_handled - else + @device_code = OidcDeviceCode.find_by_user_code(@user_code) + @state = device_code_state(@device_code) + if @state == :ok @state = :confirm @application = @device_code.application @scopes = granted_scopes(@device_code) @@ -40,21 +39,8 @@ class DeviceAuthorizationsController < ApplicationController # POST /device def verify @device_code = OidcDeviceCode.find_by_user_code(params[:user_code].to_s) - - if @device_code.nil? - @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 + @state = device_code_state(@device_code) + return render :result unless @state == :ok @application = @device_code.application @@ -82,6 +68,17 @@ class DeviceAuthorizationsController < ApplicationController 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) device_code.scope.to_s.split & OidcController::SUPPORTED_SCOPES end diff --git a/app/views/device_authorizations/_terminal_state.html.erb b/app/views/device_authorizations/_terminal_state.html.erb new file mode 100644 index 0000000..2a11075 --- /dev/null +++ b/app/views/device_authorizations/_terminal_state.html.erb @@ -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." + } %> +
+

<%= headings[state] %>

+

<%= messages[state] %>

+ <%= 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" %> +
diff --git a/app/views/device_authorizations/result.html.erb b/app/views/device_authorizations/result.html.erb index 237a567..b61afc3 100644 --- a/app/views/device_authorizations/result.html.erb +++ b/app/views/device_authorizations/result.html.erb @@ -31,13 +31,7 @@

<% else %> -

- <%= @state == :expired ? "Code expired" : (@state == :already_handled ? "Code already used" : "Code not found") %> -

-

- Start again from your tool to get a fresh code. -

- <%= 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" %> + <%= render "terminal_state", state: @state %> <% end %> diff --git a/test/controllers/oidc_device_flow_controller_test.rb b/test/controllers/oidc_device_flow_controller_test.rb index db2d6b4..f9f2518 100644 --- a/test/controllers/oidc_device_flow_controller_test.rb +++ b/test/controllers/oidc_device_flow_controller_test.rb @@ -280,6 +280,39 @@ class OidcDeviceFlowControllerTest < ActionDispatch::IntegrationTest assert_redirected_to signin_path 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. private