From b173283692b3ecfdc6f8bccdbfa2f326081da2ec Mon Sep 17 00:00:00 2001 From: augier Date: Mon, 20 Jul 2015 20:05:49 +0200 Subject: [PATCH] Test for refresh token flow --- app/models/openid_connect/authorization.rb | 9 ++- app/models/openid_connect/id_token.rb | 16 ++-- lib/openid_connect/token_endpoint.rb | 2 +- .../protected_resource_endpoint_spec.rb | 2 +- .../lib/openid_connect/token_endpoint_spec.rb | 76 +++++++++++++++++-- 5 files changed, 89 insertions(+), 16 deletions(-) diff --git a/app/models/openid_connect/authorization.rb b/app/models/openid_connect/authorization.rb index abbe8c090..341dbd67f 100644 --- a/app/models/openid_connect/authorization.rb +++ b/app/models/openid_connect/authorization.rb @@ -10,7 +10,9 @@ class OpenidConnect::Authorization < ActiveRecord::Base has_many :o_auth_access_tokens, dependent: :destroy has_many :id_tokens, dependent: :destroy - def generate_refresh_token + before_validation :setup, on: :create + + def setup self.refresh_token = SecureRandom.hex(32) end @@ -28,5 +30,10 @@ class OpenidConnect::Authorization < ActiveRecord::Base find_by(o_auth_application: app, user: user) end + def self.find_by_refresh_token(client_id, refresh_token) + OpenidConnect::Authorization.joins(:o_auth_application).where( + o_auth_applications: {client_id: client_id}, refresh_token: refresh_token).first + end + # TODO: Consider splitting into subclasses by flow type end diff --git a/app/models/openid_connect/id_token.rb b/app/models/openid_connect/id_token.rb index cd8c4d9c3..fe2cd252e 100644 --- a/app/models/openid_connect/id_token.rb +++ b/app/models/openid_connect/id_token.rb @@ -14,9 +14,17 @@ class OpenidConnect::IdToken < ActiveRecord::Base end def to_response_object(options={}) - claims = { + 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 + end + + def claims + @claims ||= { iss: AppConfig.environment.url, - sub: AppConfig.environment.url + authorization.o_auth_application.client_id.to_s + authorization.user.id.to_s, # TODO: Convert to proper PPID + # TODO: Convert to proper PPID + sub: "#{AppConfig.environment.url}#{authorization.o_auth_application.client_id}#{authorization.user.id}", aud: authorization.o_auth_application.client_id, exp: expires_at.to_i, iat: created_at.to_i, @@ -24,10 +32,6 @@ class OpenidConnect::IdToken < ActiveRecord::Base nonce: nonce, acr: 0 # TODO: Adjust ? } - 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 end # TODO: Add support for request objects diff --git a/lib/openid_connect/token_endpoint.rb b/lib/openid_connect/token_endpoint.rb index 116a5c67b..fd843110a 100644 --- a/lib/openid_connect/token_endpoint.rb +++ b/lib/openid_connect/token_endpoint.rb @@ -40,7 +40,7 @@ module OpenidConnect end def handle_refresh_flow(req, res) - auth = OpenidConnect::Authorization.where(client_id: req.client_id).where(refresh_token: req.refresh_token).first + auth = OpenidConnect::Authorization.find_by_refresh_token req.client_id, req.refresh_token if auth res.access_token = auth.create_access_token else diff --git a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb index 2ceab0972..4eb242b05 100644 --- a/spec/lib/openid_connect/protected_resource_endpoint_spec.rb +++ b/spec/lib/openid_connect/protected_resource_endpoint_spec.rb @@ -5,7 +5,7 @@ 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!(:auth) { OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: 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 diff --git a/spec/lib/openid_connect/token_endpoint_spec.rb b/spec/lib/openid_connect/token_endpoint_spec.rb index 1b60704b5..c5c6af389 100644 --- a/spec/lib/openid_connect/token_endpoint_spec.rb +++ b/spec/lib/openid_connect/token_endpoint_spec.rb @@ -2,62 +2,79 @@ require "spec_helper" describe OpenidConnect::TokenEndpoint, type: :request do let!(:client) { OpenidConnect::OAuthApplication.create!(redirect_uris: ["http://localhost"]) } + let!(:auth) { OpenidConnect::Authorization.find_or_create_by(o_auth_application: client, user: bob) } + 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 - expect(response.body).to include("'username' required") + 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 - expect(response.body).to include("'password' required") + 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 - expect(response.body).to include("invalid_grant") + 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 - expect(response.body).to include("invalid_grant") + expect(response.body).to include "invalid_grant" end end + + context "when the client_secret doesn't match" do + it "should return an invalid client error" do + post "/openid_connect/access_tokens", grant_type: "password", username: "bob", + password: "bluepin7", client_id: client.client_id, client_secret: "client.client_secret" + expect(response.body).to include "invalid_client" + 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 json = JSON.parse(response.body) + expect(json.keys).to include "expires_in" expect(json["access_token"].length).to eq(64) expect(json["token_type"]).to eq("bearer") - expect(json.keys).to include("expires_in") end 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 - expect(response.body).to include("invalid_grant") + 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 - expect(response.body).to include("invalid_client") + 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 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", @@ -65,4 +82,49 @@ describe OpenidConnect::TokenEndpoint, type: :request do expect(response.body).to include "unsupported_grant_type" end end + + describe "the refresh token flow" do + context "when the refresh token is valid" do + it "should return an access token" do + post "/openid_connect/access_tokens", grant_type: "refresh_token", + client_id: client.client_id, client_secret: client.client_secret, refresh_token: auth.refresh_token + json = JSON.parse(response.body) + expect(response.body).to include "expires_in" + expect(json["access_token"].length).to eq(64) + expect(json["token_type"]).to eq("bearer") + end + end + + context "when the refresh token is not valid" do + it "should return an invalid grant error" do + post "/openid_connect/access_tokens", grant_type: "refresh_token", + client_id: client.client_id, client_secret: client.client_secret, refresh_token: " " + 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: "refresh_token", refresh_token: auth.refresh_token, + client_id: SecureRandom.hex(16).to_s, client_secret: client.client_secret + expect(response.body).to include "invalid_client" + end + end + + context "when the refresh_token field is missing" do + it "should return an invalid request error" do + post "/openid_connect/access_tokens", grant_type: "refresh_token", + client_id: client.client_id, client_secret: client.client_secret + expect(response.body).to include "'refresh_token' required" + end + end + + context "when the client_secret doesn't match" do + it "should return an invalid client error" do + post "/openid_connect/access_tokens", grant_type: "refresh_token", refresh_token: auth.refresh_token, + client_id: client.client_id, client_secret: "client.client_secret" + expect(response.body).to include "invalid_client" + end + end + end end