Commit 1a7f2edc authored by theworldbright's avatar theworldbright

Perform major refactoring

- Add foreign_keys
- Remove unused classes/methods
- Fix pronto errors
- Add method to retrieve client id from name
- Remove TODO comments
- Fix unnecessary private key generation
parent e55a0b0d
......@@ -20,7 +20,7 @@ vendor/cache/
config/database.yml
.rvmrc_custom
.rvmrc.local
oidc_key.pem
config/oidc_key.pem
# Mailing list stuff
config/email_offset
......
......@@ -30,7 +30,7 @@ module Api
if authorization
authorization.destroy
else
raise ArgumentError, "Error while trying revoke non-existent authorization with ID #{params[:id]}"
flash[:error] = I18n.t("api.openid_connect.authorizations.destroy.fail", id: params[:id])
end
redirect_to user_applications_url
end
......@@ -54,9 +54,7 @@ module Api
def reauthenticate
sign_out current_user
params_as_get_query = params.map {|key, value| key.to_s + "=" + value }.join("&")
authorization_path_with_query = new_api_openid_connect_authorization_path + "?" + params_as_get_query
redirect_to authorization_path_with_query
redirect_to new_api_openid_connect_authorization_path(params)
end
def handle_authorization_form(auth)
......@@ -125,9 +123,9 @@ module Api
@scopes.join(" ")
end
def process_authorization_consent(approvedString)
def process_authorization_consent(approved_string)
endpoint = Api::OpenidConnect::AuthorizationPoint::EndpointConfirmationPoint.new(
current_user, to_boolean(approvedString))
current_user, to_boolean(approved_string))
handle_confirmation_endpoint_response(endpoint)
end
......@@ -166,7 +164,7 @@ module Api
def response_type_as_space_seperated_values
if session[:response_type].respond_to?(:map)
session[:response_type].map(&:to_s).join(" ")
session[:response_type].join(" ")
else
session[:response_type]
end
......@@ -189,7 +187,7 @@ module Api
def redirect_prompt_error_display(error, error_description)
redirect_params_hash = {error: error, error_description: error_description, state: params[:state]}
redirect_fragment = redirect_params_hash.compact.map {|key, value| key.to_s + "=" + value }.join("&")
redirect_to params[:redirect_uri] + "#" + redirect_fragment
redirect_to "#{params[:redirect_uri]}##{redirect_fragment}"
end
end
end
......
......@@ -15,6 +15,15 @@ module Api
render json: client.as_json(root: false)
end
def find
client = Api::OpenidConnect::OAuthApplication.find_by(client_name: params[:client_name])
if client
render json: {client_id: client.client_id}
else
render json: {error: "Client with name #{params[:client_name]} does not exist"}
end
end
private
def http_error_page_as_json(e)
......
module Api
module V0
class UsersController < Api::V0::BaseController
before_action do
require_access_token Api::OpenidConnect::Scope.find_by(name: "read")
end
def show
render json: current_user
end
end
end
end
......@@ -62,7 +62,7 @@ module Api
def self.use_code(code)
return unless code
find_by(code: code).tap do |auth|
return unless auth
next unless auth
auth.code = nil
auth.save
end
......
......@@ -16,10 +16,10 @@ module Api
end
def to_response_object(options={})
id_token = OpenIDConnect::ResponseObject::IdToken.new(claims)
id_token.code = options[:code] if options[:code]
id_token.access_token = options[:access_token] if options[:access_token]
id_token
OpenIDConnect::ResponseObject::IdToken.new(claims).tap do |id_token|
id_token.code = options[:code] if options[:code]
id_token.access_token = options[:access_token] if options[:access_token]
end
end
def claims
......@@ -45,8 +45,6 @@ module Api
authorization.user.diaspora_handle
end
end
# TODO: Add support for request objects
end
end
end
module Api
module V0
class BasePresenter
end
end
end
class UserApplicationsPresenter
def initialize(user)
@current_user = user
@user = user
end
def user_applications
# TODO: Fix and add tests
@applications ||= @current_user.o_auth_applications.each_with_object([]) do |app, array|
array << app_as_json(app)
end
@applications ||= @user.o_auth_applications.map {|app| app_as_json(app) }
end
def applications_count
......@@ -30,12 +27,14 @@ class UserApplicationsPresenter
end
def find_scopes(application)
Api::OpenidConnect::Authorization.find_by_client_id_and_user(
application.client_id, @current_user).scopes
find_auth(application).scopes
end
def find_id(application)
Api::OpenidConnect::Authorization.find_by_client_id_and_user(
application.client_id, @current_user).id
find_auth(application).id
end
def find_auth(application)
Api::OpenidConnect::Authorization.find_by_client_id_and_user(application.client_id, @user)
end
end
%h2= @o_auth_application.client_name
%p= t(".will_be_redirected")
= @redirect_uri
= t(".with_id_token")
%p= t(".redirection_message", redirect_uri: @redirect_uri)
%ul
- @scopes.each do |scope|
%li= scope
......
......@@ -9,7 +9,7 @@
%i.entypo-browser
.application-authorizations
- if app[:authorizations].count > 0
%h4="#{app[:name]} #{t("user_applications.index.access")}"
%h4=t("user_applications.index.access", name: app[:name])
%ul
- app[:authorizations].each do |authorization|
%li
......
......@@ -885,12 +885,12 @@ en:
openid_connect:
authorizations:
new:
will_be_redirected: "You will be redirected to"
with_id_token: "with an id token if approved or an error if denied"
requested_objects: "Request Objects (Currently not supported)"
redirection_message: "Are you sure you want to give access to %{redirect_uri}?"
form:
approve: "Approve"
deny: "Deny"
destroy:
fail: "The attempt to revoke the authorization with ID %{id} has failed"
people:
zero: "No people"
one: "1 person"
......@@ -1480,11 +1480,11 @@ en:
user_applications:
index:
edit_applications: "Applications"
title: "Your authorized applications"
access: "is authorized access to:"
title: "Authorized applications"
access: "%{name} is authorized access to:"
no_requirement: "This application requires no permissions"
applications_explanation: "Here is a list of applications that you to which have authorized your profile information"
no_applications: "You have no authorized applications for now"
applications_explanation: "Here is a list of applications to which you have authorized access"
no_applications: "You have no authorized applications"
revoke_autorization: "Revoke"
scopes:
openid:
......
......@@ -240,6 +240,8 @@ Diaspora::Application.routes.draw do
namespace :api do
namespace :openid_connect do
resources :clients, only: :create
get "clients/find", to: "clients#find"
post "access_tokens", to: proc {|env| Api::OpenidConnect::TokenEndpoint.new.call(env) }
# Authorization Servers MUST support the use of the HTTP GET and POST methods at the Authorization Endpoint
......
......@@ -2,7 +2,7 @@ class CreateOAuthApplications < ActiveRecord::Migration
def change
create_table :o_auth_applications do |t|
t.belongs_to :user, index: true
t.string :client_id
t.string :client_id, index: {unique: true, length: 191}
t.string :client_secret
t.string :client_name
......@@ -20,5 +20,6 @@ class CreateOAuthApplications < ActiveRecord::Migration
t.timestamps null: false
end
add_foreign_key :o_auth_applications, :users
end
end
......@@ -11,5 +11,7 @@ class CreateAuthorizations < ActiveRecord::Migration
t.timestamps null: false
end
add_foreign_key :authorizations, :users
add_foreign_key :authorizations, :o_auth_applications
end
end
......@@ -2,10 +2,11 @@ class CreateOAuthAccessTokens < ActiveRecord::Migration
def change
create_table :o_auth_access_tokens do |t|
t.belongs_to :authorization, index: true
t.string :token
t.string :token, index: {unique: true, length: 191}
t.datetime :expires_at
t.timestamps null: false
end
add_foreign_key :o_auth_access_tokens, :authorizations
end
end
......@@ -7,5 +7,6 @@ class CreateIdTokens < ActiveRecord::Migration
t.timestamps null: false
end
add_foreign_key :id_tokens, :authorizations
end
end
......@@ -7,5 +7,7 @@ class CreatePairwisePseudonymousIdentifiers < ActiveRecord::Migration
t.primary_key :guid, :string, limit: 32
t.string :sector_identifier
end
add_foreign_key :ppid, :o_auth_applications
add_foreign_key :ppid, :users
end
end
......@@ -270,6 +270,7 @@ ActiveRecord::Schema.define(version: 20150801074555) do
end
add_index "o_auth_access_tokens", ["authorization_id"], name: "index_o_auth_access_tokens_on_authorization_id", using: :btree
add_index "o_auth_access_tokens", ["token"], name: "index_o_auth_access_tokens_on_token", unique: true, length: {"token"=>191}, using: :btree
create_table "o_auth_applications", force: :cascade do |t|
t.integer "user_id", limit: 4
......@@ -291,6 +292,7 @@ ActiveRecord::Schema.define(version: 20150801074555) do
t.datetime "updated_at", null: false
end
add_index "o_auth_applications", ["client_id"], name: "index_o_auth_applications_on_client_id", unique: true, length: {"client_id"=>191}, using: :btree
add_index "o_auth_applications", ["user_id"], name: "index_o_auth_applications_on_user_id", using: :btree
create_table "o_embed_caches", force: :cascade do |t|
......@@ -656,18 +658,25 @@ ActiveRecord::Schema.define(version: 20150801074555) do
add_foreign_key "aspect_memberships", "aspects", name: "aspect_memberships_aspect_id_fk", on_delete: :cascade
add_foreign_key "aspect_memberships", "contacts", name: "aspect_memberships_contact_id_fk", on_delete: :cascade
add_foreign_key "aspect_visibilities", "aspects", name: "aspect_visibilities_aspect_id_fk", on_delete: :cascade
add_foreign_key "authorizations", "o_auth_applications"
add_foreign_key "authorizations", "users"
add_foreign_key "comments", "people", column: "author_id", name: "comments_author_id_fk", on_delete: :cascade
add_foreign_key "contacts", "people", name: "contacts_person_id_fk", on_delete: :cascade
add_foreign_key "conversation_visibilities", "conversations", name: "conversation_visibilities_conversation_id_fk", on_delete: :cascade
add_foreign_key "conversation_visibilities", "people", name: "conversation_visibilities_person_id_fk", on_delete: :cascade
add_foreign_key "conversations", "people", column: "author_id", name: "conversations_author_id_fk", on_delete: :cascade
add_foreign_key "id_tokens", "authorizations"
add_foreign_key "invitations", "users", column: "recipient_id", name: "invitations_recipient_id_fk", on_delete: :cascade
add_foreign_key "invitations", "users", column: "sender_id", name: "invitations_sender_id_fk", on_delete: :cascade
add_foreign_key "likes", "people", column: "author_id", name: "likes_author_id_fk", on_delete: :cascade
add_foreign_key "messages", "conversations", name: "messages_conversation_id_fk", on_delete: :cascade
add_foreign_key "messages", "people", column: "author_id", name: "messages_author_id_fk", on_delete: :cascade
add_foreign_key "notification_actors", "notifications", name: "notification_actors_notification_id_fk", on_delete: :cascade
add_foreign_key "o_auth_access_tokens", "authorizations"
add_foreign_key "o_auth_applications", "users"
add_foreign_key "posts", "people", column: "author_id", name: "posts_author_id_fk", on_delete: :cascade
add_foreign_key "ppid", "o_auth_applications"
add_foreign_key "ppid", "users"
add_foreign_key "profiles", "people", name: "profiles_person_id_fk", on_delete: :cascade
add_foreign_key "services", "users", name: "services_user_id_fk", on_delete: :cascade
add_foreign_key "share_visibilities", "contacts", name: "post_visibilities_contact_id_fk", on_delete: :cascade
......
......@@ -27,7 +27,7 @@ Feature: Access protected resources using implicit flow
Scenario: Application is authorized and uses small value for the max_age parameter
When I register a new client
And I sign in as "kent@kent.kent"
And I pass time
And I have signed in 5 minutes ago
And I send a post request from that client to the authorization endpoint with max age
And I sign in as "kent@kent.kent"
And I give my consent and authorize the client
......
......@@ -4,7 +4,6 @@ Feature: managing authorized applications
Given following users exist:
| username | email |
| Augier | augier@example.org |
And all scopes exist
And a client with a provided picture exists for user "augier@example.org"
And a client exists for user "augier@example.org"
......
......@@ -5,7 +5,6 @@ Feature: managing authorized applications
Given following users exist:
| username | email |
| Augier | augier@example.org |
And all scopes exist
And a client with a provided picture exists for user "augier@example.org"
And a client exists for user "augier@example.org"
......
o_auth_query_params = %i(
redirect_uri=http://localhost:3000
response_type=id_token%20token
scope=openid%20read
nonce=hello
state=hi
prompt=login
).join("&")
o_auth_query_params_with_max_age = %i(
redirect_uri=http://localhost:3000
response_type=id_token%20token
scope=openid%20read
nonce=hello
state=hi
prompt=login
max_age=30
).join("&")
O_AUTH_QUERY_PARAMS = {
redirect_uri: "http://localhost:3000",
response_type: "id_token token",
scope: "openid read",
nonce: "hello",
state: "hi",
prompt: "login"
}
O_AUTH_QUERY_PARAMS_WITH_MAX_AGE = {
redirect_uri: "http://localhost:3000",
response_type: "id_token token",
scope: "openid read",
nonce: "hello",
state: "hi",
prompt: "login",
max_age: 30
}
Given /^I send a post request from that client to the authorization endpoint$/ do
client_json = JSON.parse(last_response.body)
visit new_api_openid_connect_authorization_path +
"?client_id=#{client_json['client_id']}&#{o_auth_query_params}"
visit new_api_openid_connect_authorization_path(O_AUTH_QUERY_PARAMS.merge(client_id: client_json["client_id"]))
end
Given /^I pass time$/ do
Timecop.travel(Time.zone.now + 1.minute)
Given /^I have signed in (\d+) minutes ago$/ do |minutes|
@me.update_attribute(:current_sign_in_at, Time.zone.now - minutes.to_i.minute)
end
Given /^I send a post request from that client to the authorization endpoint with max age$/ do
client_json = JSON.parse(last_response.body)
visit new_api_openid_connect_authorization_path +
"?client_id=#{client_json['client_id']}&#{o_auth_query_params_with_max_age}"
visit new_api_openid_connect_authorization_path(
O_AUTH_QUERY_PARAMS_WITH_MAX_AGE.merge(client_id: client_json["client_id"]))
end
Given /^I send a post request from that client to the authorization endpoint using a invalid client id$/ do
visit new_api_openid_connect_authorization_path + "?client_id=randomid&#{o_auth_query_params}"
visit new_api_openid_connect_authorization_path(O_AUTH_QUERY_PARAMS.merge(client_id: "randomid"))
end
When /^I give my consent and authorize the client$/ do
......
......@@ -6,8 +6,8 @@ module Api
:scopes, :_request_, :request_uri, :request_object, :nonce
delegate :call, to: :app
def initialize(current_user)
@user = current_user
def initialize(user)
@user = user
@app = Rack::OAuth2::Server::Authorize.new do |req, res|
build_attributes(req, res)
if OAuthApplication.available_response_types.include? Array(req.response_type).map(&:to_s).join(" ")
......@@ -26,7 +26,7 @@ module Api
end
def handle_response_type(_req, _res)
# Implemented by subclass
raise NotImplementedError # Implemented by subclass
end
private
......
......@@ -56,9 +56,7 @@ module Api
def handle_approved_id_token(auth, res, response_types)
return unless response_types.include?(:id_token)
id_token = auth.create_id_token
auth_code_value = res.respond_to?(:code) ? res.code : nil
access_token_value = res.respond_to?(:access_token) ? res.access_token : nil
res.id_token = id_token.to_jwt(code: auth_code_value, access_token: access_token_value)
res.id_token = id_token.to_jwt(code: res.try(:code), access_token: res.try(:access_token))
end
end
end
......
......@@ -5,8 +5,6 @@ module Api
def handle_response_type(req, _res)
@response_type = req.response_type
end
# TODO: buildRequestObject(req)
end
end
end
......
module Api
module OpenidConnect
class IdTokenConfig
private_key = OpenSSL::PKey::RSA.new(2048)
key_file_path = File.join(Rails.root, "config", "oidc_key.pem")
if File.exist?(key_file_path)
private_key = OpenSSL::PKey::RSA.new(File.read(key_file_path))
else
open key_file_path, "w" do |io|
io.write private_key.to_pem
end
private_key = OpenSSL::PKey::RSA.new(2048)
File.write key_file_path, private_key.to_pem
File.chmod(0600, key_file_path)
end
PRIVATE_KEY = private_key
......
......@@ -301,7 +301,9 @@ describe Api::OpenidConnect::AuthorizationsController, type: :controller do
context "with non-existent authorization" do
it "raises an error" do
expect { delete :destroy, id: 123_456_789 }.to raise_error(ArgumentError)
delete :destroy, id: 123_456_789
expect(response).to redirect_to(user_applications_url)
expect(flash[:error]).to eq("The attempt to revoke the authorization with ID 123456789 has failed")
end
end
end
......
......@@ -14,6 +14,7 @@ describe Api::OpenidConnect::ClientsController, type: :controller do
expect(client_json["ppid"]).to eq(true)
end
end
context "when redirect uri is missing" do
it "should return a invalid_client_metadata error" do
post :create, response_types: [], grant_types: [], application_type: "web", contacts: [],
......@@ -23,6 +24,7 @@ describe Api::OpenidConnect::ClientsController, type: :controller do
expect(client_json["error"]).to have_content("invalid_client_metadata")
end
end
context "when redirect client_name is missing" do
it "should return a invalid_client_metadata error" do
post :create, redirect_uris: ["http://localhost"], response_types: [], grant_types: [],
......@@ -34,4 +36,24 @@ describe Api::OpenidConnect::ClientsController, type: :controller do
end
end
end
describe "#find" do
let!(:client) { FactoryGirl.create(:o_auth_application) }
context "when an OIDC client already exists" do
it "should return a client id" do
get :find, client_name: client.client_name
client_id_json = JSON.parse(response.body)
expect(client_id_json["client_id"]).to eq(client.client_id)
end
end
context "when an OIDC client doesn't already exist" do
it "should return the appropriate error" do
get :find, client_name: "random_name"
client_id_json = JSON.parse(response.body)
expect(client_id_json["error"]).to eq("Client with name random_name does not exist")
end
end
end
end
......@@ -3,17 +3,17 @@ require "spec_helper"
describe Api::OpenidConnect::DiscoveryController, type: :controller do
describe "#webfinger" do
before do
get :webfinger, resource: "http://test.host/bob"
get :webfinger, resource: "http://example.com/bob"
end
it "should return a url to the openid-configuration" do
json_body = JSON.parse(response.body)
expect(json_body["links"].first["href"]).to eq("http://test.host/")
expect(json_body["links"].first["href"]).to eq(root_url)
end
it "should return the resource in the subject" do
json_body = JSON.parse(response.body)
expect(json_body["subject"]).to eq("http://test.host/bob")
expect(json_body["subject"]).to eq("http://example.com/bob")
end
end
......@@ -21,9 +21,10 @@ describe Api::OpenidConnect::DiscoveryController, type: :controller do
before do
get :configuration
end
it "should have the issuer as the root url" do
json_body = JSON.parse(response.body)
expect(json_body["issuer"]).to eq("http://test.host/")
expect(json_body["issuer"]).to eq(root_url)
end
it "should have the appropriate user info endpoint" do
......
require "spec_helper"
describe Api::V0::BaseController do
end
......@@ -312,43 +312,43 @@ FactoryGirl.define do
factory :o_auth_application, class: Api::OpenidConnect::OAuthApplication do
client_name "Diaspora Test Client"
redirect_uris ["http://localhost:3000/"]
redirect_uris %w(http://localhost:3000/)
end
factory :o_auth_application_with_image, class: Api::OpenidConnect::OAuthApplication do
client_name "Diaspora Test Client"
redirect_uris ["http://localhost:3000/"]
redirect_uris %w(http://localhost:3000/)
logo_uri "/assets/user/default.png"
end
factory :o_auth_application_with_ppid, class: Api::OpenidConnect::OAuthApplication do
client_name "Diaspora Test Client"
redirect_uris ["http://localhost:3000/"]
redirect_uris %w(http://localhost:3000/)
ppid true
sector_identifier_uri "https://example.com/uri"
end
factory :o_auth_application_with_multiple_redirects, class: Api::OpenidConnect::OAuthApplication do
client_name "Diaspora Test Client"
redirect_uris ["http://localhost:3000/", "http://localhost/"]
redirect_uris %w(http://localhost:3000/ http://localhost/)
end
factory :auth_with_read, class: Api::OpenidConnect::Authorization do
o_auth_application
user
scopes ["openid","read"]
scopes %w(openid read)
end
factory :auth_with_read_and_ppid, class: Api::OpenidConnect::Authorization do
association :o_auth_application, factory: :o_auth_application_with_ppid
user
scopes ["openid","read"]
scopes %w(openid read)
end
factory :auth_with_read_and_write, class: Api::OpenidConnect::Authorization do
o_auth_application
user
scopes ["openid","read","write"]
scopes %w(openid read write)
end
# Factories for the DiasporaFederation-gem
......
require "spec_helper"
describe Api::OpenidConnect::ProtectedResourceEndpoint, type: :request do
let(:auth_with_read) { FactoryGirl.create(:auth_with_read) }
let!(:access_token_with_read) { auth_with_read.create_access_token.to_s }
......
require "spec_helper"
describe Api::OpenidConnect::TokenEndpoint, type: :request do
let!(:client) { FactoryGirl.create(:o_auth_application_with_ppid) }
let!(:auth) {
......@@ -34,6 +35,22 @@ describe Api::OpenidConnect::TokenEndpoint, type: :request do
access_token_check_num = UrlSafeBase64.encode64(OpenSSL::Digest::SHA256.digest(access_token)[0, 128 / 8])
expect(decoded_token.at_hash).to eq(access_token_check_num)
end
it "should not allow code to be reused" do
auth.reload
expect(auth.code).to eq(nil)
post api_openid_connect_access_tokens_path, grant_type: "authorization_code",
client_id: client.client_id, client_secret: client.client_secret,
redirect_uri: "http://localhost:3000/", code: code
expect(JSON.parse(response.body)["error"]).to eq("invalid_grant")
end
it "should not allow a nil code" do
post api_openid_connect_access_tokens_path, grant_type: "authorization_code",
client_id: client.client_id, client_secret: client.client_secret,
redirect_uri: "http://localhost:3000/", code: nil
expect(JSON.parse(response.body)["error"]).to eq("invalid_request")
end
end
context "when the authorization code is not valid" do
......
require "spec_helper"
describe Api::V0::BasePresenter do
end
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment