From 7b89dc63496de0b6131a93c57d283ebb4490804b Mon Sep 17 00:00:00 2001 From: Tom Hughes Date: Wed, 16 Nov 2011 21:06:47 +0000 Subject: [PATCH] Make OAuth work again --- app/controllers/application_controller.rb | 5 ++ app/controllers/oauth_controller.rb | 86 ++++--------------- app/models/access_token.rb | 2 +- app/models/client_application.rb | 20 ++--- app/models/oauth2_token.rb | 21 +++++ app/models/oauth2_verifier.rb | 34 ++++++++ app/models/oauth_token.rb | 17 +--- app/models/request_token.rb | 4 +- ...oauthorize.html.erb => authorize.html.erb} | 2 +- app/views/oauth/authorize_success.html.erb | 2 +- config/routes.rb | 3 +- db/migrate/20111116184519_update_oauth.rb | 11 +++ test/unit/oauth_token_test.rb | 11 --- 13 files changed, 109 insertions(+), 109 deletions(-) create mode 100644 app/models/oauth2_token.rb create mode 100644 app/models/oauth2_verifier.rb rename app/views/oauth/{oauthorize.html.erb => authorize.html.erb} (68%) create mode 100644 db/migrate/20111116184519_update_oauth.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e57638767..44307d482 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -366,6 +366,11 @@ private return [user, pass] end + # used by oauth plugin to get the current user + def current_user + @user + end + # used by oauth plugin to set the current user def current_user=(user) @user=user diff --git a/app/controllers/oauth_controller.rb b/app/controllers/oauth_controller.rb index 5c84be0cf..3c56a4bf4 100644 --- a/app/controllers/oauth_controller.rb +++ b/app/controllers/oauth_controller.rb @@ -1,83 +1,33 @@ +require 'oauth/controllers/provider_controller' + class OauthController < ApplicationController + include OAuth::Controllers::ProviderController + layout 'slim' - before_filter :authorize_web, :only => [:oauthorize, :revoke] - before_filter :set_locale, :only => [:oauthorize, :revoke] - before_filter :require_user, :only => [:oauthorize] - before_filter :verify_oauth_consumer_signature, :only => [:request_token] - before_filter :verify_oauth_request_token, :only => [:access_token] - # Uncomment the following if you are using restful_open_id_authentication - # skip_before_filter :verify_authenticity_token + def login_required + authorize_web + set_locale + require_user + end - def request_token - @token = current_client_application.create_request_token + def user_authorizes_token? + any_auth = false - if @token - logger.info "request token params: #{params.inspect}" - # request tokens indicate what permissions the client *wants*, not - # necessarily the same as those which the user allows. - current_client_application.permissions.each do |pref| + @token.client_application.permissions.each do |pref| + if params[pref] @token.write_attribute(pref, true) + any_auth ||= true + else + @token.write_attribute(pref, false) end - @token.save! - - render :text => @token.to_query - else - render :nothing => true, :status => 401 end - end - def access_token - @token = current_token && current_token.exchange! - if @token - render :text => @token.to_query - else - render :nothing => true, :status => 401 - end - end - - def oauthorize - @token = RequestToken.find_by_token params[:oauth_token] - unless @token.nil? or @token.invalidated? - if request.post? - any_auth = false - @token.client_application.permissions.each do |pref| - if params[pref] - @token.write_attribute(pref, true) - any_auth ||= true - else - @token.write_attribute(pref, false) - end - end - - if any_auth - @token.authorize!(@user) - if @token.oauth10? - redirect_url = params[:oauth_callback] || @token.client_application.callback_url - else - redirect_url = @token.oob? ? @token.client_application.callback_url : @token.callback_url - end - if redirect_url and not redirect_url.empty? - if @token.oauth10? - redirect_to "#{redirect_url}?oauth_token=#{@token.token}" - else - redirect_to "#{redirect_url}?oauth_token=#{@token.token}&oauth_verifier=#{@token.verifier}" - end - else - render :action => "authorize_success" - end - else - @token.invalidate! - render :action => "authorize_failure" - end - end - else - render :action => "authorize_failure" - end + any_auth end def revoke - @token = @user.oauth_tokens.find_by_token params[:token] + @token = current_user.oauth_tokens.find_by_token params[:token] if @token @token.invalidate! flash[:notice] = t('oauth.revoke.flash', :application => @token.client_application.name) diff --git a/app/models/access_token.rb b/app/models/access_token.rb index 1f172c5f0..a1888343a 100644 --- a/app/models/access_token.rb +++ b/app/models/access_token.rb @@ -4,7 +4,7 @@ class AccessToken < OauthToken scope :valid, where(:invalidated_at => nil) - validates_presence_of :user + validates_presence_of :user, :secret before_create :set_authorized_at diff --git a/app/models/client_application.rb b/app/models/client_application.rb index 04f1c0c99..55f34e6e2 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -4,6 +4,8 @@ class ClientApplication < ActiveRecord::Base belongs_to :user has_many :tokens, :class_name => "OauthToken" has_many :access_tokens + has_many :oauth2_verifiers + has_many :oauth_tokens validates_presence_of :name, :url, :key, :secret validates_uniqueness_of :key @@ -27,15 +29,10 @@ class ClientApplication < ActiveRecord::Base def self.verify_request(request, options = {}, &block) begin signature = OAuth::Signature.build(request, options, &block) - logger.info "Signature Base String: #{signature.signature_base_string}" - logger.info "Consumer: #{signature.send :consumer_key}" - logger.info "Token: #{signature.send :token}" return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp) value = signature.verify - logger.info "Signature verification returned: #{value.to_s}" value rescue OAuth::Signature::UnknownSignatureMethod => e - logger.info "ERROR"+e.to_s false end end @@ -52,8 +49,12 @@ class ClientApplication < ActiveRecord::Base @oauth_client ||= OAuth::Consumer.new(key, secret) end - def create_request_token - RequestToken.create :client_application => self, :callback_url => self.token_callback_url + def create_request_token(params={}) + params = { :client_application => self, :callback_url => self.token_callback_url } + permissions.each do |p| + params[p] = true + end + RequestToken.create(params) end def access_token_for_user(user) @@ -84,8 +85,7 @@ protected :allow_write_api, :allow_read_gpx, :allow_write_gpx ] def generate_keys - oauth_client = oauth_server.generate_consumer_credentials - self.key = oauth_client.key - self.secret = oauth_client.secret + self.key = OAuth::Helper.generate_key(40)[0,40] + self.secret = OAuth::Helper.generate_key(40)[0,40] end end diff --git a/app/models/oauth2_token.rb b/app/models/oauth2_token.rb new file mode 100644 index 000000000..9c28d886e --- /dev/null +++ b/app/models/oauth2_token.rb @@ -0,0 +1,21 @@ +class Oauth2Token < AccessToken + attr_accessor :state + + def as_json(options={}) + d = {:access_token=>token, :token_type => 'bearer'} + d[:expires_in] = expires_in if expires_at + d + end + + def to_query + q = "access_token=#{token}&token_type=bearer" + q << "&state=#{URI.escape(state)}" if @state + q << "&expires_in=#{expires_in}" if expires_at + q << "&scope=#{URI.escape(scope)}" if scope + q + end + + def expires_in + expires_at.to_i - Time.now.to_i + end +end diff --git a/app/models/oauth2_verifier.rb b/app/models/oauth2_verifier.rb new file mode 100644 index 000000000..94856d0d8 --- /dev/null +++ b/app/models/oauth2_verifier.rb @@ -0,0 +1,34 @@ +class Oauth2Verifier < OauthToken + validates_presence_of :user + attr_accessor :state + + def exchange!(params={}) + OauthToken.transaction do + token = Oauth2Token.create! :user=>user,:client_application=>client_application, :scope => scope + invalidate! + token + end + end + + def code + token + end + + def redirect_url + callback_url + end + + def to_query + q = "code=#{token}" + q << "&state=#{URI.escape(state)}" if @state + q + end + + protected + + def generate_keys + self.token = OAuth::Helper.generate_key(20)[0,20] + self.expires_at = 10.minutes.from_now + self.authorized_at = Time.now + end +end diff --git a/app/models/oauth_token.rb b/app/models/oauth_token.rb index 376ad7644..b38fe0ec0 100644 --- a/app/models/oauth_token.rb +++ b/app/models/oauth_token.rb @@ -5,20 +5,10 @@ class OauthToken < ActiveRecord::Base scope :authorized, where("authorized_at IS NOT NULL and invalidated_at IS NULL") validates_uniqueness_of :token - validates_presence_of :client_application, :token, :secret + validates_presence_of :client_application, :token before_validation :generate_keys, :on => :create - def self.find_token(token_key) - token = OauthToken.find_by_token(token_key, :include => :client_application) - if token && token.authorized? - logger.info "Loaded #{token.token} which was authorized by (user_id=#{token.user_id}) on the #{token.authorized_at}" - token - else - nil - end - end - def invalidated? invalidated_at != nil end @@ -38,8 +28,7 @@ class OauthToken < ActiveRecord::Base protected def generate_keys - @oauth_token = client_application.oauth_server.generate_credentials - self.token = @oauth_token[0] - self.secret = @oauth_token[1] + self.token = OAuth::Helper.generate_key(40)[0,40] + self.secret = OAuth::Helper.generate_key(40)[0,40] end end diff --git a/app/models/request_token.rb b/app/models/request_token.rb index 0044dde26..1ac502bc7 100644 --- a/app/models/request_token.rb +++ b/app/models/request_token.rb @@ -6,7 +6,7 @@ class RequestToken < OauthToken return false if authorized? self.user = user self.authorized_at = Time.now - self.verifier = OAuth::Helper.generate_key(16)[0,20] unless oauth10? + self.verifier = OAuth::Helper.generate_key(20)[0,20] unless oauth10? self.save end @@ -36,7 +36,7 @@ class RequestToken < OauthToken end def oob? - self.callback_url=='oob' + callback_url.nil? || callback_url.downcase == 'oob' end def oauth10? diff --git a/app/views/oauth/oauthorize.html.erb b/app/views/oauth/authorize.html.erb similarity index 68% rename from app/views/oauth/oauthorize.html.erb rename to app/views/oauth/authorize.html.erb index ffb403be1..eaf43f9d1 100644 --- a/app/views/oauth/oauthorize.html.erb +++ b/app/views/oauth/authorize.html.erb @@ -1,5 +1,5 @@

Authorize access to your account

-

<%= t('oauth.oauthorize.request_access', :app_name => link_to(@token.client_application.name, @token.client_application.url), :user => link_to(@user.display_name, :controller => :user, :action => :view, :display_name => @user.display_name)) %>

+

<%= raw t('oauth.oauthorize.request_access', :app_name => link_to(@token.client_application.name, @token.client_application.url), :user => link_to(@user.display_name, :controller => :user, :action => :view, :display_name => @user.display_name)) %>

<%= form_tag authorize_url do %> <%= hidden_field_tag "oauth_token", @token.token %> <%- if params[:oauth_callback] -%> diff --git a/app/views/oauth/authorize_success.html.erb b/app/views/oauth/authorize_success.html.erb index a25b98ec7..89512db31 100644 --- a/app/views/oauth/authorize_success.html.erb +++ b/app/views/oauth/authorize_success.html.erb @@ -1,5 +1,5 @@

You have allowed this request

-<% if @token.oob? %> +<% if @token.oob? and not @token.oauth10? %>

The verification code is <%= @token.verifier %>

<% end %> diff --git a/config/routes.rb b/config/routes.rb index 471f67e73..1916ae74a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -213,7 +213,8 @@ OpenStreetMap::Application.routes.draw do resources :oauth_clients end match '/oauth/revoke' => 'oauth#revoke' - match '/oauth/authorize' => 'oauth#oauthorize', :as => :authorize + match '/oauth/authorize' => 'oauth#authorize', :as => :authorize + match '/oauth/token' => 'oauth#token', :as => :token match '/oauth/request_token' => 'oauth#request_token', :as => :request_token match '/oauth/access_token' => 'oauth#access_token', :as => :access_token match '/oauth/test_request' => 'oauth#test_request', :as => :test_request diff --git a/db/migrate/20111116184519_update_oauth.rb b/db/migrate/20111116184519_update_oauth.rb new file mode 100644 index 000000000..59ab9c39b --- /dev/null +++ b/db/migrate/20111116184519_update_oauth.rb @@ -0,0 +1,11 @@ +class UpdateOauth < ActiveRecord::Migration + def up + add_column :oauth_tokens, :scope, :string + add_column :oauth_tokens, :valid_to, :timestamp + end + + def down + remove_column :oauth_tokens, :valid_to + remove_column :oauth_tokens, :scope + end +end diff --git a/test/unit/oauth_token_test.rb b/test/unit/oauth_token_test.rb index 655e64eda..eb8219c57 100644 --- a/test/unit/oauth_token_test.rb +++ b/test/unit/oauth_token_test.rb @@ -23,15 +23,4 @@ class OauthTokenTest < ActiveSupport::TestCase assert_equal false, tok.authorized?, "Token should now be invalid." end - ## - # test that tokens can't be found unless they're authorised - def test_find_token - tok = client_applications(:oauth_web_app).create_request_token - assert_equal false, tok.authorized?, "Token should be created unauthorised." - assert_equal nil, OauthToken.find_token(tok.token), "Shouldn't be able to find unauthorised token" - tok.authorize!(users(:public_user)) - assert_equal true, tok.authorized?, "Token should now be authorised." - assert_not_equal nil, OauthToken.find_token(tok.token), "Should be able to find authorised token" - end - end -- 2.39.5