diff --git a/app/controllers/api/v0/base_controller.rb b/app/controllers/api/v0/base_controller.rb index 982800edd..85a3913d1 100644 --- a/app/controllers/api/v0/base_controller.rb +++ b/app/controllers/api/v0/base_controller.rb @@ -3,7 +3,4 @@ class Api::V0::BaseController < ApplicationController before_filter :require_access_token - def authorization - current_token.authorization - end end diff --git a/app/controllers/api/v0/users_controller.rb b/app/controllers/api/v0/users_controller.rb index 57b2c300c..70e04f372 100644 --- a/app/controllers/api/v0/users_controller.rb +++ b/app/controllers/api/v0/users_controller.rb @@ -6,6 +6,6 @@ class Api::V0::UsersController < Api::V0::BaseController private def user - authorization.user + current_token.authorization.user end end diff --git a/app/controllers/openid_connect/authorizations_controller.rb b/app/controllers/openid_connect/authorizations_controller.rb index 46f226fb2..3d4a55eb9 100644 --- a/app/controllers/openid_connect/authorizations_controller.rb +++ b/app/controllers/openid_connect/authorizations_controller.rb @@ -11,31 +11,31 @@ class OpenidConnect::AuthorizationsController < ApplicationController end def create + restore_request_parameters process_authorization_consent(params[:approve]) end private - def request_authorization_consent_form - endpoint = OpenidConnect::Authorization::EndpointStartPoint.new(current_user) - handle_startpoint_response(endpoint) + def request_authorization_consent_form # TODO: Add support for prompt params + if OpenidConnect::Authorization.find_by_client_id_and_user(params[:client_id], current_user) + process_authorization_consent("true") + else + endpoint = OpenidConnect::AuthorizationPoint::EndpointStartPoint.new(current_user) + handle_start_point_response(endpoint) + end end - def handle_startpoint_response(endpoint) + def handle_start_point_response(endpoint) _status, header, response = *endpoint.call(request.env) if response.redirect? redirect_to header["Location"] else - saveParamsAndRenderConsentForm(endpoint) + save_params_and_render_consent_form(endpoint) end end - def process_authorization_consent(approvedString) - endpoint = OpenidConnect::Authorization::EndpointConfirmationPoint.new(current_user, to_boolean(approvedString)) - handle_confirmation_endpoint_response(endpoint) - end - - def saveParamsAndRenderConsentForm(endpoint) + def save_params_and_render_consent_form(endpoint) @o_auth_application, @response_type, @redirect_uri, @scopes, @request_object = *[ endpoint.o_auth_application, endpoint.response_type, endpoint.redirect_uri, endpoint.scopes, endpoint.request_object ] @@ -43,22 +43,22 @@ class OpenidConnect::AuthorizationsController < ApplicationController render :new end + def save_request_parameters + session[:client_id], session[:response_type], session[:redirect_uri], session[:scopes], session[:request_object], session[:nonce] = + @o_auth_application.client_id, @response_type, @redirect_uri, @scopes.map(&:name), @request_object, params[:nonce] + end + + def process_authorization_consent(approvedString) + endpoint = OpenidConnect::AuthorizationPoint::EndpointConfirmationPoint.new(current_user, to_boolean(approvedString)) + handle_confirmation_endpoint_response(endpoint) + end + def handle_confirmation_endpoint_response(endpoint) - restore_request_parameters(endpoint) _status, header, _response = *endpoint.call(request.env) delete_authorization_session_variables redirect_to header["Location"] end - 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, endpoint.nonce = - session[:scopes].map {|scope| Scope.find_by_name(scope) }, session[:request_object], session[:nonce] - end - def delete_authorization_session_variables session.delete(:client_id) session.delete(:response_type) @@ -68,12 +68,17 @@ class OpenidConnect::AuthorizationsController < ApplicationController session.delete(:nonce) end - def save_request_parameters - session[:client_id], session[:response_type], session[:redirect_uri], session[:scopes], session[:request_object], session[:nonce] = - @o_auth_application.client_id, @response_type, @redirect_uri, @scopes.map(&:name), @request_object, params[:nonce] - end - def to_boolean(str) str.downcase == "true" end + + def restore_request_parameters + 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]) + req.update_param("scopes", session[:scopes]) + req.update_param("request_object", session[:request_object]) + req.update_param("nonce", session[:nonce]) + end end diff --git a/app/models/openid_connect/authorization.rb b/app/models/openid_connect/authorization.rb index a13a861bd..6793c59ce 100644 --- a/app/models/openid_connect/authorization.rb +++ b/app/models/openid_connect/authorization.rb @@ -2,37 +2,35 @@ class OpenidConnect::Authorization < ActiveRecord::Base belongs_to :user belongs_to :o_auth_application - has_many :scopes, through: :authorization_scopes - has_many :o_auth_access_tokens - - before_validation :setup, on: :create - - validates :refresh_token, presence: true, uniqueness: true validates :user, presence: true, uniqueness: true validates :o_auth_application, presence: true, uniqueness: true - def setup + has_many :scopes, through: :authorization_scopes + has_many :o_auth_access_tokens, dependent: :destroy + has_many :id_tokens + + def generate_refresh_token self.refresh_token = SecureRandom.hex(32) end def create_access_token - OpenidConnect::OAuthAccessToken.create!(authorization: self).bearer_token + o_auth_access_tokens.create!.bearer_token end - # TODO: Actually call this method from token endpoint - def regenerate_refresh_token - self.refresh_token = SecureRandom.hex(32) + def self.find_by_client_id_and_user(client_id, user) + app = OpenidConnect::OAuthApplication.find_by(client_id: client_id) + find_by(o_auth_application: app, user: user) end - def self.find_by_client_id_and_user(app, user) + def self.find_by_app_and_user(app, user) find_by(o_auth_application: app, user: user) end # TODO: Handle creation error def self.find_or_create(client_id, user) app = OpenidConnect::OAuthApplication.find_by(client_id: client_id) - find_by_client_id_and_user(app, user) || create!(user: user, o_auth_application: app) + find_by_app_and_user(app, user) || create!(user: user, o_auth_application: app) end - # TODO: Incomplete class + # TODO: Consider splitting into subclasses by flow type end diff --git a/app/models/id_token.rb b/app/models/openid_connect/id_token.rb similarity index 66% rename from app/models/id_token.rb rename to app/models/openid_connect/id_token.rb index fc65b3751..1f48e3e16 100644 --- a/app/models/id_token.rb +++ b/app/models/openid_connect/id_token.rb @@ -1,12 +1,8 @@ -class IdToken < ActiveRecord::Base - belongs_to :user - belongs_to :o_auth_application +class OpenidConnect::IdToken < ActiveRecord::Base + belongs_to :authorization before_validation :setup, on: :create - validates :user, presence: true - validates :o_auth_application, presence: true - default_scope -> { where("expires_at >= ?", Time.now.utc) } def setup @@ -20,11 +16,11 @@ class IdToken < ActiveRecord::Base def to_response_object(options = {}) claims = { iss: AppConfig.environment.url, - sub: AppConfig.environment.url + o_auth_application.client_id.to_s + user.id.to_s, # TODO: Convert to proper PPID - aud: o_auth_application.client_id, + sub: AppConfig.environment.url + authorization.o_auth_application.client_id.to_s + authorization.user.id.to_s, # TODO: Convert to proper PPID + aud: authorization.o_auth_application.client_id, exp: expires_at.to_i, iat: created_at.to_i, - auth_time: user.current_sign_in_at.to_i, + auth_time: authorization.user.current_sign_in_at.to_i, nonce: nonce, acr: 0 # TODO: Adjust ? } diff --git a/app/models/openid_connect/o_auth_access_token.rb b/app/models/openid_connect/o_auth_access_token.rb index 707bdc223..de46e837f 100644 --- a/app/models/openid_connect/o_auth_access_token.rb +++ b/app/models/openid_connect/o_auth_access_token.rb @@ -1,5 +1,5 @@ class OpenidConnect::OAuthAccessToken < ActiveRecord::Base - belongs_to :authorization, dependent: :delete + belongs_to :authorization has_many :scopes, through: :scope_tokens before_validation :setup, on: :create @@ -17,7 +17,7 @@ class OpenidConnect::OAuthAccessToken < 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 diff --git a/app/models/user.rb b/app/models/user.rb index 88612b66d..28fa845a2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -79,7 +79,6 @@ class User < ActiveRecord::Base has_many :authorizations, class_name: "OpenidConnect::Authorization" has_many :o_auth_applications, through: :authorizations, class_name: "OpenidConnect::OAuthApplication" has_many :o_auth_access_tokens, through: :authorizations, class_name: "OpenidConnect::OAuthAccessToken" - has_many :id_tokens, class_name: "OpenidConnect::IdToken" before_save :guard_unconfirmed_email, :save_person! diff --git a/db/migrate/20150714055110_create_id_tokens.rb b/db/migrate/20150714055110_create_id_tokens.rb index 0aadbe880..19887d252 100644 --- a/db/migrate/20150714055110_create_id_tokens.rb +++ b/db/migrate/20150714055110_create_id_tokens.rb @@ -1,8 +1,7 @@ class CreateIdTokens < ActiveRecord::Migration def change create_table :id_tokens do |t| - t.belongs_to :user, index: true - t.belongs_to :o_auth_application, index: true + t.belongs_to :authorization, index: true t.datetime :expires_at t.string :nonce diff --git a/db/schema.rb b/db/schema.rb index 2a7e29092..0b6547dbc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -157,16 +157,14 @@ ActiveRecord::Schema.define(version: 20150714055110) do add_index "conversations", ["author_id"], name: "conversations_author_id_fk", using: :btree create_table "id_tokens", force: :cascade do |t| - t.integer "user_id", limit: 4 - t.integer "o_auth_application_id", limit: 4 + t.integer "authorization_id", limit: 4 t.datetime "expires_at" - t.string "nonce", limit: 255 - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.string "nonce", limit: 255 + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end - add_index "id_tokens", ["o_auth_application_id"], name: "index_id_tokens_on_o_auth_application_id", using: :btree - add_index "id_tokens", ["user_id"], name: "index_id_tokens_on_user_id", using: :btree + add_index "id_tokens", ["authorization_id"], name: "index_id_tokens_on_authorization_id", using: :btree create_table "invitation_codes", force: :cascade do |t| t.string "token", limit: 255 diff --git a/lib/openid_connect/endpoints/endpoint.rb b/lib/openid_connect/authorization_point/endpoint.rb similarity index 68% rename from lib/openid_connect/endpoints/endpoint.rb rename to lib/openid_connect/authorization_point/endpoint.rb index 9a1b35eca..9d31c4957 100644 --- a/lib/openid_connect/endpoints/endpoint.rb +++ b/lib/openid_connect/authorization_point/endpoint.rb @@ -1,5 +1,5 @@ module OpenidConnect - module Authorization + module AuthorizationPoint class Endpoint attr_accessor :app, :user, :o_auth_application, :redirect_uri, :response_type, :scopes, :_request_, :request_uri, :request_object, :nonce @@ -20,6 +20,8 @@ module OpenidConnect def build_attributes(req, res) build_client(req) build_redirect_uri(req, res) + verify_nonce(req, res) + build_scopes(req) end def handle_response_type(req, res) @@ -35,6 +37,22 @@ module OpenidConnect def build_redirect_uri(req, res) res.redirect_uri = @redirect_uri = req.verify_redirect_uri!(@o_auth_application.redirect_uris) end + + def verify_nonce(req, res) + if res.protocol_params_location == :fragment && req.nonce.blank? + req.invalid_request! "nonce required" + end + end + + def build_scopes(req) + @scopes = req.scope.map {|scope| + OpenidConnect::Scope.where(name: scope).first.tap do |scope| + req.invalid_scope! "Unknown scope: #{scope}" unless scope + end + } + end + + # TODO: buildResponseType(req) end end end diff --git a/lib/openid_connect/endpoints/endpoint_confirmation_point.rb b/lib/openid_connect/authorization_point/endpoint_confirmation_point.rb similarity index 78% rename from lib/openid_connect/endpoints/endpoint_confirmation_point.rb rename to lib/openid_connect/authorization_point/endpoint_confirmation_point.rb index 2400d6db1..7b91d0123 100644 --- a/lib/openid_connect/endpoints/endpoint_confirmation_point.rb +++ b/lib/openid_connect/authorization_point/endpoint_confirmation_point.rb @@ -1,16 +1,11 @@ module OpenidConnect - module Endpoints + module AuthorizationPoint class EndpointConfirmationPoint < Endpoint def initialize(current_user, approved=false) super(current_user) @approved = approved end - def build_attributes(req, res) - super(req, res) - # TODO: buildResponseType(req) - end - def handle_response_type(req, res) handle_approval(@approved, req, res) end @@ -24,9 +19,10 @@ module OpenidConnect end def approved!(req, res) + auth = OpenidConnect::Authorization.find_or_create(req.client_id, @user) response_types = Array(req.response_type) if response_types.include?(:id_token) - id_token = @user.id_tokens.create!(o_auth_application: o_auth_application, nonce: @nonce) + id_token = auth.id_tokens.create!(nonce: req.nonce) options = %i(code access_token).map{|option| ["res.#{option}", res.respond_to?(option) ? res.option : nil]}.to_h res.id_token = id_token.to_jwt(options) # TODO: Add support for request object diff --git a/lib/openid_connect/authorization_point/endpoint_start_point.rb b/lib/openid_connect/authorization_point/endpoint_start_point.rb new file mode 100644 index 000000000..94fa20770 --- /dev/null +++ b/lib/openid_connect/authorization_point/endpoint_start_point.rb @@ -0,0 +1,11 @@ +module OpenidConnect + module AuthorizationPoint + class EndpointStartPoint < Endpoint + def handle_response_type(req, res) + @response_type = req.response_type + end + + # TODO: buildRequestObject(req) + end + end +end diff --git a/lib/openid_connect/endpoints/endpoint_start_point.rb b/lib/openid_connect/endpoints/endpoint_start_point.rb deleted file mode 100644 index 0a167578f..000000000 --- a/lib/openid_connect/endpoints/endpoint_start_point.rb +++ /dev/null @@ -1,30 +0,0 @@ -module OpenidConnect - module Endpoints - class EndpointStartPoint < Endpoint - def handle_response_type(req, res) - @response_type = req.response_type - end - - def build_attributes(req, res) - super(req, res) - verify_nonce(req, res) - build_scopes(req) - # TODO: buildRequestObject(req) - end - - def verify_nonce(req, res) - if res.protocol_params_location == :fragment && req.nonce.blank? - req.invalid_request! "nonce required" - end - end - - def build_scopes(req) - @scopes = req.scope.map {|scope| - OpenidConnect::Scope.where(name: scope).first.tap do |scope| - req.invalid_scope! "Unknown scope: #{scope}" unless scope - end - } - end - end - end -end diff --git a/lib/openid_connect/token_endpoint.rb b/lib/openid_connect/token_endpoint.rb index 14dcfa94c..a19d7766c 100644 --- a/lib/openid_connect/token_endpoint.rb +++ b/lib/openid_connect/token_endpoint.rb @@ -55,6 +55,5 @@ module OpenidConnect def app_valid?(o_auth_app, req) o_auth_app.client_secret == req.client_secret end - end end diff --git a/spec/controllers/openid_connect/authorizations_controller_spec.rb b/spec/controllers/openid_connect/authorizations_controller_spec.rb index a8f9d4f60..08b65bc17 100644 --- a/spec/controllers/openid_connect/authorizations_controller_spec.rb +++ b/spec/controllers/openid_connect/authorizations_controller_spec.rb @@ -14,76 +14,100 @@ describe OpenidConnect::AuthorizationsController, type: :controller do end describe "#new" do - context "when valid parameters are passed" 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) - expect(response.body).to match("Diaspora Test Client") + context "when not yet authorized" do + context "when valid parameters are passed" 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) + 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) + expect(response.body).to match("Diaspora Test Client") + end end end - context "as POST request" do - it "should return a form page" do + 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) + 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) + 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) + 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) + 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: "openid", nonce: SecureRandom.hex(16), state: SecureRandom.hex(16) - expect(response.body).to match("Diaspora Test Client") + 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) + expect(response.location).to match("error=invalid_request") end end end + context "when already authorized" do + let!(:auth) { OpenidConnect::Authorization.find_or_create(client.client_id, alice) } - 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) - 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) - expect(response.body).to match("Diaspora Test Client") + context "when valid parameters are passed" do + before do + get :new, client_id: client.client_id, redirect_uri: "http://localhost:3000/", response_type: "id_token", + scope: "openid", nonce: 413_093_098_3, state: 413_093_098_3 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) - expect(response.body).to match("bad_request") - end - end + it "should return the id token in a fragment" do + expect(response.location).to have_content("id_token=") + encoded_id_token = response.location[/(?<=id_token=)[^&]+/] + decoded_token = OpenIDConnect::ResponseObject::IdToken.decode encoded_id_token, OpenidConnect::IdTokenConfig.public_key + expect(decoded_token.nonce).to eq("4130930983") + expect(decoded_token.exp).to be > Time.now.utc.to_i + 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) - 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) - 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) - expect(response.location).to match("error=invalid_request") + it "should return the passed in state" do + expect(response.location).to have_content("state=4130930983") + end 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 bb95fda69..2ceab0972 100644 --- a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb +++ b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb @@ -35,16 +35,38 @@ describe OpenidConnect::ProtectedResourceEndpoint, type: :request do end context "when an invalid access token is provided" do - it "should respond with a 401 Unauthorized response" do + before do get "/api/v0/user/", access_token: invalid_token + end + + it "should respond with a 401 Unauthorized response" do 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 expect(response.headers["WWW-Authenticate"]).to include("Bearer") end + + it "should contain an invalid_token error" do + expect(response.body).to include("invalid_token") + end + end + + context "when authorization has been destroyed" do + before do + auth.destroy + get "/api/v0/user/", access_token: access_token + end + + it "should respond with a 401 Unauthorized response" do + expect(response.status).to be(401) + end + + it "should have an auth-scheme value of Bearer" do + 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 expect(response.body).to include("invalid_token") end end