From 50ea54b3ed125477656893a67d9f552bb53e8ba5 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 1 Mar 2022 16:48:58 +0100 Subject: Change authorized applications page (#17656) * Change authorized applications page * Hide revoke button for superapps and suspended accounts * Clean up db/schema.rb --- app/controllers/api/base_controller.rb | 1 + .../concerns/access_token_tracking_concern.rb | 21 +++++++ .../concerns/session_tracking_concern.rb | 4 +- app/controllers/concerns/user_tracking_concern.rb | 4 +- app/helpers/application_helper.rb | 15 +++++ app/javascript/styles/mastodon/admin.scss | 10 +++ app/javascript/styles/mastodon/containers.scss | 13 ++-- app/javascript/styles/mastodon/forms.scss | 71 +++++++++++++++++++++- app/lib/access_token_extension.rb | 4 ++ app/lib/application_extension.rb | 4 ++ app/lib/scope_parser.rb | 10 +++ app/lib/scope_transformer.rb | 40 ++++++++++++ app/views/layouts/modal.html.haml | 3 +- app/views/oauth/authorizations/new.html.haml | 50 +++++++++------ .../oauth/authorized_applications/index.html.haml | 62 ++++++++++++------- app/workers/scheduler/ip_cleanup_scheduler.rb | 1 + 16 files changed, 258 insertions(+), 55 deletions(-) create mode 100644 app/controllers/concerns/access_token_tracking_concern.rb create mode 100644 app/lib/scope_parser.rb create mode 100644 app/lib/scope_transformer.rb (limited to 'app') diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index b863d8643..72c30dec7 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -5,6 +5,7 @@ class Api::BaseController < ApplicationController DEFAULT_ACCOUNTS_LIMIT = 40 include RateLimitHeaders + include AccessTokenTrackingConcern skip_before_action :store_current_location skip_before_action :require_functional!, unless: :whitelist_mode? diff --git a/app/controllers/concerns/access_token_tracking_concern.rb b/app/controllers/concerns/access_token_tracking_concern.rb new file mode 100644 index 000000000..cf60cfb99 --- /dev/null +++ b/app/controllers/concerns/access_token_tracking_concern.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module AccessTokenTrackingConcern + extend ActiveSupport::Concern + + ACCESS_TOKEN_UPDATE_FREQUENCY = 24.hours.freeze + + included do + before_action :update_access_token_last_used + end + + private + + def update_access_token_last_used + doorkeeper_token.update_last_used(request) if access_token_needs_update? + end + + def access_token_needs_update? + doorkeeper_token.present? && (doorkeeper_token.last_used_at.nil? || doorkeeper_token.last_used_at < ACCESS_TOKEN_UPDATE_FREQUENCY.ago) + end +end diff --git a/app/controllers/concerns/session_tracking_concern.rb b/app/controllers/concerns/session_tracking_concern.rb index 45361b019..eaaa4ac59 100644 --- a/app/controllers/concerns/session_tracking_concern.rb +++ b/app/controllers/concerns/session_tracking_concern.rb @@ -3,7 +3,7 @@ module SessionTrackingConcern extend ActiveSupport::Concern - UPDATE_SIGN_IN_HOURS = 24 + SESSION_UPDATE_FREQUENCY = 24.hours.freeze included do before_action :set_session_activity @@ -17,6 +17,6 @@ module SessionTrackingConcern end def session_needs_update? - !current_session.nil? && current_session.updated_at < UPDATE_SIGN_IN_HOURS.hours.ago + !current_session.nil? && current_session.updated_at < SESSION_UPDATE_FREQUENCY.ago end end diff --git a/app/controllers/concerns/user_tracking_concern.rb b/app/controllers/concerns/user_tracking_concern.rb index 45f3aab0d..e960cce53 100644 --- a/app/controllers/concerns/user_tracking_concern.rb +++ b/app/controllers/concerns/user_tracking_concern.rb @@ -3,7 +3,7 @@ module UserTrackingConcern extend ActiveSupport::Concern - UPDATE_SIGN_IN_FREQUENCY = 24.hours.freeze + SIGN_IN_UPDATE_FREQUENCY = 24.hours.freeze included do before_action :update_user_sign_in @@ -16,6 +16,6 @@ module UserTrackingConcern end def user_needs_sign_in_update? - user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < UPDATE_SIGN_IN_FREQUENCY.ago) + user_signed_in? && (current_user.current_sign_in_at.nil? || current_user.current_sign_in_at < SIGN_IN_UPDATE_FREQUENCY.ago) end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 36c66b7d1..c5d9bbc19 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -224,4 +224,19 @@ module ApplicationHelper content_tag(:script, json_escape(json).html_safe, id: 'initial-state', type: 'application/json') # rubocop:enable Rails/OutputSafety end + + def grouped_scopes(scopes) + scope_parser = ScopeParser.new + scope_transformer = ScopeTransformer.new + + scopes.each_with_object({}) do |str, h| + scope = scope_transformer.apply(scope_parser.parse(str)) + + if h[scope.key] + h[scope.key].merge!(scope) + else + h[scope.key] = scope + end + end.values + end end diff --git a/app/javascript/styles/mastodon/admin.scss b/app/javascript/styles/mastodon/admin.scss index 2e212eca5..f49a354dc 100644 --- a/app/javascript/styles/mastodon/admin.scss +++ b/app/javascript/styles/mastodon/admin.scss @@ -907,6 +907,12 @@ a.name-tag, text-decoration: none; margin-bottom: 10px; + .account-role { + vertical-align: middle; + } + } + + a.announcements-list__item__title { &:hover, &:focus, &:active { @@ -925,6 +931,10 @@ a.name-tag, align-items: center; } + &__permissions { + margin-top: 10px; + } + &:last-child { border-bottom: 0; } diff --git a/app/javascript/styles/mastodon/containers.scss b/app/javascript/styles/mastodon/containers.scss index e40ad18ff..a180df437 100644 --- a/app/javascript/styles/mastodon/containers.scss +++ b/app/javascript/styles/mastodon/containers.scss @@ -1,7 +1,6 @@ .container-alt { width: 700px; margin: 0 auto; - margin-top: 40px; @media screen and (max-width: 740px) { width: 100%; @@ -67,22 +66,20 @@ line-height: 18px; box-sizing: border-box; padding: 20px 0; - padding-bottom: 0; - margin-bottom: -30px; margin-top: 40px; + margin-bottom: 10px; + border-bottom: 1px solid $ui-base-color; @media screen and (max-width: 440px) { width: 100%; margin: 0; - margin-bottom: 10px; padding: 20px; - padding-bottom: 0; } .avatar { width: 40px; height: 40px; - margin-right: 8px; + margin-right: 10px; img { width: 100%; @@ -96,7 +93,7 @@ .name { flex: 1 1 auto; color: $secondary-text-color; - width: calc(100% - 88px); + width: calc(100% - 90px); .username { display: block; @@ -110,7 +107,7 @@ display: block; font-size: 32px; line-height: 40px; - margin-left: 8px; + margin-left: 10px; } } diff --git a/app/javascript/styles/mastodon/forms.scss b/app/javascript/styles/mastodon/forms.scss index 65f53471d..6e02e2332 100644 --- a/app/javascript/styles/mastodon/forms.scss +++ b/app/javascript/styles/mastodon/forms.scss @@ -800,9 +800,41 @@ code { } } } +} - @media screen and (max-width: 740px) and (min-width: 441px) { - margin-top: 40px; +.oauth-prompt { + h3 { + color: $ui-secondary-color; + font-size: 17px; + line-height: 22px; + font-weight: 500; + margin-bottom: 30px; + } + + p { + font-size: 14px; + line-height: 18px; + margin-bottom: 30px; + } + + .permissions-list { + border: 1px solid $ui-base-color; + border-radius: 4px; + background: darken($ui-base-color, 4%); + margin-bottom: 30px; + } + + .actions { + margin: 0 -10px; + display: flex; + + form { + box-sizing: border-box; + padding: 0 10px; + flex: 1 1 auto; + min-height: 1px; + width: 50%; + } } } @@ -1005,3 +1037,38 @@ code { display: none; } } + +.permissions-list { + &__item { + padding: 15px; + color: $ui-secondary-color; + border-bottom: 1px solid lighten($ui-base-color, 4%); + display: flex; + align-items: center; + + &__text { + flex: 1 1 auto; + + &__title { + font-weight: 500; + } + + &__type { + color: $darker-text-color; + } + } + + &__icon { + flex: 0 0 auto; + font-size: 18px; + width: 30px; + color: $valid-value-color; + display: flex; + align-items: center; + } + + &:last-child { + border-bottom: 0; + } + } +} diff --git a/app/lib/access_token_extension.rb b/app/lib/access_token_extension.rb index 3e184e775..2cafaaa20 100644 --- a/app/lib/access_token_extension.rb +++ b/app/lib/access_token_extension.rb @@ -11,6 +11,10 @@ module AccessTokenExtension update(revoked_at: clock.now.utc) end + def update_last_used(request, clock = Time) + update(last_used_at: clock.now.utc, last_used_ip: request.remote_ip) + end + def push_to_streaming_api Redis.current.publish("timeline:access_token:#{id}", Oj.dump(event: :kill)) if revoked? || destroyed? end diff --git a/app/lib/application_extension.rb b/app/lib/application_extension.rb index e61cd0721..a1fea6430 100644 --- a/app/lib/application_extension.rb +++ b/app/lib/application_extension.rb @@ -8,4 +8,8 @@ module ApplicationExtension validates :website, url: true, length: { maximum: 2_000 }, if: :website? validates :redirect_uri, length: { maximum: 2_000 } end + + def most_recently_used_access_token + @most_recently_used_access_token ||= access_tokens.where.not(last_used_at: nil).order(last_used_at: :desc).first + end end diff --git a/app/lib/scope_parser.rb b/app/lib/scope_parser.rb new file mode 100644 index 000000000..d268688c8 --- /dev/null +++ b/app/lib/scope_parser.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class ScopeParser < Parslet::Parser + rule(:term) { match('[a-z]').repeat(1).as(:term) } + rule(:colon) { str(':') } + rule(:access) { (str('write') | str('read')).as(:access) } + rule(:namespace) { str('admin').as(:namespace) } + rule(:scope) { ((namespace >> colon).maybe >> ((access >> colon >> term) | access | term)).as(:scope) } + root(:scope) +end diff --git a/app/lib/scope_transformer.rb b/app/lib/scope_transformer.rb new file mode 100644 index 000000000..fdfc6cf13 --- /dev/null +++ b/app/lib/scope_transformer.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +class ScopeTransformer < Parslet::Transform + class Scope + DEFAULT_TERM = 'all' + DEFAULT_ACCESS = %w(read write).freeze + + attr_reader :namespace, :term + + def initialize(scope) + @namespace = scope[:namespace]&.to_s + @access = scope[:access] ? [scope[:access].to_s] : DEFAULT_ACCESS.dup + @term = scope[:term]&.to_s || DEFAULT_TERM + end + + def key + @key ||= [@namespace, @term].compact.join('/') + end + + def access + @access.join('/') + end + + def merge(other_scope) + clone.merge!(other_scope) + end + + def merge!(other_scope) + raise ArgumentError unless other_scope.namespace == namespace && other_scope.term == term + + @access.concat(other_scope.instance_variable_get('@access')) + @access.uniq! + @access.sort! + + self + end + end + + rule(scope: subtree(:scope)) { Scope.new(scope) } +end diff --git a/app/views/layouts/modal.html.haml b/app/views/layouts/modal.html.haml index a2cd1193f..c0ea211ff 100644 --- a/app/views/layouts/modal.html.haml +++ b/app/views/layouts/modal.html.haml @@ -12,8 +12,9 @@ = fa_icon 'sign-out' .container-alt= yield + .modal-layout__mastodon %div - %img{alt:'', draggable:'false', src:"#{mascot_url}"} + %img{alt: '', draggable: 'false', src: mascot_url } = render template: 'layouts/application' diff --git a/app/views/oauth/authorizations/new.html.haml b/app/views/oauth/authorizations/new.html.haml index 05ff9582e..50f671b26 100644 --- a/app/views/oauth/authorizations/new.html.haml +++ b/app/views/oauth/authorizations/new.html.haml @@ -1,26 +1,38 @@ - content_for :page_title do = t('doorkeeper.authorizations.new.title') -.form-container +.form-container.simple_form .oauth-prompt - %h2= t('doorkeeper.authorizations.new.prompt', client_name: @pre_auth.client.name) + %h3= t('doorkeeper.authorizations.new.title') - %p - = t('doorkeeper.authorizations.new.able_to') - != @pre_auth.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.map { |s| "#{s}" }.to_sentence + %p= t('doorkeeper.authorizations.new.prompt_html', client_name: content_tag(:strong, @pre_auth.client.name)) - = form_tag oauth_authorization_path, method: :post, class: 'simple_form' do - = hidden_field_tag :client_id, @pre_auth.client.uid - = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri - = hidden_field_tag :state, @pre_auth.state - = hidden_field_tag :response_type, @pre_auth.response_type - = hidden_field_tag :scope, @pre_auth.scope - = button_tag t('doorkeeper.authorizations.buttons.authorize'), type: :submit + %h3= t('doorkeeper.authorizations.new.review_permissions') - = form_tag oauth_authorization_path, method: :delete, class: 'simple_form' do - = hidden_field_tag :client_id, @pre_auth.client.uid - = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri - = hidden_field_tag :state, @pre_auth.state - = hidden_field_tag :response_type, @pre_auth.response_type - = hidden_field_tag :scope, @pre_auth.scope - = button_tag t('doorkeeper.authorizations.buttons.deny'), type: :submit, class: 'negative' + %ul.permissions-list + - grouped_scopes(@pre_auth.scopes).each do |scope| + %li.permissions-list__item + .permissions-list__item__icon + = fa_icon('check') + .permissions-list__item__text + .permissions-list__item__text__title + = t(scope.key, scope: [:doorkeeper, :grouped_scopes, :title]) + .permissions-list__item__text__type + = t(scope.access, scope: [:doorkeeper, :grouped_scopes, :access]) + + .actions + = form_tag oauth_authorization_path, method: :post do + = hidden_field_tag :client_id, @pre_auth.client.uid + = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri + = hidden_field_tag :state, @pre_auth.state + = hidden_field_tag :response_type, @pre_auth.response_type + = hidden_field_tag :scope, @pre_auth.scope + = button_tag t('doorkeeper.authorizations.buttons.authorize'), type: :submit + + = form_tag oauth_authorization_path, method: :delete do + = hidden_field_tag :client_id, @pre_auth.client.uid + = hidden_field_tag :redirect_uri, @pre_auth.redirect_uri + = hidden_field_tag :state, @pre_auth.state + = hidden_field_tag :response_type, @pre_auth.response_type + = hidden_field_tag :scope, @pre_auth.scope + = button_tag t('doorkeeper.authorizations.buttons.deny'), type: :submit, class: 'negative' diff --git a/app/views/oauth/authorized_applications/index.html.haml b/app/views/oauth/authorized_applications/index.html.haml index fbb733db4..fead56f4a 100644 --- a/app/views/oauth/authorized_applications/index.html.haml +++ b/app/views/oauth/authorized_applications/index.html.haml @@ -1,24 +1,44 @@ - content_for :page_title do = t('doorkeeper.authorized_applications.index.title') -.table-wrapper - %table.table - %thead - %tr - %th= t('doorkeeper.authorized_applications.index.application') - %th= t('doorkeeper.authorized_applications.index.scopes') - %th= t('doorkeeper.authorized_applications.index.created_at') - %th - %tbody - - @applications.each do |application| - %tr - %td - - if application.website.blank? - = application.name - - else - = link_to application.name, application.website, target: '_blank', rel: 'noopener noreferrer' - %th!= application.scopes.map { |scope| t(scope, scope: [:doorkeeper, :scopes]) }.join(', ') - %td= l application.created_at - %td - - unless application.superapp? || current_account.suspended? - = table_link_to 'times', t('doorkeeper.authorized_applications.buttons.revoke'), oauth_authorized_application_path(application), method: :delete, data: { confirm: t('doorkeeper.authorized_applications.confirmations.revoke') } +%p= t('doorkeeper.authorized_applications.index.description_html') + +%hr.spacer/ + +.announcements-list + - @applications.each do |application| + .announcements-list__item + - if application.website.present? + = link_to application.name, application.website, target: '_blank', rel: 'noopener noreferrer', class: 'announcements-list__item__title' + - else + %strong.announcements-list__item__title + = application.name + - if application.superapp? + %span.account-role.moderator= t('doorkeeper.authorized_applications.index.superapp') + + .announcements-list__item__action-bar + .announcements-list__item__meta + - if application.most_recently_used_access_token + = t('doorkeeper.authorized_applications.index.last_used_at', date: l(application.most_recently_used_access_token.last_used_at.to_date)) + - else + = t('doorkeeper.authorized_applications.index.never_used') + + • + + = t('doorkeeper.authorized_applications.index.authorized_at', date: l(application.created_at.to_date)) + + - unless application.superapp? || current_account.suspended? + %div + = table_link_to 'times', t('doorkeeper.authorized_applications.buttons.revoke'), oauth_authorized_application_path(application), method: :delete, data: { confirm: t('doorkeeper.authorized_applications.confirmations.revoke') } + + .announcements-list__item__permissions + %ul.permissions-list + - grouped_scopes(application.scopes).each do |scope| + %li.permissions-list__item + .permissions-list__item__icon + = fa_icon('check') + .permissions-list__item__text + .permissions-list__item__text__title + = t(scope.key, scope: [:doorkeeper, :grouped_scopes, :title]) + .permissions-list__item__text__type + = t(scope.access, scope: [:doorkeeper, :grouped_scopes, :access]) diff --git a/app/workers/scheduler/ip_cleanup_scheduler.rb b/app/workers/scheduler/ip_cleanup_scheduler.rb index adc99c605..7afad2f58 100644 --- a/app/workers/scheduler/ip_cleanup_scheduler.rb +++ b/app/workers/scheduler/ip_cleanup_scheduler.rb @@ -18,6 +18,7 @@ class Scheduler::IpCleanupScheduler SessionActivation.where('updated_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all User.where('current_sign_in_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(sign_up_ip: nil) LoginActivity.where('created_at < ?', IP_RETENTION_PERIOD.ago).in_batches.destroy_all + Doorkeeper::AccessToken.where('last_used_at < ?', IP_RETENTION_PERIOD.ago).in_batches.update_all(last_used_ip: nil) end def clean_expired_ip_blocks! -- cgit