From e41dadcb33fda44ee274daa673bd933e13aa90eb Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Fri, 19 Dec 2014 16:15:29 +0200 Subject: [PATCH 1/6] Doorkeeper integration --- Gemfile | 2 + Gemfile.lock | 12 ++ .../oauth/applications_controller.rb | 25 +++ .../oauth/authorizations_controller.rb | 57 ++++++ .../authorized_applications_controller.rb | 8 + .../profiles/accounts_controller.rb | 2 + app/models/user.rb | 1 + .../oauth2/access_token_validation_service.rb | 41 ++++ .../applications/_delete_form.html.haml | 4 + .../doorkeeper/applications/_form.html.haml | 25 +++ .../doorkeeper/applications/edit.html.haml | 2 + .../doorkeeper/applications/index.html.haml | 16 ++ .../doorkeeper/applications/new.html.haml | 2 + .../doorkeeper/applications/show.html.haml | 21 +++ .../doorkeeper/authorizations/error.html.haml | 3 + .../doorkeeper/authorizations/new.html.haml | 28 +++ .../doorkeeper/authorizations/show.html.haml | 3 + .../_delete_form.html.haml | 4 + .../authorized_applications/index.html.haml | 16 ++ app/views/layouts/doorkeeper/admin.html.erb | 34 ++++ .../layouts/doorkeeper/application.html.erb | 23 +++ app/views/layouts/nav/_profile.html.haml | 2 +- app/views/profiles/accounts/show.html.haml | 35 ++++ config/initializers/doorkeeper.rb | 91 +++++++++ config/locales/doorkeeper.en.yml | 73 ++++++++ config/routes.rb | 5 + ...20141216155758_create_doorkeeper_tables.rb | 42 +++++ ...20141217125223_add_owner_to_application.rb | 7 + db/schema.rb | 45 ++++- features/profile/profile.feature | 14 ++ features/steps/profile/profile.rb | 50 +++++ lib/api/api.rb | 1 + lib/api/api_guard.rb | 175 ++++++++++++++++++ lib/api/helpers.rb | 2 +- spec/requests/api/api_helpers_spec.rb | 1 + spec/requests/api/doorkeeper_access_spec.rb | 31 ++++ 36 files changed, 900 insertions(+), 3 deletions(-) create mode 100644 app/controllers/oauth/applications_controller.rb create mode 100644 app/controllers/oauth/authorizations_controller.rb create mode 100644 app/controllers/oauth/authorized_applications_controller.rb create mode 100644 app/services/oauth2/access_token_validation_service.rb create mode 100644 app/views/doorkeeper/applications/_delete_form.html.haml create mode 100644 app/views/doorkeeper/applications/_form.html.haml create mode 100644 app/views/doorkeeper/applications/edit.html.haml create mode 100644 app/views/doorkeeper/applications/index.html.haml create mode 100644 app/views/doorkeeper/applications/new.html.haml create mode 100644 app/views/doorkeeper/applications/show.html.haml create mode 100644 app/views/doorkeeper/authorizations/error.html.haml create mode 100644 app/views/doorkeeper/authorizations/new.html.haml create mode 100644 app/views/doorkeeper/authorizations/show.html.haml create mode 100644 app/views/doorkeeper/authorized_applications/_delete_form.html.haml create mode 100644 app/views/doorkeeper/authorized_applications/index.html.haml create mode 100644 app/views/layouts/doorkeeper/admin.html.erb create mode 100644 app/views/layouts/doorkeeper/application.html.erb create mode 100644 config/initializers/doorkeeper.rb create mode 100644 config/locales/doorkeeper.en.yml create mode 100644 db/migrate/20141216155758_create_doorkeeper_tables.rb create mode 100644 db/migrate/20141217125223_add_owner_to_application.rb create mode 100644 lib/api/api_guard.rb create mode 100644 spec/requests/api/doorkeeper_access_spec.rb diff --git a/Gemfile b/Gemfile index ce9b83308f..85e7bba444 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,8 @@ gem 'omniauth-twitter' gem 'omniauth-github' gem 'omniauth-shibboleth' gem 'omniauth-kerberos' +gem 'doorkeeper', '2.0.1' +gem "rack-oauth2", "~> 1.0.5" # Extracting information from a git repository # Provide access to Gitlab::Git library diff --git a/Gemfile.lock b/Gemfile.lock index cf96677f87..0d089305fe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -37,6 +37,7 @@ GEM rake (>= 0.8.7) arel (5.0.1.20140414130214) asciidoctor (0.1.4) + attr_required (1.0.0) awesome_print (1.2.0) axiom-types (0.0.5) descendants_tracker (~> 0.0.1) @@ -107,6 +108,8 @@ GEM diff-lcs (1.2.5) diffy (3.0.3) docile (1.1.5) + doorkeeper (2.0.1) + railties (>= 3.1) dotenv (0.9.0) dropzonejs-rails (0.4.14) rails (> 3.1) @@ -250,6 +253,7 @@ GEM json (~> 1.8) multi_xml (>= 0.5.2) httpauth (0.2.1) + httpclient (2.5.3.3) i18n (0.6.11) ice_nine (0.10.0) jasmine (2.0.2) @@ -368,6 +372,12 @@ GEM rack (>= 1.1.3) rack-mount (0.8.3) rack (>= 1.0.0) + rack-oauth2 (1.0.8) + activesupport (>= 2.3) + attr_required (>= 0.0.5) + httpclient (>= 2.2.0.2) + multi_json (>= 1.3.6) + rack (>= 1.1) rack-protection (1.5.1) rack rack-test (0.6.2) @@ -616,6 +626,7 @@ DEPENDENCIES devise (= 3.2.4) devise-async (= 0.9.0) diffy (~> 3.0.3) + doorkeeper (= 2.0.1) dropzonejs-rails email_spec enumerize @@ -672,6 +683,7 @@ DEPENDENCIES rack-attack rack-cors rack-mini-profiler + rack-oauth2 (~> 1.0.5) rails (~> 4.1.0) rails_autolink (~> 1.1) rails_best_practices diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb new file mode 100644 index 0000000000..8eafe5e3b3 --- /dev/null +++ b/app/controllers/oauth/applications_controller.rb @@ -0,0 +1,25 @@ +class Oauth::ApplicationsController < Doorkeeper::ApplicationsController + before_filter :authenticate_user! + layout "profile" + + def index + @applications = current_user.oauth_applications + end + + def create + @application = Doorkeeper::Application.new(application_params) + @application.owner = current_user if Doorkeeper.configuration.confirm_application_owner? + if @application.save + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) + redirect_to oauth_application_url(@application) + else + render :new + end + end + + def destroy + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) if @application.destroy + redirect_to profile_account_url + end + +end \ No newline at end of file diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb new file mode 100644 index 0000000000..c46707e2c7 --- /dev/null +++ b/app/controllers/oauth/authorizations_controller.rb @@ -0,0 +1,57 @@ +class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController + before_filter :authenticate_resource_owner! + layout "profile" + + def new + if pre_auth.authorizable? + if skip_authorization? || matching_token? + auth = authorization.authorize + redirect_to auth.redirect_uri + else + render "doorkeeper/authorizations/new" + end + else + render "doorkeeper/authorizations/error" + end + end + + # TODO: Handle raise invalid authorization + def create + redirect_or_render authorization.authorize + end + + def destroy + redirect_or_render authorization.deny + end + + private + + def matching_token? + Doorkeeper::AccessToken.matching_token_for pre_auth.client, + current_resource_owner.id, + pre_auth.scopes + end + + def redirect_or_render(auth) + if auth.redirectable? + redirect_to auth.redirect_uri + else + render json: auth.body, status: auth.status + end + end + + def pre_auth + @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, + server.client_via_uid, + params) + end + + def authorization + @authorization ||= strategy.request + end + + def strategy + @strategy ||= server.authorization_request pre_auth.response_type + end +end + diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb new file mode 100644 index 0000000000..b6d4a99c0a --- /dev/null +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -0,0 +1,8 @@ +class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicationsController + layout "profile" + + def destroy + Doorkeeper::AccessToken.revoke_all_for params[:id], current_resource_owner + redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) + end +end \ No newline at end of file diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index fe121691a1..5f15378c83 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -3,5 +3,7 @@ class Profiles::AccountsController < ApplicationController def show @user = current_user + @applications = current_user.oauth_applications + @authorized_applications = Doorkeeper::Application.authorized_for(current_user) end end diff --git a/app/models/user.rb b/app/models/user.rb index 7faeef1b5b..6518fc50b7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -106,6 +106,7 @@ class User < ActiveRecord::Base has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id, class_name: "Event" has_many :assigned_issues, dependent: :destroy, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :destroy, foreign_key: :assignee_id, class_name: "MergeRequest" + has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy # diff --git a/app/services/oauth2/access_token_validation_service.rb b/app/services/oauth2/access_token_validation_service.rb new file mode 100644 index 0000000000..9528348975 --- /dev/null +++ b/app/services/oauth2/access_token_validation_service.rb @@ -0,0 +1,41 @@ +module Oauth2::AccessTokenValidationService + # Results: + VALID = :valid + EXPIRED = :expired + REVOKED = :revoked + INSUFFICIENT_SCOPE = :insufficient_scope + + class << self + def validate(token, scopes: []) + if token.expired? + return EXPIRED + + elsif token.revoked? + return REVOKED + + elsif !self.sufficent_scope?(token, scopes) + return INSUFFICIENT_SCOPE + + else + return VALID + end + end + + protected + # True if the token's scope is a superset of required scopes, + # or the required scopes is empty. + def sufficent_scope?(token, scopes) + if scopes.blank? + # if no any scopes required, the scopes of token is sufficient. + return true + else + # If there are scopes required, then check whether + # the set of authorized scopes is a superset of the set of required scopes + required_scopes = Set.new(scopes) + authorized_scopes = Set.new(token.scopes) + + return authorized_scopes >= required_scopes + end + end + end +end \ No newline at end of file diff --git a/app/views/doorkeeper/applications/_delete_form.html.haml b/app/views/doorkeeper/applications/_delete_form.html.haml new file mode 100644 index 0000000000..bf8098f38d --- /dev/null +++ b/app/views/doorkeeper/applications/_delete_form.html.haml @@ -0,0 +1,4 @@ +- submit_btn_css ||= 'btn btn-link btn-remove btn-small' += form_tag oauth_application_path(application) do + %input{:name => "_method", :type => "hidden", :value => "delete"}/ + = submit_tag 'Destroy', onclick: "return confirm('Are you sure?')", class: submit_btn_css \ No newline at end of file diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml new file mode 100644 index 0000000000..45ddf16ad0 --- /dev/null +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -0,0 +1,25 @@ += form_for application, url: doorkeeper_submit_path(application), html: {class: 'form-horizontal', role: 'form'} do |f| + - if application.errors.any? + .alert.alert-danger{"data-alert" => ""} + %p Whoops! Check your form for possible errors + = content_tag :div, class: "form-group#{' has-error' if application.errors[:name].present?}" do + = f.label :name, class: 'col-sm-2 control-label' + .col-sm-10 + = f.text_field :name, class: 'form-control' + = doorkeeper_errors_for application, :name + = content_tag :div, class: "form-group#{' has-error' if application.errors[:redirect_uri].present?}" do + = f.label :redirect_uri, class: 'col-sm-2 control-label' + .col-sm-10 + = f.text_area :redirect_uri, class: 'form-control' + = doorkeeper_errors_for application, :redirect_uri + %span.help-block + Use one line per URI + - if Doorkeeper.configuration.native_redirect_uri + %span.help-block + Use + %code= Doorkeeper.configuration.native_redirect_uri + for local tests + .form-group + .col-sm-offset-2.col-sm-10 + = f.submit 'Submit', class: "btn btn-primary wide" + = link_to "Cancel", profile_account_path, :class => "btn btn-default" \ No newline at end of file diff --git a/app/views/doorkeeper/applications/edit.html.haml b/app/views/doorkeeper/applications/edit.html.haml new file mode 100644 index 0000000000..61584eb9c4 --- /dev/null +++ b/app/views/doorkeeper/applications/edit.html.haml @@ -0,0 +1,2 @@ +%h3.page-title Edit application += render 'form', application: @application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/index.html.haml b/app/views/doorkeeper/applications/index.html.haml new file mode 100644 index 0000000000..e5be4b4bca --- /dev/null +++ b/app/views/doorkeeper/applications/index.html.haml @@ -0,0 +1,16 @@ +%h3.page-title Your applications +%p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' +%table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.redirect_uri + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link' + %td= render 'delete_form', application: application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/new.html.haml b/app/views/doorkeeper/applications/new.html.haml new file mode 100644 index 0000000000..655845e4af --- /dev/null +++ b/app/views/doorkeeper/applications/new.html.haml @@ -0,0 +1,2 @@ +%h3.page-title New application += render 'form', application: @application \ No newline at end of file diff --git a/app/views/doorkeeper/applications/show.html.haml b/app/views/doorkeeper/applications/show.html.haml new file mode 100644 index 0000000000..5236b86589 --- /dev/null +++ b/app/views/doorkeeper/applications/show.html.haml @@ -0,0 +1,21 @@ +%h3.page-title + Application: #{@application.name} +.row + .col-md-8 + %h4 Application Id: + %p + %code#application_id= @application.uid + %h4 Secret: + %p + %code#secret= @application.secret + %h4 Callback urls: + %table + - @application.redirect_uri.split.each do |uri| + %tr + %td + %code= uri + %td + = link_to 'Authorize', oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: 'code'), class: 'btn btn-success', target: '_blank' +.prepend-top-20 + %p= link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' + %p= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/error.html.haml b/app/views/doorkeeper/authorizations/error.html.haml new file mode 100644 index 0000000000..7561ec85ed --- /dev/null +++ b/app/views/doorkeeper/authorizations/error.html.haml @@ -0,0 +1,3 @@ +%h3.page-title An error has occurred +%main{:role => "main"} + %pre= @pre_auth.error_response.body[:error_description] \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/new.html.haml b/app/views/doorkeeper/authorizations/new.html.haml new file mode 100644 index 0000000000..15f9ee266c --- /dev/null +++ b/app/views/doorkeeper/authorizations/new.html.haml @@ -0,0 +1,28 @@ +%h3.page-title Authorize required +%main{:role => "main"} + %p.h4 + Authorize + %strong.text-info= @pre_auth.client.name + to use your account? + - if @pre_auth.scopes + #oauth-permissions + %p This application will be able to: + %ul.text-info + - @pre_auth.scopes.each do |scope| + %li= t scope, scope: [:doorkeeper, :scopes] + %hr/ + .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 + = submit_tag "Authorize", class: "btn btn-success wide pull-left" + = 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 + = submit_tag "Deny", class: "btn btn-danger prepend-left-10" \ No newline at end of file diff --git a/app/views/doorkeeper/authorizations/show.html.haml b/app/views/doorkeeper/authorizations/show.html.haml new file mode 100644 index 0000000000..9a40200719 --- /dev/null +++ b/app/views/doorkeeper/authorizations/show.html.haml @@ -0,0 +1,3 @@ +%h3.page-title Authorization code: +%main{:role => "main"} + %code#authorization_code= params[:code] \ No newline at end of file diff --git a/app/views/doorkeeper/authorized_applications/_delete_form.html.haml b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml new file mode 100644 index 0000000000..5cbb4a70c1 --- /dev/null +++ b/app/views/doorkeeper/authorized_applications/_delete_form.html.haml @@ -0,0 +1,4 @@ +- submit_btn_css ||= 'btn btn-link btn-remove' += form_tag oauth_authorized_application_path(application) do + %input{:name => "_method", :type => "hidden", :value => "delete"}/ + = submit_tag 'Revoke', onclick: "return confirm('Are you sure?')", class: 'btn btn-link btn-remove btn-small' \ No newline at end of file diff --git a/app/views/doorkeeper/authorized_applications/index.html.haml b/app/views/doorkeeper/authorized_applications/index.html.haml new file mode 100644 index 0000000000..814cdc987e --- /dev/null +++ b/app/views/doorkeeper/authorized_applications/index.html.haml @@ -0,0 +1,16 @@ +%header.page-header + %h1 Your authorized applications +%main{:role => "main"} + %table.table.table-striped + %thead + %tr + %th Application + %th Created At + %th + %th + %tbody + - @applications.each do |application| + %tr + %td= application.name + %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') + %td= render 'delete_form', application: application \ No newline at end of file diff --git a/app/views/layouts/doorkeeper/admin.html.erb b/app/views/layouts/doorkeeper/admin.html.erb new file mode 100644 index 0000000000..baeb5eb63f --- /dev/null +++ b/app/views/layouts/doorkeeper/admin.html.erb @@ -0,0 +1,34 @@ + + + + + + + Doorkeeper + <%= stylesheet_link_tag "doorkeeper/admin/application" %> + <%= csrf_meta_tags %> + + + +
+ <%- if flash[:notice].present? %> +
+ <%= flash[:notice] %> +
+ <% end -%> + + <%= yield %> +
+ + diff --git a/app/views/layouts/doorkeeper/application.html.erb b/app/views/layouts/doorkeeper/application.html.erb new file mode 100644 index 0000000000..fd7a31584f --- /dev/null +++ b/app/views/layouts/doorkeeper/application.html.erb @@ -0,0 +1,23 @@ + + + + OAuth authorize required + + + + + <%= stylesheet_link_tag "doorkeeper/application" %> + <%= csrf_meta_tags %> + + +
+ <%- if flash[:notice].present? %> +
+ <%= flash[:notice] %> +
+ <% end -%> + + <%= yield %> +
+ + diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 05ba20e361..f68fe87a75 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -3,7 +3,7 @@ = link_to profile_path, title: "Profile" do %i.fa.fa-user Profile - = nav_link(controller: :accounts) do + = nav_link(controller: [:accounts, :applications]) do = link_to profile_account_path do %i.fa.fa-gear Account diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index a21dcff41c..1d0b6d7718 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -75,3 +75,38 @@ The following groups will be abandoned. You should transfer or remove them: %strong #{current_user.solo_owned_groups.map(&:name).join(', ')} = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" + + %h3.page-title + OAuth2 + %fieldset.oauth-applications + %legend Your applications + %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' + %table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.redirect_uri + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' + %td= render 'doorkeeper/applications/delete_form', application: application + + %fieldset.oauth-authorized-applications + %legend Your authorized applications + %table.table.table-striped + %thead + %tr + %th Name + %th Created At + %th + %tbody + - @authorized_applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') + %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb new file mode 100644 index 0000000000..b2db3a7ea7 --- /dev/null +++ b/config/initializers/doorkeeper.rb @@ -0,0 +1,91 @@ +Doorkeeper.configure do + # Change the ORM that doorkeeper will use. + # Currently supported options are :active_record, :mongoid2, :mongoid3, :mongo_mapper + orm :active_record + + # This block will be called to check whether the resource owner is authenticated or not. + resource_owner_authenticator do + # Put your resource owner authentication logic here. + # Example implementation: + current_user || redirect_to(new_user_session_url) + end + + # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below. + # admin_authenticator do + # # Put your admin authentication logic here. + # # Example implementation: + # Admin.find_by_id(session[:admin_id]) || redirect_to(new_admin_session_url) + # end + + # Authorization Code expiration time (default 10 minutes). + # authorization_code_expires_in 10.minutes + + # Access token expiration time (default 2 hours). + # If you want to disable expiration, set this to nil. + # access_token_expires_in 2.hours + + # Reuse access token for the same resource owner within an application (disabled by default) + # Rationale: https://github.com/doorkeeper-gem/doorkeeper/issues/383 + # reuse_access_token + + # Issue access tokens with refresh token (disabled by default) + use_refresh_token + + # Provide support for an owner to be assigned to each registered application (disabled by default) + # Optional parameter :confirmation => true (default false) if you want to enforce ownership of + # a registered application + # Note: you must also run the rails g doorkeeper:application_owner generator to provide the necessary support + enable_application_owner :confirmation => true + + # Define access token scopes for your provider + # For more information go to + # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes + default_scopes :api + #optional_scopes :write, :update + + # Change the way client credentials are retrieved from the request object. + # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then + # falls back to the `:client_id` and `:client_secret` params from the `params` object. + # Check out the wiki for more information on customization + # client_credentials :from_basic, :from_params + + # Change the way access token is authenticated from the request object. + # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then + # falls back to the `:access_token` or `:bearer_token` params from the `params` object. + # Check out the wiki for more information on customization + access_token_methods :from_access_token_param, :from_bearer_authorization, :from_bearer_param + + # Change the native redirect uri for client apps + # When clients register with the following redirect uri, they won't be redirected to any server and the authorization code will be displayed within the provider + # The value can be any string. Use nil to disable this feature. When disabled, clients must provide a valid URL + # (Similar behaviour: https://developers.google.com/accounts/docs/OAuth2InstalledApp#choosingredirecturi) + # + native_redirect_uri nil#'urn:ietf:wg:oauth:2.0:oob' + + # Specify what grant flows are enabled in array of Strings. The valid + # strings and the flows they enable are: + # + # "authorization_code" => Authorization Code Grant Flow + # "implicit" => Implicit Grant Flow + # "password" => Resource Owner Password Credentials Grant Flow + # "client_credentials" => Client Credentials Grant Flow + # + # If not specified, Doorkeeper enables all the four grant flows. + # + # grant_flows %w(authorization_code implicit password client_credentials) + + # Under some circumstances you might want to have applications auto-approved, + # so that the user skips the authorization step. + # For example if dealing with trusted a application. + # skip_authorization do |resource_owner, client| + # client.superapp? or resource_owner.admin? + # end + + # WWW-Authenticate Realm (default "Doorkeeper"). + # realm "Doorkeeper" + + # Allow dynamic query parameters (disabled by default) + # Some applications require dynamic query parameters on their request_uri + # set to true if you want this to be allowed + # wildcard_redirect_uri false +end diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml new file mode 100644 index 0000000000..c5b6b75e7f --- /dev/null +++ b/config/locales/doorkeeper.en.yml @@ -0,0 +1,73 @@ +en: + activerecord: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + mongoid: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + mongo_mapper: + errors: + models: + application: + attributes: + redirect_uri: + fragment_present: 'cannot contain a fragment.' + invalid_uri: 'must be a valid URI.' + relative_uri: 'must be an absolute URI.' + doorkeeper: + errors: + messages: + # Common error messages + invalid_request: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' + invalid_redirect_uri: 'The redirect uri included is not valid.' + unauthorized_client: 'The client is not authorized to perform this request using this method.' + access_denied: 'The resource owner or authorization server denied the request.' + invalid_scope: 'The requested scope is invalid, unknown, or malformed.' + server_error: 'The authorization server encountered an unexpected condition which prevented it from fulfilling the request.' + temporarily_unavailable: 'The authorization server is currently unable to handle the request due to a temporary overloading or maintenance of the server.' + + #configuration error messages + credential_flow_not_configured: 'Resource Owner Password Credentials flow failed due to Doorkeeper.configure.resource_owner_from_credentials being unconfigured.' + resource_owner_authenticator_not_configured: 'Resource Owner find failed due to Doorkeeper.configure.resource_owner_authenticator being unconfiged.' + + # Access grant errors + unsupported_response_type: 'The authorization server does not support this response type.' + + # Access token errors + invalid_client: 'Client authentication failed due to unknown client, no client authentication included, or unsupported authentication method.' + invalid_grant: 'The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.' + unsupported_grant_type: 'The authorization grant type is not supported by the authorization server.' + + # Password Access token errors + invalid_resource_owner: 'The provided resource owner credentials are not valid, or resource owner cannot be found' + + invalid_token: + revoked: "The access token was revoked" + expired: "The access token expired" + unknown: "The access token is invalid" + scopes: + api: Access your API + + flash: + applications: + create: + notice: 'Application created.' + destroy: + notice: 'Application deleted.' + update: + notice: 'Application updated.' + authorized_applications: + destroy: + notice: 'Application revoked.' diff --git a/config/routes.rb b/config/routes.rb index b6c5bb5b90..4d3039ce11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,6 +2,11 @@ require 'sidekiq/web' require 'api/api' Gitlab::Application.routes.draw do + use_doorkeeper do + controllers :applications => 'oauth/applications', + :authorized_applications => 'oauth/authorized_applications', + :authorizations => 'oauth/authorizations' + end # # Search # diff --git a/db/migrate/20141216155758_create_doorkeeper_tables.rb b/db/migrate/20141216155758_create_doorkeeper_tables.rb new file mode 100644 index 0000000000..af5aa7d8b7 --- /dev/null +++ b/db/migrate/20141216155758_create_doorkeeper_tables.rb @@ -0,0 +1,42 @@ +class CreateDoorkeeperTables < ActiveRecord::Migration + def change + create_table :oauth_applications do |t| + t.string :name, null: false + t.string :uid, null: false + t.string :secret, null: false + t.text :redirect_uri, null: false + t.string :scopes, null: false, default: '' + t.timestamps + end + + add_index :oauth_applications, :uid, unique: true + + create_table :oauth_access_grants do |t| + t.integer :resource_owner_id, null: false + t.integer :application_id, null: false + t.string :token, null: false + t.integer :expires_in, null: false + t.text :redirect_uri, null: false + t.datetime :created_at, null: false + t.datetime :revoked_at + t.string :scopes + end + + add_index :oauth_access_grants, :token, unique: true + + create_table :oauth_access_tokens do |t| + t.integer :resource_owner_id + t.integer :application_id + t.string :token, null: false + t.string :refresh_token + t.integer :expires_in + t.datetime :revoked_at + t.datetime :created_at, null: false + t.string :scopes + end + + add_index :oauth_access_tokens, :token, unique: true + add_index :oauth_access_tokens, :resource_owner_id + add_index :oauth_access_tokens, :refresh_token, unique: true + end +end diff --git a/db/migrate/20141217125223_add_owner_to_application.rb b/db/migrate/20141217125223_add_owner_to_application.rb new file mode 100644 index 0000000000..7d5e6d07d0 --- /dev/null +++ b/db/migrate/20141217125223_add_owner_to_application.rb @@ -0,0 +1,7 @@ +class AddOwnerToApplication < ActiveRecord::Migration + def change + add_column :oauth_applications, :owner_id, :integer, null: true + add_column :oauth_applications, :owner_type, :string, null: true + add_index :oauth_applications, [:owner_id, :owner_type] + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index b8335c5841..73ddb14503 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20141205134006) do +ActiveRecord::Schema.define(version: 20141217125223) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -249,6 +249,49 @@ ActiveRecord::Schema.define(version: 20141205134006) do add_index "notes", ["project_id"], name: "index_notes_on_project_id", using: :btree add_index "notes", ["updated_at"], name: "index_notes_on_updated_at", using: :btree + create_table "oauth_access_grants", force: true do |t| + t.integer "resource_owner_id", null: false + t.integer "application_id", null: false + t.string "token", null: false + t.integer "expires_in", null: false + t.text "redirect_uri", null: false + t.datetime "created_at", null: false + t.datetime "revoked_at" + t.string "scopes" + end + + add_index "oauth_access_grants", ["token"], name: "index_oauth_access_grants_on_token", unique: true, using: :btree + + create_table "oauth_access_tokens", force: true do |t| + t.integer "resource_owner_id" + t.integer "application_id" + t.string "token", null: false + t.string "refresh_token" + t.integer "expires_in" + t.datetime "revoked_at" + t.datetime "created_at", null: false + t.string "scopes" + end + + add_index "oauth_access_tokens", ["refresh_token"], name: "index_oauth_access_tokens_on_refresh_token", unique: true, using: :btree + add_index "oauth_access_tokens", ["resource_owner_id"], name: "index_oauth_access_tokens_on_resource_owner_id", using: :btree + add_index "oauth_access_tokens", ["token"], name: "index_oauth_access_tokens_on_token", unique: true, using: :btree + + create_table "oauth_applications", force: true do |t| + t.string "name", null: false + t.string "uid", null: false + t.string "secret", null: false + t.text "redirect_uri", null: false + t.string "scopes", default: "", null: false + t.datetime "created_at" + t.datetime "updated_at" + t.integer "owner_id" + t.string "owner_type" + end + + add_index "oauth_applications", ["owner_id", "owner_type"], name: "index_oauth_applications_on_owner_id_and_owner_type", using: :btree + add_index "oauth_applications", ["uid"], name: "index_oauth_applications_on_uid", unique: true, using: :btree + create_table "projects", force: true do |t| t.string "name" t.string "path" diff --git a/features/profile/profile.feature b/features/profile/profile.feature index d7fa370fe2..88a7a3e726 100644 --- a/features/profile/profile.feature +++ b/features/profile/profile.feature @@ -71,6 +71,20 @@ Feature: Profile And I click on my profile picture Then I should see my user page + Scenario: I can manage application + Given I visit profile account page + Then I click on new application button + And I should see application form + Then I fill application form out and submit + And I see application + Then I click edit + And I see edit application form + Then I change name of application and submit + And I see that application was changed + Then I visit profile account page + And I click to remove application + Then I see that application is removed + @javascript Scenario: I change my application theme Given I visit profile design page diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 38aaadcd28..29fc7e68da 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -221,4 +221,54 @@ class Spinach::Features::Profile < Spinach::FeatureSteps step 'I should see groups I belong to' do page.should have_css('.profile-groups-avatars', visible: true) end + + step 'I click on new application button' do + click_on 'New Application' + end + + step 'I should see application form' do + page.should have_content "New application" + end + + step 'I fill application form out and submit' do + fill_in :doorkeeper_application_name, with: 'test' + fill_in :doorkeeper_application_redirect_uri, with: 'https://test.com' + click_on "Submit" + end + + step 'I see application' do + page.should have_content "Application: test" + page.should have_content "Application Id" + page.should have_content "Secret" + end + + step 'I click edit' do + click_on "Edit" + end + + step 'I see edit application form' do + page.should have_content "Edit application" + end + + step 'I change name of application and submit' do + page.should have_content "Edit application" + fill_in :doorkeeper_application_name, with: 'test_changed' + click_on "Submit" + end + + step 'I see that application was changed' do + page.should have_content "test_changed" + page.should have_content "Application Id" + page.should have_content "Secret" + end + + step 'I click to remove application' do + within '.oauth-applications' do + click_on "Destroy" + end + end + + step "I see that application is removed" do + page.find(".oauth-applications").should_not have_content "test_changed" + end end diff --git a/lib/api/api.rb b/lib/api/api.rb index d26667ba3f..cb46f477ff 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -2,6 +2,7 @@ Dir["#{Rails.root}/lib/api/*.rb"].each {|file| require file} module API class API < Grape::API + include APIGuard version 'v3', using: :path rescue_from ActiveRecord::RecordNotFound do diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb new file mode 100644 index 0000000000..2397551818 --- /dev/null +++ b/lib/api/api_guard.rb @@ -0,0 +1,175 @@ +# Guard API with OAuth 2.0 Access Token + +require 'rack/oauth2' + +module APIGuard + extend ActiveSupport::Concern + + included do |base| + # OAuth2 Resource Server Authentication + use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| + # The authenticator only fetches the raw token string + + # Must yield access token to store it in the env + request.access_token + end + + helpers HelperMethods + + install_error_responders(base) + end + + # Helper Methods for Grape Endpoint + module HelperMethods + # Invokes the doorkeeper guard. + # + # If token is presented and valid, then it sets @current_user. + # + # If the token does not have sufficient scopes to cover the requred scopes, + # then it raises InsufficientScopeError. + # + # If the token is expired, then it raises ExpiredError. + # + # If the token is revoked, then it raises RevokedError. + # + # If the token is not found (nil), then it raises TokenNotFoundError. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def doorkeeper_guard!(scopes: []) + if (access_token = find_access_token).nil? + raise TokenNotFoundError + + else + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError + + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError + + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + + end + end + end + + def doorkeeper_guard(scopes: []) + if access_token = find_access_token + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError + + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError + + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + end + end + end + + def current_user + @current_user + end + + private + def find_access_token + @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) + end + + def doorkeeper_request + @doorkeeper_request ||= ActionDispatch::Request.new(env) + end + + def validate_access_token(access_token, scopes) + Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes) + end + end + + module ClassMethods + # Installs the doorkeeper guard on the whole Grape API endpoint. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def guard_all!(scopes: []) + before do + guard! scopes: scopes + end + end + + private + def install_error_responders(base) + error_classes = [ MissingTokenError, TokenNotFoundError, + ExpiredError, RevokedError, InsufficientScopeError] + + base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler + end + + def oauth2_bearer_token_error_handler + Proc.new {|e| + response = case e + when MissingTokenError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new + + when TokenNotFoundError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Bad Access Token.") + + when ExpiredError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token is expired. You can either do re-authorization or token refresh.") + + when RevokedError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token was revoked. You have to re-authorize from the user.") + + when InsufficientScopeError + # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) + # does not include WWW-Authenticate header, which breaks the standard. + Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( + :insufficient_scope, + Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope], + { :scope => e.scopes}) + end + + response.finish + } + end + end + + # + # Exceptions + # + + class MissingTokenError < StandardError; end + + class TokenNotFoundError < StandardError; end + + class ExpiredError < StandardError; end + + class RevokedError < StandardError; end + + class InsufficientScopeError < StandardError + attr_reader :scopes + def initialize(scopes) + @scopes = scopes + end + end +end \ No newline at end of file diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 027fb20ec4..2f2342840f 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -11,7 +11,7 @@ module API def current_user private_token = (params[PRIVATE_TOKEN_PARAM] || env[PRIVATE_TOKEN_HEADER]).to_s - @current_user ||= User.find_by(authentication_token: private_token) + @current_user ||= (User.find_by(authentication_token: private_token) || doorkeeper_guard) unless @current_user && Gitlab::UserAccess.allowed?(@current_user) return nil diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index e2f222c0d3..cc071342d7 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -41,6 +41,7 @@ describe API, api: true do describe ".current_user" do it "should return nil for an invalid token" do env[API::APIHelpers::PRIVATE_TOKEN_HEADER] = 'invalid token' + self.class.any_instance.stub(:doorkeeper_guard){ false } current_user.should be_nil end diff --git a/spec/requests/api/doorkeeper_access_spec.rb b/spec/requests/api/doorkeeper_access_spec.rb new file mode 100644 index 0000000000..ddef99d77a --- /dev/null +++ b/spec/requests/api/doorkeeper_access_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe API::API, api: true do + include ApiHelpers + + let!(:user) { create(:user) } + let!(:application) { Doorkeeper::Application.create!(:name => "MyApp", :redirect_uri => "https://app.com", :owner => user) } + let!(:token) { Doorkeeper::AccessToken.create! :application_id => application.id, :resource_owner_id => user.id } + + + describe "when unauthenticated" do + it "returns authentication success" do + get api("/user"), :access_token => token.token + response.status.should == 200 + end + end + + describe "when token invalid" do + it "returns authentication error" do + get api("/user"), :access_token => "123a" + response.status.should == 401 + end + end + + describe "authorization by private token" do + it "returns authentication success" do + get api("/user", user) + response.status.should == 200 + end + end +end From a61ccd4ad2d83b2422561a374c300260e5a6d240 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 24 Dec 2014 15:44:17 +0200 Subject: [PATCH 2/6] convert erb to haml --- app/views/layouts/doorkeeper/admin.html.erb | 34 ------------------- app/views/layouts/doorkeeper/admin.html.haml | 22 ++++++++++++ .../layouts/doorkeeper/application.html.erb | 23 ------------- .../layouts/doorkeeper/application.html.haml | 15 ++++++++ 4 files changed, 37 insertions(+), 57 deletions(-) delete mode 100644 app/views/layouts/doorkeeper/admin.html.erb create mode 100644 app/views/layouts/doorkeeper/admin.html.haml delete mode 100644 app/views/layouts/doorkeeper/application.html.erb create mode 100644 app/views/layouts/doorkeeper/application.html.haml diff --git a/app/views/layouts/doorkeeper/admin.html.erb b/app/views/layouts/doorkeeper/admin.html.erb deleted file mode 100644 index baeb5eb63f..0000000000 --- a/app/views/layouts/doorkeeper/admin.html.erb +++ /dev/null @@ -1,34 +0,0 @@ - - - - - - - Doorkeeper - <%= stylesheet_link_tag "doorkeeper/admin/application" %> - <%= csrf_meta_tags %> - - - -
- <%- if flash[:notice].present? %> -
- <%= flash[:notice] %> -
- <% end -%> - - <%= yield %> -
- - diff --git a/app/views/layouts/doorkeeper/admin.html.haml b/app/views/layouts/doorkeeper/admin.html.haml new file mode 100644 index 0000000000..bd9adfab66 --- /dev/null +++ b/app/views/layouts/doorkeeper/admin.html.haml @@ -0,0 +1,22 @@ +!!! +%html + %head + %meta{:charset => "utf-8"} + %meta{:content => "IE=edge", "http-equiv" => "X-UA-Compatible"} + %meta{:content => "width=device-width, initial-scale=1.0", :name => "viewport"} + %title Doorkeeper + = stylesheet_link_tag "doorkeeper/admin/application" + = csrf_meta_tags + %body + .navbar.navbar-inverse.navbar-fixed-top{:role => "navigation"} + .container + .navbar-header + = link_to 'OAuth2 Provider', oauth_applications_path, class: 'navbar-brand' + %ul.nav.navbar-nav + = content_tag :li, class: "#{'active' if request.path == oauth_applications_path}" do + = link_to 'Applications', oauth_applications_path + .container + - if flash[:notice].present? + .alert.alert-info + = flash[:notice] + = yield \ No newline at end of file diff --git a/app/views/layouts/doorkeeper/application.html.erb b/app/views/layouts/doorkeeper/application.html.erb deleted file mode 100644 index fd7a31584f..0000000000 --- a/app/views/layouts/doorkeeper/application.html.erb +++ /dev/null @@ -1,23 +0,0 @@ - - - - OAuth authorize required - - - - - <%= stylesheet_link_tag "doorkeeper/application" %> - <%= csrf_meta_tags %> - - -
- <%- if flash[:notice].present? %> -
- <%= flash[:notice] %> -
- <% end -%> - - <%= yield %> -
- - diff --git a/app/views/layouts/doorkeeper/application.html.haml b/app/views/layouts/doorkeeper/application.html.haml new file mode 100644 index 0000000000..e5f37fad1f --- /dev/null +++ b/app/views/layouts/doorkeeper/application.html.haml @@ -0,0 +1,15 @@ +!!! +%html + %head + %title OAuth authorize required + %meta{:charset => "utf-8"} + %meta{:content => "IE=edge", "http-equiv" => "X-UA-Compatible"} + %meta{:content => "width=device-width, initial-scale=1.0", :name => "viewport"} + = stylesheet_link_tag "doorkeeper/application" + = csrf_meta_tags + %body + #container + - if flash[:notice].present? + .alert.alert-info + = flash[:notice] + = yield \ No newline at end of file From 7fe8d41d88f744b16e6e12c1c07ef3f956994110 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 16:46:28 +0200 Subject: [PATCH 3/6] Improve code style Signed-off-by: Dmitriy Zaporozhets --- app/controllers/oauth/applications_controller.rb | 14 ++++++++++---- app/controllers/oauth/authorizations_controller.rb | 11 ++++++----- .../oauth/authorized_applications_controller.rb | 4 ++-- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 8eafe5e3b3..b53e9662af 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -8,7 +8,11 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController def create @application = Doorkeeper::Application.new(application_params) - @application.owner = current_user if Doorkeeper.configuration.confirm_application_owner? + + if Doorkeeper.configuration.confirm_application_owner? + @application.owner = current_user + end + if @application.save flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :create]) redirect_to oauth_application_url(@application) @@ -18,8 +22,10 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController end def destroy - flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) if @application.destroy + if @application.destroy + flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) + end + redirect_to profile_account_url end - -end \ No newline at end of file +end diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index c46707e2c7..72cbbf2e61 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -27,9 +27,9 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController private def matching_token? - Doorkeeper::AccessToken.matching_token_for pre_auth.client, - current_resource_owner.id, - pre_auth.scopes + Doorkeeper::AccessToken.matching_token_for(pre_auth.client, + current_resource_owner.id, + pre_auth.scopes) end def redirect_or_render(auth) @@ -41,7 +41,8 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end def pre_auth - @pre_auth ||= Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, + @pre_auth ||= + Doorkeeper::OAuth::PreAuthorization.new(Doorkeeper.configuration, server.client_via_uid, params) end @@ -51,7 +52,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController end def strategy - @strategy ||= server.authorization_request pre_auth.response_type + @strategy ||= server.authorization_request(pre_auth.response_type) end end diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index b6d4a99c0a..202421b4ab 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -2,7 +2,7 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio layout "profile" def destroy - Doorkeeper::AccessToken.revoke_all_for params[:id], current_resource_owner + Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) end -end \ No newline at end of file +end From 592e396869ba5dc116cec333733cea8dfbf4a9b5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 18:35:04 +0200 Subject: [PATCH 4/6] Rework oauth2 feature * improve UI * add authorization * add separate page for oauth applications Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/generic/tables.scss | 20 +++++++++ app/assets/stylesheets/sections/tree.scss | 13 ------ .../oauth/applications_controller.rb | 12 +++++- .../oauth/authorizations_controller.rb | 1 - .../authorized_applications_controller.rb | 2 +- .../profiles/accounts_controller.rb | 2 - app/controllers/profiles_controller.rb | 5 +++ app/models/user.rb | 4 ++ .../doorkeeper/applications/_form.html.haml | 7 ++- .../doorkeeper/applications/show.html.haml | 37 +++++++++------- app/views/layouts/nav/_profile.html.haml | 6 ++- app/views/profiles/accounts/show.html.haml | 34 --------------- app/views/profiles/applications.html.haml | 43 +++++++++++++++++++ config/routes.rb | 1 + 14 files changed, 114 insertions(+), 73 deletions(-) create mode 100644 app/assets/stylesheets/generic/tables.scss create mode 100644 app/views/profiles/applications.html.haml diff --git a/app/assets/stylesheets/generic/tables.scss b/app/assets/stylesheets/generic/tables.scss new file mode 100644 index 0000000000..71a7d4abae --- /dev/null +++ b/app/assets/stylesheets/generic/tables.scss @@ -0,0 +1,20 @@ +table { + &.table { + tr { + td, th { + padding: 8px 10px; + line-height: 20px; + vertical-align: middle; + } + th { + font-weight: normal; + font-size: 15px; + border-bottom: 1px solid #CCC !important; + } + td { + border-color: #F1F1F1 !important; + border-bottom: 1px solid; + } + } + } +} diff --git a/app/assets/stylesheets/sections/tree.scss b/app/assets/stylesheets/sections/tree.scss index 678a6cd716..bc7451e2d5 100644 --- a/app/assets/stylesheets/sections/tree.scss +++ b/app/assets/stylesheets/sections/tree.scss @@ -17,19 +17,6 @@ @include border-radius(0); tr { - td, th { - padding: 8px 10px; - line-height: 20px; - } - th { - font-weight: normal; - font-size: 15px; - border-bottom: 1px solid #CCC !important; - } - td { - border-color: #F1F1F1 !important; - border-bottom: 1px solid; - } &:hover { td { background: $hover; diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index b53e9662af..93201eff30 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -3,7 +3,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController layout "profile" def index - @applications = current_user.oauth_applications + head :forbidden and return end def create @@ -28,4 +28,14 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController redirect_to profile_account_url end + + private + + def set_application + @application = current_user.oauth_applications.find(params[:id]) + end + + rescue_from ActiveRecord::RecordNotFound do |exception| + render "errors/not_found", layout: "errors", status: 404 + end end diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index 72cbbf2e61..a57b4a60c2 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -55,4 +55,3 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController @strategy ||= server.authorization_request(pre_auth.response_type) end end - diff --git a/app/controllers/oauth/authorized_applications_controller.rb b/app/controllers/oauth/authorized_applications_controller.rb index 202421b4ab..0b27ce7da7 100644 --- a/app/controllers/oauth/authorized_applications_controller.rb +++ b/app/controllers/oauth/authorized_applications_controller.rb @@ -3,6 +3,6 @@ class Oauth::AuthorizedApplicationsController < Doorkeeper::AuthorizedApplicatio def destroy Doorkeeper::AccessToken.revoke_all_for(params[:id], current_resource_owner) - redirect_to profile_account_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) + redirect_to applications_profile_url, notice: I18n.t(:notice, scope: [:doorkeeper, :flash, :authorized_applications, :destroy]) end end diff --git a/app/controllers/profiles/accounts_controller.rb b/app/controllers/profiles/accounts_controller.rb index 5f15378c83..fe121691a1 100644 --- a/app/controllers/profiles/accounts_controller.rb +++ b/app/controllers/profiles/accounts_controller.rb @@ -3,7 +3,5 @@ class Profiles::AccountsController < ApplicationController def show @user = current_user - @applications = current_user.oauth_applications - @authorized_applications = Doorkeeper::Application.authorized_for(current_user) end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index e877f9b904..c0b7e2223a 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -13,6 +13,11 @@ class ProfilesController < ApplicationController def design end + def applications + @applications = current_user.oauth_applications + @authorized_tokens = current_user.oauth_authorized_tokens + end + def update user_params.except!(:email) if @user.ldap_user? diff --git a/app/models/user.rb b/app/models/user.rb index 6518fc50b7..7dae318e78 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -565,4 +565,8 @@ class User < ActiveRecord::Base namespaces += masters_groups end end + + def oauth_authorized_tokens + Doorkeeper::AccessToken.where(resource_owner_id: self.id, revoked_at: nil) + end end diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml index 45ddf16ad0..a5fec2fabd 100644 --- a/app/views/doorkeeper/applications/_form.html.haml +++ b/app/views/doorkeeper/applications/_form.html.haml @@ -19,7 +19,6 @@ Use %code= Doorkeeper.configuration.native_redirect_uri for local tests - .form-group - .col-sm-offset-2.col-sm-10 - = f.submit 'Submit', class: "btn btn-primary wide" - = link_to "Cancel", profile_account_path, :class => "btn btn-default" \ No newline at end of file + .form-actions + = f.submit 'Submit', class: "btn btn-primary wide" + = link_to "Cancel", applications_profile_path, class: "btn btn-default" diff --git a/app/views/doorkeeper/applications/show.html.haml b/app/views/doorkeeper/applications/show.html.haml index 5236b86589..82e78b4af1 100644 --- a/app/views/doorkeeper/applications/show.html.haml +++ b/app/views/doorkeeper/applications/show.html.haml @@ -1,21 +1,26 @@ %h3.page-title Application: #{@application.name} -.row - .col-md-8 - %h4 Application Id: - %p + + +%table.table + %tr + %td + Application Id + %td %code#application_id= @application.uid - %h4 Secret: - %p + %tr + %td + Secret: + %td %code#secret= @application.secret - %h4 Callback urls: - %table + + %tr + %td + Callback url + %td - @application.redirect_uri.split.each do |uri| - %tr - %td - %code= uri - %td - = link_to 'Authorize', oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: 'code'), class: 'btn btn-success', target: '_blank' -.prepend-top-20 - %p= link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' - %p= render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' \ No newline at end of file + %div + %span.monospace= uri +.form-actions + = link_to 'Edit', edit_oauth_application_path(@application), class: 'btn btn-primary wide pull-left' + = render 'delete_form', application: @application, submit_btn_css: 'btn btn-danger prepend-left-10' diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index f68fe87a75..8bb45e4a6d 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -3,10 +3,14 @@ = link_to profile_path, title: "Profile" do %i.fa.fa-user Profile - = nav_link(controller: [:accounts, :applications]) do + = nav_link(controller: [:accounts]) do = link_to profile_account_path do %i.fa.fa-gear Account + = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new']) do + = link_to applications_profile_path do + %i.fa.fa-cloud + Applications = nav_link(controller: :emails) do = link_to profile_emails_path do %i.fa.fa-envelope-o diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 1d0b6d7718..53a50f6796 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -75,38 +75,4 @@ The following groups will be abandoned. You should transfer or remove them: %strong #{current_user.solo_owned_groups.map(&:name).join(', ')} = link_to 'Delete account', user_registration_path, data: { confirm: "REMOVE #{current_user.name}? Are you sure?" }, method: :delete, class: "btn btn-remove" - - %h3.page-title - OAuth2 - %fieldset.oauth-applications - %legend Your applications - %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' - %table.table.table-striped - %thead - %tr - %th Name - %th Callback URL - %th - %th - %tbody - - @applications.each do |application| - %tr{:id => "application_#{application.id}"} - %td= link_to application.name, oauth_application_path(application) - %td= application.redirect_uri - %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' - %td= render 'doorkeeper/applications/delete_form', application: application - %fieldset.oauth-authorized-applications - %legend Your authorized applications - %table.table.table-striped - %thead - %tr - %th Name - %th Created At - %th - %tbody - - @authorized_applications.each do |application| - %tr{:id => "application_#{application.id}"} - %td= link_to application.name, oauth_application_path(application) - %td= application.created_at.strftime('%Y-%m-%d %H:%M:%S') - %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml new file mode 100644 index 0000000000..cdb188dc1a --- /dev/null +++ b/app/views/profiles/applications.html.haml @@ -0,0 +1,43 @@ +%h3.page-title + OAuth2 + +%fieldset.oauth-applications + %legend Your applications + %p= link_to 'New Application', new_oauth_application_path, class: 'btn btn-success' + - if @applications.any? + %table.table.table-striped + %thead + %tr + %th Name + %th Callback URL + %th Clients + %th + %th + %tbody + - @applications.each do |application| + %tr{:id => "application_#{application.id}"} + %td= link_to application.name, oauth_application_path(application) + %td + - application.redirect_uri.split.each do |uri| + %div= uri + %td= application.access_tokens.count + %td= link_to 'Edit', edit_oauth_application_path(application), class: 'btn btn-link btn-small' + %td= render 'doorkeeper/applications/delete_form', application: application + +%fieldset.oauth-authorized-applications.prepend-top-20 + %legend Authorized applications + %table.table.table-striped + %thead + %tr + %th Name + %th Authorized At + %th Scope + %th + %tbody + - @authorized_tokens.each do |token| + - application = token.application + %tr{:id => "application_#{application.id}"} + %td= application.name + %td= token.created_at + %td= token.scopes + %td= render 'doorkeeper/authorized_applications/delete_form', application: application diff --git a/config/routes.rb b/config/routes.rb index 4d3039ce11..1d571e21b8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -118,6 +118,7 @@ Gitlab::Application.routes.draw do member do get :history get :design + get :applications put :reset_private_token put :update_username From aadfb3665f39e5886254bac856ebd1cc47f8c652 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 18:46:19 +0200 Subject: [PATCH 5/6] Fix tests and add message if no oauth apps Signed-off-by: Dmitriy Zaporozhets --- .../oauth/applications_controller.rb | 2 +- app/views/profiles/applications.html.haml | 34 +++++++++++-------- features/profile/profile.feature | 6 ++-- features/steps/shared/paths.rb | 4 +++ 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/app/controllers/oauth/applications_controller.rb b/app/controllers/oauth/applications_controller.rb index 93201eff30..3407490e49 100644 --- a/app/controllers/oauth/applications_controller.rb +++ b/app/controllers/oauth/applications_controller.rb @@ -26,7 +26,7 @@ class Oauth::ApplicationsController < Doorkeeper::ApplicationsController flash[:notice] = I18n.t(:notice, scope: [:doorkeeper, :flash, :applications, :destroy]) end - redirect_to profile_account_url + redirect_to applications_profile_url end private diff --git a/app/views/profiles/applications.html.haml b/app/views/profiles/applications.html.haml index cdb188dc1a..cb24e4a3dd 100644 --- a/app/views/profiles/applications.html.haml +++ b/app/views/profiles/applications.html.haml @@ -26,18 +26,22 @@ %fieldset.oauth-authorized-applications.prepend-top-20 %legend Authorized applications - %table.table.table-striped - %thead - %tr - %th Name - %th Authorized At - %th Scope - %th - %tbody - - @authorized_tokens.each do |token| - - application = token.application - %tr{:id => "application_#{application.id}"} - %td= application.name - %td= token.created_at - %td= token.scopes - %td= render 'doorkeeper/authorized_applications/delete_form', application: application + + - if @authorized_tokens.any? + %table.table.table-striped + %thead + %tr + %th Name + %th Authorized At + %th Scope + %th + %tbody + - @authorized_tokens.each do |token| + - application = token.application + %tr{:id => "application_#{application.id}"} + %td= application.name + %td= token.created_at + %td= token.scopes + %td= render 'doorkeeper/authorized_applications/delete_form', application: application + - else + %p.light You dont have any authorized applications diff --git a/features/profile/profile.feature b/features/profile/profile.feature index 88a7a3e726..fd132e1cd8 100644 --- a/features/profile/profile.feature +++ b/features/profile/profile.feature @@ -72,7 +72,7 @@ Feature: Profile Then I should see my user page Scenario: I can manage application - Given I visit profile account page + Given I visit profile applications page Then I click on new application button And I should see application form Then I fill application form out and submit @@ -81,7 +81,7 @@ Feature: Profile And I see edit application form Then I change name of application and submit And I see that application was changed - Then I visit profile account page + Then I visit profile applications page And I click to remove application Then I see that application is removed @@ -115,4 +115,4 @@ Feature: Profile Scenario: I see the password strength indicator with success Given I visit profile password page When I try to set a strong password - Then I should see the input field green \ No newline at end of file + Then I should see the input field green diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index 5f292255ce..ca03873223 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -94,6 +94,10 @@ module SharedPaths visit profile_path end + step 'I visit profile applications page' do + visit applications_profile_path + end + step 'I visit profile password page' do visit edit_profile_password_path end From d2bd5e833fdcf5bbb039936a3f71eaf7ff829063 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Dec 2014 20:51:07 +0200 Subject: [PATCH 6/6] Fix nav_link support for several path options Signed-off-by: Dmitriy Zaporozhets --- app/helpers/tab_helper.rb | 52 +++++++++++++++--------- app/views/layouts/nav/_profile.html.haml | 3 +- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/app/helpers/tab_helper.rb b/app/helpers/tab_helper.rb index bc43e07856..639fc98c22 100644 --- a/app/helpers/tab_helper.rb +++ b/app/helpers/tab_helper.rb @@ -28,6 +28,10 @@ module TabHelper # nav_link(controller: [:tree, :refs]) { "Hello" } # # => '
  • Hello
  • ' # + # # Several paths + # nav_link(path: ['tree#show', 'profile#show']) { "Hello" } + # # => '
  • Hello
  • ' + # # # Shorthand path # nav_link(path: 'tree#show') { "Hello" } # # => '
  • Hello
  • ' @@ -38,25 +42,7 @@ module TabHelper # # Returns a list item element String def nav_link(options = {}, &block) - if path = options.delete(:path) - if path.respond_to?(:each) - c = path.map { |p| p.split('#').first } - a = path.map { |p| p.split('#').last } - else - c, a, _ = path.split('#') - end - else - c = options.delete(:controller) - a = options.delete(:action) - end - - if c && a - # When given both options, make sure BOTH are active - klass = current_controller?(*c) && current_action?(*a) ? 'active' : '' - else - # Otherwise check EITHER option - klass = current_controller?(*c) || current_action?(*a) ? 'active' : '' - end + klass = active_nav_link?(options) ? 'active' : '' # Add our custom class into the html_options, which may or may not exist # and which may or may not already have a :class key @@ -72,6 +58,34 @@ module TabHelper end end + def active_nav_link?(options) + if path = options.delete(:path) + unless path.respond_to?(:each) + path = [path] + end + + path.any? do |single_path| + current_path?(single_path) + end + else + c = options.delete(:controller) + a = options.delete(:action) + + if c && a + # When given both options, make sure BOTH are true + current_controller?(*c) && current_action?(*a) + else + # Otherwise check EITHER option + current_controller?(*c) || current_action?(*a) + end + end + end + + def current_path?(path) + c, a, _ = path.split('#') + current_controller?(c) && current_action?(a) + end + def project_tab_class return "active" if current_page?(controller: "/projects", action: :edit, id: @project) diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 2821e5c066..36b48a5d02 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -11,7 +11,8 @@ = nav_link(path: ['profiles#applications', 'applications#edit', 'applications#show', 'applications#new']) do = link_to applications_profile_path do %i.fa.fa-cloud - Applications + %span + Applications = nav_link(controller: :emails) do = link_to profile_emails_path do %i.fa.fa-envelope-o