From 1475672d720494bbcfea9f45f77ad093e14cd595 Mon Sep 17 00:00:00 2001 From: theworldbright Date: Wed, 15 Jul 2015 16:16:50 +0900 Subject: [PATCH] Fix authorization and related models Squashed commits: [a844d37] Remove unnecessary class_name's from models [529a30c] Further adjust authorization and related models --- app/controllers/api/v0/base_controller.rb | 4 ++ app/controllers/api/v0/users_controller.rb | 2 +- app/models/openid_connect/authorization.rb | 39 ++++++++----------- .../openid_connect/authorization_scope.rb | 10 ++--- .../openid_connect/o_auth_access_token.rb | 4 +- .../openid_connect/o_auth_application.rb | 2 - app/models/openid_connect/scope.rb | 2 +- app/models/openid_connect/scopes_tokens.rb | 2 +- app/models/user.rb | 6 +-- ...50614134031_create_o_auth_access_tokens.rb | 4 +- ...create_authorizations_scopes_join_table.rb | 2 +- db/schema.rb | 3 +- lib/openid_connect/token_endpoint.rb | 8 ++-- .../protected_resource_endpoint_spec.rb | 10 ++--- 14 files changed, 46 insertions(+), 52 deletions(-) diff --git a/app/controllers/api/v0/base_controller.rb b/app/controllers/api/v0/base_controller.rb index f95c9c5c7..982800edd 100644 --- a/app/controllers/api/v0/base_controller.rb +++ b/app/controllers/api/v0/base_controller.rb @@ -2,4 +2,8 @@ class Api::V0::BaseController < ApplicationController include OpenidConnect::ProtectedResourceEndpoint 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 c09f4cdb4..57b2c300c 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 - current_token.user + authorization.user end end diff --git a/app/models/openid_connect/authorization.rb b/app/models/openid_connect/authorization.rb index 1b0b97f76..a13a861bd 100644 --- a/app/models/openid_connect/authorization.rb +++ b/app/models/openid_connect/authorization.rb @@ -1,43 +1,38 @@ 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, uniqueness: true - validates :user, :o_auth_application, uniqueness: true - - # TODO: Incomplete class + validates :refresh_token, presence: true, uniqueness: true + validates :user, presence: true, uniqueness: true + validates :o_auth_application, presence: true, uniqueness: true def setup - self.refresh_token = nil - end - - def self.valid?(token) - OpenidConnect::Authorization.exists? refresh_token: token - end - - def create_refresh_token self.refresh_token = SecureRandom.hex(32) end - def create_token - o_auth_access_tokens.create!.bearer_token + def create_access_token + OpenidConnect::OAuthAccessToken.create!(authorization: self).bearer_token end - def self.find_by_client_id_and_user(client_id, user) - app = OpenidConnect::OAuthApplication.find_by(client_id: client_id) + # TODO: Actually call this method from token endpoint + def regenerate_refresh_token + self.refresh_token = SecureRandom.hex(32) + end + + def self.find_by_client_id_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) - auth = find_by_client_id_and_user client_id, user - unless auth - # TODO: Handle creation error - auth = create! user: user, o_auth_application: OpenidConnect::OAuthApplication.find_by(client_id: client_id) - end - auth + app = OpenidConnect::OAuthApplication.find_by(client_id: client_id) + find_by_client_id_and_user(app, user) || create!(user: user, o_auth_application: app) end + + # TODO: Incomplete class end diff --git a/app/models/openid_connect/authorization_scope.rb b/app/models/openid_connect/authorization_scope.rb index d8a88d779..30e160281 100644 --- a/app/models/openid_connect/authorization_scope.rb +++ b/app/models/openid_connect/authorization_scope.rb @@ -1,7 +1,7 @@ -class AuthorizationScope < ActiveRecord::Base - belongs_to authorization - belongs_to scope +class OpenidConnect::AuthorizationScope < ActiveRecord::Base + belongs_to :authorization + belongs_to :scope - validates authorization, presence: true - validates scope, presence: true + validates :authorization, presence: true + validates :scope, presence: true end diff --git a/app/models/openid_connect/o_auth_access_token.rb b/app/models/openid_connect/o_auth_access_token.rb index bb7c8d90e..707bdc223 100644 --- a/app/models/openid_connect/o_auth_access_token.rb +++ b/app/models/openid_connect/o_auth_access_token.rb @@ -1,11 +1,11 @@ class OpenidConnect::OAuthAccessToken < ActiveRecord::Base - belongs_to :user - belongs_to :authorization + belongs_to :authorization, dependent: :delete has_many :scopes, through: :scope_tokens before_validation :setup, on: :create validates :token, presence: true, uniqueness: true + validates :authorization, presence: true, uniqueness: true scope :valid, ->(time) { where("expires_at >= ?", time) } diff --git a/app/models/openid_connect/o_auth_application.rb b/app/models/openid_connect/o_auth_application.rb index a5b1de35e..2ba85ba0b 100644 --- a/app/models/openid_connect/o_auth_application.rb +++ b/app/models/openid_connect/o_auth_application.rb @@ -1,6 +1,4 @@ class OpenidConnect::OAuthApplication < ActiveRecord::Base - belongs_to :user - has_many :authorizations has_many :user, through: :authorizations diff --git a/app/models/openid_connect/scope.rb b/app/models/openid_connect/scope.rb index 8fb8b28ed..e5449b648 100644 --- a/app/models/openid_connect/scope.rb +++ b/app/models/openid_connect/scope.rb @@ -1,5 +1,5 @@ class OpenidConnect::Scope < ActiveRecord::Base - has_many :tokens, through: :scope_tokens + has_many :o_auth_access_token, through: :scope_tokens has_many :authorizations, through: :authorization_scopes validates :name, presence: true, uniqueness: true diff --git a/app/models/openid_connect/scopes_tokens.rb b/app/models/openid_connect/scopes_tokens.rb index de790ef63..3c336ac73 100644 --- a/app/models/openid_connect/scopes_tokens.rb +++ b/app/models/openid_connect/scopes_tokens.rb @@ -1,6 +1,6 @@ class OpenidConnect::ScopeToken < ActiveRecord::Base belongs_to :scope - belongs_to :token + belongs_to :o_auth_access_token validates :scope, presence: true validates :token, presence: true diff --git a/app/models/user.rb b/app/models/user.rb index 945ae434e..28fa845a2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -76,9 +76,9 @@ class User < ActiveRecord::Base has_many :reports - 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 :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" before_save :guard_unconfirmed_email, :save_person! diff --git a/db/migrate/20150614134031_create_o_auth_access_tokens.rb b/db/migrate/20150614134031_create_o_auth_access_tokens.rb index 8607500d1..5084d61e5 100644 --- a/db/migrate/20150614134031_create_o_auth_access_tokens.rb +++ b/db/migrate/20150614134031_create_o_auth_access_tokens.rb @@ -1,9 +1,7 @@ class CreateOAuthAccessTokens < ActiveRecord::Migration def self.up create_table :o_auth_access_tokens do |t| - t.belongs_to :user, index: true - t.belongs_to :authorizations - t.belongs_to :endpoints + t.belongs_to :authorization, index: true t.string :token t.datetime :expires_at diff --git a/db/migrate/20150708155202_create_authorizations_scopes_join_table.rb b/db/migrate/20150708155202_create_authorizations_scopes_join_table.rb index 48a5929ca..c91e50d11 100644 --- a/db/migrate/20150708155202_create_authorizations_scopes_join_table.rb +++ b/db/migrate/20150708155202_create_authorizations_scopes_join_table.rb @@ -1,7 +1,7 @@ class CreateAuthorizationsScopesJoinTable < ActiveRecord::Migration def change create_table :authorizations_scopes, id: false do |t| - t.belongs_to :endpoints, index: true + t.belongs_to :authorization, index: true t.belongs_to :scope, index: true end end diff --git a/db/schema.rb b/db/schema.rb index d5b98ce39..7b3993f06 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -256,7 +256,6 @@ ActiveRecord::Schema.define(version: 20150708155747) do add_index "notifications", ["target_type", "target_id"], name: "index_notifications_on_target_type_and_target_id", length: {"target_type"=>190, "target_id"=>nil}, using: :btree create_table "o_auth_access_tokens", force: :cascade do |t| - t.integer "user_id", limit: 4 t.integer "authorization_id", limit: 4 t.string "token", limit: 255 t.datetime "expires_at" @@ -264,7 +263,7 @@ ActiveRecord::Schema.define(version: 20150708155747) do t.datetime "updated_at", null: false end - add_index "o_auth_access_tokens", ["user_id"], name: "index_o_auth_access_tokens_on_user_id", using: :btree + add_index "o_auth_access_tokens", ["authorization_id"], name: "index_o_auth_access_tokens_on_authorization_id", using: :btree create_table "o_auth_applications", force: :cascade do |t| t.integer "user_id", limit: 4 diff --git a/lib/openid_connect/token_endpoint.rb b/lib/openid_connect/token_endpoint.rb index 4714ad481..14dcfa94c 100644 --- a/lib/openid_connect/token_endpoint.rb +++ b/lib/openid_connect/token_endpoint.rb @@ -30,7 +30,7 @@ module OpenidConnect if user if user.valid_password?(req.password) auth = OpenidConnect::Authorization.find_or_create(req.client_id, user) - res.access_token = auth.create_token + res.access_token = auth.create_access_token else req.invalid_grant! end @@ -40,9 +40,9 @@ module OpenidConnect end def handle_refresh_flow(req, res) - auth = OpenidConnect::Authorization.find_by_client_id req.client_id - if OpenidConnect::Authorization.valid? req.refresh_token - res.access_token = auth.create_token + auth = OpenidConnect::Authorization.where(client_id: req.client_id).where(refresh_token: req.refresh_token).first + if auth + res.access_token = auth.create_access_token else req.invalid_grant! end diff --git a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb index 11df9e303..bb95fda69 100644 --- a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb +++ b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb @@ -5,20 +5,20 @@ describe OpenidConnect::ProtectedResourceEndpoint, type: :request do let!(:client) do OpenidConnect::OAuthApplication.create!(name: "Diaspora Test Client", redirect_uris: ["http://localhost:3000/"]) end - let(:auth) { OpenidConnect::Authorization.find_or_create(client.client_id, bob) } - let!(:token) { auth.create_token.to_s } - let(:invalid_token) { SecureRandom.hex(32).to_s } + let!(:auth) { OpenidConnect::Authorization.find_or_create(client.client_id, bob) } + let!(:access_token) { auth.create_access_token.to_s } + let!(:invalid_token) { SecureRandom.hex(32).to_s } # TODO: Add tests for expired access tokens context "when access token is valid" do it "shows the user's username and email" do - get "/api/v0/user/", access_token: token + get "/api/v0/user/", access_token: access_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: access_token expect(response.headers["Cache-Control"]).to include("private") end end