From 73cc55940df9fd466f3bbb1e9bc25612277b94fc Mon Sep 17 00:00:00 2001 From: Augier Date: Thu, 13 Aug 2015 13:20:22 +0900 Subject: [PATCH] Fix travis errors and refactor --- app/controllers/api/v0/users_controller.rb | 4 +- app/controllers/openid_connect/LICENSE | 22 --- .../authorizations_controller.rb | 33 +++-- .../openid_connect/clients_controller.rb | 31 +++-- .../openid_connect/discovery_controller.rb | 42 +++--- app/controllers/users_controller.rb | 3 +- app/models/authorization.rb | 2 +- app/models/authorization_scope.rb | 7 + app/models/o_auth_application.rb | 22 +-- app/models/scope.rb | 4 +- app/models/scopes_tokens.rb | 7 + app/models/token.rb | 6 +- config/application.rb | 2 +- config/routes.rb | 10 +- ...150613202109_create_o_auth_applications.rb | 2 +- features/step_definitions/openid_steps.rb | 64 ++++----- lib/account_deleter.rb | 3 +- lib/openid_connect/LICENSE | 22 --- lib/openid_connect/authorization/endpoint.rb | 25 ++-- .../endpoint_confirmation_point.rb | 10 +- .../authorization/endpoint_start_point.rb | 24 ++-- .../protected_resource_endpoint.rb | 1 - lib/openid_connect/token_endpoint.rb | 22 +-- .../api/v0/base_controller_spec.rb | 3 +- .../authorizations_controller_spec.rb | 129 ++++++------------ .../openid_connect/clients_controller_spec.rb | 15 +- .../protected_resource_endpoint_spec.rb | 33 ++--- .../lib/openid_connect/token_endpoint_spec.rb | 85 +++--------- spec/models/o_auth_application_spec.rb | 5 - spec/models/token_spec.rb | 5 - spec/presenters/api/v0/base_presenter_spec.rb | 3 +- spec/requests/api/v2/base_controller_spec.rb | 3 +- spec/spec_helper.rb | 8 -- 33 files changed, 237 insertions(+), 420 deletions(-) delete mode 100644 app/controllers/openid_connect/LICENSE create mode 100644 app/models/authorization_scope.rb create mode 100644 app/models/scopes_tokens.rb delete mode 100644 lib/openid_connect/LICENSE delete mode 100644 spec/models/o_auth_application_spec.rb delete mode 100644 spec/models/token_spec.rb diff --git a/app/controllers/api/v0/users_controller.rb b/app/controllers/api/v0/users_controller.rb index 60bd14589..c09f4cdb4 100644 --- a/app/controllers/api/v0/users_controller.rb +++ b/app/controllers/api/v0/users_controller.rb @@ -1,10 +1,10 @@ class Api::V0::UsersController < Api::V0::BaseController - def show render json: user end -private + private + def user current_token.user end diff --git a/app/controllers/openid_connect/LICENSE b/app/controllers/openid_connect/LICENSE deleted file mode 100644 index ace31447b..000000000 --- a/app/controllers/openid_connect/LICENSE +++ /dev/null @@ -1,22 +0,0 @@ -This code is based on https://github.com/nov/openid_connect_sample - -Copyright (c) 2011 nov matake - -Permission is hereby granted, free of charge, to any person obtaining -a copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to -permit persons to whom the Software is furnished to do so, subject to -the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/app/controllers/openid_connect/authorizations_controller.rb b/app/controllers/openid_connect/authorizations_controller.rb index 3b02cc625..c16d0ae9c 100644 --- a/app/controllers/openid_connect/authorizations_controller.rb +++ b/app/controllers/openid_connect/authorizations_controller.rb @@ -1,7 +1,7 @@ class OpenidConnect::AuthorizationsController < ApplicationController rescue_from Rack::OAuth2::Server::Authorize::BadRequest do |e| logger.info e.backtrace[0,10].join("\n") - render json: {error: e.message || :error, status: e.status} + render json: { error: e.message || :error, status: e.status } end before_action :authenticate_user! @@ -14,50 +14,49 @@ class OpenidConnect::AuthorizationsController < ApplicationController process_authorization_consent(params[:approve]) end -private + private def request_authorization_consent_form endpoint = OpenidConnect::Authorization::EndpointStartPoint.new(current_user) - handleStartPointResponse(endpoint) + handle_startpoint_response(endpoint) end - def handleStartPointResponse(endpoint) - status, header, response = *endpoint.call(request.env) + def handle_startpoint_response(endpoint) + _status, header, response = *endpoint.call(request.env) if response.redirect? - redirect_to header['Location'] + redirect_to header["Location"] else @client, @response_type, @redirect_uri, @scopes, @request_object = *[ endpoint.client, endpoint.response_type, endpoint.redirect_uri, endpoint.scopes, endpoint.request_object ] - saveRequestParameters + save_request_parameters render :new end end def process_authorization_consent(approvedString) endpoint = OpenidConnect::Authorization::EndpointConfirmationPoint.new(current_user, to_boolean(approvedString)) - restoreRequestParameters(endpoint) - handleConfirmationPointResponse(endpoint) + restore_request_parameters(endpoint) + handle_confirmation_endpoint_response(endpoint) end - def handleConfirmationPointResponse(endpoint) - status, header, response = *endpoint.call(request.env) - redirect_to header['Location'] + def handle_confirmation_endpoint_response(endpoint) + _status, header, _response = *endpoint.call(request.env) + redirect_to header["Location"] end - - def saveRequestParameters + def save_request_parameters session[:client_id], session[:response_type], session[:redirect_uri], session[:scopes], session[:request_object] = - @client.client_id, @response_type, @redirect_uri, @scopes.collect { |scope| scope.name }, @request_object + @client.client_id, @response_type, @redirect_uri, @scopes.map(&:name), @request_object end - def restoreRequestParameters(endpoint) + def restore_request_parameters(endpoint) req = Rack::Request.new(request.env) req.update_param("client_id", session[:client_id]) req.update_param("redirect_uri", session[:redirect_uri]) req.update_param("response_type", session[:response_type]) endpoint.scopes, endpoint.request_object = - session[:scopes].collect {|scope| Scope.find_by_name(scope)}, session[:request_object] + session[:scopes].map {|scope| Scope.find_by_name(scope) }, session[:request_object] end def to_boolean(str) diff --git a/app/controllers/openid_connect/clients_controller.rb b/app/controllers/openid_connect/clients_controller.rb index e067b9ab2..bc28752e3 100644 --- a/app/controllers/openid_connect/clients_controller.rb +++ b/app/controllers/openid_connect/clients_controller.rb @@ -1,10 +1,10 @@ class OpenidConnect::ClientsController < ApplicationController - rescue_from OpenIDConnect::HttpError do |e| - rewriteHTTPErrorPageAsJSON(e) + http_error_page_as_json(e) end + rescue_from OpenIDConnect::ValidationFailed do |e| - rewriteValidationFailErrorPageAsJSON(e) + validation_fail_as_json(e) end def create @@ -13,18 +13,21 @@ class OpenidConnect::ClientsController < ApplicationController render json: client end -private + private - def rewriteHTTPErrorPageAsJSON(e) - render json: { - error: :invalid_request, - error_description: e.message - }, status: 400 + def http_error_page_as_json(e) + render json: + { + error: :invalid_request, + error_description: e.message + }, status: 400 end - def rewriteValidationFailErrorPageAsJSON(e) - render json: { - error: :invalid_client_metadata, - error_description: e.message - }, status: 400 + + def validation_fail_as_json(e) + render json: + { + error: :invalid_client_metadata, + error_description: e.message + }, status: 400 end end diff --git a/app/controllers/openid_connect/discovery_controller.rb b/app/controllers/openid_connect/discovery_controller.rb index 5b53e5c13..e9b0a80b5 100644 --- a/app/controllers/openid_connect/discovery_controller.rb +++ b/app/controllers/openid_connect/discovery_controller.rb @@ -1,12 +1,12 @@ class DiscoveryController < ApplicationController def show case params[:id] - when 'webfinger' - webfinger_discovery - when 'openid-configuration' - openid_configuration - else - raise HttpError::NotFound + when "webfinger" + webfinger_discovery + when "openid-configuration" + openid_configuration + else + raise HttpError::NotFound end end @@ -15,7 +15,7 @@ class DiscoveryController < ApplicationController def webfinger_discovery jrd = { links: [{ - rel: OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE, + rel: OpenIDConnect::Discovery::Provider::Issuer::REL_VALUE, href: root_path }] } @@ -25,20 +25,20 @@ class DiscoveryController < ApplicationController def openid_configuration config = OpenIDConnect::Discovery::Provider::Config::Response.new( - issuer: root_path, - authorization_endpoint: "#{authorizations_url}/new", - token_endpoint: access_tokens_url, - userinfo_endpoint: user_info_url, - jwks_uri: "#{authorizations_url}/jwks.json", - registration_endpoint: "#{root_path}/connect", - scopes_supported: "iss", - response_types_supported: "Client.available_response_types", - grant_types_supported: "Client.available_grant_types", - request_object_signing_alg_values_supported: [:HS256, :HS384, :HS512], - subject_types_supported: ['public', 'pairwise'], - id_token_signing_alg_values_supported: [:RS256], - token_endpoint_auth_methods_supported: ['client_secret_basic', 'client_secret_post'], - claims_supported: ['sub', 'iss', 'name', 'email'] + issuer: root_path, + authorization_endpoint: "#{authorizations_url}/new", + token_endpoint: access_tokens_url, + userinfo_endpoint: user_info_url, + jwks_uri: "#{authorizations_url}/jwks.json", + registration_endpoint: "#{root_path}/connect", + scopes_supported: "iss", + response_types_supported: "Client.available_response_types", + grant_types_supported: "Client.available_grant_types", + request_object_signing_alg_values_supported: %i(HS256 HS384 HS512), + subject_types_supported: %w(public pairwise), + id_token_signing_alg_values_supported: %i(RS256), + token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post), + claims_supported: %w(sub iss name email) ) render json: config end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 91376d45f..7bce10206 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,8 +3,7 @@ # the COPYRIGHT file. class UsersController < ApplicationController - - before_action :authenticate_user!, except: [:new, :create, :public, :user_photo] + before_action :authenticate_user!, except: %i(new create public user_photo) respond_to :html def edit diff --git a/app/models/authorization.rb b/app/models/authorization.rb index 157b4f77a..a94e9edf2 100644 --- a/app/models/authorization.rb +++ b/app/models/authorization.rb @@ -1,7 +1,7 @@ class Authorization < ActiveRecord::Base belongs_to :user belongs_to :o_auth_application - has_and_belongs_to_many :scopes + has_many :scopes, through: :authorization_scopes # TODO: Incomplete class end diff --git a/app/models/authorization_scope.rb b/app/models/authorization_scope.rb new file mode 100644 index 000000000..d8a88d779 --- /dev/null +++ b/app/models/authorization_scope.rb @@ -0,0 +1,7 @@ +class AuthorizationScope < ActiveRecord::Base + belongs_to authorization + belongs_to scope + + validates authorization, presence: true + validates scope, presence: true +end diff --git a/app/models/o_auth_application.rb b/app/models/o_auth_application.rb index 1f9a12ccb..0b6b2b9df 100644 --- a/app/models/o_auth_application.rb +++ b/app/models/o_auth_application.rb @@ -19,25 +19,13 @@ class OAuthApplication < ActiveRecord::Base ["id_token"] end - def register!(registrarHash) - registrarHash.validate! - buildClientApplication(registrarHash) + def register!(registrar) + registrar.validate! + build_client_application(registrar) end - def buildClientApplication(registrarHash) - client = OAuthApplication.create! - client.attributes = filterNilValues(registrarHash) - client.save! - client - end - - def filterNilValues(registrarHash) - { - name: registrarHash.client_name, - redirect_uris: registrarHash.redirect_uris - }.delete_if do |key, value| - value.nil? - end + def build_client_application(registrar) + create! redirect_uris: registrar.redirect_uris end end end diff --git a/app/models/scope.rb b/app/models/scope.rb index a586fe631..a2cccb2da 100644 --- a/app/models/scope.rb +++ b/app/models/scope.rb @@ -1,6 +1,6 @@ class Scope < ActiveRecord::Base - has_and_belongs_to_many :tokens - has_and_belongs_to_many :authorizations + has_many :tokens, through: :scope_tokens + has_many :authorizations, through: :authorization_scopes validates :name, presence: true, uniqueness: true diff --git a/app/models/scopes_tokens.rb b/app/models/scopes_tokens.rb new file mode 100644 index 000000000..82f3d1c5e --- /dev/null +++ b/app/models/scopes_tokens.rb @@ -0,0 +1,7 @@ +class ScopeToken < ActiveRecord::Base + belongs_to :scope + belongs_to :token + + validates :scope, presence: true + validates :token, presence: true +end diff --git a/app/models/token.rb b/app/models/token.rb index da4aea70d..7a36a3398 100644 --- a/app/models/token.rb +++ b/app/models/token.rb @@ -1,6 +1,6 @@ class Token < ActiveRecord::Base belongs_to :user - has_and_belongs_to_many :scopes + has_many :scopes, through: :scope_tokens before_validation :setup, on: :create @@ -16,11 +16,11 @@ class Token < ActiveRecord::Base def bearer_token @bearer_token ||= Rack::OAuth2::AccessToken::Bearer.new( access_token: token, - expires_in: (expires_at - Time.now.utc).to_i + expires_in: (expires_at - Time.now.utc).to_i ) end - def accessible?(_scopes_or_claims_ = nil) + def accessible?(_scopes_or_claims_=nil) true # TODO: For now don't support scopes end end diff --git a/config/application.rb b/config/application.rb index 3745fec5c..49ddc6b1b 100644 --- a/config/application.rb +++ b/config/application.rb @@ -108,7 +108,7 @@ module Diaspora } config.action_mailer.asset_host = AppConfig.pod_uri.to_s - config.middleware.use Rack::OAuth2::Server::Resource::Bearer, 'OpenID Connect' do |req| + config.middleware.use Rack::OAuth2::Server::Resource::Bearer, "OpenID Connect" do |req| Token.valid(Time.now.utc).find_by(token: req.access_token) || req.invalid_token! end end diff --git a/config/routes.rb b/config/routes.rb index d1b1b1c7c..8920ef42a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -233,17 +233,17 @@ Diaspora::Application.routes.draw do # Startpage root :to => 'home#show' - #OpenID Connect & OAuth + # OpenID Connect & OAuth namespace :openid_connect do resources :clients, only: :create - post 'access_tokens', to: proc { |env| OpenidConnect::TokenEndpoint.new.call(env) } + post "access_tokens", to: proc {|env| OpenidConnect::TokenEndpoint.new.call(env) } # Authorization Servers MUST support the use of the HTTP GET and POST methods at the Authorization Endpoint (see http://openid.net/specs/openid-connect-core-1_0.html#AuthResponseValidation). - resources :authorizations, only: [:new, :create] - post 'authorizations/new', to: 'authorizations#new' + resources :authorizations, only: %i(new create) + post "authorizations/new", to: "authorizations#new" end api_version(module: "Api::V0", path: {value: "api/v0"}, default: true) do - match 'user', to: 'users#show', via: [:get, :post] + match "user", to: "users#show", via: [:get, :post] end end diff --git a/db/migrate/20150613202109_create_o_auth_applications.rb b/db/migrate/20150613202109_create_o_auth_applications.rb index 8137f2b54..c5f44bfef 100644 --- a/db/migrate/20150613202109_create_o_auth_applications.rb +++ b/db/migrate/20150613202109_create_o_auth_applications.rb @@ -1,5 +1,5 @@ class CreateOAuthApplications < ActiveRecord::Migration - def self.up + def change create_table :o_auth_applications do |t| t.belongs_to :user, index: true t.string :client_id diff --git a/features/step_definitions/openid_steps.rb b/features/step_definitions/openid_steps.rb index a90cc35d3..3aa5bdf36 100644 --- a/features/step_definitions/openid_steps.rb +++ b/features/step_definitions/openid_steps.rb @@ -1,64 +1,46 @@ When /^I register a new client$/ do - clientRegistrationURL = "/openid_connect/clients" - post clientRegistrationURL, - { - redirect_uris: ["http://localhost:3000"] # Not actually used - } + client_registration_url = "/openid_connect/clients" + post client_registration_url, redirect_uris: ["http://localhost:3000"] # Not actually used end Given /^I send a post request from that client to the token endpoint using "([^\"]*)"'s credentials$/ do |username| - clientJSON = JSON.parse(last_response.body) + client_json = JSON.parse(last_response.body) user = User.find_by(username: username) - tokenEndpointURL = "/openid_connect/access_tokens" - post tokenEndpointURL, - { - grant_type: "password", - username: user.username, + token_endpoint_url = "/openid_connect/access_tokens" + post token_endpoint_url, grant_type: "password", username: user.username, password: "password", # Password has been hard coded as all test accounts seem to have a password of "password" - client_id: clientJSON["o_auth_application"]["client_id"], - client_secret: clientJSON["o_auth_application"]["client_secret"] - } + client_id: client_json["o_auth_application"]["client_id"], + client_secret: client_json["o_auth_application"]["client_secret"] end Given /^I send a post request from that client to the token endpoint using invalid credentials$/ do - clientJSON = JSON.parse(last_response.body) - tokenEndpointURL = "/openid_connect/access_tokens" - post tokenEndpointURL, - { - grant_type: "password", - username: User.find_by(username: "bob"), - password: "wrongpassword", - client_id: clientJSON["o_auth_application"]["client_id"], - client_secret: clientJSON["o_auth_application"]["client_secret"] - } + client_json = JSON.parse(last_response.body) + token_endpoint_url = "/openid_connect/access_tokens" + post token_endpoint_url, grant_type: "password", username: "bob", password: "wrongpassword", + client_id: client_json["o_auth_application"]["client_id"], + client_secret: client_json["o_auth_application"]["client_secret"] end When /^I use received valid bearer tokens to access user info$/ do - accessTokenJson = JSON.parse(last_response.body) - userInfoEndPointURL = "/api/v0/user/" - get userInfoEndPointURL, - { - access_token: accessTokenJson["access_token"] - } + access_token_json = JSON.parse(last_response.body) + user_info_endpoint_url = "/api/v0/user/" + get user_info_endpoint_url, access_token: access_token_json["access_token"] end When /^I use invalid bearer tokens to access user info$/ do - userInfoEndPointURL = "/api/v0/user/" - get userInfoEndPointURL, - { - access_token: SecureRandom.hex(32) - } + user_info_endpoint_url = "/api/v0/user/" + get user_info_endpoint_url, access_token: SecureRandom.hex(32) end Then /^I should receive "([^\"]*)"'s id, username, and email$/ do |username| - userInfoJson = JSON.parse(last_response.body) + user_info_json = JSON.parse(last_response.body) user = User.find_by_username(username) - expect(userInfoJson["username"]).to have_content(user.username) - expect(userInfoJson["language"]).to have_content(user.language) - expect(userInfoJson["email"]).to have_content(user.email) + expect(user_info_json["username"]).to have_content(user.username) + expect(user_info_json["language"]).to have_content(user.language) + expect(user_info_json["email"]).to have_content(user.email) end Then /^I should receive an "([^\"]*)" error$/ do |error_message| - userInfoJson = JSON.parse(last_response.body) - expect(userInfoJson["error"]).to have_content(error_message) + user_info_json = JSON.parse(last_response.body) + expect(user_info_json["error"]).to have_content(error_message) end diff --git a/lib/account_deleter.rb b/lib/account_deleter.rb index 7aaf40937..6018657cf 100644 --- a/lib/account_deleter.rb +++ b/lib/account_deleter.rb @@ -46,7 +46,8 @@ class AccountDeleter #user deletions def normal_ar_user_associates_to_delete - [:tag_followings, :invitations_to_me, :services, :aspects, :user_preferences, :notifications, :blocks] + %i(tag_followings invitations_to_me services aspects user_preferences + notifications blocks authorizations o_auth_applications tokens) end def special_ar_user_associations diff --git a/lib/openid_connect/LICENSE b/lib/openid_connect/LICENSE deleted file mode 100644 index ace31447b..000000000 --- a/lib/openid_connect/LICENSE +++ /dev/null @@ -1,22 +0,0 @@ -This code is based on https://github.com/nov/openid_connect_sample - -Copyright (c) 2011 nov matake - -Permission is hereby granted, free of charge, to any person obtaining -a copy of this software and associated documentation files (the -"Software"), to deal in the Software without restriction, including -without limitation the rights to use, copy, modify, merge, publish, -distribute, sublicense, and/or sell copies of the Software, and to -permit persons to whom the Software is furnished to do so, subject to -the following conditions: - -The above copyright notice and this permission notice shall be -included in all copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, -EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND -NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE -LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION -WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/lib/openid_connect/authorization/endpoint.rb b/lib/openid_connect/authorization/endpoint.rb index 885e17022..0d1adc5b1 100644 --- a/lib/openid_connect/authorization/endpoint.rb +++ b/lib/openid_connect/authorization/endpoint.rb @@ -1,35 +1,38 @@ module OpenidConnect module Authorization class Endpoint - attr_accessor :app, :user, :client, :redirect_uri, :response_type, :scopes, :_request_, :request_uri, :request_object + attr_accessor :app, :user, :client, :redirect_uri, :response_type, + :scopes, :_request_, :request_uri, :request_object delegate :call, to: :app def initialize(current_user) @user = current_user @app = Rack::OAuth2::Server::Authorize.new do |req, res| - buildAttributes(req, res) - if OAuthApplication.available_response_types.include? Array(req.response_type).collect(&:to_s).join(' ') - handleResponseType(req, res) + build_attributes(req, res) + if OAuthApplication.available_response_types.include? Array(req.response_type).map(&:to_s).join(" ") + handle_response_type(req, res) else req.unsupported_response_type! end end end - def buildAttributes(req, res) - buildClient(req) - buildRedirectURI(req, res) + + def build_attributes(req, res) + build_client(req) + build_redirect_uri(req, res) end - def handleResponseType(req, res) + def handle_response_type(req, res) # Implemented by subclass end - private + private - def buildClient(req) + def build_client(req) @client = OAuthApplication.find_by_client_id(req.client_id) || req.bad_request! end - def buildRedirectURI(req, res) + + def build_redirect_uri(req, res) res.redirect_uri = @redirect_uri = req.verify_redirect_uri!(@client.redirect_uris) end end diff --git a/lib/openid_connect/authorization/endpoint_confirmation_point.rb b/lib/openid_connect/authorization/endpoint_confirmation_point.rb index ff171e53e..1e8a61423 100644 --- a/lib/openid_connect/authorization/endpoint_confirmation_point.rb +++ b/lib/openid_connect/authorization/endpoint_confirmation_point.rb @@ -1,21 +1,21 @@ module OpenidConnect module Authorization class EndpointConfirmationPoint < Endpoint - def initialize(current_user, approved = false) + def initialize(current_user, approved=false) super(current_user) @approved = approved end - def buildAttributes(req, res) + def build_attributes(req, res) super(req, res) # TODO: buildResponseType(req) end - def handleResponseType(req, res) - handleApproval(@approved, req, res) + def handle_response_type(req, res) + handle_approval(@approved, req, res) end - def handleApproval(approved, req, res) + def handle_approval(approved, req, res) if approved approved!(req, res) else diff --git a/lib/openid_connect/authorization/endpoint_start_point.rb b/lib/openid_connect/authorization/endpoint_start_point.rb index d8fae10c4..5d44e7dc5 100644 --- a/lib/openid_connect/authorization/endpoint_start_point.rb +++ b/lib/openid_connect/authorization/endpoint_start_point.rb @@ -4,24 +4,30 @@ module OpenidConnect def initialize(current_user) super(current_user) end - def handleResponseType(req, res) + + def handle_response_type(req, res) @response_type = req.response_type end - def buildAttributes(req, res) + + def build_attributes(req, res) super(req, res) - verifyNonce(req, res) - buildScopes(req) + verify_nonce(req, res) + build_scopes(req) # TODO: buildRequestObject(req) end - def verifyNonce(req, res) + + def verify_nonce(req, res) if res.protocol_params_location == :fragment && req.nonce.blank? req.invalid_request! "nonce required" end end - def buildScopes(req) - @scopes = req.scope.inject([]) do |_scopes_, scope| - _scopes_ << (Scope.find_by_name(scope) or req.invalid_scope! "Unknown scope: #{scope}") - end + + def build_scopes(req) + @scopes = req.scope.map {|scope| + Scope.where(name: scope).first.tap do |scope| + req.invalid_scope! "Unknown scope: #{scope}" unless scope + end + } end end end diff --git a/lib/openid_connect/protected_resource_endpoint.rb b/lib/openid_connect/protected_resource_endpoint.rb index fa840c18b..4b7a7ea9c 100644 --- a/lib/openid_connect/protected_resource_endpoint.rb +++ b/lib/openid_connect/protected_resource_endpoint.rb @@ -1,6 +1,5 @@ module OpenidConnect module ProtectedResourceEndpoint - def self.included(klass) klass.send :include, ProtectedResourceEndpoint::Helper end diff --git a/lib/openid_connect/token_endpoint.rb b/lib/openid_connect/token_endpoint.rb index 2f8e9c3d7..a524de61d 100644 --- a/lib/openid_connect/token_endpoint.rb +++ b/lib/openid_connect/token_endpoint.rb @@ -5,25 +5,25 @@ module OpenidConnect def initialize @app = Rack::OAuth2::Server::Token.new do |req, res| - o_auth_app = retrieveClient(req) - if isAppValid(o_auth_app, req) - handleFlows(req, res) + o_auth_app = retrieve_client(req) + if app_valid?(o_auth_app, req) + handle_flows(req, res) else req.invalid_client! end end end - def handleFlows(req, res) + def handle_flows(req, res) case req.grant_type - when :password - handlePasswordFlow(req, res) - else - req.unsupported_grant_type! + when :password + handle_password_flow(req, res) + else + req.unsupported_grant_type! end end - def handlePasswordFlow(req, res) + def handle_password_flow(req, res) user = User.find_for_database_authentication(username: req.username) if user if user.valid_password?(req.password) @@ -36,11 +36,11 @@ module OpenidConnect end end - def retrieveClient(req) + def retrieve_client(req) OAuthApplication.find_by_client_id req.client_id end - def isAppValid(o_auth_app, req) + def app_valid?(o_auth_app, req) o_auth_app.client_secret == req.client_secret end end diff --git a/spec/controllers/api/v0/base_controller_spec.rb b/spec/controllers/api/v0/base_controller_spec.rb index f637d15b8..dc359bbe2 100644 --- a/spec/controllers/api/v0/base_controller_spec.rb +++ b/spec/controllers/api/v0/base_controller_spec.rb @@ -1,5 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe Api::V0::BaseController do - end diff --git a/spec/controllers/openid_connect/authorizations_controller_spec.rb b/spec/controllers/openid_connect/authorizations_controller_spec.rb index fedc62e60..974e0620b 100644 --- a/spec/controllers/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/openid_connect/authorizations_controller_spec.rb @@ -1,13 +1,16 @@ -require 'spec_helper' +require "spec_helper" describe OpenidConnect::AuthorizationsController, type: :controller do let!(:client) { OAuthApplication.create!(name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/"]) } - let!(:client_with_multiple_redirects) { OAuthApplication.create!(name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/","http://localhost/"]) } + let!(:client_with_multiple_redirects) do + OAuthApplication.create!( + name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/", "http://localhost/"]) + end before do sign_in :user, alice allow(@controller).to receive(:current_user).and_return(alice) - Scope.create!(name:"openid") + Scope.create!(name: "openid") end describe "#new" do @@ -15,113 +18,71 @@ describe OpenidConnect::AuthorizationsController, type: :controller do render_views context "as GET request" do it "should return a form page" do - get :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end end + context "as POST request" do it "should return a form page" do - post :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end end end + context "when client id is missing" do it "should return an bad request error" do - post :new, - { - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + post :new, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("bad_request") end end + context "when redirect uri is missing" do context "when only one redirect URL is pre-registered" do it "should return a form pager" do # Note this intentionally behavior diverts from OIDC spec http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + # When client has only one redirect uri registered, only that redirect uri can be used. Hence, + # we should implicitly assume the client wants to use that registered URI. # See https://github.com/nov/rack-oauth2/blob/master/lib/rack/oauth2/server/authorize.rb#L63 - post :new, - { - client_id: client.client_id, - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + post :new, client_id: client.client_id, response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("Diaspora Test Client") end end end + context "when multiple redirect URLs are pre-registered" do it "should return an invalid request error" do - post :new, - { - client_id: client_with_multiple_redirects.client_id, - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + post :new, client_id: client_with_multiple_redirects.client_id, response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("bad_request") end end + context "when redirect URI does not match pre-registered URIs" do it "should return an invalid request error" do - post :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:2000/", - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16) - } + post :new, client_id: client.client_id, redirect_uri: "http://localhost:2000/", + response_type: "id_token", scope: "openid", nonce: SecureRandom.hex(16) expect(response.body).to match("bad_request") end end + context "when an unsupported scope is passed in" do it "should return an invalid scope error" do - post :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "random", - nonce: SecureRandom.hex(16), - state: SecureRandom.hex(16) - } + post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "random", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) expect(response.body).to match("error=invalid_scope") end end + context "when nonce is missing" do it "should return an invalid request error" do - post :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "openid", - state: SecureRandom.hex(16) - } + post :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", + response_type: "id_token", scope: "openid", state: SecureRandom.hex(16) expect(response.location).to match("error=invalid_request") end end @@ -129,44 +90,36 @@ describe OpenidConnect::AuthorizationsController, type: :controller do describe "#create" do before do - get :new, - { - client_id: client.client_id, - redirect_uri: "http://localhost:3000/", - response_type: "id_token", - scope: "openid", - nonce: SecureRandom.hex(16), - state: 4180930983 - } + get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "openid", nonce: SecureRandom.hex(16), state: 418_093_098_3 end + context "when authorization is approved" do before do - post :create, - { - approve: "true" - } + post :create, approve: "true" end + it "should return the id token in a fragment" do expect(response.location).to have_content("id_token=") end + it "should return the passed in state" do expect(response.location).to have_content("state=4180930983") end end + context "when authorization is denied" do before do - post :create, - { - approve: "false" - } + post :create, approve: "false" end + it "should return an error in the fragment" do expect(response.location).to have_content("error=") end + it "should NOT contain a id token in the fragment" do expect(response.location).to_not have_content("id_token=") end end end - end diff --git a/spec/controllers/openid_connect/clients_controller_spec.rb b/spec/controllers/openid_connect/clients_controller_spec.rb index bce1f9295..1f718e408 100644 --- a/spec/controllers/openid_connect/clients_controller_spec.rb +++ b/spec/controllers/openid_connect/clients_controller_spec.rb @@ -1,22 +1,19 @@ -require 'spec_helper' +require "spec_helper" describe OpenidConnect::ClientsController, type: :controller do describe "#create" do context "when valid parameters are passed" do it "should return a client id" do - post :create, - { - redirect_uris: ["http://localhost"] - } - clientJSON = JSON.parse(response.body) - expect(clientJSON["o_auth_application"]["client_id"].length).to eq(32) + post :create, redirect_uris: ["http://localhost"] + client_json = JSON.parse(response.body) + expect(client_json["o_auth_application"]["client_id"].length).to eq(32) end end context "when redirect uri is missing" do it "should return a invalid_client_metadata error" do post :create - clientJSON = JSON.parse(response.body) - expect(clientJSON["error"]).to have_content("invalid_client_metadata") + client_json = JSON.parse(response.body) + expect(client_json["error"]).to have_content("invalid_client_metadata") end end end diff --git a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb index 5a8251b0a..247df0661 100644 --- a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb +++ b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe OpenidConnect::ProtectedResourceEndpoint, type: :request do describe "getting the user info" do @@ -8,19 +8,13 @@ describe OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when access token is valid" do it "shows the user's username and email" do - get "/api/v0/user/", - { - access_token: token - } - jsonBody = JSON.parse(response.body) - expect(jsonBody["username"]).to eq(bob.username) - expect(jsonBody["email"]).to eq(bob.email) + get "/api/v0/user/", access_token: token + json_body = JSON.parse(response.body) + expect(json_body["username"]).to eq(bob.username) + expect(json_body["email"]).to eq(bob.email) end it "should include private in the cache-control header" do - get "/api/v0/user/", - { - access_token: token - } + get "/api/v0/user/", access_token: token expect(response.headers["Cache-Control"]).to include("private") end end @@ -38,24 +32,15 @@ describe OpenidConnect::ProtectedResourceEndpoint, type: :request do context "when an invalid access token is provided" do it "should respond with a 401 Unauthorized response" do - get "/api/v0/user/", - { - access_token: invalid_token - } + get "/api/v0/user/", access_token: invalid_token expect(response.status).to be(401) end it "should have an auth-scheme value of Bearer" do - get "/api/v0/user/", - { - access_token: invalid_token - } + get "/api/v0/user/", access_token: invalid_token expect(response.headers["WWW-Authenticate"]).to include("Bearer") end it "should contain an invalid_token error" do - get "/api/v0/user/", - { - access_token: invalid_token - } + get "/api/v0/user/", access_token: invalid_token expect(response.body).to include("invalid_token") end end diff --git a/spec/lib/openid_connect/token_endpoint_spec.rb b/spec/lib/openid_connect/token_endpoint_spec.rb index 01de06161..b0d36aafe 100644 --- a/spec/lib/openid_connect/token_endpoint_spec.rb +++ b/spec/lib/openid_connect/token_endpoint_spec.rb @@ -1,68 +1,40 @@ -require 'spec_helper' +require "spec_helper" describe OpenidConnect::TokenEndpoint, type: :request do let!(:client) { OAuthApplication.create!(redirect_uris: ["http://localhost"]) } describe "the password grant type" do context "when the username field is missing" do it "should return an invalid request error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - password: "bluepin7", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", password: "bluepin7", + client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include("'username' required") end end context "when the password field is missing" do it "should return an invalid request error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "bob", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", + client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include("'password' required") end end context "when the username does not match an existing user" do it "should return an invalid request error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "randomnoexist", - password: "bluepin7", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "randomnoexist", + password: "bluepin7", client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include("invalid_grant") end end context "when the password is invalid" do it "should return an invalid request error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "bob", - password: "wrongpassword", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", + password: "wrongpassword", client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include("invalid_grant") end end context "when the request is valid" do it "should return an access token" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "bob", - password: "bluepin7", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", + password: "bluepin7", client_id: client.client_id, client_secret: client.client_secret json = JSON.parse(response.body) expect(json["access_token"].length).to eq(64) expect(json["token_type"]).to eq("bearer") @@ -71,44 +43,25 @@ describe OpenidConnect::TokenEndpoint, type: :request do end context "when there are duplicate fields" do it "should return an invalid request error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "bob", - password: "bluepin7", - username: "bob", - password: "bluepin6", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", password: "bluepin7", + username: "bob", password: "bluepin6", client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include("invalid_grant") end end context "when the client is unregistered" do it "should return an error" do - post "/openid_connect/access_tokens", - { - grant_type: "password", - username: "bob", - password: "bluepin7", - client_id: SecureRandom.hex(16).to_s, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", + password: "bluepin7", client_id: SecureRandom.hex(16).to_s, client_secret: client.client_secret expect(response.body).to include("invalid_client") end end - # TODO: Support a way to prevent brute force attacks using rate-limitation? as specified by RFC 6749 4.3.2 Access Token Request + # TODO: Support a way to prevent brute force attacks using rate-limitation + # as specified by RFC 6749 4.3.2 Access Token Request end describe "an unsupported grant type" do it "should return an unsupported grant type error" do - post "/openid_connect/access_tokens", - { - grant_type: "noexistgrant", - username: "bob", - password: "bluepin7", - client_id: client.client_id, - client_secret: client.client_secret - } + post "/openid_connect/access_tokens", grant_type: "noexistgrant", username: "bob", + password: "bluepin7", client_id: client.client_id, client_secret: client.client_secret expect(response.body).to include "unsupported_grant_type" end end diff --git a/spec/models/o_auth_application_spec.rb b/spec/models/o_auth_application_spec.rb deleted file mode 100644 index 1f0bb9ef7..000000000 --- a/spec/models/o_auth_application_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -RSpec.describe OAuthApplication, type: :model do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/models/token_spec.rb b/spec/models/token_spec.rb deleted file mode 100644 index 18bba17d4..000000000 --- a/spec/models/token_spec.rb +++ /dev/null @@ -1,5 +0,0 @@ -require 'rails_helper' - -RSpec.describe Token, type: :model do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/presenters/api/v0/base_presenter_spec.rb b/spec/presenters/api/v0/base_presenter_spec.rb index 36d38fbe7..01261461a 100644 --- a/spec/presenters/api/v0/base_presenter_spec.rb +++ b/spec/presenters/api/v0/base_presenter_spec.rb @@ -1,5 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe Api::V0::BasePresenter do - end diff --git a/spec/requests/api/v2/base_controller_spec.rb b/spec/requests/api/v2/base_controller_spec.rb index f637d15b8..dc359bbe2 100644 --- a/spec/requests/api/v2/base_controller_spec.rb +++ b/spec/requests/api/v2/base_controller_spec.rb @@ -1,5 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe Api::V0::BaseController do - end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dcdb16d05..ca444256a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -59,14 +59,6 @@ def photo_fixture_name @photo_fixture_name = File.join(File.dirname(__FILE__), "fixtures", "button.png") end -def retrieveAccessToken(user) - o_auth_app = OAuthApplication.create!(client_id: 4, client_secret: "azerty") - user = User.find_for_database_authentication(username: user.username) - if o_auth_app && user && user.valid_password?("bluepin7") # Hard coded password for bob - o_auth_app.tokens.create!.bearer_token.to_s - end -end - # Force fixture rebuild FileUtils.rm_f(Rails.root.join("tmp", "fixture_builder.yml"))