From ca5c1ba0d37b9bb047ef731b35bac68e746e1148 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 19 Oct 2026 23:00:29 -0700 Subject: [PATCH 01/16] Design the consent screen and Connected apps: who's asking, what it may do, where it goes The consent screen names a self-registered app by where its access goes, its own name quoted beside it, warns that Fizzy hasn't checked it, explains each access level in plain words, says who is signed in and where the browser goes next. An account picker partial is ready for account binding. Errors shown on Fizzy say what to do next. Connected apps groups apps by kind (web, on your devices, Fizzy's own), with access, when connected and last active, and an empty state. --- app/assets/stylesheets/oauth.css | 108 +++++++++++++++++ app/helpers/oauth_helper.rb | 72 +++++++++++ app/models/oauth/client.rb | 21 ++++ .../my/connected_apps/_connected_app.html.erb | 112 +++++++++++------- app/views/my/connected_apps/index.html.erb | 30 +++-- .../oauth/authorizations/_access.html.erb | 24 ++++ .../authorizations/_account_picker.html.erb | 22 ++++ .../oauth/authorizations/_client.html.erb | 28 +++++ app/views/oauth/authorizations/error.html.erb | 18 +-- app/views/oauth/authorizations/new.html.erb | 45 +++---- .../my/connected_apps_controller_test.rb | 29 +++++ test/helpers/oauth_helper_test.rb | 56 +++++++++ test/integration/oauth_consent_screen_test.rb | 92 ++++++++++++++ test/integration/oauth_flow_test.rb | 12 +- test/models/oauth/client_test.rb | 8 ++ 15 files changed, 584 insertions(+), 93 deletions(-) create mode 100644 app/assets/stylesheets/oauth.css create mode 100644 app/helpers/oauth_helper.rb create mode 100644 app/views/oauth/authorizations/_access.html.erb create mode 100644 app/views/oauth/authorizations/_account_picker.html.erb create mode 100644 app/views/oauth/authorizations/_client.html.erb create mode 100644 test/helpers/oauth_helper_test.rb create mode 100644 test/integration/oauth_consent_screen_test.rb diff --git a/app/assets/stylesheets/oauth.css b/app/assets/stylesheets/oauth.css new file mode 100644 index 0000000000..5a9e966c01 --- /dev/null +++ b/app/assets/stylesheets/oauth.css @@ -0,0 +1,108 @@ +/* Authorizing an app (oauth/authorizations) and the apps you've connected (my/connected_apps) */ + +@layer components { + .oauth-consent { + display: flex; + flex-direction: column; + gap: var(--block-space); + } + + .oauth-client__name { + overflow-wrap: anywhere; + } + + .oauth-client__self-name { + overflow-wrap: anywhere; + } + + .oauth-client__warning { + background-color: var(--color-highlight); + border-radius: 0.5em; + padding: var(--block-space-half) var(--inline-space); + } + + .oauth-access { + border: 0; + margin: 0; + padding: 0; + } + + .oauth-access__legend { + margin-block-end: var(--block-space-half); + padding: 0; + } + + .oauth-access__choice { + align-items: start; + border: var(--border); + border-radius: 0.5em; + cursor: pointer; + display: flex; + gap: var(--inline-space); + padding: var(--block-space-half) var(--inline-space); + + &:has(:checked) { + border-color: var(--color-ink); + } + + &:has(:focus-visible) { + outline: 2px solid var(--color-ink); + outline-offset: 2px; + } + } + + .oauth-access__radio { + accent-color: var(--color-ink); + flex-shrink: 0; + margin-block-start: 0.2em; + } + + .oauth-consent__actions { + flex-wrap: wrap; + } + + .connected-apps { + display: flex; + flex-direction: column; + gap: var(--block-space); + } + + .connected-apps__group { + display: flex; + flex-direction: column; + } + + .connected-apps__empty { + border: 2px dashed var(--color-ink-lighter); + border-radius: 0.5em; + padding: var(--block-space) var(--inline-space-double); + } + + .connected-app { + align-items: start; + border-block-end: var(--border); + display: flex; + gap: var(--inline-space); + justify-content: space-between; + padding-block: var(--block-space-half); + + &:last-child { + border-block-end: 0; + } + + /* Narrow screens: the disconnect button drops under the details */ + @media (max-width: 480px) { + flex-direction: column; + } + } + + .connected-app__disconnect { + flex-shrink: 0; + } + + .connected-app__details { + gap: 0.15em; + min-inline-size: 0; + overflow-wrap: anywhere; + } +} diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb new file mode 100644 index 0000000000..d01ce3063d --- /dev/null +++ b/app/helpers/oauth_helper.rb @@ -0,0 +1,72 @@ +module OauthHelper + # Bidirectional controls can make a name read as something else, or push the + # host beside it out of place. A self-registered name is the app's own claim, + # so they go before it is shown anywhere. + BIDI_CONTROLS = /[؜‎‏‪-‮⁦-⁩]/ + + # Grouped by what the app is (Oauth::Client#connection_kind), in this order; + # a group with nothing in it isn't shown. + CONNECTED_APP_GROUPS = { + web: "Apps and assistants on the web", + local: "Apps on your devices", + fizzy: "Fizzy apps" + } + + ACCESS_LEVELS = { + "read" => { + label: "Read only", + description: "See your boards, cards, comments and the people in your accounts. It can't change anything." + }, + "read write" => { + label: "Read and write", + description: "See everything you can, and act as you: create, edit, move, close and delete cards, comments and boards." + } + } + + def oauth_client_self_name(client) + client.name.to_s.gsub(BIDI_CONTROLS, "") + end + + # Who the person is dealing with, as far as we can vouch for it. A trusted or + # operator-registered app is known by its name. A self-registered app's name + # is only its claim, so it goes by where its access goes: the callback's + # host, port included when it isn't the default, or this device. + def oauth_client_display_identity(client, redirect_uri) + if client.dynamically_registered? + oauth_redirect_host(redirect_uri) || "an app on your device" + else + oauth_client_self_name(client) + end + end + + # nil for a callback that stays on this device: loopback, or an app's own + # URL scheme. + def oauth_redirect_host(redirect_uri) + uri = URI.parse(redirect_uri.to_s) + + if uri.host.present? && !Oauth.loopback_host?(uri.host) && uri.scheme.to_s.downcase.in?(%w[ http https ]) + uri.port == uri.default_port ? uri.host : "#{uri.host}:#{uri.port}" + end + rescue URI::InvalidURIError + nil + end + + # The access levels a client may be granted, weakest first, each with what + # it lets the app do in plain words. + def oauth_access_levels(client) + levels = client.scopes.include?("write") ? ACCESS_LEVELS.keys : %w[ read ] + levels.map { |scope| [ scope, ACCESS_LEVELS.fetch(scope) ] } + end + + def oauth_access_label(permission) + ACCESS_LEVELS.fetch(Oauth.canonical_scope(permission))[:label] + end + + def connected_app_groups(connected_apps) + by_kind = connected_apps.group_by { |client, _| client.connection_kind } + + CONNECTED_APP_GROUPS.filter_map do |kind, heading| + [ kind, heading, by_kind[kind] ] if by_kind[kind].present? + end + end +end diff --git a/app/models/oauth/client.rb b/app/models/oauth/client.rb index 28d103eb52..07d17078d8 100644 --- a/app/models/oauth/client.rb +++ b/app/models/oauth/client.rb @@ -73,6 +73,21 @@ def redeem(authorization_code, identity:, permission:) end end + # What kind of app this is, as this server sees it rather than as it names + # itself: Connected Apps groups by it. Our own apps are the trusted ones. An + # app whose every callback stays on this device, a loopback address or an + # app's own URL scheme, is a tool running here, such as a CLI or a desktop + # assistant. Anything else receives its access somewhere on the web. + def connection_kind + if trusted? + :fizzy + elsif redirect_uris.all? { |uri| on_device_uri?(uri) } + :local + else + :web + end + end + def confidential? token_endpoint_auth_method.in?(SECRET_AUTH_METHODS) end @@ -133,6 +148,12 @@ def validate_redirect_uri(uri) errors.add :redirect_uris, "includes an invalid URI" end + def on_device_uri?(uri) + loopback_uri?(uri) || !URI.parse(uri).scheme.to_s.downcase.in?(%w[ http https ]) + rescue URI::InvalidURIError + false + end + def loopback_uri?(uri) Oauth.loopback_host?(URI.parse(uri).host) rescue URI::InvalidURIError diff --git a/app/views/my/connected_apps/_connected_app.html.erb b/app/views/my/connected_apps/_connected_app.html.erb index 3c8b2a2f35..46c4e57ec0 100644 --- a/app/views/my/connected_apps/_connected_app.html.erb +++ b/app/views/my/connected_apps/_connected_app.html.erb @@ -1,44 +1,72 @@ - - <%= client.name %> - <%= tokens.map { |t| t.permission.humanize }.uniq.sort.join(", ") %> - <%= local_datetime_tag tokens.map(&:created_at).min, style: :datetime %> - -
- - - -

Disconnect <%= client.name %>?

-

This will revoke all of its access.

- - <%= form_with url: my_connected_app_path(client), method: :delete, class: "flex flex-column gap" do |form| %> - <% if webhooks.any? %> -
-

It set up <%= webhooks.one? ? "this webhook" : "these webhooks" %>, which will keep running unless you remove <%= webhooks.one? ? "it" : "them" %>:

-
    - <% webhooks.each do |webhook| %> -
  • - <%= webhook.name %> on <%= webhook.board.name %><%= " in #{webhook.account.name}" unless webhook.account == Current.account %> - <%= hidden_field_tag "webhook_ids[]", webhook.id, id: nil %> -
    • - <% end %> -
- - -
- <% end %> - -
- - <%= form.button "Disconnect", class: "btn txt-negative" %> +<% redirect_host = client.dynamically_registered? ? oauth_redirect_host(client.redirect_uris.first) : nil %> +<% permission = tokens.any?(&:write?) ? "write" : "read" %> + +
+
+ <%# A self-registered app goes by the name it gave itself, since that's the + name people know it by, but never without where its access goes %> +

<%= oauth_client_self_name(client) %>

+ + <% if client.dynamically_registered? %> +

+ <% if redirect_host %> + Name set by the app at <%= redirect_host %> + <% else %> + Name set by an app on your device + <% end %> +

+ <% elsif client.trusted? %> +

Made by 37signals

+ <% end %> + +

+ <%= oauth_access_label(permission) %>, in every account you belong to +

+ + <%# updated_at moves each time the app renews its access, so it's the last + time the app was active, to within an access token's lifetime %> +

+ Connected <%= local_datetime_tag tokens.map(&:created_at).min, style: :date %> + · last active <%= local_datetime_tag tokens.map(&:updated_at).max, style: :daysago %> + <% if tokens.many? %>· <%= pluralize(tokens.size, "connection") %><% end %> +

+
+ +
+ + + +

Disconnect <%= oauth_client_self_name(client) %>?

+

It loses its access to Fizzy right away. To use it again, you'd connect it again.

+ + <%= form_with url: my_connected_app_path(client), method: :delete, class: "flex flex-column gap" do |form| %> + <% if webhooks.any? %> +
+

It set up <%= webhooks.one? ? "this webhook" : "these webhooks" %>, which will keep running unless you remove <%= webhooks.one? ? "it" : "them" %>:

+
    + <% webhooks.each do |webhook| %> +
  • + <%= webhook.name %> on <%= webhook.board.name %><%= " in #{webhook.account.name}" unless webhook.account == Current.account %> + <%= hidden_field_tag "webhook_ids[]", webhook.id, id: nil %> +
    • + <% end %> +
+ +
<% end %> - -
- - + +
+ + <%= form.button "Disconnect", class: "btn txt-negative" %> +
+ <% end %> +
+
+ diff --git a/app/views/my/connected_apps/index.html.erb b/app/views/my/connected_apps/index.html.erb index 49cb77e80b..83eca19f0a 100644 --- a/app/views/my/connected_apps/index.html.erb +++ b/app/views/my/connected_apps/index.html.erb @@ -12,25 +12,23 @@

<%= @page_title %>

<% end %> -
+
<% if @connected_apps.any? %> -

Apps you've authorized to access Fizzy on your behalf.

- - - - - - - - - - - <% @connected_apps.each do |client, tokens| %> +

Apps you've authorized to use Fizzy as you. Each one can reach what you can, with the access shown. Don't recognize one? Disconnect it, and its access ends right away.

+ + <% connected_app_groups(@connected_apps).each do |kind, heading, apps| %> +
"> +

" class="divider txt-medium margin-none"><%= heading %>

+ + <% apps.each do |client, tokens| %> <%= render "my/connected_apps/connected_app", client: client, tokens: tokens, webhooks: @webhooks_by_client.fetch(client.id, []) %> <% end %> -
-
AppPermissionsConnected
+
+ <% end %> <% else %> -

No apps are connected to your account. When you authorize an app to access Fizzy, it will appear here.

+
+

Nothing connected yet.

+

AI assistants and other apps you connect to Fizzy show up here, so you can see what each one can do and disconnect it whenever you like.

+
<% end %>
diff --git a/app/views/oauth/authorizations/_access.html.erb b/app/views/oauth/authorizations/_access.html.erb new file mode 100644 index 0000000000..9cd372c0c9 --- /dev/null +++ b/app/views/oauth/authorizations/_access.html.erb @@ -0,0 +1,24 @@ +<%# Fizzy's two access levels, each in plain words. Write includes read, so + the choice is one or the other; a client registered for read alone gets + the single option, shown so the person still sees what they're granting. %> +
+ Access + + <% oauth_access_levels(client).each do |value, level| %> + + <% end %> + +

+ <% if local_assigns[:one_account] %> + It works as you, so it can only reach what you can, in the account above. + <% else %> + It works as you, so it can only reach what you can, in every account you belong to. + <% end %> +

+
diff --git a/app/views/oauth/authorizations/_account_picker.html.erb b/app/views/oauth/authorizations/_account_picker.html.erb new file mode 100644 index 0000000000..f1372a0f5a --- /dev/null +++ b/app/views/oauth/authorizations/_account_picker.html.erb @@ -0,0 +1,22 @@ +<%# Which account this connection reaches. The controller sets accounts (the + identity's accounts) and the preselected one; the choice posts as + account_id. With one account there is nothing to choose, so it's named, + and carried in a hidden field. %> +
+ Account + + <% if accounts.one? %> + <% account = accounts.first %> + <%= form.hidden_field :account_id, value: account.id %> +

<%= account.name %>

+ <% else %> + <% accounts.each_with_index do |account, index| %> + + <% end %> + <% end %> + +

The app reaches this account only. Connect it again to add another.

+
diff --git a/app/views/oauth/authorizations/_client.html.erb b/app/views/oauth/authorizations/_client.html.erb new file mode 100644 index 0000000000..672455d0b2 --- /dev/null +++ b/app/views/oauth/authorizations/_client.html.erb @@ -0,0 +1,28 @@ +<%# Who is asking, as far as Fizzy can vouch for it. A self-registered app's + name is only its claim, so it never stands alone: it's shown in quotes, + subdued, beside where the access goes (bc3's "name set by" line). bdi keeps + any right-to-left letters in the name from moving the quotes and host. %> +
+

<%= client.dynamically_registered? && redirect_host.nil? ? identity.upcase_first : identity %>

+ + <% if client.dynamically_registered? %> +

+ “<%= oauth_client_self_name(client) %>” · + <% if redirect_host %> + name set by the app at <%= redirect_host %> + <% else %> + name set by an app running on your device + <% end %> +

+ +

+ <% if redirect_host %> + Fizzy hasn't checked this app. Anyone can run an app at any address, so only authorize it if you trust <%= redirect_host %>. + <% else %> + Fizzy hasn't checked this app. An app running on your device will get this access, so only authorize it if you started it. + <% end %> +

+ <% elsif client.trusted? %> +

Made by 37signals

+ <% end %> +
diff --git a/app/views/oauth/authorizations/error.html.erb b/app/views/oauth/authorizations/error.html.erb index 3493d9a7c2..40b7bb0413 100644 --- a/app/views/oauth/authorizations/error.html.erb +++ b/app/views/oauth/authorizations/error.html.erb @@ -1,15 +1,15 @@ -
+<%# Shown on Fizzy, never redirected: the request couldn't be trusted enough + to send the browser back to the app (see redirect_with_error) %> +<% @page_title = "Can't authorize this app" %> + +
-

Authorization Error

+

<%= @page_title %>

-

- <%= @error_description %> -

+

<%= @error_description %>

-

- Error code: <%= @error %> +

+ Nothing was shared. Close this window and start again from the app. If it keeps happening, let whoever made the app know, and mention this error code: <%= @error %>

- - <%= link_to "Go back", request.referer || root_path, class: "btn btn--outline center txt-medium" %>
diff --git a/app/views/oauth/authorizations/new.html.erb b/app/views/oauth/authorizations/new.html.erb index 40bd97e190..d6f3d2b05b 100644 --- a/app/views/oauth/authorizations/new.html.erb +++ b/app/views/oauth/authorizations/new.html.erb @@ -1,10 +1,18 @@ -<% @page_title = "Authorize #{@client.name}" %> +<% identity = oauth_client_display_identity(@client, @redirect_uri) %> +<% redirect_host = oauth_redirect_host(@redirect_uri) %> +<% @page_title = "Authorize #{identity}" %> <% content_for :header do %>

<%= @page_title %>

<% end %> -
+
+ <%= render "oauth/authorizations/client", client: @client, identity: identity, redirect_host: redirect_host %> + +

+ Signed in as <%= Current.identity.email_address %> +

+ <%= form_with url: oauth_authorization_path, method: :post, data: { turbo: false }, html: { class: "flex flex-column gap" } do |f| %> <%= f.hidden_field :client_id, value: @client.client_id %> <%= f.hidden_field :redirect_uri, value: @redirect_uri %> @@ -13,26 +21,23 @@ <%= f.hidden_field :code_challenge_method, value: "S256" %> <%= f.hidden_field :response_type, value: "code" %> -
- Application -

<%= @client.name %>

- <% if @client.dynamically_registered? %> - <% redirect = URI.parse(@redirect_uri) %> - <% if Oauth.loopback_host?(redirect.host) %> -

Registered by a local tool. Only authorize if you trust it.

- <% else %> -

Self-registered app that will receive access at <%= redirect.port == redirect.default_port ? redirect.host : "#{redirect.host}:#{redirect.port}" %>. Only authorize if you trust it.

- <% end %> - <% end %> -
+ <%# Account binding sets @accounts: a connection then reaches the one account chosen here %> + <% if @accounts %> + <%= render "oauth/authorizations/account_picker", form: f, accounts: @accounts, selected_account_id: @account_id %> + <% end %> -
- <%= f.label :scope, "Permission" %> - <% scope_options = @client.scopes.include?("write") ? { "Read" => "read", "Read + Write" => "read write" } : { "Read" => "read" } %> - <%= f.select :scope, options_for_select(scope_options, @scope), {}, class: "input input--select" %> -
+ <%= render "oauth/authorizations/access", form: f, client: @client, scope: @scope, one_account: @accounts.present? %> + +

+ <% if redirect_host %> + Either way, you'll go back to <%= redirect_host %>. + <% else %> + Either way, you'll go back to the app on your device. + <% end %> + You can disconnect it at any time in <%= link_to "Connected apps", my_connected_apps_path, class: "txt-link" %>. +

-
+
<%= f.button type: :submit, class: "btn btn--link txt-medium" do %> Authorize <% end %> diff --git a/test/controllers/my/connected_apps_controller_test.rb b/test/controllers/my/connected_apps_controller_test.rb index 9eb5eb9dd4..9d51ff6064 100644 --- a/test/controllers/my/connected_apps_controller_test.rb +++ b/test/controllers/my/connected_apps_controller_test.rb @@ -75,4 +75,33 @@ class My::ConnectedAppsControllerTest < ActionDispatch::IntegrationTest delete my_connected_app_path(client) assert_response :not_found end + + test "index groups apps by kind and says what each can do" do + @identity.access_tokens.create!(oauth_client: oauth_clients(:mcp_client), permission: :read) + @identity.access_tokens.create!(oauth_client: oauth_clients(:confidential_client), permission: :write) + + get my_connected_apps_path + + assert_response :success + assert_select ".connected-apps__group", count: 2 + assert_select "#connected_apps_web + .connected-app, #connected_apps_web ~ .connected-app", text: /Confidential Connector.*Read and write/m + assert_select "#connected_apps_local ~ .connected-app", text: /Test MCP Client.*Name set by an app on your device.*Read only/m + assert_select "#connected_apps_fizzy", count: 0 + end + + test "index names where a self-registered web app's access goes" do + @identity.access_tokens.create!(oauth_client: oauth_clients(:confidential_client), permission: :read) + + get my_connected_apps_path + + assert_select ".connected-app", text: /Name set by the app at connector.example.com/ + end + + test "index shows an empty state when nothing is connected" do + get my_connected_apps_path + + assert_response :success + assert_select ".connected-apps__empty", text: /Nothing connected yet/ + assert_select ".connected-app", count: 0 + end end diff --git a/test/helpers/oauth_helper_test.rb b/test/helpers/oauth_helper_test.rb new file mode 100644 index 0000000000..4ca2b36ee2 --- /dev/null +++ b/test/helpers/oauth_helper_test.rb @@ -0,0 +1,56 @@ +require "test_helper" + +class OauthHelperTest < ActionView::TestCase + test "a self-registered app is identified by its callback's host, port included when it isn't the default" do + client = Oauth::Client.new name: "Hosted", dynamically_registered: true + + assert_equal "connector.example.com", oauth_client_display_identity(client, "https://connector.example.com/callback") + assert_equal "connector.example.com:8443", oauth_client_display_identity(client, "https://connector.example.com:8443/callback") + assert_equal "an app on your device", oauth_client_display_identity(client, "http://127.0.0.1:8888/callback") + assert_equal "an app on your device", oauth_client_display_identity(client, "com.example.app:/callback") + end + + test "a registered app is identified by its name" do + assert_equal "Trusted First-Party App", oauth_client_display_identity(oauth_clients(:trusted_client), "https://app.example.com/oauth/callback") + end + + test "a read-only client is offered read alone" do + reader = Oauth::Client.new scopes: %w[ read ] + + assert_equal %w[ read ], oauth_access_levels(reader).map(&:first) + assert_equal [ "read", "read write" ], oauth_access_levels(oauth_clients(:mcp_client)).map(&:first) + end + + test "connected apps are grouped by kind, in a fixed order, skipping empty groups" do + web = oauth_clients(:confidential_client) + local = oauth_clients(:mcp_client) + + groups = connected_app_groups([ [ local, [] ], [ web, [] ] ]) + + assert_equal %i[ web local ], groups.map(&:first) + assert_equal [ [ web, [] ] ], groups.first.last + end + + test "the account picker offers each account, preselecting the chosen one" do + accounts = Account.order(:name).first(2) + + form_with(url: "/oauth/authorization") do |form| + render partial: "oauth/authorizations/account_picker", locals: { form: form, accounts: accounts, selected_account_id: accounts.second.id } + end + + assert_select "input[type=radio][name=account_id]", count: accounts.size + assert_select "input[type=radio][name=account_id][checked][value=?]", accounts.second.id.to_s + end + + test "the account picker names a lone account and carries it hidden" do + account = Account.first + + form_with(url: "/oauth/authorization") do |form| + render partial: "oauth/authorizations/account_picker", locals: { form: form, accounts: [ account ], selected_account_id: nil } + end + + assert_select "input[type=radio]", count: 0 + assert_select "input[type=hidden][name=account_id][value=?]", account.id.to_s + assert_select "bdi", text: account.name + end +end diff --git a/test/integration/oauth_consent_screen_test.rb b/test/integration/oauth_consent_screen_test.rb new file mode 100644 index 0000000000..90ab15f3a3 --- /dev/null +++ b/test/integration/oauth_consent_screen_test.rb @@ -0,0 +1,92 @@ +require "test_helper" + +class OauthConsentScreenTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :david + end + + test "a self-registered web app goes by its callback's host, its own name quoted beside it" do + client = Oauth::Client.create! name: "Totally Fizzy", dynamically_registered: true, + redirect_uris: %w[ https://connector.example.com/callback ] + + get_consent_screen client + + assert_response :success + assert_select "title", text: /Authorize connector.example.com/ + assert_select ".oauth-client__name", text: "connector.example.com" + assert_select ".oauth-client__self-name", text: /“Totally Fizzy”\s+·\s+name set by the app at\s+connector.example.com/ + assert_select ".oauth-client__warning", text: /Fizzy hasn't checked this app/ + assert_select ".oauth-consent__destination strong", text: "connector.example.com" + end + + test "a self-registered app on this device is named as one" do + get_consent_screen oauth_clients(:mcp_client) + + assert_response :success + assert_select ".oauth-client__name", text: "An app on your device" + assert_select ".oauth-client__self-name", text: /“Test MCP Client”\s+·\s+name set by an app running on your device/ + assert_select ".oauth-client__warning", text: /only authorize it if you started it/ + assert_select ".oauth-consent__destination", text: /the app on your device/ + end + + test "a trusted app is known by its name and carries no warning" do + get_consent_screen oauth_clients(:trusted_client) + + assert_response :success + assert_select ".oauth-client__name", text: "Trusted First-Party App" + assert_select ".oauth-client__made-by", text: "Made by 37signals" + assert_select ".oauth-client__self-name", count: 0 + assert_select ".oauth-client__warning", count: 0 + end + + test "bidirectional controls are stripped from a self-registered name" do + client = Oauth::Client.create! name: "Good‮ppa‬ App", dynamically_registered: true, + redirect_uris: %w[ http://127.0.0.1:8888/callback ] + + get_consent_screen client + + assert_response :success + assert_no_match "‮", response.body + assert_select ".oauth-client__self-name bdi", text: "Goodppa App" + end + + test "each access level says what it lets the app do" do + get_consent_screen oauth_clients(:mcp_client), scope: "read write" + + assert_response :success + assert_select ".oauth-access__choice", count: 2 + assert_select ".oauth-access__choice", text: /Read only.*can't change anything/m + assert_select ".oauth-access__choice", text: /Read and write.*delete cards/m + assert_select ".oauth-access__reach", text: /every account you belong to/ + end + + test "the person sees who they're signed in as" do + get_consent_screen oauth_clients(:mcp_client) + + assert_select ".oauth-consent__identity strong", text: identities(:david).email_address + end + + test "an error before consent is shown on Fizzy, with what to do next" do + get_consent_screen oauth_clients(:mcp_client), redirect_uri: "https://evil.example.com/callback" + + assert_response :bad_request + assert_select "h2", text: "Can't authorize this app" + assert_select "p", text: /Nothing was shared/ + assert_select "code", text: "invalid_request" + end + + private + def get_consent_screen(client, **overrides) + untenanted do + get new_oauth_authorization_path, params: { + client_id: client.client_id, + redirect_uri: client.redirect_uris.first, + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "S256", + scope: "read", + state: "xyz123" + }.merge(overrides) + end + end +end diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index bcf3b6851f..1f1f3112d3 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -367,8 +367,8 @@ class OauthFlowTest < ActionDispatch::IntegrationTest get_consent_screen scope: "read write" assert_response :success - assert_select "select[name=scope] option[selected]", count: 1 - assert_select "select[name=scope] option[selected][value=?]", "read write" + assert_select "input[type=radio][name=scope][checked]", count: 1 + assert_select "input[type=radio][name=scope][checked][value=?]", "read write" end test "authorization preselects Read + Write when the client requests write" do @@ -377,7 +377,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest get_consent_screen scope: "write" assert_response :success - assert_select "select[name=scope] option[selected][value=?]", "read write" + assert_select "input[type=radio][name=scope][checked][value=?]", "read write" end test "authorization preselects Read for a read request" do @@ -386,7 +386,7 @@ class OauthFlowTest < ActionDispatch::IntegrationTest get_consent_screen scope: "read" assert_response :success - assert_select "select[name=scope] option[selected][value=?]", "read" + assert_select "input[type=radio][name=scope][checked][value=?]", "read" end test "authorization offers no write option to a read-only client" do @@ -396,8 +396,8 @@ class OauthFlowTest < ActionDispatch::IntegrationTest get_consent_screen client: client, scope: "read" assert_response :success - assert_select "select[name=scope] option", count: 1 - assert_select "select[name=scope] option[value=?]", "read" + assert_select "input[type=radio][name=scope]", count: 1 + assert_select "input[type=radio][name=scope][value=?]", "read" end test "consent grants the canonical read write scope" do diff --git a/test/models/oauth/client_test.rb b/test/models/oauth/client_test.rb index fdbae59c0d..e39460a768 100644 --- a/test/models/oauth/client_test.rb +++ b/test/models/oauth/client_test.rb @@ -457,6 +457,14 @@ class Oauth::ClientTest < ActiveSupport::TestCase assert_not Oauth::Client.exists?(client.id) end + test "connection_kind tells our apps, tools on this device and web apps apart" do + assert_equal :fizzy, oauth_clients(:trusted_client).connection_kind + assert_equal :local, oauth_clients(:mcp_client).connection_kind + assert_equal :web, oauth_clients(:confidential_client).connection_kind + assert_equal :local, Oauth::Client.new(redirect_uris: %w[ com.example.app:/callback http://[::1]/cb ]).connection_kind + assert_equal :web, Oauth::Client.new(redirect_uris: %w[ http://127.0.0.1/cb https://example.com/cb ]).connection_kind + end + private def register_client(**attributes) Oauth::Client.create! name: "Abandoned", redirect_uris: %w[ http://127.0.0.1:8888/callback ], dynamically_registered: true, **attributes From 56bd25ce08a9bb0c3b409eacdaec1255a74830ac Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 19 Oct 2026 23:11:50 -0700 Subject: [PATCH 02/16] Name every remote host a connected app can send its access to A self-registered client may list a loopback callback first and an https one later, and its grant may have come through either, so the first callback doesn't speak for the app. --- app/helpers/oauth_helper.rb | 7 +++++++ app/views/my/connected_apps/_connected_app.html.erb | 6 +++--- test/controllers/my/connected_apps_controller_test.rb | 11 +++++++++++ test/helpers/oauth_helper_test.rb | 6 ++++++ 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb index d01ce3063d..3cc463d76e 100644 --- a/app/helpers/oauth_helper.rb +++ b/app/helpers/oauth_helper.rb @@ -51,6 +51,13 @@ def oauth_redirect_host(redirect_uri) nil end + # Every place outside this device a client's access can go, as consent would + # name each one. A client may register several callbacks, and its grants may + # have come through any of them, so none speaks for the rest. + def oauth_client_remote_hosts(client) + client.redirect_uris.filter_map { |uri| oauth_redirect_host(uri) }.uniq + end + # The access levels a client may be granted, weakest first, each with what # it lets the app do in plain words. def oauth_access_levels(client) diff --git a/app/views/my/connected_apps/_connected_app.html.erb b/app/views/my/connected_apps/_connected_app.html.erb index 46c4e57ec0..a225628365 100644 --- a/app/views/my/connected_apps/_connected_app.html.erb +++ b/app/views/my/connected_apps/_connected_app.html.erb @@ -1,4 +1,4 @@ -<% redirect_host = client.dynamically_registered? ? oauth_redirect_host(client.redirect_uris.first) : nil %> +<% remote_hosts = oauth_client_remote_hosts(client) %> <% permission = tokens.any?(&:write?) ? "write" : "read" %>
@@ -9,8 +9,8 @@ <% if client.dynamically_registered? %>

- <% if redirect_host %> - Name set by the app at <%= redirect_host %> + <% if remote_hosts.any? %> + Name set by the app at <%= remote_hosts.to_sentence(two_words_connector: " or ", last_word_connector: " or ") %> <% else %> Name set by an app on your device <% end %> diff --git a/test/controllers/my/connected_apps_controller_test.rb b/test/controllers/my/connected_apps_controller_test.rb index 9d51ff6064..d02c3cbf84 100644 --- a/test/controllers/my/connected_apps_controller_test.rb +++ b/test/controllers/my/connected_apps_controller_test.rb @@ -104,4 +104,15 @@ class My::ConnectedAppsControllerTest < ActionDispatch::IntegrationTest assert_select ".connected-apps__empty", text: /Nothing connected yet/ assert_select ".connected-app", count: 0 end + + test "index names every remote host a self-registered app can send its access to, not just the first" do + client = Oauth::Client.create! name: "Two Homes", dynamically_registered: true, + redirect_uris: %w[ http://127.0.0.1:8888/callback https://one.example.com/cb https://two.example.com:8443/cb ] + @identity.access_tokens.create!(oauth_client: client, permission: :read) + + get my_connected_apps_path + + assert_select "#connected_apps_web ~ .connected-app", text: /Name set by the app at one.example.com or two.example.com:8443/ + assert_no_match "Name set by an app on your device", response.body + end end diff --git a/test/helpers/oauth_helper_test.rb b/test/helpers/oauth_helper_test.rb index 4ca2b36ee2..326e7ffefb 100644 --- a/test/helpers/oauth_helper_test.rb +++ b/test/helpers/oauth_helper_test.rb @@ -53,4 +53,10 @@ class OauthHelperTest < ActionView::TestCase assert_select "input[type=hidden][name=account_id][value=?]", account.id.to_s assert_select "bdi", text: account.name end + + test "a client's remote hosts skip its on-device callbacks and repeats" do + client = Oauth::Client.new redirect_uris: %w[ http://127.0.0.1/cb https://a.example.com/x https://a.example.com/y com.example.app:/cb https://b.example.com:8443/cb ] + + assert_equal %w[ a.example.com b.example.com:8443 ], oauth_client_remote_hosts(client) + end end From 0cc090101c4c8c6050e47ff4343592498ac0937b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 23 Oct 2026 15:24:07 -0700 Subject: [PATCH 03/16] Offer "Not you?" on the consent screen, and call last activity what it is Switching signs out and brings whoever signs in next back to the same authorization request, rebuilt from parameters that passed the consent screen's checks rather than taken as a URL. Connected apps says when an app last renewed its access, which is what the timestamp records. --- .../authorizations/switches_controller.rb | 24 +++++ .../my/connected_apps/_connected_app.html.erb | 5 +- app/views/oauth/authorizations/new.html.erb | 11 ++- config/routes.rb | 6 +- .../my/connected_apps_controller_test.rb | 1 + .../oauth_authorization_switch_test.rb | 97 +++++++++++++++++++ 6 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 app/controllers/oauth/authorizations/switches_controller.rb create mode 100644 test/integration/oauth_authorization_switch_test.rb diff --git a/app/controllers/oauth/authorizations/switches_controller.rb b/app/controllers/oauth/authorizations/switches_controller.rb new file mode 100644 index 0000000000..144eab693f --- /dev/null +++ b/app/controllers/oauth/authorizations/switches_controller.rb @@ -0,0 +1,24 @@ +# "Not you?" on the consent screen. Signs the person out, then brings whoever +# signs in next back to the same authorization request. The request is rebuilt +# from its parameters once they have passed every check the consent screen's +# did, never taken as a URL, so the only place this can return anyone to is +# consent for that request. The account choice isn't carried: it was the +# signed-out identity's. +class Oauth::Authorizations::SwitchesController < Oauth::AuthorizationsController + AUTHORIZATION_PARAMS = %i[ client_id redirect_uri response_type scope state code_challenge code_challenge_method ] + + def create + return_to = new_oauth_authorization_url(script_name: nil, **authorization_params) + + terminate_session + reset_session + session[:return_to_after_authenticating] = return_to if return_to.bytesize <= RETURN_URL_MAXIMUM_LENGTH + + redirect_to_login_url + end + + private + def authorization_params + params.permit(*AUTHORIZATION_PARAMS).to_h.symbolize_keys + end +end diff --git a/app/views/my/connected_apps/_connected_app.html.erb b/app/views/my/connected_apps/_connected_app.html.erb index a225628365..644fa1792e 100644 --- a/app/views/my/connected_apps/_connected_app.html.erb +++ b/app/views/my/connected_apps/_connected_app.html.erb @@ -23,11 +23,10 @@ <%= oauth_access_label(permission) %>, in every account you belong to

- <%# updated_at moves each time the app renews its access, so it's the last - time the app was active, to within an access token's lifetime %> + <%# updated_at moves each time the app renews its access %>

Connected <%= local_datetime_tag tokens.map(&:created_at).min, style: :date %> - · last active <%= local_datetime_tag tokens.map(&:updated_at).max, style: :daysago %> + · access last renewed <%= local_datetime_tag tokens.map(&:updated_at).max, style: :daysago %> <% if tokens.many? %>· <%= pluralize(tokens.size, "connection") %><% end %>

diff --git a/app/views/oauth/authorizations/new.html.erb b/app/views/oauth/authorizations/new.html.erb index d6f3d2b05b..7552bba97c 100644 --- a/app/views/oauth/authorizations/new.html.erb +++ b/app/views/oauth/authorizations/new.html.erb @@ -9,9 +9,14 @@
<%= render "oauth/authorizations/client", client: @client, identity: identity, redirect_host: redirect_host %> -

- Signed in as <%= Current.identity.email_address %> -

+ <%# Not you? signs out and returns whoever signs in to this same request %> +
+ Signed in as <%= Current.identity.email_address %> + <%= button_to "Not you?", oauth_authorization_switch_path, + params: { client_id: @client.client_id, redirect_uri: @redirect_uri, response_type: "code", scope: @scope, + state: @state, code_challenge: @code_challenge, code_challenge_method: "S256" }.compact, + class: "btn btn--plain txt-link txt-small", form: { data: { turbo: false } } %> +
<%= form_with url: oauth_authorization_path, method: :post, data: { turbo: false }, html: { class: "flex flex-column gap" } do |f| %> <%= f.hidden_field :client_id, value: @client.client_id %> diff --git a/config/routes.rb b/config/routes.rb index a84bd46df8..7c0c32c74d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,7 +5,11 @@ get "/.well-known/oauth-protected-resource", to: "oauth/protected_resource_metadata#show" namespace :oauth do - resource :authorization, only: %i[ new create ] + resource :authorization, only: %i[ new create ] do + scope module: :authorizations do + resource :switch, only: :create + end + end resource :token, only: :create resource :revocation, only: :create resources :clients, only: :create diff --git a/test/controllers/my/connected_apps_controller_test.rb b/test/controllers/my/connected_apps_controller_test.rb index d02c3cbf84..b15ce47aa4 100644 --- a/test/controllers/my/connected_apps_controller_test.rb +++ b/test/controllers/my/connected_apps_controller_test.rb @@ -87,6 +87,7 @@ class My::ConnectedAppsControllerTest < ActionDispatch::IntegrationTest assert_select "#connected_apps_web + .connected-app, #connected_apps_web ~ .connected-app", text: /Confidential Connector.*Read and write/m assert_select "#connected_apps_local ~ .connected-app", text: /Test MCP Client.*Name set by an app on your device.*Read only/m assert_select "#connected_apps_fizzy", count: 0 + assert_select ".connected-app", text: /access last renewed/ end test "index names where a self-registered web app's access goes" do diff --git a/test/integration/oauth_authorization_switch_test.rb b/test/integration/oauth_authorization_switch_test.rb new file mode 100644 index 0000000000..b9ea2e8d86 --- /dev/null +++ b/test/integration/oauth_authorization_switch_test.rb @@ -0,0 +1,97 @@ +require "test_helper" + +class OauthAuthorizationSwitchTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :david + @client = oauth_clients(:mcp_client) + end + + test "the consent screen offers to switch who is signed in" do + untenanted { get new_oauth_authorization_path, params: authorization_params } + + assert_select "form[action=?] button", oauth_authorization_switch_path(script_name: nil), text: "Not you?" + end + + test "switching signs out and saves the authorization request, rebuilt from its checked parameters" do + assert_difference -> { identities(:david).sessions.count }, -1 do + post_switch authorization_params.merge(return_to: "https://evil.example.com/", account_id: "123") + end + + assert_redirected_to new_session_path(script_name: nil) + assert_not cookies[:session_token].present? + assert_equal expected_return_url, session[:return_to_after_authenticating] + end + + test "switching requires a same-origin request" do + with_forgery_protection do + assert_no_difference -> { identities(:david).sessions.count } do + post_switch authorization_params, headers: { "Sec-Fetch-Site" => "cross-site" } + end + end + + assert_response :unprocessable_entity + end + + test "a redirect the client didn't register is refused, leaving the person signed in and nothing saved" do + assert_no_difference -> { identities(:david).sessions.count } do + post_switch authorization_params(redirect_uri: "https://evil.example.com/callback") + end + + assert_response :bad_request + assert cookies[:session_token].present? + assert_nil session[:return_to_after_authenticating] + end + + test "an unknown client is refused, leaving the person signed in" do + assert_no_difference -> { identities(:david).sessions.count } do + post_switch authorization_params(client_id: "forged") + end + + assert_response :bad_request + assert_nil session[:return_to_after_authenticating] + end + + test "whoever signs in next lands back on consent for the same request, and the saved request is used once" do + post_switch authorization_params + sign_in_as :kevin + + assert_equal expected_return_url, response.location + assert_nil session[:return_to_after_authenticating] + + get response.location + assert_response :success + assert_select ".oauth-consent__identity strong", text: identities(:kevin).email_address + + sign_in_as :kevin + assert_not_equal expected_return_url, response.location + end + + private + def authorization_params(**overrides) + { + client_id: @client.client_id, + redirect_uri: @client.redirect_uris.first, + response_type: "code", + code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + code_challenge_method: "S256", + scope: "read", + state: "xyz123" + }.merge(overrides) + end + + def expected_return_url + new_oauth_authorization_url(script_name: nil, **authorization_params) + end + + def post_switch(params, **options) + untenanted { post oauth_authorization_switch_path, params: params, **options } + end + + def with_forgery_protection + original = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = true + yield + ensure + ActionController::Base.allow_forgery_protection = original + end +end From cc7788497ef7537e23bfa26a8e0dc0ee90c092bb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 23 Oct 2026 15:56:02 -0700 Subject: [PATCH 04/16] Tell people about an unknown client on the error page, and switch only when the request can be resumed An authorization request naming no known client has nowhere to send the browser back to, so it gets the error page rather than raw JSON. Not you? refuses a request too long to keep in the session before signing anyone out, since there would be nothing to return to. --- .../oauth/authorizations/switches_controller.rb | 14 ++++++++++---- .../oauth/authorizations_controller.rb | 15 ++++++++++----- .../oauth_authorization_switch_test.rb | 12 ++++++++++++ test/integration/oauth_flow_test.rb | 4 +++- 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/app/controllers/oauth/authorizations/switches_controller.rb b/app/controllers/oauth/authorizations/switches_controller.rb index 144eab693f..c4eaff996e 100644 --- a/app/controllers/oauth/authorizations/switches_controller.rb +++ b/app/controllers/oauth/authorizations/switches_controller.rb @@ -7,14 +7,20 @@ class Oauth::Authorizations::SwitchesController < Oauth::AuthorizationsController AUTHORIZATION_PARAMS = %i[ client_id redirect_uri response_type scope state code_challenge code_challenge_method ] + # A request too long to keep in the session can't be resumed, so the person + # stays signed in rather than signing out to nowhere. def create return_to = new_oauth_authorization_url(script_name: nil, **authorization_params) - terminate_session - reset_session - session[:return_to_after_authenticating] = return_to if return_to.bytesize <= RETURN_URL_MAXIMUM_LENGTH + if return_to.bytesize <= RETURN_URL_MAXIMUM_LENGTH + terminate_session + reset_session + session[:return_to_after_authenticating] = return_to - redirect_to_login_url + redirect_to_login_url + else + render_error "invalid_request", "This request is too long to pick up again after switching. Sign out, sign in as the right person, then connect from the app again." + end end private diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 7aec59607b..60941ae0e3 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -51,7 +51,7 @@ def create redirect_to success_redirect_uri(code), allow_other_host: true else - oauth_error "invalid_request", "Unknown client" + render_error "invalid_request", "Unknown client" end end @@ -70,9 +70,10 @@ def save_oauth_return_url end end + # There's no client to send the browser back to, so the person is told here def set_client @client = Oauth::Client.find_by(client_id: params[:client_id]) - oauth_error("invalid_request", "Unknown client") unless @client + render_error("invalid_request", "Unknown client") unless @client end def validate_redirect_uri @@ -128,12 +129,16 @@ def redirect_with_error(error, description) if params[:redirect_uri].present? && @client&.allows_redirect?(params[:redirect_uri]) && @client.vetted_redirect?(params[:redirect_uri]) redirect_to error_redirect_uri(error, description), allow_other_host: true else - @error = error - @error_description = description - render :error, status: :bad_request + render_error error, description end end + def render_error(error, description) + @error = error + @error_description = description + render :error, status: :bad_request + end + def success_redirect_uri(code) build_redirect_uri params[:redirect_uri], code: code, diff --git a/test/integration/oauth_authorization_switch_test.rb b/test/integration/oauth_authorization_switch_test.rb index b9ea2e8d86..3a1e10840c 100644 --- a/test/integration/oauth_authorization_switch_test.rb +++ b/test/integration/oauth_authorization_switch_test.rb @@ -48,6 +48,18 @@ class OauthAuthorizationSwitchTest < ActionDispatch::IntegrationTest end assert_response :bad_request + assert_select "code", text: "invalid_request" + assert_nil session[:return_to_after_authenticating] + end + + test "a request too long to resume is refused before signing out" do + assert_no_difference -> { identities(:david).sessions.count } do + post_switch authorization_params(state: "x" * 2048) + end + + assert_response :bad_request + assert_select "p", text: /too long to pick up again/ + assert cookies[:session_token].present? assert_nil session[:return_to_after_authenticating] end diff --git a/test/integration/oauth_flow_test.rb b/test/integration/oauth_flow_test.rb index 1f1f3112d3..039dc34466 100644 --- a/test/integration/oauth_flow_test.rb +++ b/test/integration/oauth_flow_test.rb @@ -57,7 +57,9 @@ class OauthFlowTest < ActionDispatch::IntegrationTest end assert_response :bad_request - assert_equal "invalid_request", response.parsed_body["error"] + assert_select "h2", text: "Can't authorize this app" + assert_select "p", text: "Unknown client" + assert_select "code", text: "invalid_request" end test "authorization rejects mismatched redirect_uri" do From c2efad212eca432b9735ebd296ea9c51884d8355 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 24 Oct 2026 14:41:36 -0700 Subject: [PATCH 05/16] Say which accounts a read grant's people come from, and read hosts in lowercase Read's description names the people in every account the person belongs to, or in the one account an audienced grant is bound to, matching the reach line beside it. A redirect's host is shown in lowercase, as a browser reads it. --- app/helpers/oauth_helper.rb | 19 ++++++++++++------- .../oauth/authorizations/_access.html.erb | 2 +- test/helpers/oauth_helper_test.rb | 11 +++++++++++ test/integration/oauth_consent_screen_test.rb | 2 +- 4 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/helpers/oauth_helper.rb b/app/helpers/oauth_helper.rb index 3cc463d76e..b29edcf470 100644 --- a/app/helpers/oauth_helper.rb +++ b/app/helpers/oauth_helper.rb @@ -15,7 +15,7 @@ module OauthHelper ACCESS_LEVELS = { "read" => { label: "Read only", - description: "See your boards, cards, comments and the people in your accounts. It can't change anything." + description: "See your boards, cards, comments and the people in your %{accounts}. It can't change anything." }, "read write" => { label: "Read and write", @@ -39,13 +39,13 @@ def oauth_client_display_identity(client, redirect_uri) end end - # nil for a callback that stays on this device: loopback, or an app's own - # URL scheme. + # Lowercase, as a browser reads it. nil for a callback that stays on this + # device: loopback, or an app's own URL scheme. def oauth_redirect_host(redirect_uri) uri = URI.parse(redirect_uri.to_s) if uri.host.present? && !Oauth.loopback_host?(uri.host) && uri.scheme.to_s.downcase.in?(%w[ http https ]) - uri.port == uri.default_port ? uri.host : "#{uri.host}:#{uri.port}" + uri.port == uri.default_port ? uri.host.downcase : "#{uri.host.downcase}:#{uri.port}" end rescue URI::InvalidURIError nil @@ -59,10 +59,15 @@ def oauth_client_remote_hosts(client) end # The access levels a client may be granted, weakest first, each with what - # it lets the app do in plain words. - def oauth_access_levels(client) + # it lets the app do in plain words. A grant reaches every account the + # person belongs to, unless it's bound to the one picked at consent. + def oauth_access_levels(client, one_account: false) levels = client.scopes.include?("write") ? ACCESS_LEVELS.keys : %w[ read ] - levels.map { |scope| [ scope, ACCESS_LEVELS.fetch(scope) ] } + + levels.map do |scope| + level = ACCESS_LEVELS.fetch(scope) + [ scope, level.merge(description: format(level[:description], accounts: one_account ? "account" : "accounts")) ] + end end def oauth_access_label(permission) diff --git a/app/views/oauth/authorizations/_access.html.erb b/app/views/oauth/authorizations/_access.html.erb index 9cd372c0c9..9173a6d0f2 100644 --- a/app/views/oauth/authorizations/_access.html.erb +++ b/app/views/oauth/authorizations/_access.html.erb @@ -4,7 +4,7 @@
Access - <% oauth_access_levels(client).each do |value, level| %> + <% oauth_access_levels(client, one_account: local_assigns[:one_account]).each do |value, level| %>