Design the OAuth consent screen and Connected apps (Decision 8)

app/assets/stylesheets/oauth.css

@@ -0,0 +1,125 @@
+/* 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;
+  }
+
+  /* A long email still leaves room for "Not you?": the row wraps, and the address breaks */
+  .oauth-consent__identity {
+    flex-wrap: wrap;
+
+    > span {
+      min-inline-size: 0;
+      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;
+
+    /* A long account name breaks inside its card rather than widening it */
+    > span {
+      min-inline-size: 0;
+      overflow-wrap: anywhere;
+    }
+
+    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;
+  }
+}

app/controllers/my/connected_apps_controller.rb

@@ -29,7 +29,7 @@ def destroy
 
   private
     def set_connected_apps
-      tokens = oauth_tokens.unlapsed.includes(:oauth_client).order(:created_at)
+      tokens = oauth_tokens.unlapsed.includes(:oauth_client, :account).order(:created_at)
       @connected_apps = tokens.group_by(&:oauth_client).sort_by { |client, _| client.name.downcase }
     end
 

app/controllers/oauth/authorizations/switches_controller.rb

@@ -0,0 +1,50 @@
+# "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, so the account check that guards granting doesn't
+# apply here. The resource is carried, so an audienced request comes back as
+# one, and its consent asks the new identity to pick an account.
+class Oauth::Authorizations::SwitchesController < Oauth::AuthorizationsController
+  AUTHORIZATION_PARAMS = %i[ client_id redirect_uri response_type scope state code_challenge code_challenge_method resource ]
+
+  # Account eligibility belongs to the identity being signed out, so neither
+  # account check applies; the request-level audience check still does.
+  skip_before_action :set_account, :set_accounts
+  before_action :require_browser_session
+
+
+  # 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)
+
+    if return_to.bytesize <= RETURN_URL_MAXIMUM_LENGTH
+      terminate_session
+      reset_session
+      session[:return_to_after_authenticating] = return_to
+
+      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
+    # Switching signs a browser out. A request authenticated by a bearer token
+    # has no session to end, so it is refused rather than half-handled.
+    def require_browser_session
+      head :forbidden unless Current.session
+    end
+
+    # Switching identity is a POST but authorizes nothing, so a metadata
+    # document client is only read here, never registered.
+    def persist_metadata_document?
+      false
+    end
+
+    def authorization_params
+      params.permit(*AUTHORIZATION_PARAMS).to_h.symbolize_keys
+    end
+end

app/controllers/oauth/authorizations_controller.rb

@@ -71,7 +71,7 @@ def create
       Oauth::Audit.record "code.issued", client: @client, identity_id: Current.identity.id, actor: "user"
       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
 
@@ -118,8 +118,9 @@ def set_client
       end
 
       unless performed?
+        # There's no client to send the browser back to, so the person is told here.
         if @client.nil?
-          oauth_error "invalid_request", "Unknown client"
+          render_error "invalid_request", "Unknown client"
         elsif !@client.allows_grant_type?("authorization_code")
           oauth_error "unauthorized_client", "Client may not use the authorization code grant"
         end
@@ -134,11 +135,15 @@ def metadata_document_client
       if denial?
         metadata_document_client_for_denial
       elsif Oauth::Availability.metadata_documents_enabled?(params[:client_id])
-        resolution = Oauth::ClientMetadata::Resolver.new(params[:client_id], requester: Current.identity).resolve(persist: request.post?)
+        resolution = Oauth::ClientMetadata::Resolver.new(params[:client_id], requester: Current.identity).resolve(persist: persist_metadata_document?)
         resolution.client || refuse_metadata_document(resolution)
       end
     end
 
+    def persist_metadata_document?
+      request.post?
+    end
+
     # A denial never registers a client, and stays reachable while metadata
     # documents are off: the stored row answers when it allows the redirect.
     # Otherwise (no row, or a stale one the consent screen saw past) the
@@ -267,10 +272,17 @@ def redirect_with_error(error, description)
       end
     end
 
+    # Shown here rather than sent back to the app, and counted like any OAuth
+    # error the app would have been sent.
     def render_error(error, description)
+      instrument_oauth_error error
       @error = error
       @error_description = description
-      render :error, status: :bad_request
+
+      respond_to do |format|
+        format.html { render :error, status: :bad_request }
+        format.json { render_oauth_error error, description, status: :bad_request }
+      end
     end
 
     def success_redirect_uri(code)

app/controllers/oauth/base_controller.rb

@@ -168,10 +168,13 @@ def prevent_referrer
     # them in Yabeda). The client is named by its type alone: a self-registered
     # client's id is anyone's to mint, so it can't be a metric label.
     def oauth_error(error, description = nil, status: :bad_request)
+      instrument_oauth_error error
+      render_oauth_error error, description, status: status
+    end
+
+    def instrument_oauth_error(error)
       ActiveSupport::Notifications.instrument "error.oauth",
         endpoint: controller_name, error_code: error, client_type: oauth_error_client&.client_type || "unknown"
-
-      render_oauth_error error, description, status: status
     end
 
     # A throttled request is counted as a rate-limit hit and not as an error,

app/helpers/oauth_helper.rb

@@ -0,0 +1,118 @@
+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."
+    }
+  }
+
+  # A name of nothing but direction controls is no name, so it reads as one.
+  def oauth_client_self_name(client)
+    client.name.to_s.gsub(BIDI_CONTROLS, "").presence || "Unnamed app"
+  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
+
+  # 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.downcase : "#{uri.host.downcase}:#{uri.port}"
+    end
+  rescue URI::InvalidURIError
+    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. 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 do |scope|
+      level = ACCESS_LEVELS.fetch(scope)
+      [ scope, level.merge(description: format(level[:description], accounts: one_account ? "account" : "accounts")) ]
+    end
+  end
+
+  def oauth_strongest_permission(tokens)
+    if tokens.any?(&:write?)
+      "write"
+    elsif tokens.any?
+      "read"
+    end
+  end
+
+  def oauth_access_label(permission)
+    ACCESS_LEVELS.fetch(Oauth.canonical_scope(permission))[:label]
+  end
+
+  # What an app's grants together let it do, as [ permission, accounts ]
+  # pairs, accounts nil for every account the person belongs to. An unbound
+  # grant reaches every account; a bound one adds only its own, and only
+  # where it grants more, so no pair claims more than some grant gives.
+  def oauth_connected_app_reach(tokens)
+    # A grant bound to an account since deleted, cancelled, or left reaches
+    # nothing: refreshing it would be refused, so it isn't shown as reach.
+    unbound, bound = tokens.partition { |token| !token.account_bound? }
+    bound = bound.select { |token| token.account && oauth_active_account_ids.include?(token.account_id) }
+    everywhere = oauth_strongest_permission(unbound)
+
+    reach = bound.group_by(&:account).filter_map do |account, account_tokens|
+      permission = oauth_strongest_permission(account_tokens)
+      [ permission, account ] if everywhere.nil? || (permission == "write" && everywhere == "read")
+    end
+
+    [ (everywhere && [ everywhere, nil ]), *reach.group_by(&:first).map { |permission, pairs| [ permission, pairs.map(&:last) ] } ].compact
+  end
+
+  # The accounts the signed-in person is still active in, read once per render
+  # however many apps the page lists.
+  def oauth_active_account_ids
+    @oauth_active_account_ids ||= Current.identity.users_with_active_accounts.pluck(:account_id).to_set
+  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