From 2c19dbc689fcdb2c1f73cf6da360f88436393663 Mon Sep 17 00:00:00 2001 From: 24c02 <163450896+24c02@users.noreply.github.com> Date: Thu, 11 Dec 2025 20:28:05 -0500 Subject: [PATCH] [oauth2] another pass --- app/frontend/stylesheets/snippets/oauth.scss | 50 ++++++++---- app/models/oauth_scope.rb | 77 +++++++++++++++++-- .../doorkeeper/authorizations/new.html.erb | 30 +++++--- config/locales/doorkeeper.en.yml | 11 --- config/locales/en.yml | 22 +++--- 5 files changed, 137 insertions(+), 53 deletions(-) diff --git a/app/frontend/stylesheets/snippets/oauth.scss b/app/frontend/stylesheets/snippets/oauth.scss index 8d1ac62..dbb19cc 100644 --- a/app/frontend/stylesheets/snippets/oauth.scss +++ b/app/frontend/stylesheets/snippets/oauth.scss @@ -43,29 +43,46 @@ background: var(--surface-2); border: 1px solid var(--surface-2-border); border-radius: $radius-md; - padding: 1rem 1.25rem; + padding: 0.875rem 1rem; - > strong { - display: block; - font-size: 0.9375rem; - font-weight: 600; + .permission-header { + display: flex; + align-items: flex-start; + gap: 0.625rem; + + svg { + opacity: 0.7; + flex-shrink: 0; + margin-top: 0.1875rem; + } + } + + .permission-title { + font-size: 0.875rem; + font-weight: 500; color: var(--text-strong); - margin-bottom: 0.5rem; + line-height: 1.4; + } + + &--action { + padding: 0.75rem 1rem; + + .permission-title { + font-weight: 500; + color: var(--text-muted-strong); + } } } .permission-details { list-style: none !important; - padding: $space-2 0 0 !important; + padding: 0.5rem 0 0 1.625rem !important; margin: 0 !important; - border-top: 1px solid var(--surface-2-border); li { - font-size: 0.875rem; - color: var(--pico-muted-color); - padding: 0.375rem 0; - display: flex; - align-items: baseline; + font-size: 0.8125rem; + color: var(--text-strong); + padding: 0.125rem 0; list-style: none !important; &::before { @@ -73,10 +90,11 @@ } span { - font-weight: 500; color: var(--text-muted-strong); - min-width: 140px; - flex-shrink: 0; + + &::after { + content: " "; + } } } } diff --git a/app/models/oauth_scope.rb b/app/models/oauth_scope.rb index a3ebb16..c41a845 100644 --- a/app/models/oauth_scope.rb +++ b/app/models/oauth_scope.rb @@ -1,23 +1,38 @@ # frozen_string_literal: true class OAuthScope - attr_reader :name, :description, :consent_fields, :includes + attr_reader :name, :description, :consent_fields, :includes, :icon - def initialize(name:, description:, consent_fields: [], includes: []) + def initialize(name:, description:, consent_fields: [], includes: [], icon: nil) @name = name @description = description @consent_fields = consent_fields @includes = includes + @icon = icon end + FIELD_ICONS = { + email: "email", + name: "person", + phone: "phone", + birthdate: "event-check", + address: "home", + verification_status: "check", + ysws_eligible: "check", + slack_id: "slack", + legal_name: "card-id" + }.freeze + ALL = [ new( name: "openid", - description: "Enable OpenID Connect authentication" + description: "Enable OpenID Connect authentication", + icon: "private" ), new( name: "email", description: "See your email address", + icon: "email", consent_fields: [ { key: :email, value: ->(ident) { ident.primary_email } } ] @@ -25,6 +40,7 @@ class OAuthScope new( name: "name", description: "See your name", + icon: "person", consent_fields: [ { key: :name, value: ->(ident) { "#{ident.first_name} #{ident.last_name}" } } ] @@ -32,6 +48,7 @@ class OAuthScope new( name: "profile", description: "See your name and profile information", + icon: "profile", consent_fields: [ { key: :name, value: ->(ident) { "#{ident.first_name} #{ident.last_name}" } } ] @@ -39,6 +56,7 @@ class OAuthScope new( name: "phone", description: "See your phone number", + icon: "phone", consent_fields: [ { key: :phone, value: ->(ident) { ident.phone_number } } ] @@ -46,6 +64,7 @@ class OAuthScope new( name: "birthdate", description: "See your date of birth", + icon: "event-check", consent_fields: [ { key: :birthdate, value: ->(ident) { ident.birthday&.strftime("%B %d, %Y") } } ] @@ -53,6 +72,7 @@ class OAuthScope new( name: "address", description: "View your mailing address(es)", + icon: "home", consent_fields: [ { key: :address, value: ->(ident) { addr = ident.primary_address @@ -63,6 +83,7 @@ class OAuthScope new( name: "verification_status", description: "See your verification status and YSWS eligibility", + icon: "check", consent_fields: [ { key: :verification_status, value: ->(ident) { ident.verification_status } }, { key: :ysws_eligible, value: ->(ident) { ident.ysws_eligible ? "Yes" : "No" } } @@ -71,6 +92,7 @@ class OAuthScope new( name: "slack_id", description: "See your Slack ID", + icon: "slack", consent_fields: [ { key: :slack_id, value: ->(ident) { ident.slack_id } } ] @@ -78,6 +100,7 @@ class OAuthScope new( name: "legal_name", description: "See your legal name", + icon: "card-id", consent_fields: [ { key: :legal_name, value: ->(ident) { [ident.legal_first_name, ident.legal_last_name].compact.join(" ").presence } } ] @@ -85,11 +108,13 @@ class OAuthScope new( name: "basic_info", description: "See basic information about you (email, name, verification status)", + icon: "docs", includes: %w[email name slack_id phone birthdate verification_status] ), new( name: "set_slack_id", - description: "Associate Slack IDs with identities" + description: "Associate Slack IDs with identities", + icon: "slack" ) ].freeze @@ -113,13 +138,55 @@ class OAuthScope scope.consent_fields.each do |field| next if seen_keys.include?(field[:key]) seen_keys << field[:key] - fields << { key: field[:key], value: field[:value].call(identity) } + fields << { + key: field[:key], + value: field[:value].call(identity), + icon: FIELD_ICONS[field[:key]] + } end end fields end + def self.consent_data_by_scope(scope_names, identity) + result = {} + seen_field_keys = Set.new + + scope_names.each do |name| + scope = find(name) + next unless scope + next if scope.name == "openid" + + fields = scope.consent_fields.map do |field| + { key: field[:key], value: field[:value].call(identity) } + end + + scope.includes.each do |included_name| + included_scope = find(included_name) + next unless included_scope + included_scope.consent_fields.each do |field| + fields << { key: field[:key], value: field[:value].call(identity) } + end + end + + unique_fields = fields.uniq { |f| f[:key] } + new_fields = unique_fields.reject { |f| seen_field_keys.include?(f[:key]) } + + next if new_fields.empty? && unique_fields.any? + + new_fields.each { |f| seen_field_keys << f[:key] } + + result[name] = { + description: scope.description, + icon: scope.icon, + fields: new_fields + } + end + + result + end + def self.expanded_scopes(scope_names) scope_names.flat_map do |name| scope = find(name) diff --git a/app/views/doorkeeper/authorizations/new.html.erb b/app/views/doorkeeper/authorizations/new.html.erb index 232ce3d..53f003e 100644 --- a/app/views/doorkeeper/authorizations/new.html.erb +++ b/app/views/doorkeeper/authorizations/new.html.erb @@ -16,26 +16,36 @@ <% scopes = @pre_auth.scopes.map(&:to_s) not_set = t('.data.not_set') - - consent_fields = OAuthScope.consent_fields_for(scopes, current_identity) - data_items = consent_fields.map { |f| { label: t(".data.#{f[:key]}"), value: f[:value] || not_set } } + scope_data = OAuthScope.consent_data_by_scope(scopes, current_identity) unknown_scopes = scopes.reject { |s| OAuthScope.known?(s) } %> - <% if data_items.any? || unknown_scopes.any? %> + <% if scope_data.any? || unknown_scopes.any? %>
<%= t('.able_to') %> diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index d1350c3..a3af293 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -68,10 +68,6 @@ en: title: 'An error has occurred' new: title: 'Grant access to your information' - prompt: "You're signing in to %{client_name} with your Hack Club account." - able_to: 'This application will be able to see:' - authorize: 'Authorize' - deny: 'Deny' data: email: 'Email' name: 'Name' @@ -84,13 +80,6 @@ en: address: 'Mailing address' rejection_reason: 'Rejection reason' not_set: 'Not set' - banners: - community_untrusted: - title: 'Community Application' - message_html: 'This application was created by a community member and has not been reviewed by Hack Club.' - community_trusted: - title: 'Trusted Community Application' - message_html: 'This application was created by a community member and has been reviewed by Hack Club.' show: title: 'Authorization code' form_post: diff --git a/config/locales/en.yml b/config/locales/en.yml index 28687cf..10841ea 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -222,7 +222,7 @@ en: authorizations: new: prompt: Authorize %{client_name} to use your account? - able_to: This application will be able to + able_to: "This application will be able to:" deny: Deny authorize: Authorize → banners: @@ -235,26 +235,26 @@ en: scopes: basic_info: title: See basic information about you - email_label: "Email:" - name_label: "Name:" - verification_status_label: "Verification status:" email: title: See your email address - email_label: "Email:" name: title: See your name - name_label: "Name:" slack_id: title: See your Slack ID - slack_id_label: "Slack ID:" - not_set: Not set verification_status: title: See your verification status - verification_status_label: "Verification status:" - ysws_eligible_label: "YSWS eligible:" profile: title: See your profile information - name_label: "Name:" + phone: + title: See your phone number + birthdate: + title: See your date of birth + address: + title: See your mailing address + legal_name: + title: See your legal name + set_slack_id: + title: Link Slack accounts to identities authorized_applications: none: You haven't authorized any apps yet. created_at: Authorized